Skip to content

Commit

Permalink
🐛 Fix duplicate question bug when same text exists in multiple sectio…
Browse files Browse the repository at this point in the history
…ns (#1524)

Signed-off-by: ibolton336 <[email protected]>
  • Loading branch information
ibolton336 authored Nov 8, 2023
1 parent 84cc866 commit 6bea42c
Show file tree
Hide file tree
Showing 10 changed files with 120 additions and 44 deletions.
12 changes: 12 additions & 0 deletions client/src/app/api/models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
}
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<
Expand Down Expand Up @@ -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({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -23,7 +23,7 @@ interface QuestionnairesTableProps {
isReadonly?: boolean;
application?: Application;
archetype?: Archetype;
assessments?: Assessment[];
assessments?: AssessmentWithSectionOrder[];
questionnaires?: Questionnaire[];
}

Expand All @@ -50,10 +50,9 @@ const QuestionnairesTable: React.FC<QuestionnairesTableProps> = ({
propHelpers: { tableProps, getThProps, getTrProps, getTdProps },
} = tableControls;

const [createdAssessment, setCreatedAssessment] = useState<Assessment | null>(
null
);
const handleModalOpen = (assessment: Assessment) => {
const [createdAssessment, setCreatedAssessment] =
useState<AssessmentWithSectionOrder | null>(null);
const handleModalOpen = (assessment: AssessmentWithSectionOrder) => {
setCreatedAssessment(assessment);
};

Expand Down
Original file line number Diff line number Diff line change
@@ -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<AssessmentModalProps> = ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -65,7 +66,7 @@ export interface AssessmentWizardValues {
}

export interface AssessmentWizardProps {
assessment?: Assessment;
assessment?: AssessmentWithSectionOrder;
onClose: () => void;
}

Expand Down Expand Up @@ -169,26 +170,30 @@ export const AssessmentWizard: React.FC<AssessmentWizardProps> = ({
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);
}) ?? false
);
};

const shouldDisableSaveAsDraft = (sections: Section[]): boolean => {
const shouldDisableSaveAsDraft = (
sections: SectionWithQuestionOrder[]
): boolean => {
const noAnswers = sections.every((section) => {
return section.questions.every((question) => !questionHasValue(question));
});
Expand All @@ -200,7 +205,9 @@ export const AssessmentWizard: React.FC<AssessmentWizardProps> = ({
return noAnswers || allQuestionsAnswered;
};

const shouldNextBtnBeEnabled = (section: Section): boolean => {
const shouldNextBtnBeEnabled = (
section: SectionWithQuestionOrder
): boolean => {
const allQuestionsValid = section?.questions.every((question) =>
isQuestionValid(question)
);
Expand All @@ -213,13 +220,13 @@ export const AssessmentWizard: React.FC<AssessmentWizardProps> = ({

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);
Expand Down Expand Up @@ -270,7 +277,7 @@ export const AssessmentWizard: React.FC<AssessmentWizardProps> = ({
: [];

const assessmentStatus: AssessmentStatus = "started";
const payload: Assessment = {
const payload: AssessmentWithSectionOrder = {
...assessment,

stakeholders: stakeholdersToPayload(values.stakeholders),
Expand Down Expand Up @@ -305,7 +312,7 @@ export const AssessmentWizard: React.FC<AssessmentWizardProps> = ({
? buildSectionsFromFormValues(formValues)
: [];

const payload: Assessment = {
const payload: AssessmentWithSectionOrder = {
...assessment,

stakeholders: stakeholdersToPayload(values.stakeholders),
Expand Down Expand Up @@ -342,7 +349,7 @@ export const AssessmentWizard: React.FC<AssessmentWizardProps> = ({
? buildSectionsFromFormValues(formValues)
: [];

const payload: Assessment = {
const payload: AssessmentWithSectionOrder = {
...assessment,

stakeholders: stakeholdersToPayload(values.stakeholders),
Expand Down Expand Up @@ -457,7 +464,10 @@ export const AssessmentWizard: React.FC<AssessmentWizardProps> = ({
onClose();
};

const getWizardFooter = (step: number, section?: Section) => {
const getWizardFooter = (
step: number,
section?: SectionWithQuestionOrder
) => {
return (
<CustomWizardFooter
enableNext={
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React, { useMemo } from "react";
import { Icon, Radio, Stack, StackItem, Tooltip } from "@patternfly/react-core";
import InfoCircleIcon from "@patternfly/react-icons/dist/esm/icons/info-circle-icon";

import { Question } from "@app/api/models";
import { QuestionWithSectionOrder } from "@app/api/models";
import { HookFormPFGroupController } from "@app/components/HookFormPFFields";
import { useFormContext } from "react-hook-form";
import { getQuestionFieldName } from "../../../form-utils";
Expand All @@ -11,7 +11,7 @@ import useIsArchetype from "@app/hooks/useIsArchetype";
import { useTranslation } from "react-i18next";

export interface MultiInputSelectionProps {
question: Question;
question: QuestionWithSectionOrder;
}

export const MultiInputSelection: React.FC<MultiInputSelectionProps> = ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<QuestionnaireFormProps> = ({
Expand Down
14 changes: 9 additions & 5 deletions client/src/app/pages/assessment/form-utils.test.ts
Original file line number Diff line number Diff line change
@@ -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", () => {
Expand All @@ -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");
});
});
16 changes: 11 additions & 5 deletions client/src/app/pages/assessment/form-utils.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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;
};
Loading

0 comments on commit 6bea42c

Please sign in to comment.