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
This commit is contained in:
Waheed Ahmed
2020-12-23 18:07:37 +05:00
committed by GitHub
parent ebcc8bd02f
commit fcdce291bb
9 changed files with 259 additions and 111 deletions

1
.gitignore vendored
View File

@@ -4,6 +4,7 @@
node_modules
npm-debug.log
coverage
module.config.js
dist/
src/i18n/transifex_input.json

View File

@@ -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(
<>
<React.Fragment key={link}>
{beforeLink}
<Hyperlink destination={link}>{linkText}</Hyperlink>
</>,
</React.Fragment>,
);
} while (afterLink.includes('a href'));
nodes.push(<>{afterLink}</>);
nodes.push(<React.Fragment key={afterLink}>{afterLink}</React.Fragment>);
return (
<>
<p {...props} />
<React.Fragment key={field.type}>
<input {...props} />
{ nodes }
</>
</React.Fragment>
);
}
if (field.type === 'checkbox') {
@@ -314,6 +308,7 @@ class RegistrationPage extends React.Component {
return (
<ValidationFormGroup
for={field.name}
key={field.name}
invalid={this.state.errors[stateVar] !== ''}
invalidMessage={field.errorMessages.required}
className="custom-control"
@@ -335,6 +330,7 @@ class RegistrationPage extends React.Component {
return (
<ValidationFormGroup
for={field.name}
key={field.name}
invalid={this.state.errors[stateVar] !== ''}
invalidMessage={field.errorMessages.required}
>
@@ -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 (
<ValidationFormGroup
for={field.name}
>
<label htmlFor={field.name} className="h6 pt-3">{field.label} (optional)</label>
<Input {...props} />
</ValidationFormGroup>
);
if (field.type === 'select') {
options = field.options.map((item) => ({
value: item.value,
label: item.name,
}));
props.options = options;
}
return (
<ValidationFormGroup
for={field.name}
key={field.name}
>
<label htmlFor={field.name} className="h6 pt-3">
{field.label} {this.props.intl.formatMessage(messages['logistration.register.potional.label'])}
</label>
<Input {...props} />
</ValidationFormGroup>
);
}
}
return (<></>);
return null;
});
return fields;
}
@@ -541,7 +538,7 @@ class RegistrationPage extends React.Component {
</ValidationFormGroup>
{ this.state.enableOptionalField ? this.addExtraOptionalFields() : null}
<StatefulButton
type="submit"
type="button"
className="btn-primary submit mt-4"
state={submitState}
labels={{

View File

@@ -1,6 +1,7 @@
import { call, put, takeEvery } from 'redux-saga/effects';
import { camelCaseObject } from '@edx/frontend-platform';
import { logError } from '@edx/frontend-platform/logging';
// Actions
import {
@@ -31,15 +32,15 @@ import {
getFieldsValidations,
getRegistrationForm,
getThirdPartyAuthContext,
postNewUser,
login,
registerRequest,
loginRequest,
} from './service';
export function* handleNewUserRegistration(action) {
try {
yield put(registerNewUserBegin());
const { redirectUrl, success } = yield call(postNewUser, action.payload.registrationInfo);
const { redirectUrl, success } = yield call(registerRequest, action.payload.registrationInfo);
yield put(registerNewUserSuccess(
redirectUrl,
@@ -50,6 +51,7 @@ export function* handleNewUserRegistration(action) {
if (e.response && statusCodes.includes(e.response.status)) {
yield put(registerNewUserFailure(e.response.data));
}
logError(e);
}
}
@@ -57,7 +59,7 @@ export function* handleLoginRequest(action) {
try {
yield put(loginRequestBegin());
const { redirectUrl, success } = yield call(login, action.payload.creds);
const { redirectUrl, success } = yield call(loginRequest, action.payload.creds);
yield put(loginRequestSuccess(
redirectUrl,
@@ -68,6 +70,7 @@ export function* handleLoginRequest(action) {
if (e.response && statusCodes.includes(e.response.status)) {
yield put(loginRequestFailure(camelCaseObject(e.response.data)));
}
logError(e);
}
}
@@ -81,7 +84,7 @@ export function* fetchThirdPartyAuthContext(action) {
));
} catch (e) {
yield put(getThirdPartyAuthContextFailure());
throw e;
logError(e);
}
}
@@ -95,7 +98,7 @@ export function* fetchRegistrationForm() {
));
} catch (e) {
yield put(fetchRegistrationFormFailure());
throw e;
logError(e);
}
}
@@ -109,7 +112,7 @@ export function* fetchRealtimeValidations(action) {
));
} catch (e) {
yield put(fetchRealtimeValidationsFailure());
throw e;
logError(e);
}
}

View File

@@ -2,7 +2,7 @@ import { camelCaseObject, getConfig } from '@edx/frontend-platform';
import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth';
import querystring from 'querystring';
export async function postNewUser(registrationInformation) {
export async function registerRequest(registrationInformation) {
const requestConfig = {
headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
isPublic: true,
@@ -24,7 +24,7 @@ export async function postNewUser(registrationInformation) {
};
}
export async function login(creds) {
export async function loginRequest(creds) {
const requestConfig = {
headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
isPublic: true,

View File

@@ -1,18 +1,19 @@
import { runSaga } from 'redux-saga';
import { camelCaseObject } from '@edx/frontend-platform';
import * as actions from '../actions';
import {
fetchRealtimeValidationsBegin,
fetchRealtimeValidationsSuccess,
fetchRealtimeValidationsFailure,
getThirdPartyAuthContextBegin,
getThirdPartyAuthContextSuccess,
getThirdPartyAuthContextFailure,
fetchRegistrationFormBegin,
fetchRegistrationFormSuccess,
fetchRegistrationFormFailure,
} from '../actions';
import { fetchRealtimeValidations, fetchThirdPartyAuthContext, fetchRegistrationForm } from '../sagas';
fetchRealtimeValidations,
fetchThirdPartyAuthContext,
fetchRegistrationForm,
handleLoginRequest,
handleNewUserRegistration,
} from '../sagas';
import * as api from '../service';
import initializeMockLogging from '../../../setupTest';
const { loggingService } = initializeMockLogging();
describe('fetchThirdPartyAuthContext', () => {
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();
});
});

View File

@@ -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;

View File

@@ -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(<IntlRegistrationPage {...props} />));
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(<IntlRegistrationPage {...props} />));
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(<IntlRegistrationPage {...props} />));
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(<IntlRegistrationPage {...props} />));
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: '',

View File

@@ -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"
>
<span
className="d-flex align-items-center justify-content-center"
@@ -382,7 +382,7 @@ exports[`./RegistrationPage.js should match default section 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"
>
<span
className="d-flex align-items-center justify-content-center"
@@ -559,7 +559,7 @@ exports[`./RegistrationPage.js should match pending button state snapshot 1`] =
className="pgn__stateful-btn pgn__stateful-btn-state-pending btn-primary submit mt-4 disabled btn btn-primary"
disabled={false}
onClick={[Function]}
type="submit"
type="button"
>
<span
className="d-flex align-items-center justify-content-center"
@@ -783,7 +783,7 @@ exports[`./RegistrationPage.js should show error message on 409 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"
>
<span
className="d-flex align-items-center justify-content-center"

View File

@@ -3,4 +3,20 @@
import Enzyme from 'enzyme';
import Adapter from 'enzyme-adapter-react-16';
import { getConfig } from '@edx/frontend-platform';
import { configure as configureLogging } from '@edx/frontend-platform/logging';
Enzyme.configure({ adapter: new Adapter() });
class MockLoggingService {
logInfo = jest.fn();
logError = jest.fn();
}
export default function initializeMockLogging() {
const loggingService = configureLogging(MockLoggingService, {
config: getConfig(),
});
return { loggingService };
}