fix: incorrect field validation on the grading page

openedx · Mar 26, 2024 · cccc106 · cccc106
1 parent 1dde30a
commit cccc106
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 19 deletions.
diff --git a/src/grading-settings/assignment-section/AssignmentSection.test.jsx b/src/grading-settings/assignment-section/AssignmentSection.test.jsx
@@ -9,7 +9,7 @@ const testObj = {};
 
 const defaultAssignments = {
   type: 'Test type',
-  minCount: 1,
+  minCount: 2,
   dropCount: 1,
   shortLabel: 'TT',
   weight: 100,
@@ -59,6 +59,20 @@ describe('<AssignmentSection />', () => {
     fireEvent.change(assignmentShortLabelInput, { target: { value: '123' } });
     expect(testObj.graders[0].shortLabel).toBe('123');
   });
+  it('checking correct assignmentTypeNameTitle value', () => {
+    const { getByTestId } = render(<RootWrapper setGradingData={setGradingData} />);
+    const assignmentShortLabelInput = getByTestId('assignment-type-name-input');
+    expect(assignmentShortLabelInput.value).toBe('Test type');
+    fireEvent.change(assignmentShortLabelInput, { target: { value: 'New Test Type' } });
+    expect(testObj.graders[0].type).toBe('New Test Type');
+  });
+  it('checking invalid assignmentTypeNameTitle value', () => {
+    const { getByText, getByTestId } = render(<RootWrapper setGradingData={setGradingData} />);
+    const assignmentShortLabelInput = getByTestId('assignment-type-name-input');
+    expect(assignmentShortLabelInput.value).toBe('Test type');
+    fireEvent.change(assignmentShortLabelInput, { target: { value: '   ' } });
+    expect(getByText(messages.assignmentTypeNameErrorMessage1.defaultMessage)).toBeInTheDocument();
+  });
   it('checking correct assignment weight of total grade value', async () => {
     const { getByTestId } = render(<RootWrapper setGradingData={setGradingData} />);
     await waitFor(() => {
@@ -72,7 +86,7 @@ describe('<AssignmentSection />', () => {
     const { getByTestId } = render(<RootWrapper setGradingData={setGradingData} />);
     await waitFor(() => {
       const assignmentTotalNumberInput = getByTestId('assignment-minCount-input');
-      expect(assignmentTotalNumberInput.value).toBe('1');
+      expect(assignmentTotalNumberInput.value).toBe('2');
       fireEvent.change(assignmentTotalNumberInput, { target: { value: '123' } });
       expect(testObj.graders[0].minCount).toBe(123);
     });
@@ -82,26 +96,26 @@ describe('<AssignmentSection />', () => {
     await waitFor(() => {
       const assignmentNumberOfDroppableInput = getByTestId('assignment-dropCount-input');
       expect(assignmentNumberOfDroppableInput.value).toBe('1');
-      fireEvent.change(assignmentNumberOfDroppableInput, { target: { value: '2' } });
-      expect(testObj.graders[0].dropCount).toBe(2);
+      fireEvent.change(assignmentNumberOfDroppableInput, { target: { value: '0' } });
+      expect(testObj.graders[0].dropCount).toBe(0);
     });
   });
-  it('checking correct error msg if dropCount have negative number or empty string', async () => {
+  it('checking correct error msg if dropCount is empty or negative integer', async () => {
     const { getByText, getByTestId } = render(<RootWrapper />);
     await waitFor(() => {
       const assignmentNumberOfDroppableInput = getByTestId('assignment-dropCount-input');
       expect(assignmentNumberOfDroppableInput.value).toBe('1');
-      fireEvent.change(assignmentNumberOfDroppableInput, { target: { value: '-2' } });
-      expect(getByText(messages.numberOfDroppableErrorMessage.defaultMessage)).toBeInTheDocument();
       fireEvent.change(assignmentNumberOfDroppableInput, { target: { value: '' } });
       expect(getByText(messages.numberOfDroppableErrorMessage.defaultMessage)).toBeInTheDocument();
+      fireEvent.change(assignmentNumberOfDroppableInput, { target: { value: '-5' } });
+      expect(getByText(messages.numberOfDroppableErrorMessage.defaultMessage)).toBeInTheDocument();
     });
   });
   it('checking correct error msg if minCount have negative number or empty string', async () => {
     const { getByText, getByTestId } = render(<RootWrapper />);
     await waitFor(() => {
       const assignmentMinCountInput = getByTestId('assignment-minCount-input');
-      expect(assignmentMinCountInput.value).toBe('1');
+      expect(assignmentMinCountInput.value).toBe('2');
       fireEvent.change(assignmentMinCountInput, { target: { value: '-2' } });
       expect(getByText(messages.totalNumberErrorMessage.defaultMessage)).toBeInTheDocument();
       fireEvent.change(assignmentMinCountInput, { target: { value: '' } });

diff --git a/src/grading-settings/assignment-section/assignments/AssignmentItem.jsx b/src/grading-settings/assignment-section/assignments/AssignmentItem.jsx
@@ -42,14 +42,15 @@ const AssignmentItem = ({
         {descriptions}
       </Form.Control.Feedback>
       {errorEffort && (
-        <Form.Control.Feedback className="feedback-error" type="invalid">
-          {errorMsg}
-        </Form.Control.Feedback>
-      )}
-      {gradeField?.dropCount !== 0 && gradeField?.dropCount > gradeField?.minCount && (
-        <Form.Control.Feedback className="feedback-error" type="invalid">
-          {secondErrorMsg}
-        </Form.Control.Feedback>
+        gradeField?.dropCount ? (
+          <Form.Control.Feedback className="feedback-error" type="invalid">
+            {gradeField?.dropCount !== 0 && gradeField?.dropCount >= gradeField?.minCount ? secondErrorMsg : errorMsg}
+          </Form.Control.Feedback>
+        ) : (
+          <Form.Control.Feedback className="feedback-error" type="invalid">
+            {errorMsg}
+          </Form.Control.Feedback>
+        )
       )}
     </Form.Group>
   </li>

diff --git a/src/grading-settings/assignment-section/utils/validation.js b/src/grading-settings/assignment-section/utils/validation.js
@@ -51,10 +51,11 @@ export const validationAssignmentFields = (
   assignmentDropCount,
 ) => {
   const courseGradingTypes = courseGraders?.map(grade => grade.type);
+  const minCountValue = courseGraders?.find(grade => grade.id === assignmentId).minCount;
 
   switch (assignmentName) {
   case assignmentType:
-    if (assignmentValue === '') {
+    if (assignmentValue.trim() === '') {
       updateAssignmentErrorList(assignmentName, assignmentId, setErrorList, setShowSavePrompt);
       return;
     }
@@ -77,7 +78,7 @@ export const validationAssignmentFields = (
     );
     break;
   case weightOfTotalGrade:
-    if (assignmentValue < 0 || assignmentValue > 100 || assignmentValue === '-0') {
+    if (assignmentValue === '' || assignmentValue < 0 || assignmentValue > 100 || assignmentValue === '-0') {
       updateAssignmentErrorList(
         assignmentName,
         assignmentId,
@@ -113,7 +114,12 @@ export const validationAssignmentFields = (
     );
     break;
   case assignmentDropCount:
-    if (assignmentValue < 0 || assignmentValue === '' || assignmentValue === '-0') {
+    if (
+      assignmentValue >= minCountValue
+      || assignmentValue < 0
+      || assignmentValue === ''
+      || assignmentValue === '-0'
+    ) {
       updateAssignmentErrorList(
         assignmentName,
         assignmentId,