From 7e62b41bffb9156c10b32e3f1ed14e75b5cf8597 Mon Sep 17 00:00:00 2001 From: farres1 Date: Tue, 16 Apr 2024 11:07:14 +0100 Subject: [PATCH 01/15] Add mutually exclusive answers to logic selector --- .../schema/resolvers/logic/binaryExpression2/index.js | 9 ++++++--- .../src/businessLogic/answerTypeToConditions.js | 1 + .../MultipleChoiceAnswerOptionsSelector.js | 10 ++++++---- .../src/App/page/Logic/BinaryExpressionEditor/index.js | 2 ++ eq-author/src/constants/answer-types.js | 3 ++- 5 files changed, 17 insertions(+), 8 deletions(-) diff --git a/eq-author-api/schema/resolvers/logic/binaryExpression2/index.js b/eq-author-api/schema/resolvers/logic/binaryExpression2/index.js index 53c331fcbf..9638041f68 100644 --- a/eq-author-api/schema/resolvers/logic/binaryExpression2/index.js +++ b/eq-author-api/schema/resolvers/logic/binaryExpression2/index.js @@ -91,9 +91,12 @@ Resolvers.LeftSide2 = { __resolveType: ({ type, sideType }) => { if (sideType === "Answer") { if ( - [answerTypes.RADIO, answerTypes.CHECKBOX, answerTypes.SELECT].includes( - type - ) + [ + answerTypes.RADIO, + answerTypes.CHECKBOX, + answerTypes.SELECT, + answerTypes.MUTUALLY_EXCLUSIVE, + ].includes(type) ) { return "MultipleChoiceAnswer"; } diff --git a/eq-author-api/src/businessLogic/answerTypeToConditions.js b/eq-author-api/src/businessLogic/answerTypeToConditions.js index b5195f0ecc..51bdcf0aef 100644 --- a/eq-author-api/src/businessLogic/answerTypeToConditions.js +++ b/eq-author-api/src/businessLogic/answerTypeToConditions.js @@ -31,6 +31,7 @@ const answerConditions = { conditions.COUNT_OF, ], [answerTypes.SELECT]: [conditions.ONE_OF, conditions.UNANSWERED], + [answerTypes.MUTUALLY_EXCLUSIVE]: [conditions.ONE_OF, conditions.UNANSWERED], }; const isAnswerTypeSupported = (answerType) => diff --git a/eq-author/src/App/page/Logic/BinaryExpressionEditor/MultipleChoiceAnswerOptionsSelector.js b/eq-author/src/App/page/Logic/BinaryExpressionEditor/MultipleChoiceAnswerOptionsSelector.js index 2b73ffd5a1..ae0c39fae4 100644 --- a/eq-author/src/App/page/Logic/BinaryExpressionEditor/MultipleChoiceAnswerOptionsSelector.js +++ b/eq-author/src/App/page/Logic/BinaryExpressionEditor/MultipleChoiceAnswerOptionsSelector.js @@ -16,7 +16,7 @@ import { ERR_COUNT_OF_GREATER_THAN_AVAILABLE_OPTIONS, } from "constants/validationMessages"; import { colors } from "constants/theme"; -import { RADIO, SELECT } from "constants/answer-types"; +import { RADIO, SELECT, MUTUALLY_EXCLUSIVE } from "constants/answer-types"; import { Select } from "components/Forms"; import TextButton from "components/buttons/TextButton"; @@ -32,6 +32,8 @@ const answerConditions = { NOTANYOF: "NotAnyOf", }; +const exclusiveAnswers = [RADIO, SELECT, MUTUALLY_EXCLUSIVE]; + const MultipleChoiceAnswerOptions = styled.div` align-items: center; display: inline-flex; @@ -206,7 +208,7 @@ class MultipleChoiceAnswerOptionsSelector extends React.Component { return message ? {message} : null; }; - renderRadioOptionSelector(hasError) { + renderExclusiveOptionSelector(hasError) { const { expression } = this.props; const options = get(expression, "left.options", []); @@ -331,8 +333,8 @@ class MultipleChoiceAnswerOptionsSelector extends React.Component { const hasConditionError = errors.filter(({ field }) => field === "condition").length > 0; - if (answerType === RADIO || answerType === SELECT) { - return this.renderRadioOptionSelector(hasError); + if (exclusiveAnswers.includes(answerType)) { + return this.renderExclusiveOptionSelector(hasError); } else { return this.renderCheckboxOptionSelector(hasError, hasConditionError); } diff --git a/eq-author/src/App/page/Logic/BinaryExpressionEditor/index.js b/eq-author/src/App/page/Logic/BinaryExpressionEditor/index.js index a7a95d8eb5..53123572c7 100644 --- a/eq-author/src/App/page/Logic/BinaryExpressionEditor/index.js +++ b/eq-author/src/App/page/Logic/BinaryExpressionEditor/index.js @@ -14,6 +14,7 @@ import { CHECKBOX, DATE, SELECT, + MUTUALLY_EXCLUSIVE, } from "constants/answer-types"; import { TEXT, TEXT_OPTIONAL } from "constants/metadata-types"; @@ -110,6 +111,7 @@ const ANSWER_TYPE_TO_RIGHT_EDITOR = { [RADIO]: MultipleChoiceAnswerOptionsSelector, [SELECT]: MultipleChoiceAnswerOptionsSelector, [CHECKBOX]: MultipleChoiceAnswerOptionsSelector, + [MUTUALLY_EXCLUSIVE]: MultipleChoiceAnswerOptionsSelector, [NUMBER]: NumberAnswerSelector, [PERCENTAGE]: NumberAnswerSelector, [CURRENCY]: NumberAnswerSelector, diff --git a/eq-author/src/constants/answer-types.js b/eq-author/src/constants/answer-types.js index c0f3ac7aee..11860d9757 100644 --- a/eq-author/src/constants/answer-types.js +++ b/eq-author/src/constants/answer-types.js @@ -11,6 +11,7 @@ export const DATE_RANGE = "DateRange"; export const UNIT = "Unit"; export const DURATION = "Duration"; export const SELECT = "Select"; +export const MUTUALLY_EXCLUSIVE = "MutuallyExclusive"; export const ROUTING_ANSWER_TYPES = [ RADIO, @@ -21,6 +22,7 @@ export const ROUTING_ANSWER_TYPES = [ UNIT, DATE, SELECT, + MUTUALLY_EXCLUSIVE, ]; export const ROUTING_METADATA_TYPES = [TEXT.value, TEXT_OPTIONAL.value]; @@ -28,7 +30,6 @@ export const ROUTING_METADATA_TYPES = [TEXT.value, TEXT_OPTIONAL.value]; export const RADIO_OPTION = "RadioOption"; export const CHECKBOX_OPTION = "CheckboxOption"; export const SELECT_OPTION = "SelectOption"; -export const MUTUALLY_EXCLUSIVE = "MutuallyExclusive"; export const MUTUALLY_EXCLUSIVE_OPTION = "MutuallyExclusiveOption"; export const ANSWER_OPTION_TYPES = { From 4cbd93c3f50b646e64ed77372d1c13addfc70c5d Mon Sep 17 00:00:00 2001 From: farres1 Date: Tue, 16 Apr 2024 11:12:18 +0100 Subject: [PATCH 02/15] Fix selecting mutually exclusive options in logic page --- eq-author-api/schema/resolvers/logic/binaryExpression2/index.js | 1 + 1 file changed, 1 insertion(+) diff --git a/eq-author-api/schema/resolvers/logic/binaryExpression2/index.js b/eq-author-api/schema/resolvers/logic/binaryExpression2/index.js index 9638041f68..127f1dc64f 100644 --- a/eq-author-api/schema/resolvers/logic/binaryExpression2/index.js +++ b/eq-author-api/schema/resolvers/logic/binaryExpression2/index.js @@ -37,6 +37,7 @@ const isLeftSideAnswerTypeCompatible = ( [answerTypes.CHECKBOX]: "SelectedOptions", [answerTypes.DATE]: "DateValue", [answerTypes.SELECT]: "SelectedOptions", + [answerTypes.MUTUALLY_EXCLUSIVE]: "SelectedOptions", }; if (secondaryCondition) { From 9e89e4ff13c59199da1643bfa591dc39ff6b1910 Mon Sep 17 00:00:00 2001 From: farres1 Date: Wed, 17 Apr 2024 10:57:01 +0100 Subject: [PATCH 03/15] Remove answer types from content picker based on mutually exclusive answers --- .../getQuestionnaireQuery.graphql | 3 ++ .../RoutingAnswerContentPicker.js | 4 ++- .../Logic/BinaryExpressionEditor/index.js | 1 + eq-author/src/utils/getContentBeforeEntity.js | 29 +++++++++++++++++-- 4 files changed, 33 insertions(+), 4 deletions(-) diff --git a/eq-author/src/App/QuestionnaireDesignPage/getQuestionnaireQuery.graphql b/eq-author/src/App/QuestionnaireDesignPage/getQuestionnaireQuery.graphql index 73a2b2f701..8bacacc678 100644 --- a/eq-author/src/App/QuestionnaireDesignPage/getQuestionnaireQuery.graphql +++ b/eq-author/src/App/QuestionnaireDesignPage/getQuestionnaireQuery.graphql @@ -365,6 +365,9 @@ fragment Answers on Answer { type properties advancedProperties + page { + id + } ... on BasicAnswer { secondaryQCode } diff --git a/eq-author/src/App/page/Logic/BinaryExpressionEditor/RoutingAnswerContentPicker.js b/eq-author/src/App/page/Logic/BinaryExpressionEditor/RoutingAnswerContentPicker.js index 4eba90fc6b..30fd0ad2b7 100644 --- a/eq-author/src/App/page/Logic/BinaryExpressionEditor/RoutingAnswerContentPicker.js +++ b/eq-author/src/App/page/Logic/BinaryExpressionEditor/RoutingAnswerContentPicker.js @@ -26,6 +26,7 @@ export const preprocessMetadata = (metadata) => const RoutingAnswerContentPicker = ({ includeSelf, selectedContentDisplayName, + expressionGroup, ...otherProps }) => { const { questionnaire } = useQuestionnaire(); @@ -38,8 +39,9 @@ const RoutingAnswerContentPicker = ({ id: pageId, includeTargetPage: includeSelf, preprocessAnswers, + expressionGroup, }), - [questionnaire, pageId, includeSelf] + [questionnaire, pageId, includeSelf, expressionGroup] ); const filteredPreviousAnswers = previousAnswers.map((answer) => { diff --git a/eq-author/src/App/page/Logic/BinaryExpressionEditor/index.js b/eq-author/src/App/page/Logic/BinaryExpressionEditor/index.js index 53123572c7..e7de84795e 100644 --- a/eq-author/src/App/page/Logic/BinaryExpressionEditor/index.js +++ b/eq-author/src/App/page/Logic/BinaryExpressionEditor/index.js @@ -213,6 +213,7 @@ export const UnwrappedBinaryExpressionEditor = ({ ? expression.left : undefined } + expressionGroup={expressionGroup} onSubmit={handleLeftSideChange} selectedId={expression?.left?.id} data-test="routing-answer-picker" diff --git a/eq-author/src/utils/getContentBeforeEntity.js b/eq-author/src/utils/getContentBeforeEntity.js index 472dcf38cc..ab68375d17 100644 --- a/eq-author/src/utils/getContentBeforeEntity.js +++ b/eq-author/src/utils/getContentBeforeEntity.js @@ -2,13 +2,16 @@ import { remove } from "lodash"; import isListCollectorPageType from "utils/isListCollectorPageType"; +import { MUTUALLY_EXCLUSIVE } from "constants/answer-types"; + const identity = (x) => x; const getContentBeforeEntity = ( questionnaire, id, preprocessAnswers, - includeTarget + includeTarget, + expressionGroup ) => { const sections = []; @@ -37,9 +40,27 @@ const getContentBeforeEntity = ( return sections; } - const answers = + let answers = !isListCollectorPageType(page.pageType) && (page?.answers?.flatMap(preprocessAnswers) || []); + + if (expressionGroup?.operator === "And") { + expressionGroup.expressions.forEach((expression) => { + if (expression?.left?.page?.id) { + answers = answers.filter((answer) => { + if (expression.left.page.id === answer.page.id) { + if (expression.left.type === MUTUALLY_EXCLUSIVE) { + return false; + } else { + return answer.type !== MUTUALLY_EXCLUSIVE; + } + } + return true; + }); + } + }); + } + if (answers.length) { sections[sections.length - 1].folders[ sections[sections.length - 1].folders.length - 1 @@ -75,6 +96,7 @@ export default ({ id, preprocessAnswers = identity, includeTargetPage = false, + expressionGroup, } = {}) => { if (!questionnaire || !id || questionnaire?.introduction?.id === id) { return []; @@ -85,7 +107,8 @@ export default ({ questionnaire, id, preprocessAnswers, - includeTargetPage + includeTargetPage, + expressionGroup ).filter(({ folders }) => folders.length) ); }; From edd2dcccfbfe54cfe7fc8b5d5a86e1167f029254 Mon Sep 17 00:00:00 2001 From: farres1 Date: Tue, 21 May 2024 15:03:03 +0100 Subject: [PATCH 04/15] Fix failing tests --- .../answerTypeToCondition.test.js | 4 ++- .../RoutingAnswerContentPicker.js | 1 + .../__snapshots__/index.test.js.snap | 30 +++++++++++++++++++ eq-author/src/utils/getContentBeforeEntity.js | 11 ++++++- 4 files changed, 44 insertions(+), 2 deletions(-) diff --git a/eq-author-api/src/businessLogic/answerTypeToCondition.test.js b/eq-author-api/src/businessLogic/answerTypeToCondition.test.js index 26367f6e74..99ca546f61 100644 --- a/eq-author-api/src/businessLogic/answerTypeToCondition.test.js +++ b/eq-author-api/src/businessLogic/answerTypeToCondition.test.js @@ -16,6 +16,7 @@ const VALID_TYPES = [ answerTypes.CHECKBOX, answerTypes.DATE, answerTypes.SELECT, + answerTypes.MUTUALLY_EXCLUSIVE, ]; describe("AnswerTypeToCondition", () => { @@ -29,7 +30,7 @@ describe("AnswerTypeToCondition", () => { }); describe("getDefault()", () => { - it("should return equal for all apart from radio", () => { + it("should return correct default condition for all valid answer types", () => { const expectedDefaults = { [answerTypes.NUMBER]: conditions.SELECT, [answerTypes.PERCENTAGE]: conditions.SELECT, @@ -39,6 +40,7 @@ describe("AnswerTypeToCondition", () => { [answerTypes.CHECKBOX]: conditions.ALL_OF, [answerTypes.DATE]: conditions.SELECT, [answerTypes.SELECT]: conditions.ONE_OF, + [answerTypes.MUTUALLY_EXCLUSIVE]: conditions.ONE_OF, }; VALID_TYPES.forEach((type) => { expect(getDefault(type)).toEqual(expectedDefaults[type]); diff --git a/eq-author/src/App/page/Logic/BinaryExpressionEditor/RoutingAnswerContentPicker.js b/eq-author/src/App/page/Logic/BinaryExpressionEditor/RoutingAnswerContentPicker.js index 30fd0ad2b7..e2d272d970 100644 --- a/eq-author/src/App/page/Logic/BinaryExpressionEditor/RoutingAnswerContentPicker.js +++ b/eq-author/src/App/page/Logic/BinaryExpressionEditor/RoutingAnswerContentPicker.js @@ -80,6 +80,7 @@ RoutingAnswerContentPicker.propTypes = { PropTypes.object, PropTypes.string, ]), + expressionGroup: PropTypes.object, //eslint-disable-line }; export default RoutingAnswerContentPicker; diff --git a/eq-author/src/App/page/Logic/BinaryExpressionEditor/__snapshots__/index.test.js.snap b/eq-author/src/App/page/Logic/BinaryExpressionEditor/__snapshots__/index.test.js.snap index a2f212d948..4f0852468d 100644 --- a/eq-author/src/App/page/Logic/BinaryExpressionEditor/__snapshots__/index.test.js.snap +++ b/eq-author/src/App/page/Logic/BinaryExpressionEditor/__snapshots__/index.test.js.snap @@ -24,6 +24,36 @@ exports[`BinaryExpressionEditor should render consistently 1`] = ` > { if (expression?.left?.page?.id) { answers = answers.filter((answer) => { + // If the expression's left side answer is on the same page as the looped answer if (expression.left.page.id === answer.page.id) { + // If the expression's left side answer is mutually exclusive, do not include the looped answer (as looped answer and expression answer are on the same page) if (expression.left.type === MUTUALLY_EXCLUSIVE) { return false; - } else { + } + // If the expression's left side answer is not mutually exclusive, only include the looped answer if it is also not mutually exclusive (as looped answer and expression answer are on the same page) + else { return answer.type !== MUTUALLY_EXCLUSIVE; } } From a2a30d5bc3f42f77fcbe1b2d83efa40cfa7e7dcb Mon Sep 17 00:00:00 2001 From: farres1 Date: Wed, 22 May 2024 12:17:42 +0100 Subject: [PATCH 05/15] Add getContentBeforeEntity tests --- .../src/utils/getContentBeforeEntity.test.js | 140 ++++++++++++++++++ 1 file changed, 140 insertions(+) diff --git a/eq-author/src/utils/getContentBeforeEntity.test.js b/eq-author/src/utils/getContentBeforeEntity.test.js index 79aa565593..fbea5ca298 100644 --- a/eq-author/src/utils/getContentBeforeEntity.test.js +++ b/eq-author/src/utils/getContentBeforeEntity.test.js @@ -4,6 +4,8 @@ import { } from "tests/utils/createMockQuestionnaire"; import getPreviousContent from "./getContentBeforeEntity"; +import { MUTUALLY_EXCLUSIVE } from "constants/answer-types"; + let questionnaire = buildQuestionnaire({ sectionCount: 2, folderCount: 2, @@ -15,10 +17,148 @@ questionnaire.introduction = { }; describe("utils/getPreviousAnswers", () => { + beforeEach(() => { + // Adds page ID to each answer + questionnaire.sections.forEach((section) => { + section.folders.forEach((folder) => { + folder.pages.forEach((page) => { + page.answers.forEach((answer) => { + answer.page = { + id: page.id, + }; + }); + }); + }); + }); + }); + it("should return empty array when questionnaire or ID not provided", () => { expect(getPreviousContent()).toHaveLength(0); }); + it("should return mutually exclusive answers when expression's left side answer is on a different page", () => { + questionnaire.sections[1].folders[0].pages[1].answers[1].type = + MUTUALLY_EXCLUSIVE; + + const previousContent = getPreviousContent({ + questionnaire, + id: questionnaire.sections[1].folders[0].pages[1].id, + includeTargetPage: true, + expressionGroup: { + operator: "And", + expressions: [ + { + left: { + id: questionnaire.sections[1].folders[0].pages[0].answers[0].id, + page: { + id: questionnaire.sections[1].folders[0].pages[0].id, + }, + }, + }, + ], + }, + }); + + // Tests previousContent[0] matches the questionnaire's first section + expect(previousContent).toHaveLength(2); + expect(previousContent[0]).toMatchObject(questionnaire.sections[0]); + + // Tests previousContent[1]'s first folder's first page matches the questionnaire's second section's first folder's first page + expect(previousContent[1].folders[0].pages).toHaveLength(2); + expect(previousContent[1].folders[0].pages[0].answers).toHaveLength(2); + expect(previousContent[1].folders[0].pages[0]).toMatchObject( + questionnaire.sections[1].folders[0].pages[0] + ); + + // Tests previousContent[1]'s first folder's second page matches the questionnaire's second section's first folder's second page + expect(previousContent[1].folders[0].pages[1].answers).toHaveLength(2); + expect(previousContent[1].folders[0].pages[1]).toMatchObject( + questionnaire.sections[1].folders[0].pages[1] + ); + }); + + it("should not return mutually exclusive answers on the same page as the expression's left side answer", () => { + questionnaire.sections[1].folders[0].pages[1].answers[1].type = + MUTUALLY_EXCLUSIVE; + + const previousContent = getPreviousContent({ + questionnaire, + id: questionnaire.sections[1].folders[0].pages[1].id, + includeTargetPage: true, + expressionGroup: { + operator: "And", + expressions: [ + { + left: { + id: questionnaire.sections[1].folders[0].pages[1].answers[0].id, + page: { + id: questionnaire.sections[1].folders[0].pages[1].id, + }, + }, + }, + ], + }, + }); + + // Tests previousContent[0] matches the questionnaire's first section + expect(previousContent).toHaveLength(2); + expect(previousContent[0]).toMatchObject(questionnaire.sections[0]); + + // Tests previousContent[1]'s first folder's first page matches the questionnaire's second section's first folder's first page + expect(previousContent[1].folders[0].pages).toHaveLength(2); + expect(previousContent[1].folders[0].pages[0].answers).toHaveLength(2); + expect(previousContent[1].folders[0].pages[0]).toMatchObject( + questionnaire.sections[1].folders[0].pages[0] + ); + + // Tests mutually exclusive answer is removed from previousContent[1]'s first folder's second page + expect(previousContent[1].folders[0].pages[1].answers).toHaveLength(1); + expect(previousContent[1].folders[0].pages[1].answers[0]).toMatchObject( + questionnaire.sections[1].folders[0].pages[1].answers[0] + ); + expect( + previousContent[1].folders[0].pages[1].answers.some( + (answer) => answer.type === MUTUALLY_EXCLUSIVE + ) + ).toBe(false); + }); + + it("should not return answers on the same page as expression's left side answer when left side answer is mutually exclusive", () => { + questionnaire.sections[1].folders[0].pages[1].answers[1].type = + MUTUALLY_EXCLUSIVE; + + const previousContent = getPreviousContent({ + questionnaire, + id: questionnaire.sections[1].folders[0].pages[1].id, + includeTargetPage: true, + expressionGroup: { + operator: "And", + expressions: [ + { + left: { + id: questionnaire.sections[1].folders[0].pages[1].answers[1].id, + type: MUTUALLY_EXCLUSIVE, + page: { + id: questionnaire.sections[1].folders[0].pages[1].id, + }, + }, + }, + ], + }, + }); + + // Tests previousContent[0] matches the questionnaire's first section + expect(previousContent).toHaveLength(2); + expect(previousContent[0]).toMatchObject(questionnaire.sections[0]); + + // Tests previousContent[1]'s first folder's first page matches the questionnaire's second section's first folder's first page, and that the second page has been removed + expect(previousContent[1].folders[0].pages).toHaveLength(1); + expect(previousContent[1].folders[0].pages[0].answers).toHaveLength(2); + expect(previousContent[1].folders[0].pages[0]).toMatchObject( + questionnaire.sections[1].folders[0].pages[0] + ); + }); + it("should return empty array on questionnaire introduction page", () => { const previousSections = getPreviousContent({ questionnaire, From 4d4209d4091939a5181eda2b9912c837d5cb6d8a Mon Sep 17 00:00:00 2001 From: farres1 Date: Thu, 23 May 2024 08:36:53 +0100 Subject: [PATCH 06/15] Update comments --- .../src/utils/getContentBeforeEntity.test.js | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/eq-author/src/utils/getContentBeforeEntity.test.js b/eq-author/src/utils/getContentBeforeEntity.test.js index fbea5ca298..dc3f6856d6 100644 --- a/eq-author/src/utils/getContentBeforeEntity.test.js +++ b/eq-author/src/utils/getContentBeforeEntity.test.js @@ -59,19 +59,23 @@ describe("utils/getPreviousAnswers", () => { }, }); - // Tests previousContent[0] matches the questionnaire's first section expect(previousContent).toHaveLength(2); + + // Tests previousContent[0] matches the questionnaire's first section expect(previousContent[0]).toMatchObject(questionnaire.sections[0]); - // Tests previousContent[1]'s first folder's first page matches the questionnaire's second section's first folder's first page + // Tests previousContent[1]'s first folder has two pages expect(previousContent[1].folders[0].pages).toHaveLength(2); + // Tests previousContent[1]'s first folder's first page has two answers expect(previousContent[1].folders[0].pages[0].answers).toHaveLength(2); + // Tests previousContent[1]'s first folder's first page matches the questionnaire's second section's first folder's first page expect(previousContent[1].folders[0].pages[0]).toMatchObject( questionnaire.sections[1].folders[0].pages[0] ); - // Tests previousContent[1]'s first folder's second page matches the questionnaire's second section's first folder's second page + // Tests previousContent[1]'s first folder's second page has two answers expect(previousContent[1].folders[0].pages[1].answers).toHaveLength(2); + // Tests previousContent[1]'s first folder's second page matches the questionnaire's second section's first folder's second page expect(previousContent[1].folders[0].pages[1]).toMatchObject( questionnaire.sections[1].folders[0].pages[1] ); @@ -100,18 +104,21 @@ describe("utils/getPreviousAnswers", () => { }, }); - // Tests previousContent[0] matches the questionnaire's first section expect(previousContent).toHaveLength(2); + + // Tests previousContent[0] matches the questionnaire's first section expect(previousContent[0]).toMatchObject(questionnaire.sections[0]); - // Tests previousContent[1]'s first folder's first page matches the questionnaire's second section's first folder's first page + // Tests previousContent[1]'s first folder has two pages expect(previousContent[1].folders[0].pages).toHaveLength(2); + // Tests previousContent[1]'s first folder's first page has two answers expect(previousContent[1].folders[0].pages[0].answers).toHaveLength(2); + // Tests previousContent[1]'s first folder's first page matches the questionnaire's second section's first folder's first page expect(previousContent[1].folders[0].pages[0]).toMatchObject( questionnaire.sections[1].folders[0].pages[0] ); - // Tests mutually exclusive answer is removed from previousContent[1]'s first folder's second page + // Tests to assert that the mutually exclusive answer is removed from previousContent[1]'s first folder's second page expect(previousContent[1].folders[0].pages[1].answers).toHaveLength(1); expect(previousContent[1].folders[0].pages[1].answers[0]).toMatchObject( questionnaire.sections[1].folders[0].pages[1].answers[0] @@ -147,13 +154,16 @@ describe("utils/getPreviousAnswers", () => { }, }); - // Tests previousContent[0] matches the questionnaire's first section expect(previousContent).toHaveLength(2); + + // Tests previousContent[0] matches the questionnaire's first section expect(previousContent[0]).toMatchObject(questionnaire.sections[0]); - // Tests previousContent[1]'s first folder's first page matches the questionnaire's second section's first folder's first page, and that the second page has been removed + // Tests previousContent[1]'s first folder has one page (the second page has been removed as left side answer is mutually exclusive) expect(previousContent[1].folders[0].pages).toHaveLength(1); + // Tests previousContent[1]'s first folder's first page has two answers expect(previousContent[1].folders[0].pages[0].answers).toHaveLength(2); + // Tests previousContent[1]'s first folder's first page matches the questionnaire's second section's first folder's first page expect(previousContent[1].folders[0].pages[0]).toMatchObject( questionnaire.sections[1].folders[0].pages[0] ); From ce30a1d5ad61b354ff83d9adf2a6c5e3a743c6ab Mon Sep 17 00:00:00 2001 From: farres1 Date: Wed, 29 May 2024 12:23:05 +0100 Subject: [PATCH 07/15] Update value to handle the same error across different answers in an expression --- eq-author-api/src/validation/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eq-author-api/src/validation/index.js b/eq-author-api/src/validation/index.js index fcc44578a8..f389c927e2 100644 --- a/eq-author-api/src/validation/index.js +++ b/eq-author-api/src/validation/index.js @@ -55,7 +55,7 @@ module.exports = (questionnaire) => { for (const err of validate.errors) { if (err.keyword === "errorMessage") { - const key = `${err.instancePath} ${err.message}`; + const key = `${err.instancePath} ${err.message} ${err.field}`; if (uniqueErrorMessages[key]) { continue; From 6190bbd5fb1c9f6f5afca39d945a4bf893ccc2a9 Mon Sep 17 00:00:00 2001 From: farres1 Date: Wed, 29 May 2024 16:03:22 +0100 Subject: [PATCH 08/15] Display error for AND logic conditions containing mutually exclusive answers on the same page as another answer in the expression group --- .../validateRoutingLogicalAND.js | 35 +++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/eq-author-api/src/validation/customKeywords/validateRoutingLogicalAND.js b/eq-author-api/src/validation/customKeywords/validateRoutingLogicalAND.js index 6ac57084c4..ad3e66979f 100644 --- a/eq-author-api/src/validation/customKeywords/validateRoutingLogicalAND.js +++ b/eq-author-api/src/validation/customKeywords/validateRoutingLogicalAND.js @@ -1,11 +1,16 @@ const { groupBy } = require("lodash"); -const { getAnswerById } = require("../../../schema/resolvers/utils"); +const { + getAnswerById, + getPageByAnswerId, +} = require("../../../schema/resolvers/utils"); + const { CURRENCY, NUMBER, PERCENTAGE, UNIT, CHECKBOX, + MUTUALLY_EXCLUSIVE, } = require("../../../constants/answerTypes"); const createValidationError = require("../createValidationError"); const { ERR_LOGICAL_AND } = require("../../../constants/validationErrorCodes"); @@ -21,6 +26,7 @@ module.exports = (ajv) => { _parentSchema, { rootData: questionnaire, instancePath } ) { + const allExpressions = expressions; const invalidAnswerIds = new Set(); const expressionsByAnswerId = groupBy(expressions, "left.answerId"); const potentialConflicts = Object.entries(expressionsByAnswerId).filter( @@ -38,8 +44,33 @@ module.exports = (ajv) => { return addError(answerId); } - // Bail out if answer isn't numerical or checkbox - remaining code validates number-type answers const answer = getAnswerById({ questionnaire }, answerId); + const expressionsContainMutuallyExclusive = allExpressions.some( + (expression) => + getAnswerById({ questionnaire }, expression.left.answerId)?.type === + MUTUALLY_EXCLUSIVE + ); + const allExpressionAnswerIds = allExpressions.map( + (expression) => expression.left.answerId + ); + const pageIdsForExpressionAnswers = allExpressionAnswerIds.map( + (answerId) => getPageByAnswerId({ questionnaire }, answerId)?.id + ); + + // Set removes duplicate values from pageIdsForExpressionAnswers array - if the length of the array is different to the size of the set then the array contains duplicates + const expressionsContainDuplicatePageIds = + new Set(pageIdsForExpressionAnswers).size !== + pageIdsForExpressionAnswers.length; + + // If any of the expressions contain a mutually exclusive answer and there are multiple answers from the same page then add an error + if ( + expressionsContainMutuallyExclusive && + expressionsContainDuplicatePageIds + ) { + return addError(answerId); + } + + // Bail out if answer isn't numerical or checkbox - remaining code validates number-type answers if ( !answer || ![CURRENCY, NUMBER, UNIT, PERCENTAGE, CHECKBOX].includes(answer.type) From ebf05173649b4dc9f960032c005b41b59718880e Mon Sep 17 00:00:00 2001 From: farres1 Date: Thu, 30 May 2024 09:34:38 +0100 Subject: [PATCH 09/15] Add test for mutually exclusive answer used in logic with answer on the same page error --- eq-author-api/schema/tests/routing.test.js | 111 ++++++++++++++++++++- 1 file changed, 110 insertions(+), 1 deletion(-) diff --git a/eq-author-api/schema/tests/routing.test.js b/eq-author-api/schema/tests/routing.test.js index 34f10ea4e4..6eaa5d0f53 100644 --- a/eq-author-api/schema/tests/routing.test.js +++ b/eq-author-api/schema/tests/routing.test.js @@ -1,5 +1,10 @@ const { buildContext } = require("../../tests/utils/contextBuilder"); -const { RADIO, NUMBER, DATE } = require("../../constants/answerTypes"); +const { + RADIO, + NUMBER, + DATE, + MUTUALLY_EXCLUSIVE, +} = require("../../constants/answerTypes"); const executeQuery = require("../../tests/utils/executeQuery"); const { @@ -21,6 +26,7 @@ const { ERR_ANSWER_NOT_SELECTED, ERR_RIGHTSIDE_NO_VALUE, ERR_RIGHTSIDE_NO_CONDITION, + ERR_LOGICAL_AND, } = require("../../constants/validationErrorCodes"); const { @@ -59,6 +65,10 @@ describe("routing", () => { { type: NUMBER, }, + { + id: "mutually-exclusive-answer", + type: MUTUALLY_EXCLUSIVE, + }, ], routing: {}, }, @@ -276,6 +286,105 @@ describe("routing", () => { expect(errors[0].errorCode).toBe(ERR_ANSWER_NOT_SELECTED); }); + it("should have validation errors when expression group contains mutually exclusive answer and answer from same page", async () => { + config.sections[0].folders[0].pages[0].routing = { + rules: [{ expressionGroup: {} }], + }; + + const ctx = await buildContext(config); + const { questionnaire } = ctx; + + const firstPage = questionnaire.sections[0].folders[0].pages[0]; + const expressionGroup = firstPage.routing.rules[0].expressionGroup; + const expressions = expressionGroup.expressions; + + await executeQuery( + createBinaryExpressionMutation, + { + input: { + expressionGroupId: expressionGroup.id, + }, + }, + ctx + ); + + await executeQuery( + updateExpressionGroupMutation, + { + input: { + id: expressionGroup.id, + operator: "And", + }, + }, + ctx + ); + + await executeQuery( + updateLeftSideMutation, + { + input: { + expressionId: expressions[0].id, + answerId: firstPage.answers[0].id, + }, + }, + ctx + ); + + await executeQuery( + updateLeftSideMutation, + { + input: { + expressionId: expressions[1].id, + answerId: firstPage.answers[1].id, + }, + }, + ctx + ); + + await executeQuery( + updateBinaryExpressionMutation, + { + input: { + id: expressions[0].id, + condition: "Equal", + }, + }, + ctx + ); + + await executeQuery( + updateRightSideMutation, + { + input: { + expressionId: expressions[0].id, + customValue: { + number: 5, + }, + }, + }, + ctx + ); + + await executeQuery( + updateRightSideMutation, + { + input: { + expressionId: expressions[1].id, + selectedOptions: [firstPage.answers[1].options[0].id], + }, + }, + ctx + ); + + const result = await queryPage(ctx, firstPage.id); + + const errors = + result.routing.rules[0].expressionGroup.validationErrorInfo.errors; + expect(errors).toHaveLength(2); + expect(errors[0].errorCode).toBe(ERR_LOGICAL_AND); + expect(errors[1].errorCode).toBe(ERR_LOGICAL_AND); + }); + it("does not have validation errors if there are none", async () => { config.sections[0].folders[0].pages[0].routing = { rules: [{ expressionGroup: {} }], From 2583cbbc198ac906aaab04b3e46a4ca67b68b553 Mon Sep 17 00:00:00 2001 From: farres1 Date: Thu, 30 May 2024 10:42:12 +0100 Subject: [PATCH 10/15] Fix validation error displayed on non-conflicting answers when expression group has mutually exclusive answer from same page as another answer in expression group --- .../customKeywords/validateRoutingLogicalAND.js | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/eq-author-api/src/validation/customKeywords/validateRoutingLogicalAND.js b/eq-author-api/src/validation/customKeywords/validateRoutingLogicalAND.js index ad3e66979f..aae8fde325 100644 --- a/eq-author-api/src/validation/customKeywords/validateRoutingLogicalAND.js +++ b/eq-author-api/src/validation/customKeywords/validateRoutingLogicalAND.js @@ -45,6 +45,7 @@ module.exports = (ajv) => { } const answer = getAnswerById({ questionnaire }, answerId); + const page = getPageByAnswerId({ questionnaire }, answerId); const expressionsContainMutuallyExclusive = allExpressions.some( (expression) => getAnswerById({ questionnaire }, expression.left.answerId)?.type === @@ -57,6 +58,14 @@ module.exports = (ajv) => { (answerId) => getPageByAnswerId({ questionnaire }, answerId)?.id ); + // Creates an array of all pageIds that are found in pageIdsForExpressionAnswers more than once - self represents the pageIdsForExpressionAnswers array itself + // If the index of the looped id is not equal to the looping index then the id is a duplicate + const duplicatePageIds = pageIdsForExpressionAnswers.filter( + (id, index, self) => { + return self.indexOf(id) !== index; + } + ); + // Set removes duplicate values from pageIdsForExpressionAnswers array - if the length of the array is different to the size of the set then the array contains duplicates const expressionsContainDuplicatePageIds = new Set(pageIdsForExpressionAnswers).size !== @@ -67,7 +76,13 @@ module.exports = (ajv) => { expressionsContainMutuallyExclusive && expressionsContainDuplicatePageIds ) { - return addError(answerId); + // Prevents the error being added to answers that are not on the same page as the mutually exclusive answer + if ( + duplicatePageIds.includes(page.id) && + page.answers.some((answer) => answer.type === MUTUALLY_EXCLUSIVE) + ) { + return addError(answerId); + } } // Bail out if answer isn't numerical or checkbox - remaining code validates number-type answers From 357ce3a4c8eed6f6672d58571c160b0a93ace552 Mon Sep 17 00:00:00 2001 From: farres1 Date: Thu, 30 May 2024 11:04:27 +0100 Subject: [PATCH 11/15] Replace value to check if expression contains duplicate pageIds --- .../customKeywords/validateRoutingLogicalAND.js | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/eq-author-api/src/validation/customKeywords/validateRoutingLogicalAND.js b/eq-author-api/src/validation/customKeywords/validateRoutingLogicalAND.js index aae8fde325..a8986139f5 100644 --- a/eq-author-api/src/validation/customKeywords/validateRoutingLogicalAND.js +++ b/eq-author-api/src/validation/customKeywords/validateRoutingLogicalAND.js @@ -66,15 +66,10 @@ module.exports = (ajv) => { } ); - // Set removes duplicate values from pageIdsForExpressionAnswers array - if the length of the array is different to the size of the set then the array contains duplicates - const expressionsContainDuplicatePageIds = - new Set(pageIdsForExpressionAnswers).size !== - pageIdsForExpressionAnswers.length; - - // If any of the expressions contain a mutually exclusive answer and there are multiple answers from the same page then add an error + // If any of the expressions contain a mutually exclusive answer and there are multiple answers from the same page if ( expressionsContainMutuallyExclusive && - expressionsContainDuplicatePageIds + duplicatePageIds.length > 0 ) { // Prevents the error being added to answers that are not on the same page as the mutually exclusive answer if ( From 564654955bdab1f70ebe951cd58eae6a1b77e223 Mon Sep 17 00:00:00 2001 From: farres1 Date: Mon, 3 Jun 2024 09:06:07 +0100 Subject: [PATCH 12/15] Fix duplicated content not displayed before refresh --- eq-author/src/graphql/fragments/answer.graphql | 3 +++ 1 file changed, 3 insertions(+) diff --git a/eq-author/src/graphql/fragments/answer.graphql b/eq-author/src/graphql/fragments/answer.graphql index 4cef7afb26..311582ad46 100644 --- a/eq-author/src/graphql/fragments/answer.graphql +++ b/eq-author/src/graphql/fragments/answer.graphql @@ -10,4 +10,7 @@ fragment Answer on Answer { displayName qCode advancedProperties + page { + id + } } From 41986b102179e51a703db24797fd1b71d6b245cf Mon Sep 17 00:00:00 2001 From: farres1 Date: Mon, 3 Jun 2024 13:16:37 +0100 Subject: [PATCH 13/15] Fix bug preventing switching answer to another answer in the same page --- .../RoutingAnswerContentPicker.js | 5 +- eq-author/src/utils/getContentBeforeEntity.js | 69 +++++++++++++++---- 2 files changed, 59 insertions(+), 15 deletions(-) diff --git a/eq-author/src/App/page/Logic/BinaryExpressionEditor/RoutingAnswerContentPicker.js b/eq-author/src/App/page/Logic/BinaryExpressionEditor/RoutingAnswerContentPicker.js index e2d272d970..f7b7a6240a 100644 --- a/eq-author/src/App/page/Logic/BinaryExpressionEditor/RoutingAnswerContentPicker.js +++ b/eq-author/src/App/page/Logic/BinaryExpressionEditor/RoutingAnswerContentPicker.js @@ -27,6 +27,7 @@ const RoutingAnswerContentPicker = ({ includeSelf, selectedContentDisplayName, expressionGroup, + selectedId, ...otherProps }) => { const { questionnaire } = useQuestionnaire(); @@ -40,8 +41,9 @@ const RoutingAnswerContentPicker = ({ includeTargetPage: includeSelf, preprocessAnswers, expressionGroup, + selectedId, }), - [questionnaire, pageId, includeSelf, expressionGroup] + [questionnaire, pageId, includeSelf, expressionGroup, selectedId] ); const filteredPreviousAnswers = previousAnswers.map((answer) => { @@ -80,6 +82,7 @@ RoutingAnswerContentPicker.propTypes = { PropTypes.object, PropTypes.string, ]), + selectedId: PropTypes.string, expressionGroup: PropTypes.object, //eslint-disable-line }; diff --git a/eq-author/src/utils/getContentBeforeEntity.js b/eq-author/src/utils/getContentBeforeEntity.js index 7244beabd6..02ebd974ba 100644 --- a/eq-author/src/utils/getContentBeforeEntity.js +++ b/eq-author/src/utils/getContentBeforeEntity.js @@ -1,6 +1,7 @@ import { remove } from "lodash"; import isListCollectorPageType from "utils/isListCollectorPageType"; +import { getPageByAnswerId } from "utils/questionnaireUtils"; import { MUTUALLY_EXCLUSIVE } from "constants/answer-types"; @@ -11,9 +12,11 @@ const getContentBeforeEntity = ( id, preprocessAnswers, includeTarget, - expressionGroup + expressionGroup, + selectedId ) => { const sections = []; + const selectedAnswerPageId = getPageByAnswerId(questionnaire, selectedId)?.id; for (const section of questionnaire.sections) { if (section.id === id) { @@ -52,20 +55,50 @@ const getContentBeforeEntity = ( if (expressionGroup?.operator === "And") { expressionGroup.expressions.forEach((expression) => { if (expression?.left?.page?.id) { - answers = answers.filter((answer) => { - // If the expression's left side answer is on the same page as the looped answer - if (expression.left.page.id === answer.page.id) { - // If the expression's left side answer is mutually exclusive, do not include the looped answer (as looped answer and expression answer are on the same page) - if (expression.left.type === MUTUALLY_EXCLUSIVE) { - return false; + // Filters answers if the expression's left side page is not the selected answer's page - allows selection of answers on the same page as the selected answer + if (expression.left.page.id !== selectedAnswerPageId) { + answers = answers.filter((answer) => { + // If the expression's left side answer is on the same page as the looped answer + if (expression.left.page.id === answer.page.id) { + // If the expression's left side answer is mutually exclusive, do not include the looped answer (as looped answer and expression answer are on the same page) + if (expression.left.type === MUTUALLY_EXCLUSIVE) { + return false; + } + // If the expression's left side answer is not mutually exclusive, only include the looped answer if it is also not mutually exclusive (as looped answer and expression answer are on the same page) + else { + return answer.type !== MUTUALLY_EXCLUSIVE; + } } - // If the expression's left side answer is not mutually exclusive, only include the looped answer if it is also not mutually exclusive (as looped answer and expression answer are on the same page) - else { - return answer.type !== MUTUALLY_EXCLUSIVE; + return true; + }); + } + // Filters answers if the expression's left side page matches the selected answer's page - allows selection of answers on the same page as the selected answer + else { + // Gets all expressions using answers from the same page as the selected answer + const expressionsFromSamePage = + expressionGroup.expressions.filter( + (expression) => + expression?.left?.page?.id === selectedAnswerPageId + ); + + /* + Checks if the expression group includes more than one expression using the selected answer's page + (to allow selection of answers on the same page as the selected answer when it is the only expression using the selected answer's page) + */ + const expressionGroupIncludesExpressionFromSamePage = + expressionsFromSamePage.length > 1; // Checks length to see if there is more than one expression in the expression group using the selected answer's page + + // Filters out answers on the same page as the selected answer if the expression group includes more than one expression using the selected answer's page + answers = answers.filter((answer) => { + if ( + answer.page.id === selectedAnswerPageId && + expressionGroupIncludesExpressionFromSamePage + ) { + return false; } - } - return true; - }); + return true; + }); + } } }); } @@ -100,12 +133,19 @@ export const stripEmpty = (sections) => { return sections; }; +// const expressionGroupHasAnswerFromSamePage = (expressionGroup, pageId) => { +// expressionGroup.expressions.forEach((expression) => +// console.log("expression", expression) +// ); +// }; + export default ({ questionnaire, id, preprocessAnswers = identity, includeTargetPage = false, expressionGroup, + selectedId, } = {}) => { if (!questionnaire || !id || questionnaire?.introduction?.id === id) { return []; @@ -117,7 +157,8 @@ export default ({ id, preprocessAnswers, includeTargetPage, - expressionGroup + expressionGroup, + selectedId ).filter(({ folders }) => folders.length) ); }; From 7c5f9c5fe3faf85607574c541f75e9ea4a9076ac Mon Sep 17 00:00:00 2001 From: farres1 Date: Mon, 3 Jun 2024 14:15:56 +0100 Subject: [PATCH 14/15] Add tests for selecting answers on same page as selected answer in expression --- eq-author/src/utils/getContentBeforeEntity.js | 21 +-- .../src/utils/getContentBeforeEntity.test.js | 171 +++++++++++++++++- 2 files changed, 176 insertions(+), 16 deletions(-) diff --git a/eq-author/src/utils/getContentBeforeEntity.js b/eq-author/src/utils/getContentBeforeEntity.js index 02ebd974ba..f623ed053b 100644 --- a/eq-author/src/utils/getContentBeforeEntity.js +++ b/eq-author/src/utils/getContentBeforeEntity.js @@ -16,7 +16,7 @@ const getContentBeforeEntity = ( selectedId ) => { const sections = []; - const selectedAnswerPageId = getPageByAnswerId(questionnaire, selectedId)?.id; + const selectedAnswerPage = getPageByAnswerId(questionnaire, selectedId); for (const section of questionnaire.sections) { if (section.id === id) { @@ -56,7 +56,7 @@ const getContentBeforeEntity = ( expressionGroup.expressions.forEach((expression) => { if (expression?.left?.page?.id) { // Filters answers if the expression's left side page is not the selected answer's page - allows selection of answers on the same page as the selected answer - if (expression.left.page.id !== selectedAnswerPageId) { + if (expression.left.page.id !== selectedAnswerPage?.id) { answers = answers.filter((answer) => { // If the expression's left side answer is on the same page as the looped answer if (expression.left.page.id === answer.page.id) { @@ -78,7 +78,7 @@ const getContentBeforeEntity = ( const expressionsFromSamePage = expressionGroup.expressions.filter( (expression) => - expression?.left?.page?.id === selectedAnswerPageId + expression?.left?.page?.id === selectedAnswerPage?.id ); /* @@ -88,11 +88,14 @@ const getContentBeforeEntity = ( const expressionGroupIncludesExpressionFromSamePage = expressionsFromSamePage.length > 1; // Checks length to see if there is more than one expression in the expression group using the selected answer's page - // Filters out answers on the same page as the selected answer if the expression group includes more than one expression using the selected answer's page + // Filters out answers on the same page as the selected answer if the expression group includes more than one expression using the selected answer's page and the selected answer's page includes a mutually exclusive answer answers = answers.filter((answer) => { if ( - answer.page.id === selectedAnswerPageId && - expressionGroupIncludesExpressionFromSamePage + answer.page.id === selectedAnswerPage?.id && + expressionGroupIncludesExpressionFromSamePage && + selectedAnswerPage?.answers.some( + (answer) => answer.type === MUTUALLY_EXCLUSIVE + ) ) { return false; } @@ -133,12 +136,6 @@ export const stripEmpty = (sections) => { return sections; }; -// const expressionGroupHasAnswerFromSamePage = (expressionGroup, pageId) => { -// expressionGroup.expressions.forEach((expression) => -// console.log("expression", expression) -// ); -// }; - export default ({ questionnaire, id, diff --git a/eq-author/src/utils/getContentBeforeEntity.test.js b/eq-author/src/utils/getContentBeforeEntity.test.js index dc3f6856d6..21c93d32f9 100644 --- a/eq-author/src/utils/getContentBeforeEntity.test.js +++ b/eq-author/src/utils/getContentBeforeEntity.test.js @@ -4,7 +4,7 @@ import { } from "tests/utils/createMockQuestionnaire"; import getPreviousContent from "./getContentBeforeEntity"; -import { MUTUALLY_EXCLUSIVE } from "constants/answer-types"; +import { MUTUALLY_EXCLUSIVE, NUMBER } from "constants/answer-types"; let questionnaire = buildQuestionnaire({ sectionCount: 2, @@ -49,7 +49,8 @@ describe("utils/getPreviousAnswers", () => { expressions: [ { left: { - id: questionnaire.sections[1].folders[0].pages[0].answers[0].id, + answerId: + questionnaire.sections[1].folders[0].pages[0].answers[0].id, page: { id: questionnaire.sections[1].folders[0].pages[0].id, }, @@ -94,7 +95,8 @@ describe("utils/getPreviousAnswers", () => { expressions: [ { left: { - id: questionnaire.sections[1].folders[0].pages[1].answers[0].id, + answerId: + questionnaire.sections[1].folders[0].pages[1].answers[0].id, page: { id: questionnaire.sections[1].folders[0].pages[1].id, }, @@ -143,7 +145,8 @@ describe("utils/getPreviousAnswers", () => { expressions: [ { left: { - id: questionnaire.sections[1].folders[0].pages[1].answers[1].id, + answerId: + questionnaire.sections[1].folders[0].pages[1].answers[1].id, type: MUTUALLY_EXCLUSIVE, page: { id: questionnaire.sections[1].folders[0].pages[1].id, @@ -169,6 +172,166 @@ describe("utils/getPreviousAnswers", () => { ); }); + it("should allow selection of answers on the same page as the selected answer when it is the only expression using the selected answer's page", () => { + questionnaire.sections[1].folders[0].pages[1].answers[1].type = + MUTUALLY_EXCLUSIVE; + + const previousContent = getPreviousContent({ + questionnaire, + id: questionnaire.sections[1].folders[0].pages[1].id, + includeTargetPage: true, + expressionGroup: { + operator: "And", + expressions: [ + { + left: { + answerId: + questionnaire.sections[1].folders[0].pages[1].answers[0].id, + page: { + id: questionnaire.sections[1].folders[0].pages[1].id, + }, + }, + }, + { + left: { + answerId: + questionnaire.sections[1].folders[0].pages[0].answers[0].id, + page: { + id: questionnaire.sections[1].folders[0].pages[0].id, + }, + }, + }, + ], + }, + selectedId: questionnaire.sections[1].folders[0].pages[1].answers[0].id, + }); + + expect(previousContent).toHaveLength(2); + + // Tests previousContent[0] matches the questionnaire's first section + expect(previousContent[0]).toMatchObject(questionnaire.sections[0]); + + // Tests previousContent[1]'s first folder has two pages + expect(previousContent[1].folders[0].pages).toHaveLength(2); + // Tests previousContent[1]'s first folder's first page has two answers + expect(previousContent[1].folders[0].pages[0].answers).toHaveLength(2); + // Tests previousContent[1]'s first folder's first page matches the questionnaire's second section's first folder's first page + expect(previousContent[1].folders[0].pages[0]).toMatchObject( + questionnaire.sections[1].folders[0].pages[0] + ); + + // Tests previousContent[1]'s first folder's second page has two answers + expect(previousContent[1].folders[0].pages[1].answers).toHaveLength(2); + // Tests previousContent[1]'s first folder's second page matches the questionnaire's second section's first folder's second page + expect(previousContent[1].folders[0].pages[1]).toMatchObject( + questionnaire.sections[1].folders[0].pages[1] + ); + }); + + it("should not allow selection of answers on the same page as the selected answer when other expressions in expression group use the selected answer's page and selected answer's page includes a mutually exclusive answer", () => { + questionnaire.sections[1].folders[0].pages[1].answers[1].type = + MUTUALLY_EXCLUSIVE; + + const previousContent = getPreviousContent({ + questionnaire, + id: questionnaire.sections[1].folders[0].pages[1].id, + includeTargetPage: true, + expressionGroup: { + operator: "And", + expressions: [ + { + left: { + answerId: + questionnaire.sections[1].folders[0].pages[1].answers[0].id, + page: { + id: questionnaire.sections[1].folders[0].pages[1].id, + }, + }, + }, + { + left: { + answerId: + questionnaire.sections[1].folders[0].pages[1].answers[1].id, + page: { + id: questionnaire.sections[1].folders[0].pages[1].id, + }, + }, + }, + ], + }, + selectedId: questionnaire.sections[1].folders[0].pages[1].answers[1].id, + }); + + expect(previousContent).toHaveLength(2); + + // Tests previousContent[0] matches the questionnaire's first section + expect(previousContent[0]).toMatchObject(questionnaire.sections[0]); + + // Tests previousContent[1]'s first folder has one page (the second page has been removed as another expression is using an answer from the same page) + expect(previousContent[1].folders[0].pages).toHaveLength(1); + // Tests previousContent[1]'s first folder's first page has two answers + expect(previousContent[1].folders[0].pages[0].answers).toHaveLength(2); + // Tests previousContent[1]'s first folder's first page matches the questionnaire's second section's first folder's first page + expect(previousContent[1].folders[0].pages[0]).toMatchObject( + questionnaire.sections[1].folders[0].pages[0] + ); + }); + + it("should allow selection of answers on the same page as the selected answer when other expressions in expression group use the selected answer's page and selected answer's page does not include a mutually exclusive answer", () => { + questionnaire.sections[1].folders[0].pages[1].answers[1].type = NUMBER; + + const previousContent = getPreviousContent({ + questionnaire, + id: questionnaire.sections[1].folders[0].pages[1].id, + includeTargetPage: true, + expressionGroup: { + operator: "And", + expressions: [ + { + left: { + answerId: + questionnaire.sections[1].folders[0].pages[1].answers[0].id, + page: { + id: questionnaire.sections[1].folders[0].pages[1].id, + }, + }, + }, + { + left: { + answerId: + questionnaire.sections[1].folders[0].pages[1].answers[1].id, + page: { + id: questionnaire.sections[1].folders[0].pages[1].id, + }, + }, + }, + ], + }, + selectedId: questionnaire.sections[1].folders[0].pages[1].answers[1].id, + }); + + expect(previousContent).toHaveLength(2); + + // Tests previousContent[0] matches the questionnaire's first section + expect(previousContent[0]).toMatchObject(questionnaire.sections[0]); + + // Tests previousContent[1]'s first folder has two pages + expect(previousContent[1].folders[0].pages).toHaveLength(2); + // Tests previousContent[1]'s first folder's first page has two answers + expect(previousContent[1].folders[0].pages[0].answers).toHaveLength(2); + // Tests previousContent[1]'s first folder's first page matches the questionnaire's second section's first folder's first page + expect(previousContent[1].folders[0].pages[0]).toMatchObject( + questionnaire.sections[1].folders[0].pages[0] + ); + + // Tests previousContent[1]'s first folder's second page has two answers + expect(previousContent[1].folders[0].pages[1].answers).toHaveLength(2); + // Tests previousContent[1]'s first folder's second page matches the questionnaire's second section's first folder's second page + expect(previousContent[1].folders[0].pages[1]).toMatchObject( + questionnaire.sections[1].folders[0].pages[1] + ); + }); + it("should return empty array on questionnaire introduction page", () => { const previousSections = getPreviousContent({ questionnaire, From 868f97b2d81b9d7936deb969937b024f7b2a99da Mon Sep 17 00:00:00 2001 From: farres1 Date: Tue, 4 Jun 2024 15:37:06 +0100 Subject: [PATCH 15/15] Fix validation error displayed when expression group contains a mutually exclusive answer from a different page --- .../validateRoutingLogicalAND.js | 50 +++++++++++-------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/eq-author-api/src/validation/customKeywords/validateRoutingLogicalAND.js b/eq-author-api/src/validation/customKeywords/validateRoutingLogicalAND.js index a8986139f5..1d639de540 100644 --- a/eq-author-api/src/validation/customKeywords/validateRoutingLogicalAND.js +++ b/eq-author-api/src/validation/customKeywords/validateRoutingLogicalAND.js @@ -46,11 +46,6 @@ module.exports = (ajv) => { const answer = getAnswerById({ questionnaire }, answerId); const page = getPageByAnswerId({ questionnaire }, answerId); - const expressionsContainMutuallyExclusive = allExpressions.some( - (expression) => - getAnswerById({ questionnaire }, expression.left.answerId)?.type === - MUTUALLY_EXCLUSIVE - ); const allExpressionAnswerIds = allExpressions.map( (expression) => expression.left.answerId ); @@ -58,28 +53,41 @@ module.exports = (ajv) => { (answerId) => getPageByAnswerId({ questionnaire }, answerId)?.id ); - // Creates an array of all pageIds that are found in pageIdsForExpressionAnswers more than once - self represents the pageIdsForExpressionAnswers array itself - // If the index of the looped id is not equal to the looping index then the id is a duplicate - const duplicatePageIds = pageIdsForExpressionAnswers.filter( - (id, index, self) => { - return self.indexOf(id) !== index; + /* + Creates an array of all pageIds that are found in pageIdsForExpressionAnswers more than once + If the index of the looped id is not equal to the looping index then the id is a duplicate + */ + const duplicatedPageIds = pageIdsForExpressionAnswers.filter( + (id, index) => { + return pageIdsForExpressionAnswers.indexOf(id) !== index; } ); - // If any of the expressions contain a mutually exclusive answer and there are multiple answers from the same page - if ( - expressionsContainMutuallyExclusive && - duplicatePageIds.length > 0 - ) { - // Prevents the error being added to answers that are not on the same page as the mutually exclusive answer - if ( - duplicatePageIds.includes(page.id) && - page.answers.some((answer) => answer.type === MUTUALLY_EXCLUSIVE) - ) { + // If there are multiple answers from the same page + if (duplicatedPageIds.length > 0) { + /* + Checks if any of the expressions' answers conflict with a mutually exclusive answer + Conflicts (returns true) if all of the following are met: + - The looping expression's answer in the expression group is a mutually exclusive answer + - The same expression's answer is on a page that has other answers from the same page in the logic rule + - The same expression's answer is on the same page as the current answer being validated + */ + const conflictsWithMutuallyExclusive = allExpressions.some( + (expression) => + getAnswerById({ questionnaire }, expression.left.answerId) + ?.type === MUTUALLY_EXCLUSIVE && + duplicatedPageIds.includes( + getPageByAnswerId({ questionnaire }, expression.left.answerId) + ?.id + ) && + getPageByAnswerId({ questionnaire }, expression.left.answerId) + ?.id === page.id + ); + + if (conflictsWithMutuallyExclusive) { return addError(answerId); } } - // Bail out if answer isn't numerical or checkbox - remaining code validates number-type answers if ( !answer ||