Skip to content

Commit

Permalink
fix: Action Plan matrix item type render bugs (M2-8348) (#560)
Browse files Browse the repository at this point in the history 
* refactor: fix logic for generating matrix answers

Simplified and fixed logic for generating response text for matrix item
types (single selection per row, multiple selection per row). Omitted
row selections should not display any "skipped" text. Also, the order
of responses shouldn't change based on the format picked from the
dropdown – responses should always be output by column left to right,
then by row top to bottom. The only thing that changes based on the
format selection is whether to output the response as
"columnLabel rowLabel" vs. "rowLabel columnLabel".

* fix: improve slider response output

- Do not treat slider 0 value as "skipped" for `sliderRows`
- Output individual `slider` response in same format as `sliderRows`
  • Loading branch information
@farmerpaul
farmerpaul authored Dec 12, 2024
1 parent 75ce9e8 commit 5a99170
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 144 deletions.
76 changes: 31 additions & 45 deletions src/entities/activity/ui/items/ActionPlan/pageComponent.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@ import {
SentencePageComponent,
TextItemResponsePageComponent,
} from './Document.type';
import { ActivitiesPhrasalData, ActivityPhrasalDataSliderRowContext } from './phrasalData';
import {
ActivitiesPhrasalData,
ActivityPhrasalDataSliderContext,
ActivityPhrasalDataSliderRowContext,
} from './phrasalData';

const isAnswersSkipped = (answers: string[]): boolean => {
if (!answers || answers.length <= 0) {
Expand All @@ -32,13 +36,12 @@ const isAnswersSkipped = (answers: string[]): boolean => {

const identity: FieldValueTransformer = (value) => value;
const sliderValueTransformer =
(
t: TFunction,
ctx: ActivityPhrasalDataSliderRowContext,
itemIndex: number,
): FieldValueTransformer =>
(t: TFunction, maxValue: number): FieldValueTransformer =>
(value) =>
t('sliderValue', { value, total: ctx.maxValues[itemIndex] });
t('sliderValue', {
value,
total: maxValue,
});

const joinWithComma: FieldValueItemsJoiner = (values) => values.join(', ');
const joinWithDash: FieldValueItemsJoiner = (values) => values.join(' - ');
Expand Down Expand Up @@ -79,11 +82,17 @@ export const buildPageComponents = (
if (fieldPhrasalData) {
let transformValue = identity;
let joinValueItems = joinWithComma;
if (fieldPhrasalData.context.itemResponseType === 'sliderRows') {
if (fieldPhrasalData.context.itemResponseType === 'slider') {
transformValue = sliderValueTransformer(
t,
(fieldPhrasalData.context as ActivityPhrasalDataSliderContext).maxValue,
);
} else if (fieldPhrasalData.context.itemResponseType === 'sliderRows') {
transformValue = sliderValueTransformer(
t,
fieldPhrasalData.context as ActivityPhrasalDataSliderRowContext,
field.itemIndex,
(fieldPhrasalData.context as ActivityPhrasalDataSliderRowContext).maxValues[
field.itemIndex
],
);
} else if (fieldPhrasalData.context.itemResponseType === 'timeRange') {
joinValueItems = joinWithDash;
Expand All @@ -100,44 +109,21 @@ export const buildPageComponents = (
? [t('questionSkipped')]
: indexedAnswers.map(transformValue);
} else if (fieldPhrasalDataType === 'matrix') {
// The admin UI actually allows matrix type items to have `sentence` as
// their display mode. So in this case, we're just going to assume the
// effective render order for the values to be "by row".
let renderByRowValues = true;
if (
field.displayMode === 'sentence_option_row' ||
field.displayMode === 'bullet_list_option_row'
) {
renderByRowValues = false;
} else if (
const rowFirst =
field.displayMode === 'sentence_row_option' ||
field.displayMode === 'bullet_list_text_row'
) {
renderByRowValues = true;
field.displayMode === 'bullet_list_text_row';

valueItems = [];
for (const { rowLabel, columnLabels } of fieldPhrasalData.values) {
for (const columnLabel of columnLabels) {
valueItems.push(
rowFirst ? `${rowLabel} ${columnLabel}` : `${columnLabel} ${rowLabel}`,
);
}
}

if (renderByRowValues) {
valueItems = fieldPhrasalData.values.byRow
.map(({ label, values }) => {
const transformedValues = isAnswersSkipped(values)
? [t('questionSkipped')]
: values.map(transformValue);
return transformedValues.map(
(transformedValue) => `${label} ${transformedValue}`,
);
})
.flat();
} else {
valueItems = fieldPhrasalData.values.byColumn
.map(({ label, values }) => {
const transformedValues = isAnswersSkipped(values)
? [t('questionSkipped')]
: values.map(transformValue);
return transformedValues.map(
(transformedValue) => `${label} ${transformedValue}`,
);
})
.flat();
if (isAnswersSkipped(valueItems)) {
valueItems = [t('questionSkipped')];
}
} else {
// This also shouldn't happen. But including a `else` here allows all
Expand Down
56 changes: 24 additions & 32 deletions src/entities/activity/ui/items/ActionPlan/phrasalData.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ describe('Action Plan', () => {
const newTimeItem = newSimpleItem('time');
const newTimeRangeItem = newSimpleItem('timeRange');
const newNumberSelectItem = newSimpleItem<number>('numberSelect');
const newSliderItem = newSimpleItem('slider');
const newTextItem = newSimpleItem('text');

const newSelectItem = (type: string) => (name: string, answer: string[], options: string[]) =>
Expand All @@ -53,14 +52,14 @@ describe('Action Plan', () => {
responseType: 'multiSelectRows',
responseValues: {
rows: options[0].map((option) => ({ rowName: option })),
options: options[1].map((option) => ({ text: option })),
options: options[1].map((option, index) => ({ id: `col:${index}`, text: option })),
},
answer,
}) as never as ItemRecord;

const newSingleSelectRowsItem = (
name: string,
answer: string[],
answer: (string | null)[],
options: [string[], string[]],
) =>
({
Expand All @@ -73,6 +72,14 @@ describe('Action Plan', () => {
answer,
}) as never as ItemRecord;

const newSliderItem = (name: string, answer: string[], maxValue: number) =>
({
name,
responseType: 'slider',
responseValues: { maxValue },
answer,
}) as never as ItemRecord;

const newSliderRowsItem = (name: string, answer: number[], options: [number, number][]) =>
({
name,
Expand Down Expand Up @@ -132,12 +139,13 @@ describe('Action Plan', () => {
});

it('should extract data from `slider` activity type', () => {
const data = extractActivitiesPhrasalData([newSliderItem('item', ['6'])]);
const data = extractActivitiesPhrasalData([newSliderItem('item', ['6'], 10)]);

expect(data).toHaveProperty('item');
expect(data.item).toHaveProperty('type', 'array');
expect(data.item).toHaveProperty('values.0', '6');
expect(data.item).toHaveProperty('context.itemResponseType', 'slider');
expect(data.item).toHaveProperty('context.maxValue', 10);
});

it('should extract data from `text` activity type', () => {
Expand Down Expand Up @@ -199,22 +207,12 @@ describe('Action Plan', () => {

expect(data).toHaveProperty('item');
expect(data.item).toHaveProperty('type', 'matrix');
expect(data.item).toHaveProperty('values.byRow.0.label', 'R1');
expect(data.item).toHaveProperty('values.byRow.0.values.0', 'C1');
expect(data.item).toHaveProperty('values.byRow.0.values.1', 'C2');
expect(data.item).toHaveProperty('values.byRow.1.label', 'R2');
expect(data.item).toHaveProperty('values.byRow.1.values.0', 'C2');
expect(data.item).toHaveProperty('values.byRow.2.label', 'R3');
expect(data.item).toHaveProperty('values.byRow.2.values.0', 'C2');
expect(data.item).toHaveProperty('values.byRow.2.values.1', 'C3');
expect(data.item).toHaveProperty('values.byColumn.0.label', 'C1');
expect(data.item).toHaveProperty('values.byColumn.0.values.0', 'R1');
expect(data.item).toHaveProperty('values.byColumn.1.label', 'C2');
expect(data.item).toHaveProperty('values.byColumn.1.values.0', 'R1');
expect(data.item).toHaveProperty('values.byColumn.1.values.1', 'R2');
expect(data.item).toHaveProperty('values.byColumn.1.values.2', 'R3');
expect(data.item).toHaveProperty('values.byColumn.2.label', 'C3');
expect(data.item).toHaveProperty('values.byColumn.2.values.0', 'R3');
expect(data.item).toHaveProperty('values.0.rowLabel', 'R1');
expect(data.item).toHaveProperty('values.0.columnLabels', ['C1', 'C2']);
expect(data.item).toHaveProperty('values.1.rowLabel', 'R2');
expect(data.item).toHaveProperty('values.1.columnLabels', ['C2']);
expect(data.item).toHaveProperty('values.2.rowLabel', 'R3');
expect(data.item).toHaveProperty('values.2.columnLabels', ['C2', 'C3']);
expect(data.item).toHaveProperty('context.itemResponseType', 'multiSelectRows');
});

Expand All @@ -232,18 +230,12 @@ describe('Action Plan', () => {

expect(data).toHaveProperty('item');
expect(data.item).toHaveProperty('type', 'matrix');
expect(data.item).toHaveProperty('values.byRow.0.label', 'R1');
expect(data.item).toHaveProperty('values.byRow.0.values.0', 'C3');
expect(data.item).toHaveProperty('values.byRow.1.label', 'R2');
expect(data.item).toHaveProperty('values.byRow.1.values.0', 'C1');
expect(data.item).toHaveProperty('values.byRow.2.label', 'R3');
expect(data.item).toHaveProperty('values.byRow.2.values.0', 'C2');
expect(data.item).toHaveProperty('values.byColumn.0.label', 'C1');
expect(data.item).toHaveProperty('values.byColumn.0.values.0', 'R2');
expect(data.item).toHaveProperty('values.byColumn.1.label', 'C2');
expect(data.item).toHaveProperty('values.byColumn.1.values.0', 'R3');
expect(data.item).toHaveProperty('values.byColumn.2.label', 'C3');
expect(data.item).toHaveProperty('values.byColumn.2.values.0', 'R1');
expect(data.item).toHaveProperty('values.0.rowLabel', 'R1');
expect(data.item).toHaveProperty('values.0.columnLabels', ['C3']);
expect(data.item).toHaveProperty('values.1.rowLabel', 'R2');
expect(data.item).toHaveProperty('values.1.columnLabels', ['C1']);
expect(data.item).toHaveProperty('values.2.rowLabel', 'R3');
expect(data.item).toHaveProperty('values.2.columnLabels', ['C2']);
expect(data.item).toHaveProperty('context.itemResponseType', 'singleSelectRows');
});

Expand Down
119 changes: 52 additions & 67 deletions src/entities/activity/ui/items/ActionPlan/phrasalData.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ type ActivityPhrasalDataGenericContext = {
itemResponseType: ActivityItemType;
};

export type ActivityPhrasalDataSliderContext = ActivityPhrasalDataGenericContext & {
itemResponseType: 'slider';
maxValue: number;
};

export type ActivityPhrasalDataSliderRowContext = ActivityPhrasalDataGenericContext & {
itemResponseType: 'sliderRows';
maxValues: number[];
Expand All @@ -33,14 +38,11 @@ type ActivityPhrasalIndexedArrayFieldData = ActivityPhrasalBaseData<
>;

type ActivityPhrasalIndexedMatrixValue = {
label: string;
values: string[];
rowLabel: string;
columnLabels: string[];
};

type ActivityPhrasalMatrixValue = {
byRow: ActivityPhrasalIndexedMatrixValue[];
byColumn: ActivityPhrasalIndexedMatrixValue[];
};
type ActivityPhrasalMatrixValue = ActivityPhrasalIndexedMatrixValue[];

type ActivityPhrasalMatrixFieldData = ActivityPhrasalBaseData<'matrix', ActivityPhrasalMatrixValue>;

Expand Down Expand Up @@ -85,13 +87,12 @@ export const extractActivitiesPhrasalData = (items: ItemRecord[]): ActivitiesPhr
fieldData = timeFieldData;
} else if (
item.responseType === 'numberSelect' ||
item.responseType === 'slider' ||
item.responseType === 'text' ||
item.responseType === 'paragraphText'
) {
const textFieldData: ActivityPhrasalArrayFieldData = {
type: 'array',
values: item.answer.map((value) => `${value || ''}`),
values: item.answer.map((value) => `${value ?? ''}`),
context: fieldDataContext,
};
fieldData = textFieldData;
Expand All @@ -104,78 +105,62 @@ export const extractActivitiesPhrasalData = (items: ItemRecord[]): ActivitiesPhr
context: fieldDataContext,
};
fieldData = selectFieldData;
} else if (item.responseType === 'multiSelectRows') {
const byRow = item.responseValues.rows.map<ActivityPhrasalIndexedMatrixValue>(
(row, rowIndex) => {
return {
label: row.rowName,
values:
item.answer[rowIndex]?.filter<string>(
(value): value is string => value !== null && value !== undefined,
) || [],
};
},
);

const byColumn = item.responseValues.options.map<ActivityPhrasalIndexedMatrixValue>(
(option) => {
const answerIndices: number[] = [];
item.answer.forEach((values, answerIndex) => {
if (values.find((value) => value === option.text)) {
answerIndices.push(answerIndex);
}
} else if (
item.responseType === 'singleSelectRows' ||
item.responseType === 'multiSelectRows'
) {
// Unifiy logic for both singleSelectRows and multiSelectRows item types:
// - for singleSelectRows, map each answer to an array of one element
// - for multiSelectRows, answers are tracked as the _label_ of the column rather than ID,
// so we need to normalize to ID to align with singleSelectRows
const answers =
item.responseType === 'singleSelectRows'
? item.answer.map((value) => [value])
: item.answer.map((values) =>
values.map(
(text) => item.responseValues.options.find((option) => option.text === text)?.id,
),
);

const values: ActivityPhrasalIndexedMatrixValue[] = [];

answers.forEach((optionIds, rowIndex) => {
const optionValues = item.responseValues.options
.filter(({ id }) => optionIds.includes(id))
.map(({ text }) => text);

if (optionValues.length) {
values.push({
rowLabel: item.responseValues.rows[rowIndex].rowName,
columnLabels: optionValues,
});
return {
label: option.text,
values: answerIndices.map(
(answerIndex) => item.responseValues.rows[answerIndex].rowName || '',
),
};
},
);

const selectFieldData: ActivityPhrasalMatrixFieldData = {
}
});

const matrixFieldData: ActivityPhrasalMatrixFieldData = {
type: 'matrix',
values: { byRow, byColumn },
values,
context: fieldDataContext,
};
fieldData = selectFieldData;
} else if (item.responseType === 'singleSelectRows') {
const byRow = item.responseValues.rows.map<ActivityPhrasalIndexedMatrixValue>(
(row, rowIndex) => {
return {
label: row.rowName,
values: [
item.responseValues.options.find((option) => option.id === item.answer[rowIndex])
?.text || '',
],
};
},
);

const byColumn = item.responseValues.options.map<ActivityPhrasalIndexedMatrixValue>(
(option) => {
return {
label: option.text,
values: [item.responseValues.rows[item.answer.indexOf(option.id)]?.rowName || ''],
};
},
);

const selectFieldData: ActivityPhrasalMatrixFieldData = {
type: 'matrix',
values: { byRow, byColumn },
fieldData = matrixFieldData;
} else if (item.responseType === 'slider') {
(fieldDataContext as ActivityPhrasalDataSliderContext).maxValue =
item.responseValues.maxValue;

const sliderFieldData: ActivityPhrasalArrayFieldData = {
type: 'array',
values: item.answer.map((value) => `${value ?? ''}`),
context: fieldDataContext,
};
fieldData = selectFieldData;
fieldData = sliderFieldData;
} else if (item.responseType === 'sliderRows') {
(fieldDataContext as ActivityPhrasalDataSliderRowContext).maxValues =
item.responseValues.rows.map(({ maxValue }) => maxValue);

const sliderRowsFieldData: ActivityPhrasalIndexedArrayFieldData = {
type: 'indexed-array',
values: item.answer.reduce((acc, answerValue, answerIndex) => {
acc[answerIndex] = [`${answerValue || ''}`];
acc[answerIndex] = [`${answerValue ?? ''}`];
return acc;
}, {} as ActivityPhrasalItemizedArrayValue),
context: fieldDataContext,
Expand Down

0 comments on commit 5a99170

Please sign in to comment.