From e543ccc2e1038dda47feaf4eba767dc6008537e0 Mon Sep 17 00:00:00 2001
From: Kristin Aoki <42981026+KristinAoki@users.noreply.github.com>
Date: Tue, 4 Jun 2024 16:41:58 -0400
Subject: [PATCH] fix: parser not saving unlimited attempts (#483)

* fix: default settings not loading for new problems

* fix: unlimited attempts not saving
---
 .../components/EditProblemView/hooks.test.js  |  3 +-
 .../SelectTypeWrapper/SelectTypeFooter.jsx    | 19 +++++++++--
 .../SelectTypeFooter.test.jsx                 |  3 +-
 .../components/SelectTypeModal/hooks.js       | 22 ++++++++++---
 .../components/SelectTypeModal/hooks.test.js  | 29 +++++++++++++----
 .../ProblemEditor/data/SettingsParser.js      |  4 ++-
 src/editors/data/redux/problem/reducers.js    |  2 +-
 src/editors/utils/index.js                    |  1 +
 src/editors/utils/snakeCaseKeys.js            | 15 +++++++++
 src/editors/utils/snakeCaseKeys.test.js       | 32 +++++++++++++++++++
 10 files changed, 113 insertions(+), 17 deletions(-)
 create mode 100644 src/editors/utils/snakeCaseKeys.js
 create mode 100644 src/editors/utils/snakeCaseKeys.test.js

diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/hooks.test.js b/src/editors/containers/ProblemEditor/components/EditProblemView/hooks.test.js
index 83226be54..fe3a4d7b0 100644
--- a/src/editors/containers/ProblemEditor/components/EditProblemView/hooks.test.js
+++ b/src/editors/containers/ProblemEditor/components/EditProblemView/hooks.test.js
@@ -305,7 +305,7 @@ describe('EditProblemView hooks parseState', () => {
       });
     });
 
-    it('returned parseState content.settings should not include default values', () => {
+    it('returned parseState content.settings should not include default values (not including maxAttempts)', () => {
       const problem = {
         ...problemState,
         problemType: ProblemTypeKeys.NUMERIC,
@@ -326,6 +326,7 @@ describe('EditProblemView hooks parseState', () => {
         openSaveWarningModal,
       });
       expect(settings).toEqual({
+        max_attempts: '',
         attempts_before_showanswer_button: 0,
         submission_wait_seconds: 0,
         weight: 1,
diff --git a/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/SelectTypeFooter.jsx b/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/SelectTypeFooter.jsx
index efb25fa80..677dc4988 100644
--- a/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/SelectTypeFooter.jsx
+++ b/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/SelectTypeFooter.jsx
@@ -11,12 +11,13 @@ import { FormattedMessage, injectIntl, intlShape } from '@edx/frontend-platform/
 import messages from './messages';
 import hooks from '../hooks';
 
-import { actions } from '../../../../../data/redux';
+import { actions, selectors } from '../../../../../data/redux';
 
 export const SelectTypeFooter = ({
   onCancel,
   selected,
   // redux
+  defaultSettings,
   updateField,
   setBlockTitle,
   // injected,
@@ -35,7 +36,12 @@ export const SelectTypeFooter = ({
         </Button>
         <Button
           aria-label={intl.formatMessage(messages.selectButtonAriaLabel)}
-          onClick={hooks.onSelect({ selected, updateField, setBlockTitle })}
+          onClick={hooks.onSelect({
+            selected,
+            updateField,
+            setBlockTitle,
+            defaultSettings,
+          })}
           disabled={!selected}
         >
           <FormattedMessage {...messages.selectButtonLabel} />
@@ -50,6 +56,12 @@ SelectTypeFooter.defaultProps = {
 };
 
 SelectTypeFooter.propTypes = {
+  defaultSettings: PropTypes.shape({
+    maxAttempts: PropTypes.number,
+    rerandomize: PropTypes.string,
+    showResetButton: PropTypes.bool,
+    showanswer: PropTypes.string,
+  }).isRequired,
   onCancel: PropTypes.func.isRequired,
   selected: PropTypes.string,
   updateField: PropTypes.func.isRequired,
@@ -58,7 +70,8 @@ SelectTypeFooter.propTypes = {
   intl: intlShape.isRequired,
 };
 
-export const mapStateToProps = () => ({
+export const mapStateToProps = (state) => ({
+  defaultSettings: selectors.problem.defaultSettings(state),
 });
 
 export const mapDispatchToProps = {
diff --git a/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/SelectTypeFooter.test.jsx b/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/SelectTypeFooter.test.jsx
index e545131a0..2a9e14236 100644
--- a/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/SelectTypeFooter.test.jsx
+++ b/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/SelectTypeFooter.test.jsx
@@ -16,6 +16,7 @@ describe('SelectTypeFooter', () => {
     onCancel: jest.fn().mockName('onCancel'),
     selected: null,
     // redux
+    defaultSettings: {},
     updateField: jest.fn().mockName('UpdateField'),
     // inject
     intl: { formatMessage },
@@ -45,7 +46,7 @@ describe('SelectTypeFooter', () => {
 
   describe('mapStateToProps', () => {
     test('is empty', () => {
-      expect(module.mapStateToProps()).toEqual({});
+      expect(module.mapDispatchToProps.defaultSettings).toEqual(actions.problem.defaultSettings);
     });
   });
   describe('mapDispatchToProps', () => {
diff --git a/src/editors/containers/ProblemEditor/components/SelectTypeModal/hooks.js b/src/editors/containers/ProblemEditor/components/SelectTypeModal/hooks.js
index c07663dfd..5854ea2e8 100644
--- a/src/editors/containers/ProblemEditor/components/SelectTypeModal/hooks.js
+++ b/src/editors/containers/ProblemEditor/components/SelectTypeModal/hooks.js
@@ -2,7 +2,7 @@ import { useEffect, useState } from 'react';
 import {
   AdvanceProblemKeys, AdvanceProblems, ProblemTypeKeys, ProblemTypes,
 } from '../../../../data/constants/problem';
-import { StrictDict } from '../../../../utils';
+import { StrictDict, snakeCaseKeys } from '../../../../utils';
 import * as module from './hooks';
 import { getDataFromOlx } from '../../../../data/redux/thunkActions/problem';
 
@@ -19,14 +19,28 @@ export const selectHooks = () => {
   };
 };
 
-export const onSelect = ({ selected, updateField, setBlockTitle }) => () => {
+export const onSelect = ({
+  selected,
+  updateField,
+  setBlockTitle,
+  defaultSettings,
+}) => () => {
   if (Object.values(AdvanceProblemKeys).includes(selected)) {
     updateField({ problemType: ProblemTypeKeys.ADVANCED, rawOLX: AdvanceProblems[selected].template });
     setBlockTitle(AdvanceProblems[selected].title);
   } else {
     const newOLX = ProblemTypes[selected].template;
-    const { settings, ...newState } = getDataFromOlx({ rawOLX: newOLX, rawSettings: {}, defaultSettings: {} });
-    updateField({ ...newState });
+    const newState = getDataFromOlx({
+      rawOLX: newOLX,
+      rawSettings: {
+        weight: 1,
+        attempts_before_showanswer_button: 0,
+        show_reset_button: null,
+        showanswer: null,
+      },
+      defaultSettings: snakeCaseKeys(defaultSettings),
+    });
+    updateField(newState);
     setBlockTitle(ProblemTypes[selected].title);
   }
 };
diff --git a/src/editors/containers/ProblemEditor/components/SelectTypeModal/hooks.test.js b/src/editors/containers/ProblemEditor/components/SelectTypeModal/hooks.test.js
index b44cc4eef..baa57670d 100644
--- a/src/editors/containers/ProblemEditor/components/SelectTypeModal/hooks.test.js
+++ b/src/editors/containers/ProblemEditor/components/SelectTypeModal/hooks.test.js
@@ -3,7 +3,7 @@ import React from 'react';
 import { MockUseState } from '../../../../../testUtils';
 import * as module from './hooks';
 import { AdvanceProblems, ProblemTypeKeys, ProblemTypes } from '../../../../data/constants/problem';
-import { OLXParser } from '../../data/OLXParser';
+import { getDataFromOlx } from '../../../../data/redux/thunkActions/problem';
 
 jest.mock('react', () => ({
   ...jest.requireActual('react'),
@@ -17,6 +17,12 @@ const mockSelected = 'multiplechoiceresponse';
 const mockAdvancedSelected = 'circuitschematic';
 const mockSetSelected = jest.fn().mockName('setSelected');
 const mocksetBlockTitle = jest.fn().mockName('setBlockTitle');
+const mockDefaultSettings = {
+  max_attempts: null,
+  rerandomize: 'never',
+  showR_reset_button: false,
+  showanswer: 'always',
+};
 
 let hook;
 
@@ -58,13 +64,24 @@ describe('SelectTypeModal hooks', () => {
       expect(mocksetBlockTitle).toHaveBeenCalledWith(AdvanceProblems[mockAdvancedSelected].title);
     });
     test('updateField is called with selected on visual propblems', () => {
-      module.onSelect({ selected: mockSelected, updateField: mockUpdateField, setBlockTitle: mocksetBlockTitle })();
-      const testOlXParser = new OLXParser(ProblemTypes[mockSelected].template);
-      const { settings, ...testState } = testOlXParser.getParsedOLXData();
-      expect(mockUpdateField).toHaveBeenCalledWith({
-        ...testState,
+      module.onSelect({
+        selected: mockSelected,
+        updateField: mockUpdateField,
+        setBlockTitle: mocksetBlockTitle,
+        defaultSettings: mockDefaultSettings,
+      })();
+      // const testOlXParser = new OLXParser(ProblemTypes[mockSelected].template);
+      const testState = getDataFromOlx({
         rawOLX: ProblemTypes[mockSelected].template,
+        rawSettings: {
+          weight: 1,
+          attempts_before_showanswer_button: 0,
+          show_reset_button: null,
+          showanswer: null,
+        },
+        defaultSettings: mockDefaultSettings,
       });
+      expect(mockUpdateField).toHaveBeenCalledWith(testState);
       expect(mocksetBlockTitle).toHaveBeenCalledWith(ProblemTypes[mockSelected].title);
     });
   });
diff --git a/src/editors/containers/ProblemEditor/data/SettingsParser.js b/src/editors/containers/ProblemEditor/data/SettingsParser.js
index 99c88c067..5836e2827 100644
--- a/src/editors/containers/ProblemEditor/data/SettingsParser.js
+++ b/src/editors/containers/ProblemEditor/data/SettingsParser.js
@@ -6,7 +6,9 @@ export const popuplateItem = (parentObject, itemName, statekey, metadata, defaul
   let parent = parentObject;
   const item = _.get(metadata, itemName, null);
   const equalsDefault = item === defaultValue;
-  if ((!_.isNil(item) || allowNull) && !equalsDefault) {
+  if (allowNull) {
+    parent = { ...parentObject, [statekey]: item };
+  } else if (!_.isNil(item) && !equalsDefault) {
     parent = { ...parentObject, [statekey]: item };
   }
   return parent;
diff --git a/src/editors/data/redux/problem/reducers.js b/src/editors/data/redux/problem/reducers.js
index c672b85a3..c61dfb240 100644
--- a/src/editors/data/redux/problem/reducers.js
+++ b/src/editors/data/redux/problem/reducers.js
@@ -21,7 +21,7 @@ const initialState = {
     scoring: {
       weight: 1,
       attempts: {
-        unlimited: false,
+        unlimited: true,
         number: null,
       },
     },
diff --git a/src/editors/utils/index.js b/src/editors/utils/index.js
index 14ecc749c..e31466915 100644
--- a/src/editors/utils/index.js
+++ b/src/editors/utils/index.js
@@ -4,3 +4,4 @@ export { default as keyStore } from './keyStore';
 export { default as camelizeKeys } from './camelizeKeys';
 export { default as removeItemOnce } from './removeOnce';
 export { default as formatDuration } from './formatDuration';
+export { default as snakeCaseKeys } from './snakeCaseKeys';
diff --git a/src/editors/utils/snakeCaseKeys.js b/src/editors/utils/snakeCaseKeys.js
new file mode 100644
index 000000000..d07acfba1
--- /dev/null
+++ b/src/editors/utils/snakeCaseKeys.js
@@ -0,0 +1,15 @@
+import { snakeCase } from 'lodash-es';
+
+const snakeCaseKeys = (obj) => {
+  if (Array.isArray(obj)) {
+    return obj.map(v => snakeCaseKeys(v));
+  }
+  if (obj != null && obj.constructor === Object) {
+    return Object.keys(obj).reduce(
+      (result, key) => ({ ...result, [snakeCase(key)]: snakeCaseKeys(obj[key]) }),
+      {},
+    );
+  }
+  return obj;
+};
+export default snakeCaseKeys;
diff --git a/src/editors/utils/snakeCaseKeys.test.js b/src/editors/utils/snakeCaseKeys.test.js
new file mode 100644
index 000000000..3bdd209c5
--- /dev/null
+++ b/src/editors/utils/snakeCaseKeys.test.js
@@ -0,0 +1,32 @@
+import { camelizeKeys } from './index';
+
+const snakeCaseObject = {
+  some_attribute:
+    {
+      another_attribute: [
+        { a_list: 'a lIsT' },
+        { of_attributes: 'iN diFferent' },
+        { different_cases: 'to Test' },
+      ],
+    },
+  a_final_attribute: null,
+  a_last_one: undefined,
+};
+const camelCaseObject = {
+  someAttribute:
+    {
+      anotherAttribute: [
+        { aList: 'a lIsT' },
+        { ofAttributes: 'iN diFferent' },
+        { differentCases: 'to Test' },
+      ],
+    },
+  aFinalAttribute: null,
+  aLastOne: undefined,
+};
+
+describe('camelizeKeys', () => {
+  it('converts keys of objects to be camelCase', () => {
+    expect(camelizeKeys(snakeCaseObject)).toEqual(camelCaseObject);
+  });
+});