feat: validate submission response required fields (#241)

* feat: add required property to both FileResponseConfig and TextResponseConfig types * feat: add a validation hook for required submission fields * feat: validate submission required fields * feat: remove unsupported multiple attribute from dropzone
openedx · Nov 7, 2024 · 80d48d2 · 80d48d2
1 parent 8185c59
commit 80d48d2
Show file tree

Hide file tree

Showing 22 changed files with 390 additions and 120 deletions.
diff --git a/src/components/FileUpload/__snapshots__/index.test.jsx.snap b/src/components/FileUpload/__snapshots__/index.test.jsx.snap
@@ -4,6 +4,7 @@ exports[`<FileUpload /> render default 1`] = `
 <div>
   <h3>
     File upload
+     
   </h3>
   <b>
     Uploaded files
@@ -57,20 +58,23 @@ exports[`<FileUpload /> render default 1`] = `
       ]
     }
   />
-  <Dropzone
-    accept={
-      {
-        "*": [
-          ".pdf",
-          ".jpg",
-        ],
+  <Form.Group
+    isInValid={true}
+  >
+    <Dropzone
+      accept={
+        {
+          "*": [
+            ".pdf",
+            ".jpg",
+          ],
+        }
       }
-    }
-    maxSize={123456}
-    multiple={true}
-    onProcessUpload={[MockFunction onProcessUpload]}
-    progressVariant="bar"
-  />
+      maxSize={123456}
+      onProcessUpload={[MockFunction onProcessUpload]}
+      progressVariant="bar"
+    />
+  </Form.Group>
 </div>
 `;
 
@@ -82,6 +86,7 @@ exports[`<FileUpload /> render extra columns when activeStepName is submission 1
 <div>
   <h3>
     File upload
+     
   </h3>
   <b>
     Uploaded files
@@ -140,27 +145,31 @@ exports[`<FileUpload /> render extra columns when activeStepName is submission 1
       ]
     }
   />
-  <Dropzone
-    accept={
-      {
-        "*": [
-          ".pdf",
-          ".jpg",
-        ],
+  <Form.Group
+    isInValid={true}
+  >
+    <Dropzone
+      accept={
+        {
+          "*": [
+            ".pdf",
+            ".jpg",
+          ],
+        }
       }
-    }
-    maxSize={123456}
-    multiple={true}
-    onProcessUpload={[MockFunction onProcessUpload]}
-    progressVariant="bar"
-  />
+      maxSize={123456}
+      onProcessUpload={[MockFunction onProcessUpload]}
+      progressVariant="bar"
+    />
+  </Form.Group>
 </div>
 `;
 
 exports[`<FileUpload /> render without dropzone and confirm modal when isReadOnly 1`] = `
 <div>
   <h3>
     File upload
+     
   </h3>
   <FilePreview
     defaultCollapsePreview={false}
@@ -224,6 +233,7 @@ exports[`<FileUpload /> render without file preview if uploadedFiles are empty a
 <div>
   <h3>
     File upload
+     
   </h3>
   <b>
     Uploaded files
@@ -312,20 +322,23 @@ exports[`<FileUpload /> render without header 1`] = `
       ]
     }
   />
-  <Dropzone
-    accept={
-      {
-        "*": [
-          ".pdf",
-          ".jpg",
-        ],
+  <Form.Group
+    isInValid={true}
+  >
+    <Dropzone
+      accept={
+        {
+          "*": [
+            ".pdf",
+            ".jpg",
+          ],
+        }
       }
-    }
-    maxSize={123456}
-    multiple={true}
-    onProcessUpload={[MockFunction onProcessUpload]}
-    progressVariant="bar"
-  />
+      maxSize={123456}
+      onProcessUpload={[MockFunction onProcessUpload]}
+      progressVariant="bar"
+    />
+  </Form.Group>
 </div>
 `;
 

diff --git a/src/components/FileUpload/index.jsx b/src/components/FileUpload/index.jsx
@@ -2,7 +2,7 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import filesize from 'filesize';
 
-import { DataTable, Dropzone } from '@openedx/paragon';
+import { DataTable, Dropzone, Form } from '@openedx/paragon';
 import { useIntl } from '@edx/frontend-platform/i18n';
 
 import { nullMethod } from 'utils';
@@ -35,6 +35,7 @@ const FileUpload = ({
   onDeletedFile,
   defaultCollapsePreview,
   hideHeader,
+  isInValid,
 }) => {
   const { formatMessage } = useIntl();
   const {
@@ -47,7 +48,7 @@ const FileUpload = ({
   const viewStep = useViewStep();
   const activeStepName = useActiveStepName();
   const {
-    enabled, fileUploadLimit, allowedExtensions, maxFileSize,
+    enabled, fileUploadLimit, allowedExtensions, maxFileSize, required,
   } = useFileUploadConfig() || {};
 
   if (!enabled || viewStep === stepNames.studentTraining) {
@@ -78,7 +79,7 @@ const FileUpload = ({
 
   return (
     <div>
-      {!hideHeader && <h3>{formatMessage(messages.fileUploadTitle)}</h3>}
+      {!hideHeader && <h3>{formatMessage(messages.fileUploadTitle)} {required && <span>(required)</span>}</h3>}
       {uploadedFiles.length > 0 && isReadOnly && (
         <FilePreview defaultCollapsePreview={defaultCollapsePreview} />
       )}
@@ -93,15 +94,17 @@ const FileUpload = ({
         columns={columns}
       />
       {!isReadOnly && fileUploadLimit > uploadedFiles.length && (
-        <Dropzone
-          multiple
-          onProcessUpload={onProcessUpload}
-          progressVariant="bar"
-          accept={{
-            '*': (allowedExtensions || []).map((ext) => `.${ext}`),
-          }}
-          maxSize={maxFileSize}
-        />
+        <Form.Group isInValid>
+          <Dropzone
+            onProcessUpload={onProcessUpload}
+            progressVariant="bar"
+            accept={{
+              '*': (allowedExtensions || []).map((ext) => `.${ext}`),
+            }}
+            maxSize={maxFileSize}
+          />
+          {isInValid && <Form.Control.Feedback type="invalid">{formatMessage(messages.required)}</Form.Control.Feedback>}
+        </Form.Group>
       )}
       {!isReadOnly && isModalOpen && (
         <UploadConfirmModal
@@ -122,6 +125,7 @@ FileUpload.defaultProps = {
   onDeletedFile: nullMethod,
   defaultCollapsePreview: false,
   hideHeader: false,
+  isInValid: false,
 };
 FileUpload.propTypes = {
   isReadOnly: PropTypes.bool,
@@ -137,6 +141,7 @@ FileUpload.propTypes = {
   onDeletedFile: PropTypes.func,
   defaultCollapsePreview: PropTypes.bool,
   hideHeader: PropTypes.bool,
+  isInValid: PropTypes.bool,
 };
 
 export default FileUpload;
diff --git a/src/components/FileUpload/messages.js b/src/components/FileUpload/messages.js
@@ -96,6 +96,11 @@ const messages = defineMessages({
     defaultMessage: 'File description',
     description: 'Popover title for file description',
   },
+  required: {
+    id: 'frontend-app-ora.FileUpload.required',
+    defaultMessage: 'File Upload is required',
+    description: 'Indicating file upload is required',
+  },
 });
 
 export default messages;
diff --git a/src/data/services/lms/types/blockInfo.ts b/src/data/services/lms/types/blockInfo.ts
@@ -17,6 +17,7 @@ export interface TextResponseConfig {
   optional: boolean,
   editorType: 'text' | 'tinymce',
   allowLatexPreview: boolean,
+  required: boolean,
 }
 
 export interface FileResponseConfig {
@@ -27,6 +28,7 @@ export interface FileResponseConfig {
   allowedExtensions: string[],
   blockedExtensions: string[],
   fileTypeDescription: string,
+  required: boolean,
 }
 
 export interface SubmissionConfig {

diff --git a/src/hooks/actions/useConfirmAction.js b/src/hooks/actions/useConfirmAction.js
@@ -13,10 +13,14 @@ export const assessmentSteps = [
   stepNames.peer,
 ];
 
-const useConfirmAction = () => {
+const useConfirmAction = (validateBeforeOpen = () => true) => {
   const [isOpen, setIsOpen] = useKeyedState(stateKeys.isOpen, false);
   const close = React.useCallback(() => setIsOpen(false), [setIsOpen]);
-  const open = React.useCallback(() => setIsOpen(true), [setIsOpen]);
+  const open = React.useCallback(() => {
+    if (validateBeforeOpen()) {
+      setIsOpen(true);
+    }
+  }, [setIsOpen, validateBeforeOpen]);
   return React.useCallback((config) => {
     const { description, title } = config;
     const action = config.action.action ? config.action.action : config.action;

diff --git a/src/hooks/actions/useConfirmAction.test.js b/src/hooks/actions/useConfirmAction.test.js
@@ -23,12 +23,14 @@ const nestedActionConfig = {
   action: { action: config.action },
 };
 
+const validateBeforeOpen = jest.fn(() => true);
+
 let out;
 describe('useConfirmAction', () => {
   beforeEach(() => {
     jest.clearAllMocks();
     state.mock();
-    out = useConfirmAction();
+    out = useConfirmAction(validateBeforeOpen);
   });
   afterEach(() => { state.resetVals(); });
   describe('behavior', () => {
@@ -44,7 +46,7 @@ describe('useConfirmAction', () => {
       state.expectSetStateCalledWith(stateKeys.isOpen, false);
     };
     const testOpen = (openFn) => {
-      expect(openFn.useCallback.prereqs).toEqual([state.setState[stateKeys.isOpen]]);
+      expect(openFn.useCallback.prereqs).toEqual([state.setState[stateKeys.isOpen], validateBeforeOpen]);
       openFn.useCallback.cb();
       state.expectSetStateCalledWith(stateKeys.isOpen, true);
     };
@@ -62,13 +64,13 @@ describe('useConfirmAction', () => {
         expect(out.useCallback.prereqs[1]).toEqual(true);
       });
       test('open callback', () => {
-        out = useConfirmAction();
+        out = useConfirmAction(validateBeforeOpen);
         testOpen(prereqs[2]);
       });
     });
     describe('callback', () => {
       it('returns action with labels from config action', () => {
-        out = useConfirmAction().useCallback.cb(config);
+        out = useConfirmAction(validateBeforeOpen).useCallback.cb(config);
         testOpen(out.action.onClick);
         expect(out.action.children).toEqual(config.action.labels.default);
         expect(out.confirmProps.isOpen).toEqual(false);
@@ -78,7 +80,7 @@ describe('useConfirmAction', () => {
         testClose(out.confirmProps.close);
       });
       it('returns nested action from config action', () => {
-        out = useConfirmAction().useCallback.cb(nestedActionConfig);
+        out = useConfirmAction(validateBeforeOpen).useCallback.cb(nestedActionConfig);
         testOpen(out.action.onClick);
         expect(out.action.children).toEqual(nestedActionConfig.action.action.labels.default);
         expect(out.confirmProps.isOpen).toEqual(false);
@@ -89,7 +91,7 @@ describe('useConfirmAction', () => {
       });
       it('returns action with children from config action', () => {
         state.mockVals({ [stateKeys.isOpen]: true });
-        out = useConfirmAction().useCallback.cb(noLabelConfig);
+        out = useConfirmAction(validateBeforeOpen).useCallback.cb(noLabelConfig);
         testOpen(out.action.onClick);
         expect(out.action.children).toEqual(noLabelConfig.action.children);
         expect(out.confirmProps.isOpen).toEqual(true);

diff --git a/src/hooks/actions/useSubmitResponseAction.js b/src/hooks/actions/useSubmitResponseAction.js
@@ -13,8 +13,8 @@ import messages, { confirmDescriptions, confirmTitles } from './messages';
  */
 const useSubmitResponseAction = ({ options }) => {
   const { formatMessage } = useIntl();
-  const { submit, submitStatus } = options;
-  const confirmAction = useConfirmAction();
+  const { submit, submitStatus, validateBeforeConfirmation } = options;
+  const confirmAction = useConfirmAction(validateBeforeConfirmation);
   return confirmAction({
     action: {
       onClick: submit,

diff --git a/src/hooks/actions/useSubmitResponseAction.test.js b/src/hooks/actions/useSubmitResponseAction.test.js
@@ -12,14 +12,15 @@ jest.mock('./useConfirmAction', () => ({
   default: jest.fn(),
 }));
 
-const mockConfirmAction = jest.fn(args => ({ confirmAction: args }));
-when(useConfirmAction).calledWith().mockReturnValue(mockConfirmAction);
-
 const options = {
   submit: jest.fn(),
   submitStatus: 'test-submit-status',
+  validateBeforeConfirmation: jest.fn(),
 };
 
+const mockConfirmAction = jest.fn(args => ({ confirmAction: args }));
+when(useConfirmAction).calledWith(options.validateBeforeConfirmation).mockReturnValue(mockConfirmAction);
+
 let out;
 describe('useSubmitResponseAction', () => {
   beforeEach(() => {
@@ -28,7 +29,7 @@ describe('useSubmitResponseAction', () => {
   describe('behavior', () => {
     it('loads internatioonalization and confirm action from hooks', () => {
       expect(useIntl).toHaveBeenCalledWith();
-      expect(useConfirmAction).toHaveBeenCalledWith();
+      expect(useConfirmAction).toHaveBeenCalledWith(options.validateBeforeConfirmation);
     });
   });
   describe('output confirmAction', () => {