feat: adding form validation to LtiConfigForm and LegacyConfigForm (#65)

* feat: adding form validation to LtiConfigForm and LegacyConfigForm

The three fields in LtiConfigForm each get their validation/feedback hooked up properly, and the blackout dates field in LegacyConfigForm now uses a regex-based validator for blackout date strings.

* doc: Improving comments for the blackout dates regex.

* fix: allow no blackout dates

src/pages-and-resources/discussions/apps/legacy/LegacyConfigForm.jsx

@@ -2,21 +2,32 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import { Form } from '@edx/paragon';
 import { useFormik } from 'formik';
+import * as Yup from 'yup';
+import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
 
 import DivisionByGroupFields from '../shared/DivisionByGroupFields';
 import AnonymousPostingFields from '../shared/AnonymousPostingFields';
-import BlackoutDatesField from '../shared/BlackoutDatesField';
+import BlackoutDatesField, { blackoutDatesRegex } from '../shared/BlackoutDatesField';
 
-export default function LegacyConfigForm({
+import messages from '../shared/messages';
-  appConfig, onSubmit, formRef,
+
+function LegacyConfigForm({
+  appConfig, onSubmit, formRef, intl,
 }) {
   const {
     handleSubmit,
     handleChange,
     handleBlur,
     values,
+    errors,
   } = useFormik({
     initialValues: appConfig,
+    validationSchema: Yup.object().shape({
+      blackoutDates: Yup.string().matches(
+        blackoutDatesRegex,
+        intl.formatMessage(messages.blackoutDatesFormattingError),
+      ),
+    }),
     onSubmit,
   });
 
@@ -37,6 +48,7 @@ export default function LegacyConfigForm({
         values={values}
       />
       <BlackoutDatesField
+        errors={errors}
         onBlur={handleBlur}
         onChange={handleChange}
         values={values}
@@ -59,4 +71,7 @@ LegacyConfigForm.propTypes = {
   onSubmit: PropTypes.func.isRequired,
   // eslint-disable-next-line react/forbid-prop-types
   formRef: PropTypes.object.isRequired,
+  intl: intlShape.isRequired,
 };
+
+export default injectIntl(LegacyConfigForm);

src/pages-and-resources/discussions/apps/lti/LtiConfigForm.jsx

@@ -47,44 +47,44 @@ function LtiConfigForm({
           }}
         />
       </p>
-      <Form.Group controlId="consumerKey">
+      <Form.Group controlId="consumerKey" isInvalid={errors.consumerKey}>
         <Form.Label>{intl.formatMessage(messages.consumerKey)}</Form.Label>
         <Form.Control
           onChange={handleChange}
           onBlur={handleBlur}
           value={values.consumerKey}
-          className={{ 'is-invalid': !!errors.consumerKey }}
-          aria-describedby="consumerKeyFeedback"
         />
-        <Form.Control.Feedback id="consumerKeyFeedback" type="invalid">
+        {errors.consumerKey && (
-          {intl.formatMessage(messages.consumerKeyRequired)}
+          <Form.Control.Feedback type="invalid">
-        </Form.Control.Feedback>
+            {errors.consumerKey}
+          </Form.Control.Feedback>
+        )}
       </Form.Group>
-      <Form.Group controlId="consumerSecret">
+      <Form.Group controlId="consumerSecret" isInvalid={errors.consumerSecret}>
         <Form.Label>{intl.formatMessage(messages.consumerSecret)}</Form.Label>
         <Form.Control
           onChange={handleChange}
           onBlur={handleBlur}
           value={values.consumerSecret}
-          className={{ 'is-invalid': !!errors.consumerSecret }}
-          aria-describedby="consumerSecretFeedback"
         />
-        <Form.Control.Feedback id="consumerSecretFeedback" type="invalid">
+        {errors.consumerSecret && (
-          {intl.formatMessage(messages.consumerSecretRequired)}
+          <Form.Control.Feedback type="invalid">
-        </Form.Control.Feedback>
+            {errors.consumerSecret}
+          </Form.Control.Feedback>
+        )}
       </Form.Group>
-      <Form.Group controlId="launchUrl">
+      <Form.Group controlId="launchUrl" isInvalid={errors.launchUrl}>
         <Form.Label>{intl.formatMessage(messages.launchUrl)}</Form.Label>
         <Form.Control
           onChange={handleChange}
           onBlur={handleBlur}
           value={values.launchUrl}
-          className={{ 'is-invalid': !!errors.launchUrl }}
-          aria-describedby="launchUrlFeedback"
         />
-        <Form.Control.Feedback id="launchUrlFeedback" type="invalid">
+        {errors.launchUrl && (
-          {intl.formatMessage(messages.launchUrlRequired)}
+          <Form.Control.Feedback type="invalid">
-        </Form.Control.Feedback>
+            {errors.launchUrl}
+          </Form.Control.Feedback>
+        )}
       </Form.Group>
     </Form>
   );

src/pages-and-resources/discussions/apps/shared/BlackoutDatesField.jsx

@@ -4,11 +4,76 @@ import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
 import { Form } from '@edx/paragon';
 import messages from './messages';
 
+/**
+ * Lets break this regex down.
+ *
+ * The goal is to accept arrays of dates like the following:
+ *
+ * [["2015-09-15", "2015-09-21"], ["2015-10-01T11:45", "2015-10-08"]]
+ *
+ * Any date can be YYYY-MM-DDTHH:MM or just YYYY-MM-DD like the above.
+ * The hours and minutes are optional, as illustrated.
+ *
+ * The regex errs on the side of being too loose, so you'll see things that are not perfect.  It's
+ * better to be too liberal than to accidentally reject something that should be fine.
+ *
+ * So let multi-line this regex and explain the parts:
+ *
+ * Beginning of the string:
+ *     ^
+ * The outer square brackets:
+ *     \[
+ * Start of a group for a pair of dates with their square brackets:
+ *     (\[
+ * A group for the first date (YYYY-MM-DDTHH:MM) with its opening double quote:
+ *     ("
+ * Any four digits for the YYYY year, and a dash:
+ *     [0-9]{4}-
+ * MM Months 00 - 12 with either a 0 followed by a digit, 1-9, OR a 1 followed by a digit 0-2
+ * Then a dash:
+ *     (0[1-9]|1[0-2])-
+ * Finally, for days, accepts any digit 0-3 followed by any digit 0-9.  Not a very exact regex.
+ *     [0-3][0-9]
+ * A sub-group for the hours and minutes.  T is just part of it:
+ *     (T
+ * The hours HH are a 0 or 1 followed by 0-9, OR a 2 followed by 0-3.  Captures digits 00-23:
+ *     ([0-1][0-9]|2[0-3])
+ * Then a colon!
+ *     :
+ * The minutes MM are any digit 0-5 followed by any digit 0-9, capturing 00-59:
+ *     ([0-5][0-9])
+ * The THH:MM is optional, so end the group by allowing it to repeat 0 or 1 times
+ *     ){0,1}
+ * Now end the first date group with its closing double quote, the group parenthesis, and a comma.
+ * The comma is a necessary separator between the first and second dates:
+ *     "),
+ * Now start the second date of the pair:
+ *     ("
+ * The second date is identical to the first, so here it is in all its glory:
+ *     [0-9]{4}-(0[1-9]|1[0-2])-[0-3][0-9] // YYYY-MM-DD, identical to above
+ *     (T([0-1][0-9]|2[0-3]):([0-5][0-9])){0,1}  // THH:MM, identical to above
+ * Close out the second date with its closing double quotes:
+ *     ")
+ * Close out the pair of dates with its closing square bracket:
+ *     \]
+ * An optional comma after the pair of dates, in case there's another pair.  If there isn't another
+ * date, there shouldn't be another comma, but this regex errors on the side of looseness.
+ *     (,){0,1}
+ * This entire group, ["YYYY-MM-DDTHH:MM", "YYYY-MM-DDTHH:MM"], can be repeated zero or more times.
+ *     )*
+ * Close out the last square bracket around all the groups:
+ *    \]
+ * End of string:
+ *    $
+ */
+export const blackoutDatesRegex = /^\[(\[("[0-9]{4}-(0[1-9]|1[0-2])-[0-3][0-9](T([0-1][0-9]|2[0-3]):([0-5][0-9])){0,1}"),("[0-9]{4}-(0[1-9]|1[0-2])-[0-3][0-9](T([0-1][0-9]|2[0-3]):([0-5][0-9])){0,1}")\](,){0,1})*\]$/;
+
 function BlackoutDatesField({
   onBlur,
   onChange,
   intl,
   values,
+  errors,
 }) {
   return (
     <>
@@ -22,6 +87,11 @@ function BlackoutDatesField({
           onBlur={onBlur}
           floatingLabel={intl.formatMessage(messages.blackoutDatesLabel)}
         />
+        {errors.blackoutDates && (
+          <Form.Control.Feedback type="invalid">
+            {errors.blackoutDates}
+          </Form.Control.Feedback>
+        )}
         <Form.Text muted>
           {intl.formatMessage(messages.blackoutDatesHelp)}
         </Form.Text>
@@ -37,6 +107,9 @@ BlackoutDatesField.propTypes = {
   values: PropTypes.shape({
     blackoutDates: PropTypes.string,
   }).isRequired,
+  errors: PropTypes.shape({
+    blackoutDates: PropTypes.string,
+  }).isRequired,
 };
 
 export default injectIntl(BlackoutDatesField);

src/pages-and-resources/discussions/apps/shared/messages.js

@@ -111,6 +111,10 @@ const messages = defineMessages({
       the date and time. For example, an entry defining two blackout periods looks like this, including
       the outer pair of square brackets: [["2015-09-15", "2015-09-21"], ["2015-10-01", "2015-10-08"]]`,
   },
+  blackoutDatesFormattingError: {
+    id: 'authoring.discussions.builtIn.blackoutDates.formattingError',
+    defaultMessage: "There's a formatting error in your blackout dates.",
+  },
 });
 
 export default messages;