[ENG-421] feat: enhance questionnaire with repeatable groups#16399
Open
abhimanyurajeesh wants to merge 13 commits into
Open
[ENG-421] feat: enhance questionnaire with repeatable groups#16399abhimanyurajeesh wants to merge 13 commits into
abhimanyurajeesh wants to merge 13 commits into
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR adds support for repeatable questionnaire groups, enabling users to create and respond to multiple instances of a group. The implementation extends ChangesRepeatable Questionnaire Groups
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Deploying care-preview with
|
| Latest commit: |
83033bb
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://008f2432.care-preview-a7w.pages.dev |
| Branch Preview URL: | https://eng-421.care-preview-a7w.pages.dev |
🎭 Playwright Test ResultsStatus: ❌ Failed
📊 Detailed results are available in the playwright-final-report artifact. Run: #9636 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/Questionnaire/QuestionnaireForm.tsx`:
- Around line 346-369: serializeRepeatableGroupResponse currently serializes
value types differently than the main submission path causing inconsistent
date/dateTime/unit formatting; extract the shared logic into a single
serializeValue function and use it inside serializeRepeatableGroupResponse and
in the main submission mapping so both paths produce identical output.
Specifically, implement serializeValue(value: QuestionnaireResponseValue) that
applies dateQueryString for 'date', toISOString for 'dateTime', and returns {
value, unit, coding } for unit-bearing values (matching the main submission
logic), then replace the inline value mapping in
serializeRepeatableGroupResponse (and the mapping in the main submission code)
to call serializeValue for each value to ensure consistent formatting.
In `@src/components/Questionnaire/QuestionTypes/QuestionGroup.tsx`:
- Around line 360-362: The component currently uses instanceIndex as the React
key for subResults which causes incorrect re-renders when items are removed;
modify instance creation logic (e.g., initializeGroupResponses and wherever new
instances are pushed/added) to attach a stable unique identifier field like
_instanceId (use a UUID or monotonic counter) to each instance object, then
change the map in QuestionGroup (the subResults.map rendering) to use
instance._instanceId as the key instead of instanceIndex so keys remain stable
across removals and reorders.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a402be7c-a43a-4a65-8f0b-320663b70801
📒 Files selected for processing (8)
public/locale/en.jsonsrc/components/Facility/ConsultationDetails/PrintAllQuestionnaireResponses.tsxsrc/components/Facility/ConsultationDetails/QuestionnaireResponsesList.tsxsrc/components/Questionnaire/QuestionRenderer.tsxsrc/components/Questionnaire/QuestionTypes/QuestionGroup.tsxsrc/components/Questionnaire/QuestionnaireEditor.tsxsrc/components/Questionnaire/QuestionnaireForm.tsxsrc/types/questionnaire/form.ts
ohc-project-manager
Bot
added
needs testing
needs review
and removed
Work in Progress
labels
Comment on lines
+134
to
+136
| const hasAnyValue = subResp.values.some( | ||
| (v) => v.value || v.coding, | ||
| ); |
Comment on lines
+153
to
+156
| {vidx > 0 && ", "} | ||
| {val.value && | ||
| formatValue(val.value, subQuestion.type)} | ||
| {val.unit && ( |
Comment on lines
+289
to
+293
| if (question.type === "structured") return false; | ||
| const resp = responses.find((r) => r.question_id === question.id); | ||
| if (!resp) return false; | ||
| return resp.values.some((v) => v.value || v.coding); | ||
| }); |
Comment on lines
+167
to
+185
| if (question.repeats) { | ||
| return ( | ||
| <RepeatableGroupRenderer | ||
| question={question} | ||
| encounterId={encounterId} | ||
| questionnaireResponses={questionnaireResponses} | ||
| updateQuestionnaireResponseCB={updateQuestionnaireResponseCB} | ||
| errors={errors} | ||
| clearError={clearError} | ||
| disabled={disabled} | ||
| activeGroupId={activeGroupId} | ||
| facilityId={facilityId} | ||
| patientId={patientId} | ||
| isSubQuestion={isSubQuestion} | ||
| questionnaireId={questionnaireId} | ||
| questionnaireSlug={questionnaireSlug} | ||
| /> | ||
| ); | ||
| } |
Greptile SummaryThis PR adds support for repeatable question groups in the questionnaire feature — allowing users to add/remove multiple instances of a group, with corresponding updates to form initialization, validation, serialization, and both the response list and print views.
Confidence Score: 3/5Safe to merge for core form functionality, but the display and print views will silently omit any explicitly-entered zero values from repeatable group responses. The form, serialization, and key-stability logic are well-implemented. The gap is in the display layer: both the response list and the print view hide rows/groups whose only filled values are numeric 0 — a legitimate clinical value (e.g., pain scale, episode count). This affects every repeatable group response containing a zero-valued answer, which in a medical questionnaire context can be significant. PrintAllQuestionnaireResponses.tsx (hasRenderableContent) and QuestionnaireResponsesList.tsx (hasAnyValue in renderQuestionsInOrder) both need the falsy value check replaced. Important Files Changed
Reviews (9): Last reviewed commit: "fix a validation issue" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
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 (2)
src/components/Questionnaire/QuestionTypes/QuestionGroup.tsx (2)
366-377:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInternationalize the instance header and aria-label.
The instance number display and remove button aria-label contain literal English strings, violating the i18n requirement for healthcare interfaces.
🌐 Proposed i18n fix
Add translation keys to
public/locale/en.json:{ "instance_number": "Instance #{{number}}", "remove_instance": "Remove instance {{number}}" }Then update the component:
<span className="text-sm font-medium text-gray-500"> - #{instanceIndex + 1} + {t("instance_number", { number: instanceIndex + 1 })} </span> ... <Button ... - aria-label={`Remove instance ${instanceIndex + 1}`} + aria-label={t("remove_instance", { number: instanceIndex + 1 })} >As per coding guidelines: "All literal strings must use i18next for multi-language support in healthcare interfaces".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Questionnaire/QuestionTypes/QuestionGroup.tsx` around lines 366 - 377, The instance header and remove button aria-label use hard-coded English; replace them with i18n lookups using the i18next hook (t) and the instanceIndex value: update the span that renders "#{instanceIndex + 1}" to use t('instance_number', { number: instanceIndex + 1 }) and change the Button aria-label to t('remove_instance', { number: instanceIndex + 1 }); ensure the component imports and uses the useTranslation hook and add the suggested keys ("instance_number" and "remove_instance") to the locale files so translations resolve at runtime, keeping the onClick handler handleRemoveInstance unchanged.
191-211: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winApply consistent simplification for styling metadata access.
The redundant
&&pattern was correctly simplified inRepeatableGroupRenderer(lines 346, 387), but the same pattern remains in the originalQuestionGroupcomponent.♻️ Proposed fix for consistency
<div className={cn( "sm:rounded-lg bg-gray-100 md:bg-transparent", isActive && "ring-2 ring-primary", - question.styling_metadata?.classes && question.styling_metadata.classes, + question.styling_metadata?.classes, )} > ... <div className={cn( "gap-1", - question.styling_metadata?.containerClasses && - question.styling_metadata.containerClasses, + question.styling_metadata?.containerClasses, )} >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Questionnaire/QuestionTypes/QuestionGroup.tsx` around lines 191 - 211, The JSX uses the redundant "&&" pattern when accessing styling metadata in QuestionGroup; replace occurrences like question.styling_metadata?.classes && question.styling_metadata.classes and question.styling_metadata?.containerClasses && question.styling_metadata.containerClasses with the simplified optional chaining form (e.g., question.styling_metadata?.classes and question.styling_metadata?.containerClasses) so the className/cn calls mirror the simplified approach already used in RepeatableGroupRenderer; update all usages inside the QuestionGroup component (including the className for the wrapper and the container className) to this simplified form.
♻️ Duplicate comments (1)
src/components/Questionnaire/QuestionnaireForm.tsx (1)
354-356:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winPotential runtime error:
dateTimevalue assumed to be Date object.The
datecase (lines 347-352) properly convertsvalue.valueto aDateobject before calling methods on it, but thedateTimecase directly calls.toISOString()without conversion. Ifvalue.valueis a string (common for form inputs), this will throwTypeError.🐛 Proposed fix: handle string values consistently with date case
if (value.type === "dateTime" && value.value) { - return { ...value, value: value.value.toISOString() }; + const date = value.value instanceof Date ? value.value : new Date(value.value); + if (isNaN(date.getTime())) { + return { ...value, value: "" }; + } + return { ...value, value: date.toISOString() }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Questionnaire/QuestionnaireForm.tsx` around lines 354 - 356, The dateTime branch in QuestionnaireForm (the block checking value.type === "dateTime") assumes value.value is a Date and calls .toISOString(), which can throw if it's a string; update that branch to mirror the date handling: coerce value.value into a Date (e.g., new Date(value.value)) only when value.value exists and is not already a Date, guard against invalid dates, then return the object with value set to the ISO string; use the same checks/logic pattern used in the date case to ensure consistency and avoid runtime TypeError.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@public/locale/en.json`:
- Line 4895: Update the value for the localization key
"repeatable_group_cannot_contain_structured" to the suggested, clearer wording;
locate the string named repeatable_group_cannot_contain_structured and replace
its current value with: "A repeatable group cannot contain structured questions.
Disable repeatable mode or remove structured questions."
---
Outside diff comments:
In `@src/components/Questionnaire/QuestionTypes/QuestionGroup.tsx`:
- Around line 366-377: The instance header and remove button aria-label use
hard-coded English; replace them with i18n lookups using the i18next hook (t)
and the instanceIndex value: update the span that renders "#{instanceIndex + 1}"
to use t('instance_number', { number: instanceIndex + 1 }) and change the Button
aria-label to t('remove_instance', { number: instanceIndex + 1 }); ensure the
component imports and uses the useTranslation hook and add the suggested keys
("instance_number" and "remove_instance") to the locale files so translations
resolve at runtime, keeping the onClick handler handleRemoveInstance unchanged.
- Around line 191-211: The JSX uses the redundant "&&" pattern when accessing
styling metadata in QuestionGroup; replace occurrences like
question.styling_metadata?.classes && question.styling_metadata.classes and
question.styling_metadata?.containerClasses &&
question.styling_metadata.containerClasses with the simplified optional chaining
form (e.g., question.styling_metadata?.classes and
question.styling_metadata?.containerClasses) so the className/cn calls mirror
the simplified approach already used in RepeatableGroupRenderer; update all
usages inside the QuestionGroup component (including the className for the
wrapper and the container className) to this simplified form.
---
Duplicate comments:
In `@src/components/Questionnaire/QuestionnaireForm.tsx`:
- Around line 354-356: The dateTime branch in QuestionnaireForm (the block
checking value.type === "dateTime") assumes value.value is a Date and calls
.toISOString(), which can throw if it's a string; update that branch to mirror
the date handling: coerce value.value into a Date (e.g., new Date(value.value))
only when value.value exists and is not already a Date, guard against invalid
dates, then return the object with value set to the ISO string; use the same
checks/logic pattern used in the date case to ensure consistency and avoid
runtime TypeError.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4f6a595e-24ea-4909-a1e3-0a0453e70c72
📒 Files selected for processing (3)
public/locale/en.jsonsrc/components/Questionnaire/QuestionTypes/QuestionGroup.tsxsrc/components/Questionnaire/QuestionnaireForm.tsx
abhimanyurajeesh
changed the title
Comment on lines
+801
to
+804
| q.questions!.forEach((subQ) => { | ||
| if (subQ.type === "group" && subQ.questions) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
When a repeatable group contains a nested non-repeatable group, validation bails out early with return instead of recursing into the nested group's children. Since the editor allows groups inside repeatable groups (only boolean, display, and structured are excluded via HIDE_REPEATABLE_QUESTION_TYPES), a user can submit without filling required questions that live inside a nested group within a repeatable group. The non-repeatable path correctly calls q.questions.forEach(validateQuestion) recursively — the repeatable path needs a similar recursive descent for subQ.type === "group" children across each instance.
yash-learner
requested changes
Jun 16, 2026
yash-learner
left a comment
yash-learner
left a comment
Member
There was a problem hiding this comment.
I think this is out of scope of this PR, maybe a separate ticket. we need to fix the font of the text for group inside in a group because it looks out when there are sub questions of non group type inside a group along with it
like same on questionnairereponselist on encounter home
| <QuestionDescription question={question} /> | ||
| </div> | ||
| )} | ||
| <div className="space-y-4 p-2"> |
Member
There was a problem hiding this comment.
I feel there is not lot of unnecessary vertical space and why can't we have the number and question name on same line ?
#2 New Sub Question
Contributor
Author
There was a problem hiding this comment.
thought so but there is a Close button to
Contributor
Author
There was a problem hiding this comment.
this looks good on phone
Comment on lines
+332
to
+368
| function isValueFilled(v: ResponseValue): boolean { | ||
| if (v.coding) return true; | ||
| if (v.value == null || v.value === "") return false; | ||
| if (Array.isArray(v.value) && v.value.length === 0) return false; | ||
| return true; | ||
| } | ||
|
|
||
| const processQuestion = (q: Question) => { | ||
| if (q.type === "group" && q.questions) { | ||
| q.questions.forEach(processQuestion); | ||
| } else { | ||
| let defaultValues: ResponseValue[] = []; | ||
| if (q.answer_option && q.answer_option.length > 0) { | ||
| const defaultOptions: AnswerOption[] = q.answer_option.filter( | ||
| (o) => o.initial_selected === true, | ||
| ); | ||
| if (defaultOptions.length > 0) { | ||
| defaultValues = defaultOptions.map((opt) => ({ | ||
| type: "string", | ||
| value: opt.value, | ||
| coding: opt.code ?? undefined, | ||
| })); | ||
| } | ||
| } | ||
| responses.push({ | ||
| question_id: q.id, | ||
| link_id: q.link_id, | ||
| values: defaultValues, | ||
| structured_type: q.structured_type ?? null, | ||
| }); | ||
| function isResponseFilled(r: QuestionnaireResponse): boolean { | ||
| if (r.structured_type) return false; | ||
| if (r.values.length > 0 && r.values.some(isValueFilled)) return true; | ||
| if (r.sub_results?.some((inst) => inst.some(isResponseFilled))) return true; | ||
| return false; | ||
| } | ||
|
|
||
| function serializeValue(value: ResponseValue): Record<string, unknown> { | ||
| if (value.type === "date" && value.value) { | ||
| const date = new Date(value.value); | ||
| if (isNaN(date.getTime())) { | ||
| return { ...value, value: "" }; | ||
| } | ||
| return { ...value, value: dateQueryString(date) }; | ||
| } | ||
| if (value.type === "dateTime" && value.value) { | ||
| return { ...value, value: value.value.toISOString() }; | ||
| } | ||
| if (value.unit) { | ||
| return { | ||
| value: value.value?.toString(), | ||
| unit: value.unit, | ||
| coding: value.coding, | ||
| }; | ||
| } | ||
| if (value.coding) return { coding: value.coding }; | ||
| return { value: String(value.value) }; | ||
| } | ||
|
|
||
| function serializeRepeatableGroupResponse( |
Member
There was a problem hiding this comment.
I feel all these methods should inside the utils src/components/Questionnaire/utils.ts
what do you think ?
Contributor
Author
There was a problem hiding this comment.
could be but i dont see a real use case
Comment on lines
+214
to
+221
| function hasStructuredQuestion(questions?: Question[]): boolean { | ||
| if (!questions) return false; | ||
| return questions.some( | ||
| (q) => | ||
| q.type === "structured" || | ||
| (q.type === "group" && hasStructuredQuestion(q.questions)), | ||
| ); | ||
| } |
Member
There was a problem hiding this comment.
This is the duplicate of the
so can we move this to util and use at both places ?
Contributor
Author
There was a problem hiding this comment.
yap these two are similar, can be moved to utility
ohc-project-manager
Bot
added
Changes Required
and removed
needs review
needs testing
labels
…n QuestionnaireForm and QuestionnaireEditor
ohc-project-manager
Bot
added
needs testing
needs review
and removed
Changes Required
labels
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed Changes
This PR adds support for repeatable question groups and fixes the repeatable question validation issue.
Fixes ENG-421
Tagging: @ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit