Skip to content

Commit

Permalink
fix: removal of content after problem type tags (openedx#479)
Browse files Browse the repository at this point in the history
* fix: removal of content after problem type tags

* fix: readability and error handling
  • Loading branch information
KristinAoki authored May 30, 2024
1 parent 6c743f8 commit a959c05
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 4 deletions.
41 changes: 39 additions & 2 deletions src/editors/containers/ProblemEditor/data/OLXParser.js
Original file line number Diff line number Diff line change
Expand Up @@ -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>');
}
Expand Down Expand Up @@ -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');
Expand Down
16 changes: 14 additions & 2 deletions src/editors/containers/ProblemEditor/data/OLXParser.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
solutionExplanationWithoutDivTest,
tablesInRichTextTest,
parseOutExplanationTests,
unexpectOlxAfterProblemTypeTags,
} from './mockData/olxTestData';
import { ProblemTypeKeys } from '../../../data/constants/problem';

Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -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', () => {
Expand Down
18 changes: 18 additions & 0 deletions src/editors/containers/ProblemEditor/data/mockData/olxTestData.js
Original file line number Diff line number Diff line change
Expand Up @@ -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>`
}

0 comments on commit a959c05

Please sign in to comment.