From fcdce291bb43eec712b4dbcbd8f6032136cfa095 Mon Sep 17 00:00:00 2001 From: Waheed Ahmed Date: Wed, 23 Dec 2020 18:07:37 +0500 Subject: [PATCH] Fix form submission issues. (#50) * Fix form submission issues. Fixed form submission issues on first click and with optional fields check. Also fixed some warnings and did some refactoring as well. VAN-225, VAN-206 * added more tests --- .gitignore | 1 + src/logistration/RegistrationPage.jsx | 77 ++++--- src/logistration/data/sagas.js | 17 +- src/logistration/data/service.js | 4 +- src/logistration/data/tests/sagas.test.js | 204 ++++++++++++++++-- src/logistration/messages.jsx | 5 + .../tests/RegistrationPage.test.jsx | 38 ---- .../RegistrationPage.test.jsx.snap | 8 +- src/setupTest.js | 16 ++ 9 files changed, 259 insertions(+), 111 deletions(-) diff --git a/.gitignore b/.gitignore index 31e63a92..88c392ae 100755 --- a/.gitignore +++ b/.gitignore @@ -4,6 +4,7 @@ node_modules npm-debug.log coverage +module.config.js dist/ src/i18n/transifex_input.json diff --git a/src/logistration/RegistrationPage.jsx b/src/logistration/RegistrationPage.jsx index 5ff81864..ec29f864 100644 --- a/src/logistration/RegistrationPage.jsx +++ b/src/logistration/RegistrationPage.jsx @@ -74,7 +74,7 @@ class RegistrationPage extends React.Component { usernameValid: false, passwordValid: false, countryValid: false, - honorCodeValid: false, + honorCodeValid: true, termsOfServiceValid: false, formValid: false, institutionLogin: false, @@ -118,8 +118,7 @@ class RegistrationPage extends React.Component { }; const fieldMap = { ...REGISTRATION_VALIDITY_MAP, ...REGISTRATION_OPTIONAL_MAP }; - Object.keys(fieldMap).forEach((key) => { - const value = fieldMap[key]; + Object.entries(fieldMap).forEach(([key, value]) => { if (value) { payload[key] = this.state[camelCase(key)]; } @@ -244,18 +243,12 @@ class RegistrationPage extends React.Component { } = this.state; const validityMap = REGISTRATION_VALIDITY_MAP; - const validStates = []; - Object.keys(validityMap).forEach((key) => { - const value = validityMap[key]; - if (value) { - const state = camelCase(key); - const stateValid = `${state}Valid`; - validStates.push(stateValid); - } - }); let extraFieldsValid = true; - validStates.forEach((value) => { - extraFieldsValid = extraFieldsValid && this.state[value]; + Object.entries(validityMap).forEach(([key, value]) => { + if (value) { + const stateValid = `${camelCase(key)}Valid`; + extraFieldsValid = extraFieldsValid && this.state[stateValid]; + } }); this.setState({ @@ -287,24 +280,25 @@ class RegistrationPage extends React.Component { REGISTRATION_VALIDITY_MAP[field.name] = true; if (field.type === 'plaintext' && field.name === 'honor_code') { // special case where honor code and tos are combined afterLink = field.label; + props.type = 'hidden'; const nodes = []; do { const matches = processLink(afterLink); [beforeLink, link, linkText, afterLink] = matches; nodes.push( - <> + {beforeLink} {linkText} - , + , ); } while (afterLink.includes('a href')); - nodes.push(<>{afterLink}); + nodes.push({afterLink}); return ( - <> -

+ + { nodes } - + ); } if (field.type === 'checkbox') { @@ -314,6 +308,7 @@ class RegistrationPage extends React.Component { return ( @@ -344,7 +340,7 @@ class RegistrationPage extends React.Component { ); } } - return (<>); + return null; }); return fields; } @@ -353,7 +349,7 @@ class RegistrationPage extends React.Component { const fields = this.props.formData.fields.map((field) => { let options = null; if (REGISTRATION_EXTRA_FIELDS.includes(field.name)) { - if (!field.required) { + if (!field.required && field.name !== 'honor_code' && field.name !== 'country') { REGISTRATION_OPTIONAL_MAP[field.name] = true; const props = { id: field.name, @@ -361,26 +357,27 @@ class RegistrationPage extends React.Component { type: field.type, onChange: e => this.handleOnChange(e), }; - if (field.name !== 'honor_code' && field.name !== 'country') { - if (field.type === 'select') { - options = field.options.map((item) => ({ - value: item.value, - label: item.name, - })); - props.options = options; - } - return ( - - - - - ); + if (field.type === 'select') { + options = field.options.map((item) => ({ + value: item.value, + label: item.name, + })); + props.options = options; } + return ( + + + + + ); } } - return (<>); + return null; }); return fields; } @@ -541,7 +538,7 @@ class RegistrationPage extends React.Component { { this.state.enableOptionalField ? this.addExtraOptionalFields() : null} { const params = { @@ -27,6 +28,10 @@ describe('fetchThirdPartyAuthContext', () => { pipelineUserDetails: {}, }; + beforeEach(() => { + loggingService.logError.mockReset(); + }); + it('should call service and dispatch success action', async () => { const getThirdPartyAuthContext = jest.spyOn(api, 'getThirdPartyAuthContext') .mockImplementation(() => Promise.resolve({ thirdPartyAuthContext: data })); @@ -39,13 +44,16 @@ describe('fetchThirdPartyAuthContext', () => { ); expect(getThirdPartyAuthContext).toHaveBeenCalledTimes(1); - expect(dispatched).toEqual([getThirdPartyAuthContextBegin(), getThirdPartyAuthContextSuccess(data)]); + expect(dispatched).toEqual([ + actions.getThirdPartyAuthContextBegin(), + actions.getThirdPartyAuthContextSuccess(data), + ]); getThirdPartyAuthContext.mockClear(); }); it('should call service and dispatch error action', async () => { const getThirdPartyAuthContext = jest.spyOn(api, 'getThirdPartyAuthContext') - .mockImplementation(() => Promise.reject()); + .mockImplementation(() => Promise.reject(new Error('something went wrong'))); const dispatched = []; await runSaga( @@ -55,7 +63,11 @@ describe('fetchThirdPartyAuthContext', () => { ); expect(getThirdPartyAuthContext).toHaveBeenCalledTimes(1); - expect(dispatched).toEqual([getThirdPartyAuthContextBegin(), getThirdPartyAuthContextFailure()]); + expect(loggingService.logError).toHaveBeenCalled(); + expect(dispatched).toEqual([ + actions.getThirdPartyAuthContextBegin(), + actions.getThirdPartyAuthContextFailure(), + ]); getThirdPartyAuthContext.mockClear(); }); }); @@ -82,6 +94,10 @@ describe('fetchRegistrationForm', () => { }], }; + beforeEach(() => { + loggingService.logError.mockReset(); + }); + it('should call service and dispatch success action', async () => { const getRegistrationForm = jest.spyOn(api, 'getRegistrationForm') .mockImplementation(() => Promise.resolve({ registrationForm: data })); @@ -93,13 +109,16 @@ describe('fetchRegistrationForm', () => { ); expect(getRegistrationForm).toHaveBeenCalledTimes(1); - expect(dispatched).toEqual([fetchRegistrationFormBegin(), fetchRegistrationFormSuccess(data)]); + expect(dispatched).toEqual([ + actions.fetchRegistrationFormBegin(), + actions.fetchRegistrationFormSuccess(data), + ]); getRegistrationForm.mockClear(); }); it('should call service and dispatch error action', async () => { const getRegistrationForm = jest.spyOn(api, 'getRegistrationForm') - .mockImplementation(() => Promise.reject()); + .mockImplementation(() => Promise.reject(new Error('something went wrong'))); const dispatched = []; await runSaga( @@ -108,7 +127,11 @@ describe('fetchRegistrationForm', () => { ); expect(getRegistrationForm).toHaveBeenCalledTimes(1); - expect(dispatched).toEqual([fetchRegistrationFormBegin(), fetchRegistrationFormFailure()]); + expect(loggingService.logError).toHaveBeenCalled(); + expect(dispatched).toEqual([ + actions.fetchRegistrationFormBegin(), + actions.fetchRegistrationFormFailure(), + ]); getRegistrationForm.mockClear(); }); }); @@ -127,6 +150,10 @@ describe('fetchRealtimeValidations', () => { }, }; + beforeEach(() => { + loggingService.logError.mockReset(); + }); + const data = { validation_decisions: { username: 'Username must be between 2 and 30 characters long.', @@ -145,13 +172,16 @@ describe('fetchRealtimeValidations', () => { ); expect(getFieldsValidations).toHaveBeenCalledTimes(1); - expect(dispatched).toEqual([fetchRealtimeValidationsBegin(), fetchRealtimeValidationsSuccess(data)]); + expect(dispatched).toEqual([ + actions.fetchRealtimeValidationsBegin(), + actions.fetchRealtimeValidationsSuccess(data), + ]); getFieldsValidations.mockClear(); }); it('should call service and dispatch error action', async () => { const getFieldsValidations = jest.spyOn(api, 'getFieldsValidations') - .mockImplementation(() => Promise.reject()); + .mockImplementation(() => Promise.reject(new Error('something went wrong'))); const dispatched = []; await runSaga( @@ -161,7 +191,141 @@ describe('fetchRealtimeValidations', () => { ); expect(getFieldsValidations).toHaveBeenCalledTimes(1); - expect(dispatched).toEqual([fetchRealtimeValidationsBegin(), fetchRealtimeValidationsFailure()]); + expect(loggingService.logError).toHaveBeenCalled(); + expect(dispatched).toEqual([ + actions.fetchRealtimeValidationsBegin(), + actions.fetchRealtimeValidationsFailure(), + ]); getFieldsValidations.mockClear(); }); }); + +describe('handleLoginRequest', () => { + const params = { + payload: { + formData: { + email: 'test@test.com', + password: 'test-password', + }, + }, + }; + + beforeEach(() => { + loggingService.logError.mockReset(); + }); + + it('should call service and dispatch success action', async () => { + const data = { redirectUrl: '/dashboard', success: true }; + const loginRequest = jest.spyOn(api, 'loginRequest') + .mockImplementation(() => Promise.resolve(data)); + + const dispatched = []; + await runSaga( + { dispatch: (action) => dispatched.push(action) }, + handleLoginRequest, + params, + ); + + expect(loginRequest).toHaveBeenCalledTimes(1); + expect(dispatched).toEqual([ + actions.loginRequestBegin(), + actions.loginRequestSuccess(data.redirectUrl, data.success), + ]); + loginRequest.mockClear(); + }); + + it('should call service and dispatch error action', async () => { + const loginErrorRespinse = { + response: { + status: 400, + data: { + login_error: 'something went wrong', + }, + }, + }; + const loginRequest = jest.spyOn(api, 'loginRequest') + .mockImplementation(() => Promise.reject(loginErrorRespinse)); + + const dispatched = []; + await runSaga( + { dispatch: (action) => dispatched.push(action) }, + handleLoginRequest, + params, + ); + + expect(loginRequest).toHaveBeenCalledTimes(1); + expect(loggingService.logError).toHaveBeenCalled(); + expect(dispatched).toEqual([ + actions.loginRequestBegin(), + actions.loginRequestFailure(camelCaseObject(loginErrorRespinse.response.data)), + ]); + loginRequest.mockClear(); + }); +}); + +describe('handleNewUserRegistration', () => { + const params = { + payload: { + formData: { + email: 'test@test.com', + username: 'test-username', + password: 'test-password', + name: 'test-name', + honor_code: true, + country: 'test-country', + }, + }, + }; + + beforeEach(() => { + loggingService.logError.mockReset(); + }); + + it('should call service and dispatch success action', async () => { + const data = { redirectUrl: '/dashboard', success: true }; + const registerRequest = jest.spyOn(api, 'registerRequest') + .mockImplementation(() => Promise.resolve(data)); + + const dispatched = []; + await runSaga( + { dispatch: (action) => dispatched.push(action) }, + handleNewUserRegistration, + params, + ); + + expect(registerRequest).toHaveBeenCalledTimes(1); + expect(dispatched).toEqual([ + actions.registerNewUserBegin(), + actions.registerNewUserSuccess(data.redirectUrl, data.success), + ]); + registerRequest.mockClear(); + }); + + it('should call service and dispatch error action', async () => { + const loginErrorRespinse = { + response: { + status: 400, + data: { + error: 'something went wrong', + }, + }, + }; + const registerRequest = jest.spyOn(api, 'registerRequest') + .mockImplementation(() => Promise.reject(loginErrorRespinse)); + + const dispatched = []; + await runSaga( + { dispatch: (action) => dispatched.push(action) }, + handleNewUserRegistration, + params, + ); + + expect(registerRequest).toHaveBeenCalledTimes(1); + expect(loggingService.logError).toHaveBeenCalled(); + expect(dispatched).toEqual([ + actions.registerNewUserBegin(), + actions.registerNewUserFailure(loginErrorRespinse.response.data), + ]); + registerRequest.mockClear(); + }); +}); diff --git a/src/logistration/messages.jsx b/src/logistration/messages.jsx index 1918e0f7..f140c1f0 100644 --- a/src/logistration/messages.jsx +++ b/src/logistration/messages.jsx @@ -186,6 +186,11 @@ const messages = defineMessages({ defaultMessage: 'Support education research by providing additional information', description: 'Text for a checkbox to ask user for if they are willing to provide extra information for education research', }, + 'logistration.register.potional.label': { + id: 'logistration.register.potional.label', + defaultMessage: '(optional)', + description: 'Text that appears with potional field labels', + }, }); export default messages; diff --git a/src/logistration/tests/RegistrationPage.test.jsx b/src/logistration/tests/RegistrationPage.test.jsx index 4c142523..e1845b9f 100644 --- a/src/logistration/tests/RegistrationPage.test.jsx +++ b/src/logistration/tests/RegistrationPage.test.jsx @@ -88,12 +88,6 @@ describe('./RegistrationPage.js', () => { it('should show error message on invalid email', () => { const validationMessage = 'Enter a valid email address that contains at least 3 characters.'; - store = mockStore({ - ...initialState, - logistration: { - ...initialState.logistration, - }, - }); const registrationPage = mount(reduxWrapper()); registrationPage.find('input#email').simulate('change', { target: { value: '', name: 'email' } }); @@ -103,12 +97,6 @@ describe('./RegistrationPage.js', () => { it('should show error message on invalid username', () => { const validationMessage = 'Username must be between 2 and 30 characters long.'; - store = mockStore({ - ...initialState, - logistration: { - ...initialState.logistration, - }, - }); const registrationPage = mount(reduxWrapper()); registrationPage.find('input#username').simulate('change', { target: { value: '', name: 'username' } }); registrationPage.update(); @@ -117,12 +105,6 @@ describe('./RegistrationPage.js', () => { it('should show error message on invalid name', () => { const validationMessage = 'Enter your full name.'; - store = mockStore({ - ...initialState, - logistration: { - ...initialState.logistration, - }, - }); const registrationPage = mount(reduxWrapper()); registrationPage.find('input#name').simulate('change', { target: { value: '', name: 'name' } }); registrationPage.update(); @@ -131,12 +113,6 @@ describe('./RegistrationPage.js', () => { it('should show error message on invalid password', () => { const validationMessage = 'This password is too short. It must contain at least 8 characters. This password must contain at least 1 number.'; - store = mockStore({ - ...initialState, - logistration: { - ...initialState.logistration, - }, - }); const registrationPage = mount(reduxWrapper()); registrationPage.find('input#password').simulate('change', { target: { value: '', name: 'password' } }); registrationPage.update(); @@ -238,13 +214,6 @@ describe('./RegistrationPage.js', () => { }); it('should dispatch fetchRealtimeValidations on Blur', () => { - store = mockStore({ - ...initialState, - logistration: { - ...initialState.logistration, - }, - }); - const formPayload = { email: '', username: '', @@ -271,13 +240,6 @@ describe('./RegistrationPage.js', () => { }); it('should not dispatch registerNewUser on Submit', () => { - store = mockStore({ - ...initialState, - logistration: { - ...initialState.logistration, - }, - }); - const formPayload = { email: '', username: '', diff --git a/src/logistration/tests/__snapshots__/RegistrationPage.test.jsx.snap b/src/logistration/tests/__snapshots__/RegistrationPage.test.jsx.snap index a7098a92..e6ffac48 100644 --- a/src/logistration/tests/__snapshots__/RegistrationPage.test.jsx.snap +++ b/src/logistration/tests/__snapshots__/RegistrationPage.test.jsx.snap @@ -205,7 +205,7 @@ exports[`./RegistrationPage.js should match TPA provider snapshot 1`] = ` className="pgn__stateful-btn pgn__stateful-btn-state-default btn-primary submit mt-4 btn btn-primary" disabled={false} onClick={[Function]} - type="submit" + type="button" >