feat: fixed assignment and course grade fields validations

openedx · Oct 24, 2023 · 0b278b7 · 0b278b7
1 parent ba9bd46
commit 0b278b7
Show file tree

Hide file tree

Showing 27 changed files with 301 additions and 51 deletions.
diff --git a/src/components/GradebookFilters/AssignmentGradeFilter/__snapshots__/index.test.jsx.snap b/src/components/GradebookFilters/AssignmentGradeFilter/__snapshots__/index.test.jsx.snap
@@ -55,7 +55,7 @@ exports[`AssignmentFilter component render without selected assignment snapshot
     className="grade-filter-action"
   >
     <Button
-      disabled={true}
+      disabled={false}
       name="assignmentGradeMinMax"
       type="submit"
       variant="outline-secondary"

diff --git a/src/components/GradebookFilters/AssignmentGradeFilter/hooks.js b/src/components/GradebookFilters/AssignmentGradeFilter/hooks.js
@@ -7,6 +7,7 @@ const useAssignmentGradeFilterData = ({ updateQueryParams }) => {
   const fetchGrades = thunkActions.grades.useFetchGrades();
   const setFilter = actions.app.useSetLocalFilter();
   const updateAssignmentLimits = actions.filters.useUpdateAssignmentLimits();
+  const isDisabled = !selectors.app.useAreAssignmentGradeFiltersValid() || !selectedAssignment;
 
   const handleSubmit = () => {
     updateAssignmentLimits(localAssignmentLimits);
@@ -27,6 +28,7 @@ const useAssignmentGradeFilterData = ({ updateQueryParams }) => {
     assignmentGradeMin,
     assignmentGradeMax,
     selectedAssignment,
+    isDisabled,
     handleSetMax,
     handleSetMin,
     handleSubmit,

diff --git a/src/components/GradebookFilters/AssignmentGradeFilter/hooks.test.js b/src/components/GradebookFilters/AssignmentGradeFilter/hooks.test.js
@@ -4,7 +4,10 @@ import useAssignmentGradeFilterData from './hooks';
 
 jest.mock('data/redux/hooks', () => ({
   selectors: {
-    app: { useAssignmentGradeLimits: jest.fn() },
+    app: {
+      useAreAssignmentGradeFiltersValid: jest.fn(),
+      useAssignmentGradeLimits: jest.fn(),
+    },
     filters: { useSelectedAssignmentLabel: jest.fn() },
   },
   actions: {
@@ -20,6 +23,7 @@ let out;
 
 const assignmentGradeLimits = { assignmentGradeMax: 200, assignmentGradeMin: 3 };
 const selectedAssignmentLabel = 'test-assignment-label';
+const useAreAssignmentGradeFiltersValid = false;
 selectors.app.useAssignmentGradeLimits.mockReturnValue(assignmentGradeLimits);
 selectors.filters.useSelectedAssignmentLabel.mockReturnValue(selectedAssignmentLabel);
 
@@ -40,6 +44,7 @@ describe('useAssignmentFilterData hook', () => {
   });
   describe('behavior', () => {
     it('initializes redux hooks', () => {
+      expect(selectors.app.useAreAssignmentGradeFiltersValid).toHaveBeenCalledWith();
       expect(selectors.app.useAssignmentGradeLimits).toHaveBeenCalledWith();
       expect(selectors.filters.useSelectedAssignmentLabel).toHaveBeenCalledWith();
       expect(actions.app.useSetLocalFilter).toHaveBeenCalledWith();
@@ -77,5 +82,8 @@ describe('useAssignmentFilterData hook', () => {
       expect(out.assignmentGradeMax).toEqual(assignmentGradeLimits.assignmentGradeMax);
       expect(out.assignmentGradeMin).toEqual(assignmentGradeLimits.assignmentGradeMin);
     });
+    it('passes isDisabled from hook', () => {
+      expect(out.isDisabled).toEqual(!useAreAssignmentGradeFiltersValid || !selectedAssignmentLabel);
+    });
   });
 });
diff --git a/src/components/GradebookFilters/AssignmentGradeFilter/index.jsx b/src/components/GradebookFilters/AssignmentGradeFilter/index.jsx
@@ -13,6 +13,7 @@ export const AssignmentGradeFilter = ({ updateQueryParams }) => {
     assignmentGradeMin,
     assignmentGradeMax,
     selectedAssignment,
+    isDisabled,
     handleSetMax,
     handleSetMin,
     handleSubmit,
@@ -39,7 +40,7 @@ export const AssignmentGradeFilter = ({ updateQueryParams }) => {
           type="submit"
           variant="outline-secondary"
           name="assignmentGradeMinMax"
-          disabled={!selectedAssignment}
+          disabled={isDisabled}
           onClick={handleSubmit}
         >
           {formatMessage(messages.apply)}

diff --git a/src/components/GradebookFilters/AssignmentGradeFilter/index.test.jsx b/src/components/GradebookFilters/AssignmentGradeFilter/index.test.jsx
@@ -17,6 +17,7 @@ const hookData = {
   selectedAssignment: 'test-assignment',
   assignmentGradeMax: 300,
   assignmentGradeMin: 23,
+  isDisabled: false,
 };
 useAssignmentGradeFilterData.mockReturnValue(hookData);
 

diff --git a/src/components/GradesView/StatusAlerts/__snapshots__/index.test.jsx.snap b/src/components/GradesView/StatusAlerts/__snapshots__/index.test.jsx.snap
@@ -16,5 +16,12 @@ exports[`StatusAlerts component render snapshot 1`] = `
   >
     hooks.grade-filter-text
   </Alert>
+  <Alert
+    dismissible={false}
+    show="hooks.show-grade-filter"
+    variant="danger"
+  >
+    hooks.grade-filter-text
+  </Alert>
 </Fragment>
 `;
diff --git a/src/components/GradesView/StatusAlerts/hooks.js b/src/components/GradesView/StatusAlerts/hooks.js
@@ -6,15 +6,27 @@ import messages from './messages';
 export const useStatusAlertsData = () => {
   const { formatMessage } = useIntl();
 
-  const limitValidity = selectors.app.useCourseGradeFilterValidity();
+  const courseLimitValidity = selectors.app.useCourseGradeFilterValidity();
+  const assignmentLimitValidity = selectors.app.useAssignmentGradeFilterValidity();
+
   const showSuccessBanner = selectors.grades.useShowSuccess();
   const handleCloseSuccessBanner = actions.grades.useCloseBanner();
 
-  const isCourseGradeFilterAlertOpen = !limitValidity.isMinValid || !limitValidity.isMaxValid;
+  const isCourseGradeFilterAlertOpen = !courseLimitValidity.isMinValid
+    || !courseLimitValidity.isMaxValid || !courseLimitValidity.isMinLessMaxValid;
+  const isAssignmentGradeFilterAlertOpen = !assignmentLimitValidity.isMinValid
+    || !assignmentLimitValidity.isMaxValid || !assignmentLimitValidity.isMinLessMaxValid;
+
+  const courseValidityMessages = {
+    min: courseLimitValidity.isMinValid ? '' : formatMessage(messages.minGradeInvalid),
+    max: courseLimitValidity.isMaxValid ? '' : formatMessage(messages.maxGradeInvalid),
+    minLessMax: courseLimitValidity.isMinLessMaxValid ? '' : formatMessage(messages.minLessMaxGradeInvalid),
+  };
 
-  const validityMessages = {
-    min: limitValidity.isMinValid ? '' : formatMessage(messages.minGradeInvalid),
-    max: limitValidity.isMaxValid ? '' : formatMessage(messages.maxGradeInvalid),
+  const assignmentValidityMessages = {
+    min: assignmentLimitValidity.isMinValid ? '' : formatMessage(messages.minAssignmentInvalid),
+    max: assignmentLimitValidity.isMaxValid ? '' : formatMessage(messages.maxAssignmentInvalid),
+    minLessMax: assignmentLimitValidity.isMinLessMaxValid ? '' : formatMessage(messages.minLessMaxAssignmentInvalid),
   };
 
   return {
@@ -23,9 +35,13 @@ export const useStatusAlertsData = () => {
       show: showSuccessBanner,
       text: formatMessage(messages.editSuccessAlert),
     },
-    gradeFilter: {
+    courseGradeFilter: {
       show: isCourseGradeFilterAlertOpen,
-      text: `${validityMessages.min}${validityMessages.max}`,
+      text: `${courseValidityMessages.min} ${courseValidityMessages.max} ${courseValidityMessages.minLessMax}`,
+    },
+    assignmentGradeFilter: {
+      show: isAssignmentGradeFilterAlertOpen,
+      text: `${assignmentValidityMessages.min} ${assignmentValidityMessages.max} ${assignmentValidityMessages.minLessMax}`,
     },
   };
 };

diff --git a/src/components/GradesView/StatusAlerts/hooks.test.js b/src/components/GradesView/StatusAlerts/hooks.test.js
@@ -11,16 +11,21 @@ jest.mock('data/redux/hooks', () => ({
     grades: { useCloseBanner: jest.fn() },
   },
   selectors: {
-    app: { useCourseGradeFilterValidity: jest.fn() },
+    app: {
+      useCourseGradeFilterValidity: jest.fn(),
+      useAssignmentGradeFilterValidity: jest.fn(),
+    },
     grades: { useShowSuccess: jest.fn() },
   },
 }));
 
 const validity = {
   isMinValid: true,
   isMaxValid: true,
+  isMinLessMaxValid: true,
 };
 selectors.app.useCourseGradeFilterValidity.mockReturnValue(validity);
+selectors.app.useAssignmentGradeFilterValidity.mockReturnValue(validity);
 const showSuccess = 'test-show-success';
 selectors.grades.useShowSuccess.mockReturnValue(showSuccess);
 const closeBanner = jest.fn().mockName('hooks.closeBanner');
@@ -39,6 +44,7 @@ describe('useStatusAlertsData', () => {
     it('initializes redux hooks', () => {
       expect(actions.grades.useCloseBanner).toHaveBeenCalled();
       expect(selectors.app.useCourseGradeFilterValidity).toHaveBeenCalled();
+      expect(selectors.app.useAssignmentGradeFilterValidity).toHaveBeenCalled();
       expect(selectors.grades.useShowSuccess).toHaveBeenCalled();
     });
   });
@@ -53,55 +59,105 @@ describe('useStatusAlertsData', () => {
       });
     });
     describe('gradeFilter', () => {
-      describe('both filters are valid', () => {
+      describe('all filters are valid', () => {
         test('do not show', () => {
-          expect(out.gradeFilter.show).toEqual(false);
+          expect(out.courseGradeFilter.show).toEqual(false);
+          expect(out.assignmentGradeFilter.show).toEqual(false);
         });
       });
       describe('min filter is invalid', () => {
         beforeEach(() => {
           selectors.app.useCourseGradeFilterValidity.mockReturnValue({
             isMinValid: false,
             isMaxValid: true,
+            isMinLessMaxValid: true,
+          });
+          selectors.app.useAssignmentGradeFilterValidity.mockReturnValue({
+            isMinValid: false,
+            isMaxValid: true,
+            isMinLessMaxValid: true,
           });
           out = useStatusAlertsData();
         });
         test('show grade filter banner', () => {
-          expect(out.gradeFilter.show).toEqual(true);
+          expect(out.courseGradeFilter.show).toEqual(true);
+          expect(out.assignmentGradeFilter.show).toEqual(true);
         });
         test('filter message', () => {
-          expect(out.gradeFilter.text).toEqual(formatMessage(messages.minGradeInvalid));
+          expect(out.courseGradeFilter.text).toEqual(`${formatMessage(messages.minGradeInvalid)}  `);
+          expect(out.assignmentGradeFilter.text).toEqual(`${formatMessage(messages.minAssignmentInvalid)}  `);
         });
       });
       describe('max filter is invalid', () => {
         beforeEach(() => {
           selectors.app.useCourseGradeFilterValidity.mockReturnValue({
             isMinValid: true,
             isMaxValid: false,
+            isMinLessMaxValid: true,
+          });
+          selectors.app.useAssignmentGradeFilterValidity.mockReturnValue({
+            isMinValid: true,
+            isMaxValid: false,
+            isMinLessMaxValid: true,
           });
           out = useStatusAlertsData();
         });
         test('show grade filter banner', () => {
-          expect(out.gradeFilter.show).toEqual(true);
+          expect(out.courseGradeFilter.show).toEqual(true);
+          expect(out.assignmentGradeFilter.show).toEqual(true);
         });
         test('filter message', () => {
-          expect(out.gradeFilter.text).toEqual(formatMessage(messages.maxGradeInvalid));
+          expect(out.courseGradeFilter.text).toEqual(` ${formatMessage(messages.maxGradeInvalid)} `);
+          expect(out.assignmentGradeFilter.text).toEqual(` ${formatMessage(messages.maxAssignmentInvalid)} `);
         });
       });
-      describe('both filters are invalid', () => {
+      describe('minLessMax filter is invalid', () => {
         beforeEach(() => {
           selectors.app.useCourseGradeFilterValidity.mockReturnValue({
+            isMinValid: true,
+            isMaxValid: true,
+            isMinLessMaxValid: false,
+          });
+          selectors.app.useAssignmentGradeFilterValidity.mockReturnValue({
+            isMinValid: true,
+            isMaxValid: true,
+            isMinLessMaxValid: false,
+          });
+          out = useStatusAlertsData();
+        });
+        test('show grade filter banner', () => {
+          expect(out.courseGradeFilter.show).toEqual(true);
+          expect(out.assignmentGradeFilter.show).toEqual(true);
+        });
+        test('filter message', () => {
+          expect(out.courseGradeFilter.text).toEqual(`  ${formatMessage(messages.minLessMaxGradeInvalid)}`);
+          expect(out.assignmentGradeFilter.text).toEqual(`  ${formatMessage(messages.minLessMaxAssignmentInvalid)}`);
+        });
+      });
+      describe('all filters are invalid', () => {
+        beforeEach(() => {
+          selectors.app.useCourseGradeFilterValidity.mockReturnValue({
+            isMinValid: false,
+            isMaxValid: false,
+            isMinLessMaxValid: false,
+          });
+          selectors.app.useAssignmentGradeFilterValidity.mockReturnValue({
             isMinValid: false,
             isMaxValid: false,
+            isMinLessMaxValid: false,
           });
           out = useStatusAlertsData();
         });
         test('show grade filter banner', () => {
-          expect(out.gradeFilter.show).toEqual(true);
+          expect(out.courseGradeFilter.show).toEqual(true);
+          expect(out.assignmentGradeFilter.show).toEqual(true);
         });
         test('filter message', () => {
-          expect(out.gradeFilter.text).toEqual(
-            `${formatMessage(messages.minGradeInvalid)}${formatMessage(messages.maxGradeInvalid)}`,
+          expect(out.courseGradeFilter.text).toEqual(
+            `${formatMessage(messages.minGradeInvalid)} ${formatMessage(messages.maxGradeInvalid)} ${formatMessage(messages.minLessMaxGradeInvalid)}`,
+          );
+          expect(out.assignmentGradeFilter.text).toEqual(
+            `${formatMessage(messages.minAssignmentInvalid)} ${formatMessage(messages.maxAssignmentInvalid)} ${formatMessage(messages.minLessMaxAssignmentInvalid)}`,
           );
         });
       });

diff --git a/src/components/GradesView/StatusAlerts/index.jsx b/src/components/GradesView/StatusAlerts/index.jsx
@@ -7,7 +7,8 @@ import useStatusAlertsData from './hooks';
 export const StatusAlerts = () => {
   const {
     successBanner,
-    gradeFilter,
+    courseGradeFilter,
+    assignmentGradeFilter,
   } = useStatusAlertsData();
 
   return (
@@ -22,9 +23,16 @@ export const StatusAlerts = () => {
       <Alert
         variant="danger"
         dismissible={false}
-        show={gradeFilter.show}
+        show={assignmentGradeFilter.show}
       >
-        {gradeFilter.text}
+        {assignmentGradeFilter.text}
+      </Alert>
+      <Alert
+        variant="danger"
+        dismissible={false}
+        show={courseGradeFilter.show}
+      >
+        {courseGradeFilter.text}
       </Alert>
     </>
   );

diff --git a/src/components/GradesView/StatusAlerts/index.test.jsx b/src/components/GradesView/StatusAlerts/index.test.jsx
@@ -14,7 +14,11 @@ const hookProps = {
     show: 'hooks.show-success-banner',
     text: 'hooks.success-banner-text',
   },
-  gradeFilter: {
+  courseGradeFilter: {
+    show: 'hooks.show-grade-filter',
+    text: 'hooks.grade-filter-text',
+  },
+  assignmentGradeFilter: {
     show: 'hooks.show-grade-filter',
     text: 'hooks.grade-filter-text',
   },
@@ -43,11 +47,13 @@ describe('StatusAlerts component', () => {
       expect(props.show).toEqual(hookProps.successBanner.show);
       expect(alert.text()).toEqual(hookProps.successBanner.text);
     });
-    test('grade filter banner', () => {
+    test('course and assignment filter banner', () => {
       const alert = el.find(Alert).at(1);
       const props = alert.props();
-      expect(props.show).toEqual(hookProps.gradeFilter.show);
-      expect(alert.text()).toEqual(hookProps.gradeFilter.text);
+      expect(props.show).toEqual(hookProps.courseGradeFilter.show);
+      expect(alert.text()).toEqual(hookProps.courseGradeFilter.text);
+      expect(props.show).toEqual(hookProps.assignmentGradeFilter.show);
+      expect(alert.text()).toEqual(hookProps.assignmentGradeFilter.text);
     });
   });
 });
diff --git a/src/components/GradesView/StatusAlerts/messages.js b/src/components/GradesView/StatusAlerts/messages.js
@@ -16,6 +16,26 @@ const messages = defineMessages({
     defaultMessage: 'Minimum course grade must be between 0 and 100',
     description: 'An alert text for selecting a minimum course grade less than 0',
   },
+  minLessMaxGradeInvalid: {
+    id: 'gradebook.GradesView.minLessMaxGradeInvalid',
+    defaultMessage: 'Minimum course grade must be less than maximum course grade',
+    description: 'An alert text for selecting a minimum course grade must be less than maximum course grade',
+  },
+  maxAssignmentInvalid: {
+    id: 'gradebook.GradesView.maxAssignmentGradeInvalid',
+    defaultMessage: 'Maximum assignment grade must be between 0 and 100',
+    description: 'An alert text for selecting a maximum assignment grade greater than 100',
+  },
+  minAssignmentInvalid: {
+    id: 'gradebook.GradesView.minAssignmentGradeInvalid',
+    defaultMessage: 'Minimum assignment grade must be between 0 and 100',
+    description: 'An alert text for selecting a minimum assignment grade less than 0',
+  },
+  minLessMaxAssignmentInvalid: {
+    id: 'gradebook.GradesView.minLessMaxAssignmentInvalid',
+    defaultMessage: 'Minimum assignment grade must be less than maximum assignment grade',
+    description: 'An alert text for selecting a minimum assignment grade must be less than maximum assignment grade',
+  },
 });
 
 export default messages;
diff --git a/src/data/redux/hooks/selectors.js b/src/data/redux/hooks/selectors.js
@@ -23,8 +23,10 @@ export const app = StrictDict({
   useActiveView: selectorHook(selectors.app.activeView),
   useAssignmentGradeLimits: selectorHook(selectors.app.assignmentGradeLimits),
   useAreCourseGradeFiltersValid: selectorHook(selectors.app.areCourseGradeFiltersValid),
+  useAreAssignmentGradeFiltersValid: selectorHook(selectors.app.areAssignmentGradeFiltersValid),
   useCourseGradeLimits: selectorHook(selectors.app.courseGradeLimits),
   useCourseGradeFilterValidity: selectorHook(selectors.app.courseGradeFilterValidity),
+  useAssignmentGradeFilterValidity: selectorHook(selectors.app.assignmentGradeFilterValidity),
   useCourseId: selectorHook(selectors.app.courseId),
   useModalData: selectorHook(selectors.app.modalData),
   useSearchValue: selectorHook(selectors.app.searchValue),