Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: removal of content after problem type tags #479

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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>`
}
Loading