From a959c0543cac6f7da2f9120949ba8e99496afe64 Mon Sep 17 00:00:00 2001
From: Kristin Aoki <42981026+KristinAoki@users.noreply.github.com>
Date: Thu, 30 May 2024 12:33:43 -0400
Subject: [PATCH] fix: removal of content after problem type tags (#479)

* fix: removal of content after problem type tags

* fix: readability and error handling
---
 .../ProblemEditor/data/OLXParser.js           | 41 ++++++++++++++++++-
 .../ProblemEditor/data/OLXParser.test.js      | 16 +++++++-
 .../data/mockData/olxTestData.js              | 18 ++++++++
 3 files changed, 71 insertions(+), 4 deletions(-)

diff --git a/src/editors/containers/ProblemEditor/data/OLXParser.js b/src/editors/containers/ProblemEditor/data/OLXParser.js
index 918aa4958..7f7f11c9b 100644
--- a/src/editors/containers/ProblemEditor/data/OLXParser.js
+++ b/src/editors/containers/ProblemEditor/data/OLXParser.js
@@ -491,6 +491,39 @@ export class OLXParser {
     return res;
   }
 
+  /** hasOLXAfterProblemTypeTag(problemType)
+   * checkTextAfterProblemTypeTag takes a problemType. The problem type is used to determine
+   * if there is olx after the answer choices the problem. Simple problems are not expected
+   * to have olx after the answer choices and returns false. In the event that a problem has
+   * olx after the answer choices it returns true and will raise an error.
+   * @param {string} problemType - string of the olx problem type
+   * @return {bool}
+   */
+  hasOLXAfterProblemTypeTag(problemType) {
+    let problemTagIndex = this.richTextProblem.length - 1;
+    let hasExtraOLX = false;
+    Object.entries(this.richTextProblem).forEach(([i, value]) => {
+      if (Object.keys(value).includes(problemType)) {
+        problemTagIndex = i;
+      }
+    });
+
+    if (problemTagIndex < this.richTextProblem.length - 1) {
+      const olxAfterProblemType = this.richTextProblem.slice(problemTagIndex + 1);
+      Object.values(olxAfterProblemType).forEach(value => {
+        const currentKey = Object.keys(value)[0];
+        const invalidText = currentKey === '#text' && value[currentKey] !== '\n';
+        const invalidKey = !nonQuestionKeys.includes(currentKey) && currentKey !== '#text';
+        if (invalidText) {
+          hasExtraOLX = true;
+        } else if (invalidKey) {
+          hasExtraOLX = true;
+        }
+      });
+    }
+    return hasExtraOLX;
+  }
+
   replaceOlxDescriptionTag(questionString) {
     return questionString.replace(/<description>/gm, '<em class="olx_description">').replace(/<\/description>/gm, '</em>');
   }
@@ -634,14 +667,18 @@ export class OLXParser {
       throw new Error('Misc Attributes asscoiated with problem, opening in advanced editor');
     }
 
+    const problemType = this.getProblemType();
+
+    if (this.hasOLXAfterProblemTypeTag(problemType)) {
+      throw new Error(`OLX was found after the ${problemType} tags, opening in advanced editor`);
+    }
+
     let answersObject = {};
     let additionalAttributes = {};
     let groupFeedbackList = [];
-    const problemType = this.getProblemType();
     const hints = this.getHints();
     const question = this.parseQuestions(problemType);
     const solutionExplanation = this.getSolutionExplanation(problemType);
-
     switch (problemType) {
       case ProblemTypeKeys.DROPDOWN:
         answersObject = this.parseMultipleChoiceAnswers(ProblemTypeKeys.DROPDOWN, 'optioninput', 'option');
diff --git a/src/editors/containers/ProblemEditor/data/OLXParser.test.js b/src/editors/containers/ProblemEditor/data/OLXParser.test.js
index ed5dbe83f..1d8cc8d5a 100644
--- a/src/editors/containers/ProblemEditor/data/OLXParser.test.js
+++ b/src/editors/containers/ProblemEditor/data/OLXParser.test.js
@@ -27,6 +27,7 @@ import {
   solutionExplanationWithoutDivTest,
   tablesInRichTextTest,
   parseOutExplanationTests,
+  unexpectOlxAfterProblemTypeTags,
 } from './mockData/olxTestData';
 import { ProblemTypeKeys } from '../../../data/constants/problem';
 
@@ -87,7 +88,7 @@ describe('OLXParser', () => {
         }
       });
     });
-    describe('when multi select problem finds partial_credit attribute', () => {
+    describe('when numerical problem finds partial_credit attribute', () => {
       it('should throw error and contain message regarding opening advanced editor', () => {
         try {
           numericalProblemPartialCreditParser.getParsedOLXData();
@@ -97,7 +98,7 @@ describe('OLXParser', () => {
         }
       });
     });
-    describe('when multi select problem finds partial_credit attribute', () => {
+    describe('when single select problem finds partial_credit attribute', () => {
       it('should throw error and contain message regarding opening advanced editor', () => {
         try {
           singleSelectPartialCreditParser.getParsedOLXData();
@@ -107,6 +108,17 @@ describe('OLXParser', () => {
         }
       });
     });
+    describe('when signle select problem has unexpected olx after multiplechoiceresponse tag', () => {
+      it('should throw error and contain message regarding opening advanced editor', () => {
+        const unexpectOlxAfterProblemTypeTagsParser = new OLXParser(unexpectOlxAfterProblemTypeTags.rawOLX);
+        try {
+          unexpectOlxAfterProblemTypeTagsParser.getParsedOLXData();
+        } catch (e) {
+          expect(e).toBeInstanceOf(Error);
+          expect(e.message).toBe('OLX found after the multiplechoiceresponse tags, opening in advanced editor');
+        }
+      });
+    });
   });
   describe('getProblemType()', () => {
     describe('given a blank problem', () => {
diff --git a/src/editors/containers/ProblemEditor/data/mockData/olxTestData.js b/src/editors/containers/ProblemEditor/data/mockData/olxTestData.js
index f913f9d53..0158428f0 100644
--- a/src/editors/containers/ProblemEditor/data/mockData/olxTestData.js
+++ b/src/editors/containers/ProblemEditor/data/mockData/olxTestData.js
@@ -1154,3 +1154,21 @@ export const numericalProblemPartialCredit = {
     </numericalresponse>
   </problem>`
 }
+
+export const unexpectOlxAfterProblemTypeTags = {
+  rawOLX: `<problem>
+    <multiplechoiceresponse>
+      <label>What Apple device competed with the portable CD player?</label>
+      <choicegroup type="MultipleChoice">
+        <choice correct="false">The iPad</choice>
+        <choice correct="false">Napster</choice>
+        <choice correct="true">The iPod</choice>
+      </choicegroup>
+    </multiplechoiceresponse>
+    <a href="#">Check out Apple's history for more information.</a>
+    <demandhint>
+      <hint><p>You can add an optional hint like this. Problems that have a hint include a hint button, and this text appears the first time learners select the button.</p></hint>
+      <hint><p>If you add more than one hint, a different hint appears each time learners select the hint button.</p></hint>
+    </demandhint>
+  </problem>`
+}