🐛 Update assessment form IDs with UUID #2626
Conversation
Updating assessment questionnaires to not construct fields IDs/names with questions&answers tests, but use UUIDs. Fixes misfunctions caused by too long questions&answers strings. Signed-off-by: Marek Aufart <[email protected]>
WalkthroughAdds stable UI identifiers via useWithUiId for sections, questions, and options; replaces index/text-based keys and IDs with _ui_unique_id across the assessment wizard and questionnaire form; changes getQuestionFieldName to omit question.text from generated field names. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Wizard as AssessmentWizard
participant UIId as useWithUiId
participant Form as QuestionnaireForm
participant Multi as MultiInputSelection
User->>Wizard: open assessment
Wizard->>Wizard: compute sortedSections (useMemo)
Wizard->>UIId: withUiId(sortedSections) → sectionsWithUiId
Wizard->>Wizard: render steps keyed by section._ui_unique_id
Wizard->>Form: render QuestionnaireForm(section._ui_unique_id)
Form->>UIId: withUiId(sortedQuestions) → questionsWithUiId
Form->>Form: render questions keyed by question._ui_unique_id
Form->>Multi: render MultiInputSelection(question._ui_unique_id)
Multi->>UIId: withUiId(sortedOptions) → optionsWithUiId
Multi->>Multi: render options keyed by option._ui_unique_id
Note right of Multi: answerUniqueId = questionFieldName + option._ui_unique_id
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
client/src/app/pages/assessment/components/questionnaire-form/multi-input-selection/multi-input-selection.tsx (1)
24-31: Avoid mutating props; prefer cloning before sort. Also prefer stable IDs over order pairs for option UI IDs..sort() mutates question.answers. And order-based IDs will churn if options are re-ordered; use an intrinsic id if available.
Apply this diff:
- const sortedOptions = useMemo(() => { - return (question.answers || []).sort((a, b) => a.order - b.order); - }, [question]); + const sortedOptions = useMemo(() => { + const items = question.answers ?? []; + return [...items].sort((a, b) => a.order - b.order); + }, [question.answers]); - const optionsWithUiId = useWithUiId( - sortedOptions, - (option) => `${question.order}-${option.order}` - ); + const optionsWithUiId = useWithUiId( + sortedOptions, + // Prefer a backend/stable id; fall back to order if none + (option) => String((option as any).id ?? `${question.order}-${option.order}`) + );client/src/app/pages/assessment/components/questionnaire-form/questionnaire-form.tsx (1)
45-52: Don’t mutate section.questions; generate stable question UI IDs without section.name..sort() mutates props. Also, composing the UI ID with section.name reintroduces long strings; prefer an intrinsic id or order-only.
Apply this diff:
- const sortedQuestions = useMemo(() => { - return section.questions.sort((a, b) => a.order - b.order); - }, [section]); + const sortedQuestions = useMemo(() => { + const items = section.questions ?? []; + return [...items].sort((a, b) => a.order - b.order); + }, [section.questions]); - const questionsWithUiId = useWithUiId( - sortedQuestions, - (question) => `${section.name}-${question.order || "no-order"}` - ); + const questionsWithUiId = useWithUiId( + sortedQuestions, + (question) => + String((question as any).id ?? `q-${question.order ?? "no-order"}`) + );client/src/app/pages/assessment/components/assessment-wizard/assessment-wizard.tsx (1)
129-145: Bug: initial answer lookup can pull from the wrong section; use the current question’s answers.Matching by text across all sections risks collisions when different sections reuse question text.
Apply this diff:
- const initialQuestions = useMemo(() => { + const initialQuestions = useMemo(() => { const questions: { [key: string]: string | undefined } = {}; if (assessment) { assessment.sections .flatMap((f) => f.questions) .forEach((question) => { - const existingAnswer = assessment?.sections - ?.flatMap((section) => section.questions) - .find((q) => q.text === question.text) - ?.answers.find((a) => a.selected === true); - - questions[getQuestionFieldName(question, false)] = - existingAnswer?.text || ""; + const existingAnswer = question.answers.find((a) => a.selected); + questions[getQuestionFieldName(question, false)] = + existingAnswer?.text ?? ""; }); } return questions; }, [assessment]);
🧹 Nitpick comments (4)
client/src/app/pages/assessment/form-utils.ts (2)
28-35: Good move: drop text from question field names. Consider using stable UI IDs, not order, to avoid churn on reordering.Using section/question order is better than text, but still fragile if orders change. Prefer a stable per-question UI ID when available, with order as a fallback.
Apply this diff:
export const getQuestionFieldName = ( question: QuestionWithSectionOrder, fullName: boolean ): string => { - const fieldName = `section-${question.sectionOrder}-question-${question.order}`; + const fieldName = + // Prefer a stable UI id if present + (question as any)._ui_unique_id ?? + `section-${question.sectionOrder}-question-${question.order}`; const sanitizedFieldName = sanitizeKey(fieldName); return fullName ? `${QUESTIONS_KEY}.${sanitizedFieldName}` : sanitizedFieldName; };
12-18: Align comments keying with the same non-text approach.Comments still key off section.name; very long names can bloat form paths and are inconsistent with the goal of avoiding text-derived keys.
Apply this diff:
export const getCommentFieldName = (section: Section, fullName: boolean) => { - const fieldName = `section-${section.name}`; + const fieldName = + (section as any)._ui_unique_id ?? + (section as any).order !== undefined + ? `section-${(section as any).order}` + : `section-${section.name}`; const sanitizedFieldName = sanitizeKey(fieldName); return fullName ? `${COMMENTS_KEY}.${sanitizedFieldName}` : sanitizedFieldName; };client/src/app/pages/assessment/components/questionnaire-form/questionnaire-form.tsx (1)
88-90: Remove redundant key on child component.The parent StackItem already has a stable key; a key on the direct child is unnecessary.
Apply this diff:
- <MultiInputSelection - key={question._ui_unique_id} - question={question} - /> + <MultiInputSelection question={question} />client/src/app/pages/assessment/components/assessment-wizard/assessment-wizard.tsx (1)
114-117: Use compact, stable section IDs; avoid section.name in UI IDs.Long names aren’t needed for React keys and can be volatile.
Apply this diff:
- const sectionsWithUiId = useWithUiId( - sortedSections, - (section) => `section-${section.order}-${section.name}` - ); + const sectionsWithUiId = useWithUiId( + sortedSections, + (section) => String((section as any).id ?? `section-${section.order}`) + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
client/src/app/pages/assessment/components/assessment-wizard/assessment-wizard.tsx(6 hunks)client/src/app/pages/assessment/components/questionnaire-form/multi-input-selection/multi-input-selection.tsx(2 hunks)client/src/app/pages/assessment/components/questionnaire-form/questionnaire-form.tsx(4 hunks)client/src/app/pages/assessment/form-utils.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-15T22:35:55.912Z
Learnt from: sjd78
PR: konveyor/tackle2-ui#2558
File: client/src/app/pages/applications/generate-assets-wizard/generate-assets-wizard.tsx:0-0
Timestamp: 2025-08-15T22:35:55.912Z
Learning: The Generate Assets Wizard in client/src/app/pages/applications/generate-assets-wizard/generate-assets-wizard.tsx is designed to accommodate additional wizard steps in the future, so the current conditional rendering of the Results step is intentional and appropriate for the planned architecture.
Applied to files:
client/src/app/pages/assessment/components/assessment-wizard/assessment-wizard.tsx
🧬 Code graph analysis (3)
client/src/app/pages/assessment/components/assessment-wizard/assessment-wizard.tsx (2)
client/src/app/utils/query-utils.ts (1)
useWithUiId(14-37)client/src/app/pages/assessment/components/questionnaire-form/questionnaire-form.tsx (1)
QuestionnaireForm(29-116)
client/src/app/pages/assessment/components/questionnaire-form/multi-input-selection/multi-input-selection.tsx (2)
client/src/app/utils/query-utils.ts (1)
useWithUiId(14-37)client/src/app/pages/assessment/form-utils.ts (1)
getQuestionFieldName(24-35)
client/src/app/pages/assessment/components/questionnaire-form/questionnaire-form.tsx (1)
client/src/app/utils/query-utils.ts (1)
useWithUiId(14-37)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: unit-test
- GitHub Check: build-and-upload-for-global-ci
🔇 Additional comments (5)
client/src/app/pages/assessment/components/questionnaire-form/multi-input-selection/multi-input-selection.tsx (1)
39-43: LGTM: DOM ids/keys now based on option UI IDs.This removes text from element IDs and avoids length issues.
client/src/app/pages/assessment/components/assessment-wizard/assessment-wizard.tsx (4)
542-553: LGTM: Footer logic now references sectionsWithUiId.The last-step and disable checks align with the new sections collection.
578-579: LGTM: Force remount on section count change.Keying the Wizard by sectionsWithUiId.length is appropriate.
605-622: LGTM: Section steps keyed by stable UI IDs.Keys and QuestionnaireForm usage now align with the UI-ID approach.
40-44: Verify no remaining call sites depend on text-based field names.Sandbox search failed (rg: "unrecognized file type: tsx"). Run these from the repo root and confirm there are no matches for homemade field-name builders or direct uses of question.text in form paths:
rg -n -C2 '\b(getQuestionFieldName|getCommentFieldName)\s*\(' rg -n -C2 '\bquestion\.text\b|\$\{\s*question\.text|question\.text\s*\+' rg -n -C2 'questions\[[^\]]+\]\.text|\bquestions\.' rg -n -C2 '\bcomments\.'Look for: homemade field-name builders, template-literals or concatenation using question.text, or any direct form-paths using question.text.
client/src/app/pages/assessment/components/assessment-wizard/assessment-wizard.tsx
Show resolved
Hide resolved
|
I installed the questionnaire attached to MTA-6143, and took the assessment for an application with the PR running. I could click all the way through the wizard but the assessment actions page shows the questionnaire as "Continue" instead of the "Retake" I expected: 
|
That is right, I can confirm this, the original questionnaire had a wrong order number for second section (already mentioned it to reporter on slack), that causes this behaviour. With questionnaire data fix (in the original file, change line 142 oder: 1->2), it works well for me locally. 
|
sjd78
added
the
cherry-pick/release-0.8
sjd78
left a comment
sjd78
left a comment
There was a problem hiding this comment.
Confirmed working when each section, question, and answer has a correct unique order assigned.
Resolves: https://issues.redhat.com/browse/MTA-6143 Resolves: #2653 Updating assessment questionnaires to not construct fields IDs/names with questions&answers tests, but use UUIDs. Fixes misfunctions caused by too long questions&answers strings. Used claude code to help create this PR. ## Summary by CodeRabbit - Bug Fixes - Stabilized Assessment Wizard steps and questionnaire items to prevent jumps, duplicates, or missing steps. - Preserves answers, input focus, and selection state when navigating or when question/option order changes. - Ensures consistent option behavior and avoids accidental resets during edits. - Performance - Smoother, more reliable rendering with fewer re-renders and reduced flicker across the wizard and forms. - Faster navigation between sections and questions. - Other - No visual design changes; improvements focus on stability and consistency of the assessment experience. --------- Signed-off-by: Marek Aufart <[email protected]> Signed-off-by: Cherry Picker <[email protected]>
Resolves: https://issues.redhat.com/browse/MTA-6143 Resolves: #2653 Updating assessment questionnaires to not construct fields IDs/names with questions&answers tests, but use UUIDs. Fixes misfunctions caused by too long questions&answers strings. Used claude code to help create this PR. ## Summary by CodeRabbit - Bug Fixes - Stabilized Assessment Wizard steps and questionnaire items to prevent jumps, duplicates, or missing steps. - Preserves answers, input focus, and selection state when navigating or when question/option order changes. - Ensures consistent option behavior and avoids accidental resets during edits. - Performance - Smoother, more reliable rendering with fewer re-renders and reduced flicker across the wizard and forms. - Faster navigation between sections and questions. - Other - No visual design changes; improvements focus on stability and consistency of the assessment experience. --------- Signed-off-by: Marek Aufart <[email protected]> Signed-off-by: Cherry Picker <[email protected]> Co-authored-by: Marek Aufart <[email protected]>
Resolves: https://issues.redhat.com/browse/MTA-6143 Resolves: konveyor#2653 Updating assessment questionnaires to not construct fields IDs/names with questions&answers tests, but use UUIDs. Fixes misfunctions caused by too long questions&answers strings. Used claude code to help create this PR. ## Summary by CodeRabbit - Bug Fixes - Stabilized Assessment Wizard steps and questionnaire items to prevent jumps, duplicates, or missing steps. - Preserves answers, input focus, and selection state when navigating or when question/option order changes. - Ensures consistent option behavior and avoids accidental resets during edits. - Performance - Smoother, more reliable rendering with fewer re-renders and reduced flicker across the wizard and forms. - Faster navigation between sections and questions. - Other - No visual design changes; improvements focus on stability and consistency of the assessment experience. --------- Signed-off-by: Marek Aufart <[email protected]>
2 participants
Resolves: https://issues.redhat.com/browse/MTA-6143
Resolves: #2653
Updating assessment questionnaires to not construct fields IDs/names with questions&answers tests, but use UUIDs.
Fixes misfunctions caused by too long questions&answers strings.
Used claude code to help create this PR.
Summary by CodeRabbit
Bug Fixes
Performance
Other