fix editor deleting description (#444)

internal issue: https://2u-internal.atlassian.net/browse/TNL-11311 This fixes a bug where the editor was deleting the OLX tag when editing in the simple editor and then saving. Also the description was being converted to em for the simple editor, but then not converted back. However, the xblock renders label and then em in reverse order for some reason. To fix it, the em gets converted back to description now, but not for every em tag (added a class "olx_description" for the tags that should be converted).
openedx · Dec 20, 2023 · 50c580c · 50c580c
1 parent 47a3fd6
commit 50c580c
Show file tree

Hide file tree

Showing 5 changed files with 75 additions and 25 deletions.
diff --git a/src/editors/containers/ProblemEditor/data/OLXParser.js b/src/editors/containers/ProblemEditor/data/OLXParser.js
@@ -487,7 +487,12 @@ export class OLXParser {
       }
     });
     const questionString = this.richTextBuilder.build(questionArray);
-    return questionString.replace(/<description>/gm, '<em>').replace(/<\/description>/gm, '</em>');
+    const res = this.replaceOlxDescriptionTag(questionString);
+    return res;
+  }
+
+  replaceOlxDescriptionTag(questionString) {
+    return questionString.replace(/<description>/gm, '<em class="olx_description">').replace(/<\/description>/gm, '</em>');
   }
 
   /** getHints()

diff --git a/src/editors/containers/ProblemEditor/data/OLXParser.test.js b/src/editors/containers/ProblemEditor/data/OLXParser.test.js
@@ -329,7 +329,7 @@ describe('OLXParser', () => {
       const olxparser = new OLXParser(labelDescriptionQuestionOLX.rawOLX);
       const problemType = olxparser.getProblemType();
       const question = olxparser.parseQuestions(problemType);
-      it('should append the label/description to the question', () => {
+      it('should append the label/description to the question, converting description to <em> with "olx_description" class', () => {
         expect(question.trim()).toBe(labelDescriptionQuestionOLX.question);
       });
     });

diff --git a/src/editors/containers/ProblemEditor/data/ReactStateOLXParser.js b/src/editors/containers/ProblemEditor/data/ReactStateOLXParser.js
@@ -193,29 +193,31 @@ class ReactStateOLXParser {
 
   /** addQuestion()
    * The editorObject saved to the class constuctor is parsed for the attribute question. The question is parsed and
-   * checked for label tags. After the question is fully updated, the questionObject is returned.
+   * checked for label and em tags. After the question is fully updated, the questionObject is returned.
+   * TODO: this is very brittle and unreliable. Needs improvement.
    * @return {object} object representaion of question
    */
   addQuestion() {
     const { question } = this.editorObject;
     const questionObject = this.richTextParser.parse(question);
-    /* Removes block tags like <p> or <h1> that surround the <label> format.
+    /* Removes block tags like <p> or <h1> that surround the <label> or <em> format.
       Block tags are required by tinyMCE but have adverse effect on css in studio.
       */
-    questionObject.forEach((tag, ind) => {
-      const tagName = Object.keys(tag)[0];
-      let label = null;
-      tag[tagName].forEach(subTag => {
-        const subTagName = Object.keys(subTag)[0];
-        if (subTagName === 'label') {
-          label = subTag;
-        }
-      });
-      if (label) {
-        questionObject[ind] = label;
+    const resultQuestion = [];
+    const relevantSubnodes = ['label', 'em'];
+
+    questionObject.forEach((tag) => {
+      const subNodes = Object.values(tag)[0];
+      const containsRelevantSubnodes = subNodes.some(subNode => relevantSubnodes.includes(Object.keys(subNode)[0]));
+
+      if (!containsRelevantSubnodes) {
+        resultQuestion.push(tag);
+      } else {
+        resultQuestion.push(...subNodes);
       }
     });
-    return questionObject;
+
+    return resultQuestion;
   }
 
   /** buildMultiSelectProblem()
@@ -259,13 +261,22 @@ class ReactStateOLXParser {
       default:
         break;
     }
-    const updatedString = `${problemTypeTag}\n${questionString}`;
+    const questionStringWithEmDescriptionReplace = this.replaceEmWithDescriptionTag(questionString);
+    const updatedString = `${problemTypeTag}\n${questionStringWithEmDescriptionReplace}`;
     const problemBodyString = problemBody.replace(problemTypeTag, updatedString);
     const fullProblemString = `<problem>${problemBodyString}${hintString}\n</problem>`;
 
     return fullProblemString;
   }
 
+  replaceEmWithDescriptionTag(xmlString) {
+    const regexPattern = /<em class="olx_description">(.*?)<\/em>/g;
+    const replacement = '<description>$1</description>';
+
+    const updatedHtml = xmlString.replace(regexPattern, replacement);
+    return updatedHtml;
+  }
+
   /** buildTextInput()
    * String response OLX builder. The question builder has a different format than the
    * other parts (demand hint, answers, and solution) of the problem so it has to be

diff --git a/src/editors/containers/ProblemEditor/data/ReactStateOLXParser.test.js b/src/editors/containers/ProblemEditor/data/ReactStateOLXParser.test.js
@@ -52,16 +52,48 @@ describe('Check React State OLXParser problem', () => {
     const buildOLX = stateParser.buildOLX();
     expect(buildOLX.replace(/\s/g, '')).toEqual(textInputWithFeedbackAndHintsOLX.buildOLX.replace(/\s/g, ''));
   });
-  test('Test multiple choice with feedback and hints problem type', () => {
+  describe('Multiple choice with feedback and hints problem type', () => {
     const olxparser = new OLXParser(multipleChoiceWithFeedbackAndHintsOLX.rawOLX);
     const problem = olxparser.getParsedOLXData();
     const stateParser = new ReactStateOLXParser({
       problem,
       editorObject: multipleChoiceWithFeedbackAndHints,
     });
-    const buildOLX = stateParser.buildOLX();
-    expect(buildOLX.replace(/\s/g, '')).toEqual(multipleChoiceWithFeedbackAndHintsOLX.buildOLX.replace(/\s/g, ''));
+
+    it('should parse correctly', () => {
+      const buildOLX = stateParser.buildOLX();
+      expect(buildOLX.replace(/\s/g, '')).toEqual(multipleChoiceWithFeedbackAndHintsOLX.buildOLX.replace(/\s/g, ''));
+    });
   });
+
+  describe('with label and em tag wrapped in div: ', () => {
+    const olxparser = new OLXParser(multipleChoiceWithFeedbackAndHintsOLX.rawOLX);
+    const problem = olxparser.getParsedOLXData();
+    const stateParser = new ReactStateOLXParser({
+      problem,
+      editorObject: multipleChoiceWithFeedbackAndHints,
+    });
+    stateParser.editorObject.question = '<p>You can use this template as a guide to the simple editor markdown and OLX markup to use for multiple choice with hints and feedback problems. Edit this component to replace this template with your own assessment.</p>\n<div><label>Add the question text, or prompt, here. This text is required.</label> <em class="olx_description">You can add an optional tip or note related to the prompt like this. </em><em>Just a generic em tag</em></div>';
+
+    it('parser should not delete <em> tags', () => {
+      const buildOLX = stateParser.buildOLX();
+      expect(buildOLX.replace(/\s/g, '')).toEqual(multipleChoiceWithFeedbackAndHintsOLX.buildOLX.replace(/\s/g, ''));
+    });
+
+    it('addQuestion method should add a question to the problemState correctly', () => {
+      const received = stateParser.addQuestion();
+      expect(received).toEqual(
+        [
+          { p: [{ '#text': 'You can use this template as a guide to the simple editor markdown and OLX markup to use for multiple choice with hints and feedback problems. Edit this component to replace this template with your own assessment.' }] },
+          { label: [{ '#text': 'Add the question text, or prompt, here. This text is required.' }] },
+          { '#text': ' ' },
+          { em: [{ '#text': 'You can add an optional tip or note related to the prompt like this. ' }], ':@': { '@_class': 'olx_description' } },
+          { em: [{ '#text': 'Just a generic em tag' }] },
+        ],
+      );
+    });
+  });
+
   test('Test numerical response with feedback and hints problem type', () => {
     const olxparser = new OLXParser(numericInputWithFeedbackAndHintsOLX.rawOLX);
     const problem = olxparser.getParsedOLXData();

diff --git a/src/editors/containers/ProblemEditor/data/mockData/olxTestData.js b/src/editors/containers/ProblemEditor/data/mockData/olxTestData.js
@@ -314,10 +314,11 @@ export const multipleChoiceWithFeedbackAndHintsOLX = {
   <p>You can use this template as a guide to the simple editor markdown and OLX markup to use for multiple choice with hints and feedback problems. Edit this component to replace this template with your own assessment.</p>
 <label>Add the question text, or prompt, here. This text is required.</label>
 <description>You can add an optional tip or note related to the prompt like this. </description>
+<em>Just a generic em tag</em>
 <choicegroup type="MultipleChoice">
     <choice correct="false"><p>an incorrect answer</p><choicehint><p>You can specify optional feedback like this, which appears after this answer is submitted.</p></choicehint></choice>
     <choice correct="true"><p>the correct answer</p></choice>
-    <choice correct="false"><p>an incorrect answer</p><choicehint><p>You can specify optional feedback for none, a subset, or all of the answers.</></choicehint></choice>
+    <choice correct="false"><p>an incorrect answer</p><choicehint><p>You can specify optional feedback for none, a subset, or all of the answers.</p></choicehint></choice>
   </choicegroup>
   <solution>
     <p>You can add a solution</p>
@@ -359,12 +360,13 @@ export const multipleChoiceWithFeedbackAndHintsOLX = {
       },
     ],
   },
-  question: '<p>You can use this template as a guide to the simple editor markdown and OLX markup to use for multiple choice with hints and feedback problems. Edit this component to replace this template with your own assessment.</p><label>Add the question text, or prompt, here. This text is required.</label><em>You can add an optional tip or note related to the prompt like this.</em>',
+  question: '<p>You can use this template as a guide to the simple editor markdown and OLX markup to use for multiple choice with hints and feedback problems. Edit this component to replace this template with your own assessment.</p><label>Add the question text, or prompt, here. This text is required.</label><em class="olx_description">You can add an optional tip or note related to the prompt like this.</em><em>Just a generic em tag</em>',
   buildOLX: `<problem>
   <multiplechoiceresponse>
     <p>You can use this template as a guide to the simple editor markdown and OLX markup to use for multiple choice with hints and feedback problems. Edit this component to replace this template with your own assessment.</p>
     <label>Add the question text, or prompt, here. This text is required.</label>
-    <em>You can add an optional tip or note related to the prompt like this.</em>
+    <description>You can add an optional tip or note related to the prompt like this.</description>
+    <em>Just a generic em tag</em>
     <choicegroup>
       <choice correct="false">
 <p>an incorrect answer</p>        <choicehint><p>You can specify optional feedback like this, which appears after this answer is submitted.</p></choicehint>
@@ -541,7 +543,7 @@ export const textInputWithFeedbackAndHintsOLX = {
   },
   question: `<p>You can use this template as a guide to the simple editor markdown and OLX markup to use for text input with hints and feedback problems. Edit this component to replace this template with your own assessment.</p>
 <label>Add the question text, or prompt, here. This text is required.</label>
-<em>You can add an optional tip or note related to the prompt like this. </em>`,
+<em class="olx_description">You can add an optional tip or note related to the prompt like this. </em>`,
   buildOLX: `<problem>
   <stringresponse answer="the correct answer" type="ci">
     <p>You can use this template as a guide to the simple editor markdown and OLX markup to use for text input with hints and feedback problems. Edit this component to replace this template with your own assessment.</p>
@@ -773,7 +775,7 @@ export const labelDescriptionQuestionOLX = {
   question: `<p style="text-align: center;"><img height="274" width="" src="/static/boiling_eggs_water_system.png" alt="boiling eggs: water system"></img></p>
   
   <label>Taking the system as just the <b>water</b>, as indicated by the red dashed line, what would be the correct expression for the first law of thermodynamics applied to this system?</label>
-  <em>Watch out, boiling water is hot</em>`,
+  <em class="olx_description">Watch out, boiling water is hot</em>`,
 };
 
 export const htmlEntityTestOLX = {