From 366117c798a4f63ed865f6dc3e9e249ae5e9d822 Mon Sep 17 00:00:00 2001 From: Joshua Melville Date: Thu, 11 Jan 2024 14:20:36 +0200 Subject: [PATCH 1/3] Fix for promptId related bugs This fixes bugs related to the lack of a promptId in certain contexts. The issue was caused by faulty selectors that assumed promptIndex (not ID!) was being passed into the stage component as a prop. I removed this during refactoring as it would cause the stage to re-render when the prompt changed (so was incorrect). The new implementation gets the promptId from the session, via the promptIndex. This is implemented within the `getPromptIndex` session selector. --- lib/interviewer/ducks/modules/session.js | 1 + .../selectors/__tests__/interface.test.js | 8 -------- lib/interviewer/selectors/interface.js | 13 ++++++++++--- lib/interviewer/selectors/name-generator.js | 17 +++++++++++------ lib/interviewer/selectors/prop.js | 13 ++++--------- 5 files changed, 26 insertions(+), 26 deletions(-) diff --git a/lib/interviewer/ducks/modules/session.js b/lib/interviewer/ducks/modules/session.js index db82d808..165c92d7 100644 --- a/lib/interviewer/ducks/modules/session.js +++ b/lib/interviewer/ducks/modules/session.js @@ -40,6 +40,7 @@ const getReducer = (network) => (state = initialState, action = {}) => { protocolUID, ...action.payload.session, network: action.payload.session.network ?? network(state.network, action), + promptIndex: 0, }, } } diff --git a/lib/interviewer/selectors/__tests__/interface.test.js b/lib/interviewer/selectors/__tests__/interface.test.js index 0d9b8b97..e970c9aa 100644 --- a/lib/interviewer/selectors/__tests__/interface.test.js +++ b/lib/interviewer/selectors/__tests__/interface.test.js @@ -139,14 +139,6 @@ const mockState = { describe('interface selector', () => { describe('memoed selectors', () => { - it('makeGetIds()', () => { - const selected = Interface.makeGetIds(); - expect(selected(mockState, mockProps)).toEqual({ - stageId: mockStage.id, - promptId: mockPrompt.id, - }); - }); - it('makeGetAdditionalAttributes()', () => { const selected = Interface.makeGetAdditionalAttributes(); expect(selected(mockState, mockProps)).toEqual({ 'b6f2c4b9-e42f-459b-8f59-a11a685f460d': 2 }); diff --git a/lib/interviewer/selectors/interface.js b/lib/interviewer/selectors/interface.js index bdc5b3f5..6a279607 100644 --- a/lib/interviewer/selectors/interface.js +++ b/lib/interviewer/selectors/interface.js @@ -1,8 +1,9 @@ import { filter, includes, intersection } from 'lodash'; import { getProtocolCodebook } from './protocol'; import { getNetwork, getNetworkEdges, getNetworkNodes } from './network'; -import { getPromptOtherVariable, getStageSubject, stagePromptIds, getPropPromptId, getPromptVariable, getSubjectType } from './prop'; +import { getPromptOtherVariable, getStageSubject, stagePromptIds, getPromptVariable, getSubjectType } from './prop'; import { createSelector } from '@reduxjs/toolkit'; +import { getPromptIndex, getPrompts } from './session'; // Selectors that are generic between interfaces @@ -117,9 +118,15 @@ export const makeGetStageNodeCount = () => { * Return a filtered node list containing only nodes where node IDs contains the current promptId. */ +export const getPromptId = createSelector( + getPrompts, + getPromptIndex, + (prompts, promptIndex) => prompts[promptIndex].id, +) + export const getNetworkNodesForPrompt = createSelector( getNetworkNodesForType, - getPropPromptId, + getPromptId, (nodes, promptId) => filter(nodes, (node) => includes(node.promptIDs, promptId)), ) @@ -133,7 +140,7 @@ export const makeNetworkNodesForPrompt = () => getNetworkNodesForPrompt; */ export const getNetworkNodesForOtherPrompts = createSelector( - getNetworkNodesForType, getPropPromptId, + getNetworkNodesForType, getPromptId, (nodes, promptId) => filter(nodes, (node) => !includes(node.promptIDs, promptId)), ); diff --git a/lib/interviewer/selectors/name-generator.js b/lib/interviewer/selectors/name-generator.js index c8fc103c..6ad3138e 100644 --- a/lib/interviewer/selectors/name-generator.js +++ b/lib/interviewer/selectors/name-generator.js @@ -1,7 +1,8 @@ import { has } from 'lodash'; import { getProtocolCodebook } from './protocol'; -import { getIds, getStageSubject, getSubjectType } from './prop'; +import { getStageSubject, getSubjectType, propStage } from './prop'; import { createSelector } from '@reduxjs/toolkit'; +import { getPromptId } from './interface'; // Selectors that are specific to the name generator @@ -25,11 +26,15 @@ const propPanels = (_, props) => props.stage.panels; export const getPromptModelData = createSelector( getStageSubject, - getIds, - ({ type }, ids) => ({ - type, - ...ids, - }), + propStage, + getPromptId, + ({ type }, { id }, promptId) => { + return { + type, + stageId: id, + promptId, + }; + }, ) export const makeGetPromptNodeModelData = () => getPromptModelData; diff --git a/lib/interviewer/selectors/prop.js b/lib/interviewer/selectors/prop.js index d864f7e7..d2b56b49 100644 --- a/lib/interviewer/selectors/prop.js +++ b/lib/interviewer/selectors/prop.js @@ -22,7 +22,10 @@ export const getAdditionalAttributes = (stage, prompt) => { export const propStage = (_, props) => props?.stage ?? null; export const propPrompt = (_, props) => props?.prompt ?? null; export const propStageId = (_, props) => props?.stage?.id ?? null; -export const getPropPromptId = (_, props) => props?.prompt?.id ?? null; + +// JRM - below not working due to prompt being removed from stage props (by +// design. If you come across something using this, refactor! +// export const getPropPromptId = (_, props) => props?.prompt?.id ?? null; export const getStageOrPromptSubject = (stage, prompt) => { return stage?.subject || prompt?.subject || null; @@ -42,14 +45,6 @@ export const getSubjectType = createSelector( export const makeGetSubject = () => getStageSubject; -// Returns current stage and prompt ID -export const getIds = createSelector( - propStageId, getPropPromptId, - (stageId, promptId) => ({ stageId, promptId }), -); - -export const makeGetIds = () => getIds; - export const getAdditionalAttributesSelector = createSelector( propStage, propPrompt, (stage, prompt) => getAdditionalAttributes(stage, prompt), From 4258004fabddf7e46bc18a95e04bdd1fe50c1adc Mon Sep 17 00:00:00 2001 From: Joshua Melville Date: Thu, 11 Jan 2024 14:24:51 +0200 Subject: [PATCH 2/3] small additional changes --- lib/interviewer/ducks/modules/session.js | 1 - lib/interviewer/selectors/session.ts | 10 +++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/interviewer/ducks/modules/session.js b/lib/interviewer/ducks/modules/session.js index 165c92d7..db82d808 100644 --- a/lib/interviewer/ducks/modules/session.js +++ b/lib/interviewer/ducks/modules/session.js @@ -40,7 +40,6 @@ const getReducer = (network) => (state = initialState, action = {}) => { protocolUID, ...action.payload.session, network: action.payload.session.network ?? network(state.network, action), - promptIndex: 0, }, } } diff --git a/lib/interviewer/selectors/session.ts b/lib/interviewer/selectors/session.ts index 02620720..7b7b655a 100644 --- a/lib/interviewer/selectors/session.ts +++ b/lib/interviewer/selectors/session.ts @@ -53,6 +53,11 @@ export const getCurrentStage = createSelector( }, ); +export const getCaseId = createDeepEqualSelector( + getActiveSession, + (session) => session?.caseId, +); + export const getPromptIndex = createSelector( getActiveSession, (session) => session?.promptIndex ?? 0, @@ -64,11 +69,6 @@ export const getCurrentPrompt = createSelector( (stage, promptIndex) => stage?.prompts?.[promptIndex], ); -export const getCaseId = createDeepEqualSelector( - getActiveSession, - (session) => session?.caseId, -); - export const getPrompts = createSelector( getCurrentStage, (stage) => stage?.prompts, From ebf165cbc4a612cc3748f7001746d3aa940675e4 Mon Sep 17 00:00:00 2001 From: Joshua Melville Date: Thu, 11 Jan 2024 14:30:25 +0200 Subject: [PATCH 3/3] small cleanup --- lib/interviewer/selectors/name-generator.js | 20 +++++++++++++++----- lib/interviewer/selectors/prop.js | 4 ---- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/lib/interviewer/selectors/name-generator.js b/lib/interviewer/selectors/name-generator.js index 6ad3138e..3ace7143 100644 --- a/lib/interviewer/selectors/name-generator.js +++ b/lib/interviewer/selectors/name-generator.js @@ -1,6 +1,6 @@ import { has } from 'lodash'; import { getProtocolCodebook } from './protocol'; -import { getStageSubject, getSubjectType, propStage } from './prop'; +import { getStageSubject, getSubjectType, propStage, propStageId } from './prop'; import { createSelector } from '@reduxjs/toolkit'; import { getPromptId } from './interface'; @@ -24,14 +24,24 @@ const stageCardOptions = (_, props) => props.stage.cardOptions; const stageSortOptions = (_, props) => props.stage.sortOptions; const propPanels = (_, props) => props.stage.panels; +const getIDs = createSelector( + propStageId, + getPromptId, + (stageId, promptId) => { + return { + stageId, + promptId, + }; + }, +) + export const getPromptModelData = createSelector( getStageSubject, - propStage, - getPromptId, - ({ type }, { id }, promptId) => { + getIDs, + ({ type }, { stageId, promptId }) => { return { type, - stageId: id, + stageId, promptId, }; }, diff --git a/lib/interviewer/selectors/prop.js b/lib/interviewer/selectors/prop.js index d2b56b49..8e380597 100644 --- a/lib/interviewer/selectors/prop.js +++ b/lib/interviewer/selectors/prop.js @@ -23,10 +23,6 @@ export const propStage = (_, props) => props?.stage ?? null; export const propPrompt = (_, props) => props?.prompt ?? null; export const propStageId = (_, props) => props?.stage?.id ?? null; -// JRM - below not working due to prompt being removed from stage props (by -// design. If you come across something using this, refactor! -// export const getPropPromptId = (_, props) => props?.prompt?.id ?? null; - export const getStageOrPromptSubject = (stage, prompt) => { return stage?.subject || prompt?.subject || null; }