Repository navigation
feat: fine-tune field widgets (multi-select, boolean radios, calendars) - #23
Conversation
…alendars Map MULTI_TEXT+optionSet to multiSelect with comma-separated codes, render BOOLEAN as Yes/No radios, and replace native date/time inputs with calendar pickers across dhis2-ui, mantine, and mui (including DATETIME).
|
Warning Review limit reached
Next review available in: 49 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR adds ChangesField contracts and validation
Adapter implementations
Shared behavior and Storybook coverage
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Storybook
participant D2Field
participant Widget
participant FieldValidation
Storybook->>D2Field: render widget kind
D2Field->>Widget: select mapped field component
Widget->>FieldValidation: validate changed value
FieldValidation-->>Widget: accept or reject value
Widget-->>Storybook: update field state
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
Strip invalid fallow-ignore reasons, suppress mirrored D2Field duplication at file scope, and simplify temporal input queries so new-only audit passes.
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
utils/hooks/src/fields/computeAgeFromDob.test.ts (1)
5-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert exact ages with a fixed clock.
The current test only proves that the result is a positive numeric string. It passes when the age is off by one. Freeze the clock and assert exact values before, on, and after the birthday. Add an invalid calendar-date case such as
2024-02-31.Proposed test improvement
-import { describe, expect, it } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import { computeAgeFromDob } from './computeAgeFromDob'; +afterEach(() => vi.useRealTimers()); + describe('computeAgeFromDob', () => { ... it('computes whole years from a date of birth', () => { - expect(computeAgeFromDob('2000-01-01')).toMatch(/^\d+$/); - expect(Number(computeAgeFromDob('2000-01-01'))).toBeGreaterThan(0); + vi.useFakeTimers(); + vi.setSystemTime(new Date(2025, 5, 15, 12)); + + expect(computeAgeFromDob('2000-01-01')).toBe('25'); + expect(computeAgeFromDob('2000-06-15')).toBe('25'); + expect(computeAgeFromDob('2000-06-16')).toBe('24'); + expect(computeAgeFromDob('2024-02-31')).toBe(''); });Verify that the repository’s Vitest setup supports this clock-control pattern.
🤖 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 `@utils/hooks/src/fields/computeAgeFromDob.test.ts` around lines 5 - 13, Strengthen the tests for computeAgeFromDob by using the repository-supported Vitest clock controls to freeze the current date, then assert exact ages for dates before, on, and after the birthday. Extend the invalid-date cases with a value such as 2024-02-31 and preserve the existing empty-string behavior.
🤖 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 `@apps/storybook/interactions/fieldStoryInteractions.ts`:
- Around line 247-273: Update the dateInput and datetimeInput play functions to
exercise the adapter-specific picker controls rather than skipping read-only
inputs or only checking control presence. Select representative date and
datetime values through those picker interactions, then assert the fields’
serialized values; keep timeInput unchanged and use the existing field-query and
assertion helpers where applicable.
In `@components/dhis2-ui/src/fields/widgets/DateFields.tsx`:
- Around line 81-87: Update the commit function so it clears the field whenever
nextDate is empty, regardless of whether nextTime has a value, preventing
persistence of time-only values such as “T14:30”. Preserve the existing
empty-date-and-time handling and datetime construction for valid dates;
optionally disable the time input until a date is selected.
- Line 5: Update the todayIso helper used by D2DateField and D2DateTimeField to
derive the date from local calendar components instead of Date.toISOString(),
preserving the YYYY-MM-DD format used by the future-date limit.
In `@components/mantine/package.json`:
- Around line 28-35: Update the peerDependencies objects in
components/mantine/package.json (lines 28-35) and components/mui/package.json
(lines 28-36) to declare `@dhis2/app-runtime` and `@dhis2/rule-engine` as peer
dependencies, ensuring both adapter packages require them without bundling them.
In `@components/mui/package.json`:
- Around line 31-33: Align the `@mui/material` and `@mui/x-date-pickers` dependency
ranges with the selected `@mui/x-date-pickers` 9.10.1 peer-dependency contract,
avoiding ranges that permit incompatible future Material UI versions. Apply the
matching versions in components/mui/package.json lines 31-33 and 42-43, and
apps/storybook/package.json lines 25-27, keeping the MUI package pair consistent
across both packages.
In `@docs/use-field-control-plan.md`:
- Around line 831-851: Update the earlier WidgetKind example and the test-plan
example to reflect the resolved option-set mapping: option sets map to select
except MULTI_TEXT with an optionSet, which maps to multiSelect. Preserve the
existing select behavior for all other option-set value types and align both
examples with the current resolver and mapping table.
In `@packages/metadata/src/buildTeaFieldSchema.ts`:
- Line 13: Update the DATETIME validation anchored by DATETIME_PATTERN in
packages/metadata/src/buildTeaFieldSchema.ts:13-13 and the event-field schema in
utils/hooks/src/fields/fieldValidation.ts:5-5 to retain the existing format
check and add the same calendar- and clock-aware refinement, rejecting invalid
dates and times such as 2026-99-99T99:99 in both schemas.
- Around line 15-18: Update rejectFutureDates so DATETIME values are compared
using only their YYYY-MM-DD portion against todayIso(), preventing valid times
on the current date from being rejected while preserving the existing
future-date validation.
In `@utils/hooks/src/fields/computeAgeFromDob.ts`:
- Around line 3-10: Update the DOB parsing in the age-calculation function to
extract year, month, and day components, construct the birth date as a local
calendar date, and reject it when the constructed date’s components differ from
the parsed values, covering invalid dates such as February 31. Preserve the
existing invalid-input return and age calculation behavior while removing
reliance on new Date(dob).
---
Nitpick comments:
In `@utils/hooks/src/fields/computeAgeFromDob.test.ts`:
- Around line 5-13: Strengthen the tests for computeAgeFromDob by using the
repository-supported Vitest clock controls to freeze the current date, then
assert exact ages for dates before, on, and after the birthday. Extend the
invalid-date cases with a value such as 2024-02-31 and preserve the existing
empty-string behavior.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9f5bdeea-d3de-4ed1-bf75-fa0f0c797424
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (41)
apps/storybook/.storybook/preview.tsxapps/storybook/fixtures/fieldMetadata.tsapps/storybook/interactions/ancInteractions.tsapps/storybook/interactions/fieldStoryInteractions.tsapps/storybook/package.jsonapps/storybook/stories/dhis2-ui/D2Field.stories.tsxapps/storybook/stories/mantine/D2Field.stories.tsxapps/storybook/stories/mui/D2Field.stories.tsxcomponents/dhis2-ui/src/fields/D2Field.tsxcomponents/dhis2-ui/src/fields/widgets/ChoiceFields.tsxcomponents/dhis2-ui/src/fields/widgets/DateFields.tsxcomponents/dhis2-ui/src/fields/widgets/NumericFields.tsxcomponents/dhis2-ui/src/fields/widgets/index.tscomponents/mantine/package.jsoncomponents/mantine/src/fields/D2Field.tsxcomponents/mantine/src/fields/widgets/ChoiceFields.tsxcomponents/mantine/src/fields/widgets/DateFields.tsxcomponents/mantine/src/fields/widgets/TextField.tsxcomponents/mantine/src/fields/widgets/index.tscomponents/mui/package.jsoncomponents/mui/src/fields/D2Field.tsxcomponents/mui/src/fields/widgets/ChoiceFields.tsxcomponents/mui/src/fields/widgets/DateFields.tsxcomponents/mui/src/fields/widgets/TextField.tsxcomponents/mui/src/fields/widgets/index.tsdocs/use-field-control-plan.mdpackages/metadata/src/buildTeaFieldSchema.tspackages/metadata/src/index.tspackages/metadata/src/multiTextValue.test.tspackages/metadata/src/multiTextValue.tsutils/hooks/src/fields/computeAgeFromDob.test.tsutils/hooks/src/fields/computeAgeFromDob.tsutils/hooks/src/fields/fieldValidation.test.tsutils/hooks/src/fields/fieldValidation.tsutils/hooks/src/fields/multiTextValue.test.tsutils/hooks/src/fields/multiTextValue.tsutils/hooks/src/fields/widgetKind.test.tsutils/hooks/src/fields/widgetKind.tsutils/hooks/src/index.tsutils/rules/src/filterPayload.test.tsutils/rules/src/filterPayload.ts
💤 Files with no reviewable changes (2)
- components/mantine/src/fields/widgets/TextField.tsx
- components/mui/src/fields/widgets/TextField.tsx
| const dateInput: PlayFunction<FieldStoryArgs> = async ({ canvasElement }) => { | ||
| await typeIntoField(canvasElement, 'date', '2024-06-15'); | ||
| const canvas = canvasOf(canvasElement); | ||
| const input = queryFieldInput(canvas, 'date'); | ||
| await expect(input).toBeInTheDocument(); | ||
| if (input instanceof HTMLInputElement && !input.readOnly) { | ||
| await userEvent.clear(input); | ||
| await userEvent.type(input, '2024-06-15'); | ||
| await assertInputValue(input, '2024-06-15'); | ||
| } | ||
| }; | ||
|
|
||
| const timeInput: PlayFunction<FieldStoryArgs> = async ({ canvasElement }) => { | ||
| const canvas = canvasOf(canvasElement); | ||
| const input = queryFieldInput(canvas, 'time'); | ||
| await expect(input).toBeInTheDocument(); | ||
| if (input instanceof HTMLInputElement && !input.readOnly) { | ||
| await userEvent.clear(input); | ||
| await userEvent.type(input, '14:30'); | ||
| await assertInputValue(input, '14:30'); | ||
| } | ||
| }; | ||
|
|
||
| const datetimeInput: PlayFunction<FieldStoryArgs> = async ({ canvasElement }) => { | ||
| const canvas = canvasOf(canvasElement); | ||
| const matches = canvas.queryAllByLabelText(labelPattern('datetime')); | ||
| await expect(matches.length).toBeGreaterThan(0); | ||
| }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Exercise temporal value changes in these plays.
dateInput does nothing when the adapter exposes a read-only picker input. datetimeInput only checks that a labeled control exists. These plays can pass while date or datetime selection does not update the field value.
Use adapter-specific picker interactions. Assert the resulting serialized date and datetime values after selection. As per coding guidelines, “Use Storybook in apps/storybook as the primary component documentation and integration/interaction test surface.”
🤖 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 `@apps/storybook/interactions/fieldStoryInteractions.ts` around lines 247 -
273, Update the dateInput and datetimeInput play functions to exercise the
adapter-specific picker controls rather than skipping read-only inputs or only
checking control presence. Select representative date and datetime values
through those picker interactions, then assert the fields’ serialized values;
keep timeInput unchanged and use the existing field-query and assertion helpers
where applicable.
Source: Coding guidelines
| import { resolveFieldValidation } from '@dhis2-form-utils/hooks'; | ||
| import { computeAgeFromDob, resolveFieldValidation } from '@dhis2-form-utils/hooks'; | ||
|
|
||
| const todayIso = (): string => new Date().toISOString().slice(0, 10); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use the local calendar date for the future-date limit.
toISOString() converts the local time to UTC. It can allow tomorrow or reject today near a UTC day boundary. This affects both D2DateField and D2DateTimeField.
Proposed fix
-const todayIso = (): string => new Date().toISOString().slice(0, 10);
+const todayIso = (): string => {
+ const now = new Date();
+ const month = String(now.getMonth() + 1).padStart(2, '0');
+ const day = String(now.getDate()).padStart(2, '0');
+ return `${now.getFullYear()}-${month}-${day}`;
+};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const todayIso = (): string => new Date().toISOString().slice(0, 10); | |
| const todayIso = (): string => { | |
| const now = new Date(); | |
| const month = String(now.getMonth() + 1).padStart(2, '0'); | |
| const day = String(now.getDate()).padStart(2, '0'); | |
| return `${now.getFullYear()}-${month}-${day}`; | |
| }; |
🤖 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 `@components/dhis2-ui/src/fields/widgets/DateFields.tsx` at line 5, Update the
todayIso helper used by D2DateField and D2DateTimeField to derive the date from
local calendar components instead of Date.toISOString(), preserving the
YYYY-MM-DD format used by the future-date limit.
| const commit = (nextDate: string, nextTime: string) => { | ||
| if (!nextDate && !nextTime) { | ||
| field.onChange(''); | ||
| return; | ||
| } | ||
| field.onChange(`${nextDate}T${nextTime || '00:00'}`); | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not persist a time without a date.
If a user clears the date while a time exists, or enters time first, commit writes values such as T14:30. The next render passes that invalid date portion to CalendarInput, and strict datetime validation rejects the field. Clear the value when nextDate is empty. Optionally disable the time input until a date exists.
Proposed fix
- if (!nextDate && !nextTime) {
+ if (!nextDate) {
field.onChange('');
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const commit = (nextDate: string, nextTime: string) => { | |
| if (!nextDate && !nextTime) { | |
| field.onChange(''); | |
| return; | |
| } | |
| field.onChange(`${nextDate}T${nextTime || '00:00'}`); | |
| }; | |
| const commit = (nextDate: string, nextTime: string) => { | |
| if (!nextDate) { | |
| field.onChange(''); | |
| return; | |
| } | |
| field.onChange(`${nextDate}T${nextTime || '00:00'}`); | |
| }; |
🤖 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 `@components/dhis2-ui/src/fields/widgets/DateFields.tsx` around lines 81 - 87,
Update the commit function so it clears the field whenever nextDate is empty,
regardless of whether nextTime has a value, preventing persistence of time-only
values such as “T14:30”. Preserve the existing empty-date-and-time handling and
datetime construction for valid dates; optionally disable the time input until a
date is selected.
| "peerDependencies": { | ||
| "@mantine/core": ">=7.0.0", | ||
| "@mantine/dates": ">=7.0.0", | ||
| "@mantine/hooks": ">=7.0.0", | ||
| "dayjs": ">=1.11.0", | ||
| "react": ">=18.0.0", | ||
| "react-dom": ">=18.0.0", | ||
| "react-hook-form": ">=7.0.0" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Declare the required DHIS2 peer dependencies in both adapter packages.
components/mantine/package.json#L28-L35: add@dhis2/app-runtimeand@dhis2/rule-enginetopeerDependencies.components/mui/package.json#L28-L36: add@dhis2/app-runtimeand@dhis2/rule-enginetopeerDependencies.
As per coding guidelines, "**/package.json: Declare @dhis2/app-runtime and @dhis2/rule-engine as peer dependencies; they must not be bundled."
📍 Affects 2 files
components/mantine/package.json#L28-L35(this comment)components/mui/package.json#L28-L36
🤖 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 `@components/mantine/package.json` around lines 28 - 35, Update the
peerDependencies objects in components/mantine/package.json (lines 28-35) and
components/mui/package.json (lines 28-36) to declare `@dhis2/app-runtime` and
`@dhis2/rule-engine` as peer dependencies, ensuring both adapter packages require
them without bundling them.
Source: Coding guidelines
| "@mui/material": ">=6.0.0", | ||
| "@mui/x-date-pickers": ">=7.0.0", | ||
| "dayjs": ">=1.11.0", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package.json files =="
git ls-files '*package.json' | sed -n '1,120p'
echo
echo "== relevant package lines =="
for f in components/mui/package.json apps/storybook/package.json; do
if [ -f "$f" ]; then
echo "--- $f ---"
nl -ba "$f" | sed -n '1,90p'
else
echo "missing $f"
fi
done
echo
echo "== lockfiles / manifests mentioning package versions =="
for f in pnpm-lock.yaml apps/playground/package.json components/mui/package.json apps/storybook/package.json; do
if [ -f "$f" ]; then
echo "--- $f relevant ---"
rg -n '`@mui/material`|`@mui/x-date-pickers`|dayjs' "$f" || true
fi
doneRepository: nnkogift/dhis2-form-utils
Length of output: 635
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import json, pathlib
for rel in ["components/mui/package.json", "apps/storybook/package.json"]:
p=pathlib.Path(rel)
print(f"--- {rel} ---")
if not p.exists():
print("missing")
continue
data=json.loads(p.read_text())
for sec in ["peerDependencies","devDependencies","dependencies"]:
deps=data.get(sec,{})
hits={k:v for k,v in deps.items() if k.startswith("`@mui/`") or k=="dayjs"}
if hits:
print(sec, hits)
print("workspace:", data.get("workspace"))
PYRepository: nnkogift/dhis2-form-utils
Length of output: 563
🌐 Web query:
@mui/x-date-pickers 9.10.1 supported @mui/material versions
💡 Result:
The @mui/x-date-pickers version 9.10.1 has the following peer dependency requirement for @mui/material: ^5.15.14 || ^6.0.0 || ^7.0.0 [1]. Although older versions of the peer dependency range may be technically listed in some manifests [1], MUI X v9 migration documentation explicitly requires upgrading to at least @mui/material version 7.3.0 or 9.0.0 (depending on the specific MUI ecosystem versioning at the time) to ensure compatibility and leverage required API changes, such as theming optimizations [2][3]. Compatibility with Material UI v5 and v6 is generally deprecated or requires specific configurations in MUI X v9, and users are strongly encouraged to upgrade to Material UI v7 or higher [2][4][3].
Citations:
- 1: https://cdn.jsdelivr.net/npm/@mui/x-date-pickers@9.10.1/README.md
- 2: https://mui.com/x/migration/migration-pickers-v8/
- 3: [question] Migration from v8 to v9 fails – unclear peer dependency requirements for @mui/material mui/mui-x#22065
- 4: https://mui.com/x/migration/usage-with-material-ui-v5-v6/
Align the MUI X and Material UI version pair.
@mui/x-date-pickers 9.10.1 declares peerDependencies as ^5.15.14 || ^6.0.0 || ^7.0.0; @mui/material and the Storybook app should use a Material UI version compatible with the selected @mui/x-date-pickers version.
📍 Affects 2 files
components/mui/package.json#L31-L33(this comment)components/mui/package.json#L42-L43apps/storybook/package.json#L25-L27
🤖 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 `@components/mui/package.json` around lines 31 - 33, Align the `@mui/material`
and `@mui/x-date-pickers` dependency ranges with the selected `@mui/x-date-pickers`
9.10.1 peer-dependency contract, avoiding ranges that permit incompatible future
Material UI versions. Apply the matching versions in components/mui/package.json
lines 31-33 and 42-43, and apps/storybook/package.json lines 25-27, keeping the
MUI package pair consistent across both packages.
| | `valueType` | `widgetKind` | DHIS2 UI | Mantine | Material UI | | ||
| | ----------------------------------- | ------------- | ----------------------------------- | -------------------------------- | --------------------------- | | ||
| | `TEXT`, `LETTER`, `URL`, `USERNAME` | `text` | `InputField` | `TextInput` | `TextField` | | ||
| | `LONG_TEXT` | `longText` | `TextAreaField` | `Textarea` | `TextField multiline` | | ||
| | `EMAIL` | `email` | `InputField type=email` | `TextInput type=email` | `TextField type=email` | | ||
| | `PHONE_NUMBER` | `phone` | `InputField type=tel` | `TextInput type=tel` | `TextField type=tel` | | ||
| | `NUMBER`, `UNIT_INTERVAL` | `number` | `InputField type=number` | `NumberInput` | `TextField type=number` | | ||
| | `INTEGER`, `INTEGER_*` | `integer` | `InputField type=number step=1` | `NumberInput allowDecimal=false` | `TextField type=number` | | ||
| | `PERCENTAGE` | `percentage` | `InputField` + `%` suffix | `NumberInput` + `%` | `TextField` + `%` adornment | | ||
| | `BOOLEAN` | `boolean` | Yes/No/(—) `Radio` | Yes/No/(—) `Radio.Group` | Yes/No/(—) `RadioGroup` | | ||
| | `TRUE_ONLY` | `trueOnly` | `Checkbox` | `Checkbox` | `Checkbox` | | ||
| | `DATE` | `date` | `CalendarInput` | `DateInput` | `DatePicker` | | ||
| | `DATETIME` | `datetime` | `CalendarInput` + `InputField` time | `DateTimePicker` | `DateTimePicker` | | ||
| | `TIME` | `time` | `InputField type=time` | `TimeInput` | `TimePicker` | | ||
| | `AGE` | `age` | `CalendarInput` + age display | `DateInput` + age display | `DatePicker` + age display | | ||
| | any + `optionSet` (not MULTI_TEXT) | `select` | `SingleSelectField` | `Select` | `Select` | | ||
| | `MULTI_TEXT` + `optionSet` | `multiSelect` | `MultiSelectField` (CSV codes) | `MultiSelect` (CSV codes) | multi `TextField select` | | ||
| | `COORDINATE` | `coordinate` | Two `InputField` (lat + lng) | Two `NumberInput` | Two `TextField` | | ||
| | `FILE_RESOURCE` | `file` | `FileInputField` | `FileInput` | `Button` file type | | ||
| | `IMAGE` | `image` | `FileInputField` + preview | `FileInput` + preview | `Button` file + preview | | ||
| | `ORGANISATION_UNIT` | `orgUnit` | DHIS2 `OrgUnitField` | Custom tree picker | Custom tree picker | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Synchronize the remaining plan examples with the resolved mapping.
The new table and the resolved multiSelect note are correct. The earlier WidgetKind example at Lines 316-340 and the test-plan example at Lines 929-932 still omit multiSelect and state that every option set maps to select. Update those examples to include the MULTI_TEXT branch.
Proposed documentation update
- | 'select' // any valueType WITH an optionSet (single-select)
+ | 'select' // any valueType WITH an optionSet except MULTI_TEXT
+ | 'multiSelect' // MULTI_TEXT WITH an optionSet
...
- if (config.optionSet) return 'select';
+ if (config.optionSet) {
+ return config.valueType === 'MULTI_TEXT' ? 'multiSelect' : 'select';
+ }
...
- case 'LONG_TEXT':
+ case 'LONG_TEXT':
+ case 'MULTI_TEXT':This follows the current resolver implementation and the updated plan table.
Also applies to: 1009-1015
🤖 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 `@docs/use-field-control-plan.md` around lines 831 - 851, Update the earlier
WidgetKind example and the test-plan example to reflect the resolved option-set
mapping: option sets map to select except MULTI_TEXT with an optionSet, which
maps to multiSelect. Preserve the existing select behavior for all other
option-set value types and align both examples with the current resolver and
mapping table.
| }; | ||
|
|
||
| const todayIso = (): string => new Date().toISOString().slice(0, 10); | ||
| const DATETIME_PATTERN = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}$/; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Validate calendar and clock ranges in both DATETIME schemas.
Both regular expressions check only digit counts. They accept invalid values such as 2026-99-99T99:99, so invalid form values can pass validation.
- packages/metadata/src/buildTeaFieldSchema.ts#L13-L13: add a calendar-aware refinement after the format check.
- utils/hooks/src/fields/fieldValidation.ts#L5-L5: apply the same refinement to the event-field schema.
📍 Affects 2 files
packages/metadata/src/buildTeaFieldSchema.ts#L13-L13(this comment)utils/hooks/src/fields/fieldValidation.ts#L5-L5
🤖 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 `@packages/metadata/src/buildTeaFieldSchema.ts` at line 13, Update the DATETIME
validation anchored by DATETIME_PATTERN in
packages/metadata/src/buildTeaFieldSchema.ts:13-13 and the event-field schema in
utils/hooks/src/fields/fieldValidation.ts:5-5 to retain the existing format
check and add the same calendar- and clock-aware refinement, rejecting invalid
dates and times such as 2026-99-99T99:99 in both schemas.
| const rejectFutureDates = (schema: z.ZodTypeAny): z.ZodTypeAny => | ||
| schema.refine((value) => typeof value !== 'string' || value === '' || value <= todayIso(), { | ||
| message: 'Date cannot be in the future', | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Compare the date portion for DATETIME future checks.
rejectFutureDates compares a YYYY-MM-DDTHH:mm value with date-only todayIso(). On August 5, 2026, 2026-08-05T09:00 is lexically greater than 2026-08-05, so allowFutureDate=false rejects every datetime on August 5, 2026. Compare value.slice(0, 10) with todayIso(), or compare full timestamps.
Minimal fix
- value <= todayIso(),
+ value.slice(0, 10) <= todayIso(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const rejectFutureDates = (schema: z.ZodTypeAny): z.ZodTypeAny => | |
| schema.refine((value) => typeof value !== 'string' || value === '' || value <= todayIso(), { | |
| message: 'Date cannot be in the future', | |
| }); | |
| const rejectFutureDates = (schema: z.ZodTypeAny): z.ZodTypeAny => | |
| schema.refine((value) => typeof value !== 'string' || value === '' || value.slice(0, 10) <= todayIso(), { | |
| message: 'Date cannot be in the future', | |
| }); |
🤖 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 `@packages/metadata/src/buildTeaFieldSchema.ts` around lines 15 - 18, Update
rejectFutureDates so DATETIME values are compared using only their YYYY-MM-DD
portion against todayIso(), preventing valid times on the current date from
being rejected while preserving the existing future-date validation.
| if (!/^\d{4}-\d{2}-\d{2}$/.test(dob)) return ''; | ||
| const birth = new Date(dob); | ||
| if (Number.isNaN(birth.getTime())) return ''; | ||
| const today = new Date(); | ||
| let age = today.getFullYear() - birth.getFullYear(); | ||
| const monthDiff = today.getMonth() - birth.getMonth(); | ||
| if (monthDiff < 0 || (monthDiff === 0 && today.getDate() < birth.getDate())) { | ||
| age -= 1; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/sh
set -eu
TZ=America/Los_Angeles node - <<'NODE'
const parsed = new Date('2000-01-01');
console.log(parsed.getFullYear(), parsed.getMonth() + 1, parsed.getDate());
console.log(new Date('2024-02-31').toString());
NODERepository: nnkogift/dhis2-form-utils
Length of output: 232
🏁 Script executed:
#!/bin/bash
set -eu
printf 'Repo files matching computeAgeFromDob:\n'
fd -a 'computeAgeFromDob\.ts$|field.*dob|Date' . | sed 's#^\./##' | head -100
printf '\nTarget file:\n'
cat -n utils/hooks/src/fields/computeAgeFromDob.ts
printf '\nUsages of computeAgeFromDob:\n'
rg -n "computeAgeFromDob|ageFromDob|Age.*DOB|Date.*DOB" utils apps . --glob '!node_modules' --glob '!dist' --glob '!build' | head -120
printf '\nLocate date widget/value adapters in hooks:\n'
fd -t f '.*' utils/hooks/src | rg -i "date|dob|field" | head -150Repository: nnkogift/dhis2-form-utils
Length of output: 4607
Parse date-only DOB values as local calendar dates.
new Date(dob) parses an ISO date-only string at UTC midnight, but getFullYear(), getMonth(), and getDate() read local time. With negative UTC offsets, 2000-01-01 can become December 31, 1999 in the local calendar and advance the age one day early.
This parser also normalizes invalid calendar dates like 2024-02-31 to 2024-03-01; the existing checks still accept this. Parse the date components, construct a local date, reject any normalized component, then calculate the age.
Proposed fix
- if (!/^\d{4}-\d{2}-\d{2}$/.test(dob)) return '';
- const birth = new Date(dob);
- if (Number.isNaN(birth.getTime())) return '';
+ const match = /^(\d{4})-(\d{2})-(\d{2})$/.exec(dob);
+ if (!match) return '';
+ const year = Number(match[1]);
+ const month = Number(match[2]);
+ const day = Number(match[3]);
+ const birth = new Date(year, month - 1, day);
+ if (
+ birth.getFullYear() !== year ||
+ birth.getMonth() !== month - 1 ||
+ birth.getDate() !== day
+ ) {
+ return '';
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!/^\d{4}-\d{2}-\d{2}$/.test(dob)) return ''; | |
| const birth = new Date(dob); | |
| if (Number.isNaN(birth.getTime())) return ''; | |
| const today = new Date(); | |
| let age = today.getFullYear() - birth.getFullYear(); | |
| const monthDiff = today.getMonth() - birth.getMonth(); | |
| if (monthDiff < 0 || (monthDiff === 0 && today.getDate() < birth.getDate())) { | |
| age -= 1; | |
| const match = /^(\d{4})-(\d{2})-(\d{2})$/.exec(dob); | |
| if (!match) return ''; | |
| const year = Number(match[1]); | |
| const month = Number(match[2]); | |
| const day = Number(match[3]); | |
| const birth = new Date(year, month - 1, day); | |
| if ( | |
| birth.getFullYear() !== year || | |
| birth.getMonth() !== month - 1 || | |
| birth.getDate() !== day | |
| ) { | |
| return ''; | |
| } | |
| const today = new Date(); | |
| let age = today.getFullYear() - birth.getFullYear(); | |
| const monthDiff = today.getMonth() - birth.getMonth(); | |
| if (monthDiff < 0 || (monthDiff === 0 && today.getDate() < birth.getDate())) { | |
| age -= 1; |
🤖 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 `@utils/hooks/src/fields/computeAgeFromDob.ts` around lines 3 - 10, Update the
DOB parsing in the age-calculation function to extract year, month, and day
components, construct the birth date as a local calendar date, and reject it
when the constructed date’s components differ from the parsed values, covering
invalid dates such as February 31. Preserve the existing invalid-input return
and age calculation behavior while removing reliance on new Date(dob).
Resolve ChoiceFields conflict keeping Radio.Group boolean widgets, drop obsolete ancInteractions, and update event/tracker program-rules plays to click Yes/No radios across adapters.
There was a problem hiding this comment.
Stale comment
Left a non-blocking comment (not approving): Cursor Security Agent was present but completed as skipped/failed to start, so required security review did not finish successfully. Cursor Bugbot was not present after the initial poll. Human review is needed before merge; no eligible non-author reviewers were available to assign.
Sent by Cursor Approval Agent: Pull Request Router and Approver
Fieldset textContent includes Yes/No options, so anchored label regexes failed; match the legend instead.
There was a problem hiding this comment.
Left a non-blocking comment (not approving): Cursor Security Agent was present but completed as skipped, so required security review did not finish successfully. Cursor Bugbot was not present after the initial poll. Human review is needed; no eligible non-author reviewers were available to assign.
Sent by Cursor Approval Agent: Pull Request Router and Approver


Summary
Fine-tunes field widgets across hooks and all three UI adapters:
multiSelectwidget with comma-separated option codes—option)main(PRT Storybook fixtures) and updated program-rules boolean plays for radiosTest plan
filterPayloadmulti-code filteringfallow auditnew-only gate passesSummary by CodeRabbit