Merge pull request #240 from openedx/kenclary/TNL-10433

fix: more correct parsing of settings, for scoring; use empty string instead of null for empty attempts. TNL-10433.

src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/hooks.js

@@ -125,7 +125,7 @@ export const scoringCardHooks = (scoring, updateSettings) => {
     let unlimitedAttempts = false;
     let attemptNumber = parseInt(event.target.value);
     if (_.isNaN(attemptNumber)) {
-      attemptNumber = null;
+      attemptNumber = '';
       unlimitedAttempts = true;
     } else if (attemptNumber < 0) {
       attemptNumber = 0;

src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/hooks.test.js

@@ -187,19 +187,19 @@ describe('Problem settings hooks', () => {
       const value = null;
       output.handleMaxAttemptChange({ target: { value } });
       expect(updateSettings)
-        .toHaveBeenCalledWith({ scoring: { ...scoring, attempts: { number: null, unlimited: true } } });
+        .toHaveBeenCalledWith({ scoring: { ...scoring, attempts: { number: '', unlimited: true } } });
     });
     test('test handleMaxAttemptChange set attempts to empty string', () => {
       const value = '';
       output.handleMaxAttemptChange({ target: { value } });
       expect(updateSettings)
-        .toHaveBeenCalledWith({ scoring: { ...scoring, attempts: { number: null, unlimited: true } } });
+        .toHaveBeenCalledWith({ scoring: { ...scoring, attempts: { number: '', unlimited: true } } });
     });
     test('test handleMaxAttemptChange set attempts to non-numeric value', () => {
       const value = 'abc';
       output.handleMaxAttemptChange({ target: { value } });
       expect(updateSettings)
-        .toHaveBeenCalledWith({ scoring: { ...scoring, attempts: { number: null, unlimited: true } } });
+        .toHaveBeenCalledWith({ scoring: { ...scoring, attempts: { number: '', unlimited: true } } });
     });
     test('test handleMaxAttemptChange set attempts to negative value', () => {
       const value = -1;

src/editors/containers/ProblemEditor/data/SettingsParser.js

@@ -15,11 +15,15 @@ export const parseScoringSettings = (metadata) => {
   let scoring = {};
 
   let attempts = popuplateItem({}, 'max_attempts', 'number', metadata);
-  if (!_.isEmpty(attempts)) {
-    const unlimited = _.isNaN(attempts.number);
-    attempts = { ...attempts, unlimited };
-    scoring = { ...scoring, attempts };
+  if (_.isEmpty(attempts) || _.isNaN(attempts.number)) {
+    attempts = { unlimited: true, number: '' };
+  } else {
+    attempts.unlimited = false;
+    if (attempts.number < 0) {
+      attempts.number = 0;
+    }
   }
+  scoring = { ...scoring, attempts };
 
   scoring = popuplateItem(scoring, 'weight', 'weight', metadata);
 

src/editors/containers/ProblemEditor/data/SettingsParser.test.js

@@ -4,7 +4,7 @@ import {
   dropdownWithFeedbackHints,
   numericWithHints,
   textInputWithHints,
-  singleSelectWithHints,
+  singleSelectWithHints, negativeAttempts,
 } from './mockData/problemTestData';
 
 describe('Test Settings to State Parser', () => {
@@ -33,13 +33,18 @@ describe('Test Settings to State Parser', () => {
   });
 
   test('Test score settings attempts missing', () => {
-    const scoreSettings = parseScoringSettings(textInputWithHints.metadata);
-    expect(scoreSettings.attempts).toBeUndefined();
+    const scoreSettings = parseScoringSettings(singleSelectWithHints.metadata);
+    expect(scoreSettings.attempts).toStrictEqual(singleSelectWithHints.state.settings.scoring.attempts);
+  });
+
+  test('Test negative attempts in score', () => {
+    const scoreSettings = parseScoringSettings(negativeAttempts.metadata);
+    expect(scoreSettings.attempts).toStrictEqual(negativeAttempts.state.settings.scoring.attempts);
   });
 
   test('Test score settings missing', () => {
     const settings = parseSettings(singleSelectWithHints.metadata);
-    expect(settings.scoring).toBeUndefined();
+    expect(settings.scoring).toStrictEqual(singleSelectWithHints.state.settings.scoring);
   });
 
   test('Test invalid randomization', () => {

src/editors/containers/ProblemEditor/data/mockData/problemTestData.js

@@ -359,10 +359,9 @@ export const singleSelectWithHints = {
         },
       ],
       scoring: {
-        weight: 0,
         attempts: {
           unlimited: true,
-          number: null,
+          number: '',
         },
       },
       timeBetween: 0,
@@ -387,3 +386,38 @@ export const singleSelectWithHints = {
 `,
   },
 };
+
+export const negativeAttempts = {
+  state: {
+    rawOLX: '<problem>\n    <numericalresponse answer="100">\n        <responseparam type="tolerance" default="5"/>\n        <formulaequationinput/>\n    </numericalresponse>\n</problem>\n',
+    problemType: 'TEXTINPUT',
+    question: '',
+    answers: [
+      {
+        id: 'A',
+        title: '100 +-5',
+        correct: true,
+      },
+    ],
+    groupFeedbackList: [],
+    settings: {
+      scoring: {
+        weight: 2.5,
+        attempts: {
+          unlimited: false,
+          number: 0,
+        },
+      },
+    },
+  },
+  metadata: {
+    markdown: `
+=100 +-5 {{You can specify optional feedback like this, which appears after this answer is submitted.}}
+`,
+    weight: 2.5,
+    max_attempts: -1,
+    rerandomize: 'invalid_input',
+    showanswer: 'invalid_input',
+    attempts_before_showanswer_button: 2,
+  },
+};

src/editors/data/redux/problem/reducers.js

@@ -20,7 +20,7 @@ const initialState = {
       weight: 0,
       attempts: {
         unlimited: true,
-        number: null,
+        number: '',
       },
     },
     hints: [],