From 6bea42c33990e794cd316502cd7deda746d3e05f Mon Sep 17 00:00:00 2001 From: Ian Bolton Date: Wed, 8 Nov 2023 11:22:21 -0500 Subject: [PATCH] :bug: Fix duplicate question bug when same text exists in multiple sections (#1524) Signed-off-by: ibolton336 --- client/src/app/api/models.ts | 12 +++++ .../dynamic-assessment-actions-row.tsx | 11 ++-- .../components/questionnaires-table.tsx | 11 ++-- .../assessment-wizard-modal.tsx | 4 +- .../assessment-wizard/assessment-wizard.tsx | 38 ++++++++------ .../multi-input-selection.tsx | 4 +- .../questionnaire-form/questionnaire-form.tsx | 4 +- .../app/pages/assessment/form-utils.test.ts | 14 ++++-- client/src/app/pages/assessment/form-utils.ts | 16 ++++-- client/src/app/queries/assessments.ts | 50 +++++++++++++++++-- 10 files changed, 120 insertions(+), 44 deletions(-) diff --git a/client/src/app/api/models.ts b/client/src/app/api/models.ts index 582522cf1f..d3f105e47f 100644 --- a/client/src/app/api/models.ts +++ b/client/src/app/api/models.ts @@ -762,3 +762,15 @@ export interface Archetype { review?: Ref; } export type ProviderType = "Java" | "Go"; + +export interface QuestionWithSectionOrder extends Question { + sectionOrder: number; +} + +export interface SectionWithQuestionOrder extends Section { + questions: QuestionWithSectionOrder[]; +} + +export interface AssessmentWithSectionOrder extends Assessment { + sections: SectionWithQuestionOrder[]; +} diff --git a/client/src/app/pages/assessment/components/assessment-actions/components/dynamic-assessment-actions-row.tsx b/client/src/app/pages/assessment/components/assessment-actions/components/dynamic-assessment-actions-row.tsx index 7a532b8938..a5491825f8 100644 --- a/client/src/app/pages/assessment/components/assessment-actions/components/dynamic-assessment-actions-row.tsx +++ b/client/src/app/pages/assessment/components/assessment-actions/components/dynamic-assessment-actions-row.tsx @@ -2,11 +2,12 @@ import { Paths } from "@app/Paths"; import { Application, Archetype, - Assessment, + AssessmentWithSectionOrder, InitialAssessment, Questionnaire, } from "@app/api/models"; import { + addSectionOrderToQuestions, assessmentsByItemIdQueryKey, useCreateAssessmentMutation, useDeleteAssessmentMutation, @@ -38,9 +39,9 @@ interface DynamicAssessmentActionsRowProps { questionnaire: Questionnaire; application?: Application; archetype?: Archetype; - assessment?: Assessment; + assessment?: AssessmentWithSectionOrder; isReadonly?: boolean; - onOpenModal: (assessment: Assessment) => void; + onOpenModal: (assessment: AssessmentWithSectionOrder) => void; } const DynamicAssessmentActionsRow: FunctionComponent< @@ -134,8 +135,10 @@ const DynamicAssessmentActionsRow: FunctionComponent< try { const result = await createAssessmentAsync(newAssessment); + const assessmentWithOrder: AssessmentWithSectionOrder = + addSectionOrderToQuestions(result); - onOpenModal(result); + onOpenModal(assessmentWithOrder); } catch (error) { console.error("Error while creating assessment:", error); pushNotification({ diff --git a/client/src/app/pages/assessment/components/assessment-actions/components/questionnaires-table.tsx b/client/src/app/pages/assessment/components/assessment-actions/components/questionnaires-table.tsx index a3aebdbbc0..587c7ad0af 100644 --- a/client/src/app/pages/assessment/components/assessment-actions/components/questionnaires-table.tsx +++ b/client/src/app/pages/assessment/components/assessment-actions/components/questionnaires-table.tsx @@ -11,7 +11,7 @@ import { NoDataEmptyState } from "@app/components/NoDataEmptyState"; import { Application, Archetype, - Assessment, + AssessmentWithSectionOrder, Questionnaire, } from "@app/api/models"; import DynamicAssessmentActionsRow from "./dynamic-assessment-actions-row"; @@ -23,7 +23,7 @@ interface QuestionnairesTableProps { isReadonly?: boolean; application?: Application; archetype?: Archetype; - assessments?: Assessment[]; + assessments?: AssessmentWithSectionOrder[]; questionnaires?: Questionnaire[]; } @@ -50,10 +50,9 @@ const QuestionnairesTable: React.FC = ({ propHelpers: { tableProps, getThProps, getTrProps, getTdProps }, } = tableControls; - const [createdAssessment, setCreatedAssessment] = useState( - null - ); - const handleModalOpen = (assessment: Assessment) => { + const [createdAssessment, setCreatedAssessment] = + useState(null); + const handleModalOpen = (assessment: AssessmentWithSectionOrder) => { setCreatedAssessment(assessment); }; diff --git a/client/src/app/pages/assessment/components/assessment-wizard/assessment-wizard-modal.tsx b/client/src/app/pages/assessment/components/assessment-wizard/assessment-wizard-modal.tsx index 9a886cbd27..0d59aa5d48 100644 --- a/client/src/app/pages/assessment/components/assessment-wizard/assessment-wizard-modal.tsx +++ b/client/src/app/pages/assessment/components/assessment-wizard/assessment-wizard-modal.tsx @@ -1,12 +1,12 @@ import { Modal, ModalVariant } from "@patternfly/react-core"; import React, { FunctionComponent } from "react"; import { AssessmentWizard } from "./assessment-wizard"; -import { Assessment } from "@app/api/models"; +import { AssessmentWithSectionOrder } from "@app/api/models"; interface AssessmentModalProps { isOpen: boolean; onRequestClose: () => void; - assessment: Assessment | null; + assessment: AssessmentWithSectionOrder | null; } const AssessmentModal: FunctionComponent = ({ diff --git a/client/src/app/pages/assessment/components/assessment-wizard/assessment-wizard.tsx b/client/src/app/pages/assessment/components/assessment-wizard/assessment-wizard.tsx index 17afc194fe..ed8e3140a7 100644 --- a/client/src/app/pages/assessment/components/assessment-wizard/assessment-wizard.tsx +++ b/client/src/app/pages/assessment/components/assessment-wizard/assessment-wizard.tsx @@ -13,9 +13,10 @@ import { import { Assessment, AssessmentStatus, - Question, + AssessmentWithSectionOrder, + QuestionWithSectionOrder, Ref, - Section, + SectionWithQuestionOrder, } from "@app/api/models"; import { CustomWizardFooter } from "../custom-wizard-footer"; import { getApplicationById, getArchetypeById } from "@app/api/rest"; @@ -65,7 +66,7 @@ export interface AssessmentWizardValues { } export interface AssessmentWizardProps { - assessment?: Assessment; + assessment?: AssessmentWithSectionOrder; onClose: () => void; } @@ -169,18 +170,20 @@ export const AssessmentWizard: React.FC = ({ return numberOfStakeholdlers + numberOfGroups > 0; }; - const isQuestionValid = (question: Question): boolean => { + const isQuestionValid = (question: QuestionWithSectionOrder): boolean => { const questionErrors = errors.questions || {}; return !questionErrors[getQuestionFieldName(question, false)]; }; - const questionHasValue = (question: Question): boolean => { + const questionHasValue = (question: QuestionWithSectionOrder): boolean => { const questionValues = values.questions || {}; const value = questionValues[getQuestionFieldName(question, false)]; return value !== null && value !== undefined && value !== ""; }; - const areAllQuestionsAnswered = (section: Section): boolean => { + const areAllQuestionsAnswered = ( + section: SectionWithQuestionOrder + ): boolean => { return ( section?.questions.every((question) => { return questionHasValue(question); @@ -188,7 +191,9 @@ export const AssessmentWizard: React.FC = ({ ); }; - const shouldDisableSaveAsDraft = (sections: Section[]): boolean => { + const shouldDisableSaveAsDraft = ( + sections: SectionWithQuestionOrder[] + ): boolean => { const noAnswers = sections.every((section) => { return section.questions.every((question) => !questionHasValue(question)); }); @@ -200,7 +205,9 @@ export const AssessmentWizard: React.FC = ({ return noAnswers || allQuestionsAnswered; }; - const shouldNextBtnBeEnabled = (section: Section): boolean => { + const shouldNextBtnBeEnabled = ( + section: SectionWithQuestionOrder + ): boolean => { const allQuestionsValid = section?.questions.every((question) => isQuestionValid(question) ); @@ -213,13 +220,13 @@ export const AssessmentWizard: React.FC = ({ const buildSectionsFromFormValues = ( formValues: AssessmentWizardValues - ): Section[] => { + ): SectionWithQuestionOrder[] => { if (!formValues || !formValues[QUESTIONS_KEY]) { return []; } const updatedQuestionsData = formValues[QUESTIONS_KEY]; - const sections: Section[] = + const sections: SectionWithQuestionOrder[] = assessment?.sections?.map((section) => { const commentValues = values["comments"]; const fieldName = getCommentFieldName(section, false); @@ -270,7 +277,7 @@ export const AssessmentWizard: React.FC = ({ : []; const assessmentStatus: AssessmentStatus = "started"; - const payload: Assessment = { + const payload: AssessmentWithSectionOrder = { ...assessment, stakeholders: stakeholdersToPayload(values.stakeholders), @@ -305,7 +312,7 @@ export const AssessmentWizard: React.FC = ({ ? buildSectionsFromFormValues(formValues) : []; - const payload: Assessment = { + const payload: AssessmentWithSectionOrder = { ...assessment, stakeholders: stakeholdersToPayload(values.stakeholders), @@ -342,7 +349,7 @@ export const AssessmentWizard: React.FC = ({ ? buildSectionsFromFormValues(formValues) : []; - const payload: Assessment = { + const payload: AssessmentWithSectionOrder = { ...assessment, stakeholders: stakeholdersToPayload(values.stakeholders), @@ -457,7 +464,10 @@ export const AssessmentWizard: React.FC = ({ onClose(); }; - const getWizardFooter = (step: number, section?: Section) => { + const getWizardFooter = ( + step: number, + section?: SectionWithQuestionOrder + ) => { return ( = ({ diff --git a/client/src/app/pages/assessment/components/questionnaire-form/questionnaire-form.tsx b/client/src/app/pages/assessment/components/questionnaire-form/questionnaire-form.tsx index dedf105558..ca746d1b4f 100644 --- a/client/src/app/pages/assessment/components/questionnaire-form/questionnaire-form.tsx +++ b/client/src/app/pages/assessment/components/questionnaire-form/questionnaire-form.tsx @@ -14,12 +14,12 @@ import { MultiInputSelection } from "./multi-input-selection"; import { Question, QuestionHeader, QuestionBody } from "./question"; import { getCommentFieldName } from "../../form-utils"; import { useFormContext } from "react-hook-form"; -import { Section } from "@app/api/models"; +import { SectionWithQuestionOrder } from "@app/api/models"; import { AssessmentWizardValues } from "@app/pages/assessment/components/assessment-wizard/assessment-wizard"; import { HookFormPFTextInput } from "@app/components/HookFormPFFields"; export interface QuestionnaireFormProps { - section: Section; + section: SectionWithQuestionOrder; } export const QuestionnaireForm: React.FC = ({ diff --git a/client/src/app/pages/assessment/form-utils.test.ts b/client/src/app/pages/assessment/form-utils.test.ts index 401648b394..53700b6f75 100644 --- a/client/src/app/pages/assessment/form-utils.test.ts +++ b/client/src/app/pages/assessment/form-utils.test.ts @@ -1,17 +1,21 @@ -import { Question, Section } from "@app/api/models"; +import { + QuestionWithSectionOrder, + SectionWithQuestionOrder, +} from "@app/api/models"; import { getCommentFieldName, getQuestionFieldName } from "./form-utils"; describe("Application assessment - form utils", () => { - const section: Section = { + const section: SectionWithQuestionOrder = { name: "section-123", order: 1, questions: [], }; - const question: Question = { + const question: QuestionWithSectionOrder = { text: "Question 321", order: 1, answers: [], explanation: "Explanation 321", + sectionOrder: 1, }; it("getCommentFieldName: fullName", () => { @@ -26,11 +30,11 @@ describe("Application assessment - form utils", () => { it("getQuestionFieldName: fullName", () => { const fieldName = getQuestionFieldName(question, true); - expect(fieldName).toBe("questions.question-1-Question_321"); + expect(fieldName).toBe("questions.section-1-question-1-Question_321"); }); it("getQuestionFieldName: singleName", () => { const fieldName = getQuestionFieldName(question, false); - expect(fieldName).toBe("question-1-Question_321"); + expect(fieldName).toBe("section-1-question-1-Question_321"); }); }); diff --git a/client/src/app/pages/assessment/form-utils.ts b/client/src/app/pages/assessment/form-utils.ts index bbbd993e6c..79c769f923 100644 --- a/client/src/app/pages/assessment/form-utils.ts +++ b/client/src/app/pages/assessment/form-utils.ts @@ -1,4 +1,4 @@ -import { Question, Section } from "@app/api/models"; +import { QuestionWithSectionOrder, Section } from "@app/api/models"; export const COMMENTS_KEY = "comments"; export const QUESTIONS_KEY = "questions"; @@ -21,9 +21,15 @@ export const sanitizeKey = (text: string) => { return text.replace(/[^a-zA-Z0-9-_:.]/g, "_"); }; -export const getQuestionFieldName = (question: Question, fullName: boolean) => { - const fieldName = `question-${question.order}-${question.text}`; +export const getQuestionFieldName = ( + question: QuestionWithSectionOrder, + fullName: boolean +): string => { + const fieldName = `section-${question.sectionOrder}-question-${question.order}-${question.text}`; + + const sanitizedFieldName = sanitizeKey(fieldName); + return fullName - ? `${QUESTIONS_KEY}.${sanitizeKey(fieldName)}` - : sanitizeKey(fieldName); + ? `${QUESTIONS_KEY}.${sanitizedFieldName}` + : sanitizedFieldName; }; diff --git a/client/src/app/queries/assessments.ts b/client/src/app/queries/assessments.ts index 5e0586bde8..d805b47ca2 100644 --- a/client/src/app/queries/assessments.ts +++ b/client/src/app/queries/assessments.ts @@ -1,4 +1,5 @@ import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; +import { useMemo } from "react"; import { createAssessment, @@ -9,7 +10,11 @@ import { updateAssessment, } from "@app/api/rest"; import { AxiosError } from "axios"; -import { Assessment, InitialAssessment } from "@app/api/models"; +import { + Assessment, + AssessmentWithSectionOrder, + InitialAssessment, +} from "@app/api/models"; import { QuestionnairesQueryKey } from "./questionnaires"; import { ARCHETYPE_QUERY_KEY } from "./archetypes"; @@ -23,8 +28,13 @@ export const useFetchAssessments = () => { queryFn: getAssessments, onError: (error: AxiosError) => console.log("error, ", error), }); + const assessmentsWithOrder: AssessmentWithSectionOrder[] = useMemo( + () => data?.map(addSectionOrderToQuestions) || [], + [data] + ); + return { - assessments: data || [], + assessments: assessmentsWithOrder || [], isFetching: isLoading, fetchError: error, }; @@ -64,7 +74,10 @@ export const useUpdateAssessmentMutation = ( const queryClient = useQueryClient(); return useMutation({ - mutationFn: (assessment: Assessment) => updateAssessment(assessment), + mutationFn: (assessmentWithOrder: AssessmentWithSectionOrder) => { + const assessment = removeSectionOrderFromQuestions(assessmentWithOrder); + return updateAssessment(assessment); + }, onSuccess: (_, args) => { onSuccess && onSuccess(args.name); const isArchetype = !!args.archetype?.id; @@ -160,11 +173,40 @@ export const useFetchAssessmentsByItemId = ( isArchetype, ]); }; + const assessmentsWithOrder: AssessmentWithSectionOrder[] = + data?.map(addSectionOrderToQuestions) || []; return { - assessments: data, + assessments: assessmentsWithOrder, isFetching: isLoading, fetchError: error, invalidateAssessmentsQuery, }; }; + +export const addSectionOrderToQuestions = ( + assessment: Assessment +): AssessmentWithSectionOrder => { + return { + ...assessment, + sections: assessment.sections.map((section) => ({ + ...section, + questions: section.questions.map((question) => ({ + ...question, + sectionOrder: section.order, + })), + })), + }; +}; + +const removeSectionOrderFromQuestions = ( + assessmentWithOrder: AssessmentWithSectionOrder +): Assessment => { + return { + ...assessmentWithOrder, + sections: assessmentWithOrder.sections.map((section) => ({ + ...section, + questions: section.questions.map(({ sectionOrder, ...rest }) => rest), // Destructure out sectionOrder + })), + }; +};