From ab83d698f94ffa644bcd97fcf7e12bda68bc8734 Mon Sep 17 00:00:00 2001 From: Shaun Hamilton <51722130+ShaunSHamilton@users.noreply.github.com> Date: Mon, 1 Feb 2021 13:34:04 +0000 Subject: [PATCH] fix(client): give useful error in solutionform (#40225) --- .../src/components/formHelpers/Form.test.js | 4 +-- .../src/components/formHelpers/FormFields.js | 35 ++++++++++++++++--- .../components/formHelpers/FormValidators.js | 14 ++++++++ client/src/components/formHelpers/index.js | 24 +++++++++++-- .../src/components/settings/Certification.js | 2 +- .../Challenges/projects/SolutionForm.js | 16 +++++++-- .../Challenges/projects/backend/Show.js | 8 +++-- .../Challenges/projects/frontend/Show.js | 14 ++++++-- .../Challenges/redux/challenge-modal-epic.js | 17 --------- .../redux/execute-challenge-saga.js | 17 ++++++--- .../src/templates/Challenges/redux/index.js | 3 +- 11 files changed, 114 insertions(+), 40 deletions(-) create mode 100644 client/src/components/formHelpers/FormValidators.js delete mode 100644 client/src/templates/Challenges/redux/challenge-modal-epic.js diff --git a/client/src/components/formHelpers/Form.test.js b/client/src/components/formHelpers/Form.test.js index be0af7fa7c..daf30c19f3 100644 --- a/client/src/components/formHelpers/Form.test.js +++ b/client/src/components/formHelpers/Form.test.js @@ -79,12 +79,12 @@ test('should submit', () => { fireEvent.click(button); expect(submit).toHaveBeenCalledTimes(1); - expect(submit.mock.calls[0][0]).toEqual({ website: websiteValue }); + expect(submit.mock.calls[0][0].values).toEqual({ website: websiteValue }); fireEvent.change(websiteInput, { target: { value: `${websiteValue}///` } }); expect(websiteInput).toHaveValue(`${websiteValue}///`); fireEvent.click(button); expect(submit).toHaveBeenCalledTimes(2); - expect(submit.mock.calls[1][0]).toEqual({ website: websiteValue }); + expect(submit.mock.calls[1][0].values).toEqual({ website: websiteValue }); }); diff --git a/client/src/components/formHelpers/FormFields.js b/client/src/components/formHelpers/FormFields.js index 485f59ce61..db087217dd 100644 --- a/client/src/components/formHelpers/FormFields.js +++ b/client/src/components/formHelpers/FormFields.js @@ -1,5 +1,6 @@ import React from 'react'; import { kebabCase } from 'lodash'; +import normalizeUrl from 'normalize-url'; import PropTypes from 'prop-types'; import { Alert, @@ -10,6 +11,11 @@ import { HelpBlock } from '@freecodecamp/react-bootstrap'; import { Field } from 'react-final-form'; +import { + editorValidator, + localhostValidator, + composeValidators +} from './FormValidators'; const propTypes = { formFields: PropTypes.arrayOf( @@ -33,6 +39,28 @@ function FormFields(props) { types = {} } = options; + const nullOrWarning = (value, error, isURL) => { + let validationError; + if (value && isURL) { + try { + normalizeUrl(value, { stripWWW: false }); + } catch (err) { + validationError = err.message; + } + } + const validationWarning = composeValidators( + editorValidator, + localhostValidator + )(value); + const message = error || validationError || validationWarning; + return message ? ( + + + {message} + + + ) : null; + }; return (
{formFields @@ -44,6 +72,7 @@ function FormFields(props) { const type = name in types ? types[name] : 'text'; const placeholder = name in placeholders ? placeholders[name] : ''; + const isURL = types[name] === 'url'; return ( @@ -61,11 +90,7 @@ function FormFields(props) { type={type} value={value} /> - {error && !pristine ? ( - - {error} - - ) : null} + {nullOrWarning(value, !pristine && error, isURL)} ); diff --git a/client/src/components/formHelpers/FormValidators.js b/client/src/components/formHelpers/FormValidators.js new file mode 100644 index 0000000000..15fa624c50 --- /dev/null +++ b/client/src/components/formHelpers/FormValidators.js @@ -0,0 +1,14 @@ +// Matches editor links for: Repl.it, Glitch, CodeSandbox +const editorRegex = /repl\.it\/@|glitch\.com\/edit\/#!|codesandbox\.io\/s\//; +const localhostRegex = /localhost:/; + +export const editorValidator = value => + editorRegex.test(value) ? 'Remember to submit the Live App URL.' : null; + +export const localhostValidator = value => + localhostRegex.test(value) + ? 'Remember to submit a publicly visible app URL.' + : null; + +export const composeValidators = (...validators) => value => + validators.reduce((error, validator) => error ?? validator(value), null); diff --git a/client/src/components/formHelpers/index.js b/client/src/components/formHelpers/index.js index 79dfd0540a..0ea401c813 100644 --- a/client/src/components/formHelpers/index.js +++ b/client/src/components/formHelpers/index.js @@ -1,4 +1,9 @@ import normalizeUrl from 'normalize-url'; +import { + localhostValidator, + editorValidator, + composeValidators +} from './FormValidators'; export { default as BlockSaveButton } from './BlockSaveButton.js'; export { default as BlockSaveWrapper } from './BlockSaveWrapper.js'; @@ -10,11 +15,26 @@ const normalizeOptions = { }; export function formatUrlValues(values, options) { - return Object.keys(values).reduce((result, key) => { + const validatedValues = { values: {}, errors: [], invalidValues: [] }; + const urlValues = Object.keys(values).reduce((result, key) => { let value = values[key]; + const nullOrWarning = composeValidators( + localhostValidator, + editorValidator + )(value); + if (nullOrWarning) { + validatedValues.invalidValues.push(nullOrWarning); + } if (value && options.types[key] === 'url') { - value = normalizeUrl(value, normalizeOptions); + try { + value = normalizeUrl(value, normalizeOptions); + } catch (err) { + // Not a valid URL for testing or submission + validatedValues.errors.push({ error: err, value }); + } } return { ...result, [key]: value }; }, {}); + validatedValues.values = urlValues; + return validatedValues; } diff --git a/client/src/components/settings/Certification.js b/client/src/components/settings/Certification.js index 5a8973febb..994d12f15b 100644 --- a/client/src/components/settings/Certification.js +++ b/client/src/components/settings/Certification.js @@ -328,7 +328,7 @@ export class CertificationSettings extends Component { }; // legacy projects rendering - handleSubmitLegacy(formChalObj) { + handleSubmitLegacy({ values: formChalObj }) { const { isHonest, createFlashMessage, diff --git a/client/src/templates/Challenges/projects/SolutionForm.js b/client/src/templates/Challenges/projects/SolutionForm.js index 4e268daccf..baed7034ab 100644 --- a/client/src/templates/Challenges/projects/SolutionForm.js +++ b/client/src/templates/Challenges/projects/SolutionForm.js @@ -27,10 +27,20 @@ export class SolutionForm extends Component { componentDidMount() { this.props.updateSolutionForm({}); } - handleSubmit(values) { - this.props.updateSolutionForm(values); - this.props.onSubmit(); + + handleSubmit(validatedValues) { + // Do not execute challenge, if errors + if (validatedValues.errors.length === 0) { + if (validatedValues.invalidValues.length === 0) { + // updates values on server + this.props.updateSolutionForm(validatedValues.values); + this.props.onSubmit({ isShouldCompletionModalOpen: true }); + } else { + this.props.onSubmit({ isShouldCompletionModalOpen: false }); + } + } } + render() { const { isSubmitting, challengeType, description, t } = this.props; diff --git a/client/src/templates/Challenges/projects/backend/Show.js b/client/src/templates/Challenges/projects/backend/Show.js index b65f5f70f6..d839d7fc18 100644 --- a/client/src/templates/Challenges/projects/backend/Show.js +++ b/client/src/templates/Challenges/projects/backend/Show.js @@ -87,6 +87,7 @@ export class BackEnd extends Component { super(props); this.state = {}; this.updateDimensions = this.updateDimensions.bind(this); + this.handleSubmit = this.handleSubmit.bind(this); } componentDidMount() { @@ -152,6 +153,10 @@ export class BackEnd extends Component { challengeMounted(challengeMeta.id); } + handleSubmit({ isShouldCompletionModalOpen }) { + this.props.executeChallenge(isShouldCompletionModalOpen); + } + render() { const { data: { @@ -172,7 +177,6 @@ export class BackEnd extends Component { }, t, tests, - executeChallenge, updateSolutionFormValues } = this.props; @@ -205,7 +209,7 @@ export class BackEnd extends Component { /> { - const challengeComplete = tests.every(test => test.pass && !test.err); - return challengeComplete ? of(openModal('completion')) : empty(); - }) - ); -} - -export default challengeModalEpic; diff --git a/client/src/templates/Challenges/redux/execute-challenge-saga.js b/client/src/templates/Challenges/redux/execute-challenge-saga.js index 0566887746..622fbc56b9 100644 --- a/client/src/templates/Challenges/redux/execute-challenge-saga.js +++ b/client/src/templates/Challenges/redux/execute-challenge-saga.js @@ -24,6 +24,7 @@ import { updateLogs, logsToConsole, updateTests, + openModal, isBuildEnabledSelector, disableBuildOnError, types @@ -43,11 +44,12 @@ import { const previewTimeout = 2500; let previewTask; -export function* executeCancellableChallengeSaga() { +export function* executeCancellableChallengeSaga(payload) { if (previewTask) { yield cancel(previewTask); } - const task = yield fork(executeChallengeSaga); + // executeChallenge with payload containing isShouldCompletionModalOpen + const task = yield fork(executeChallengeSaga, payload); previewTask = yield fork(previewChallengeSaga, { flushLogs: false }); yield take(types.cancelTests); @@ -58,7 +60,9 @@ export function* executeCancellablePreviewSaga() { previewTask = yield fork(previewChallengeSaga); } -export function* executeChallengeSaga() { +export function* executeChallengeSaga({ + payload: isShouldCompletionModalOpen +}) { const isBuildEnabled = yield select(isBuildEnabledSelector); if (!isBuildEnabled) { return; @@ -93,8 +97,13 @@ export function* executeChallengeSaga() { document ); const testResults = yield executeTests(testRunner, tests); - yield put(updateTests(testResults)); + + const challengeComplete = testResults.every(test => test.pass && !test.err); + if (challengeComplete && isShouldCompletionModalOpen) { + yield put(openModal('completion')); + } + yield put(updateConsole(i18next.t('learn.tests-completed'))); yield put(logsToConsole(i18next.t('learn.console-output'))); } catch (e) { diff --git a/client/src/templates/Challenges/redux/index.js b/client/src/templates/Challenges/redux/index.js index a910fa983b..129f639027 100644 --- a/client/src/templates/Challenges/redux/index.js +++ b/client/src/templates/Challenges/redux/index.js @@ -4,7 +4,6 @@ import { createTypes } from '../../../../utils/stateManagement'; import { createPoly } from '../../../../../utils/polyvinyl'; import { getLines } from '../../../../../utils/get-lines'; -import challengeModalEpic from './challenge-modal-epic'; import completionEpic from './completion-epic'; import codeLockEpic from './code-lock-epic'; import createQuestionEpic from './create-question-epic'; @@ -97,7 +96,6 @@ export const types = createTypes( ); export const epics = [ - challengeModalEpic, codeLockEpic, completionEpic, createQuestionEpic, @@ -195,6 +193,7 @@ export const isCompletionModalOpenSelector = state => export const isHelpModalOpenSelector = state => state[ns].modal.help; export const isVideoModalOpenSelector = state => state[ns].modal.video; export const isResetModalOpenSelector = state => state[ns].modal.reset; + export const isBuildEnabledSelector = state => state[ns].isBuildEnabled; export const successMessageSelector = state => state[ns].successMessage;