From 398839d76ca471a3e3052cf73bd0bdbdcd1ee21b Mon Sep 17 00:00:00 2001
From: Navin Karkera <navin@disroot.org>
Date: Mon, 9 Oct 2023 21:29:53 +0530
Subject: [PATCH] refactor: improve setting parser condition handling

---
 .../ProblemEditor/data/SettingsParser.js      | 33 +++++++++----------
 .../ProblemEditor/data/SettingsParser.test.js | 15 +++++++++
 2 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/src/editors/containers/ProblemEditor/data/SettingsParser.js b/src/editors/containers/ProblemEditor/data/SettingsParser.js
index 586cccad0..36307b200 100644
--- a/src/editors/containers/ProblemEditor/data/SettingsParser.js
+++ b/src/editors/containers/ProblemEditor/data/SettingsParser.js
@@ -15,24 +15,23 @@ export const parseScoringSettings = (metadata, defaultSettings) => {
   let scoring = {};
 
   let attempts = popuplateItem({}, 'max_attempts', 'number', metadata);
-  // TODO: impove below condition handling
-  if ((_.isEmpty(attempts) || _.isNaN(attempts.number)) && (_.isEmpty(defaultSettings) || _.isNaN(defaultSettings.max_attempts))) {
-    attempts = { unlimited: true, number: '' };
-  } else if (!_.isEmpty(attempts) && !_.isNaN(attempts.number)) {
-    attempts.unlimited = false;
-    attempts.number = attempts.number;
-    if (attempts.number < 0) {
-      attempts.number = 0;
-    }
-  } else if (!_.isNaN(defaultSettings.max_attempts)) {
-    attempts.unlimited = false;
-    attempts.number = defaultSettings.max_attempts;
-  } else {
-    attempts.unlimited = false;
-    if (attempts.number < 0) {
-      attempts.number = 0;
-    }
+  attempts.unlimited = false;
+
+  // isFinite checks if value is a finite primitive number.
+  if (!_.isFinite(_.get(attempts, 'number', null))) {
+    attempts.number = _.get(defaultSettings, 'max_attempts', null);
   }
+
+  // if above statement was true and no default was found, set unlimited to true
+  if (_.isNil(attempts.number)) {
+    attempts.number = '';
+    attempts.unlimited = true;
+  }
+
+  if (attempts.number < 0) {
+    attempts.number = 0;
+  }
+
   scoring = { ...scoring, attempts };
 
   scoring = popuplateItem(scoring, 'weight', 'weight', metadata);
diff --git a/src/editors/containers/ProblemEditor/data/SettingsParser.test.js b/src/editors/containers/ProblemEditor/data/SettingsParser.test.js
index c5278ae11..b415a8422 100644
--- a/src/editors/containers/ProblemEditor/data/SettingsParser.test.js
+++ b/src/editors/containers/ProblemEditor/data/SettingsParser.test.js
@@ -30,6 +30,11 @@ describe('Test Settings to State Parser', () => {
     expect(scoreSettings.attempts).toStrictEqual(singleSelectWithHints.state.settings.scoring.attempts);
   });
 
+  test('Test score settings attempts missing with default max_attempts', () => {
+    const scoreSettings = parseScoringSettings(singleSelectWithHints.metadata, defaultSettings);
+    expect(scoreSettings.attempts).toStrictEqual({number: 1, unlimited: false});
+  });
+
   test('Test negative attempts in score', () => {
     const scoreSettings = parseScoringSettings(negativeAttempts.metadata, defaultSettings);
     expect(scoreSettings.attempts).toStrictEqual(negativeAttempts.state.settings.scoring.attempts);
@@ -40,6 +45,16 @@ describe('Test Settings to State Parser', () => {
     expect(settings.scoring).toStrictEqual(singleSelectWithHints.state.settings.scoring);
   });
 
+  test('Test score settings missing with default', () => {
+    const settings = parseSettings(singleSelectWithHints.metadata, defaultSettings);
+    expect(settings.scoring).toStrictEqual({attempts: {number: 1, unlimited: false}});
+  });
+
+  test('Test score settings missing with null default', () => {
+    const settings = parseSettings(singleSelectWithHints.metadata, {max_attempts: null});
+    expect(settings.scoring).toStrictEqual({attempts: {number: '', unlimited: true}});
+  });
+
   test('Test invalid randomization', () => {
     const settings = parseSettings(numericWithHints.metadata, defaultSettings);
     expect(settings.randomization).toBeUndefined();