(fix) O3-5432: Validate completion date against enrollment date in program enrollment form#3035
Conversation
da249e2 to
ae5d851
Compare
|
@denniskigen @VeronicaMuthee Could you please review this PR? |
denniskigen
left a comment
There was a problem hiding this comment.
Thanks for adding the cross-field validation!
| selectedProgram: z.string().refine((value) => !!value, t('programRequired', 'Program is required')), | ||
| enrollmentDate: z.date(), | ||
| completionDate: z.date().optional().nullable(), | ||
| enrollmentLocation: z.string(), |
There was a problem hiding this comment.
nit: The backend (PatientProgramValidator in openmrs-core) uses a strictly-less-than comparison — it allows dateCompleted == dateEnrolled (same-day completion). This check uses <=, which is stricter than the backend. Was that intentional?
If we want to align with the backend, this should be < instead of <=, and the minDate / isDisabled constraints on the completion date picker (lines 237-239) should also be relaxed to allow same-day dates.
There was a problem hiding this comment.
@denniskigen Yes. if you check here, the existing UI already enforced that completion date must be at least 1 day after enrollment date. I matched the superRefine validation to be consistent with these existing UI constraints, that's why I used <=.
That being said should we allow same day completions?
packages/esm-patient-programs-app/src/programs/programs-form.workspace.tsx
Show resolved
Hide resolved
| await user.tab(); | ||
|
|
||
| await user.click(saveButton); | ||
|
|
There was a problem hiding this comment.
Could we also assert that the validation error message is rendered? Right now the test only checks that the submit handlers weren't called, but doesn't verify the user actually sees feedback. Something like:
expect(screen.getByText(/completion date cannot be before the enrollment date/i)).toBeInTheDocument();There was a problem hiding this comment.
@denniskigen regarding this, if you check here, the OpenmrsDatePicker mock in @openmrs/esm-framework destructures isInvalid to conditionally render invalidText but the real component (and all usages across the codebase) passes invalid, not isInvalid. So the mock never renders the error text, making screen.getByText(...) impossible for date picker validation errors.
Would it maybe make more sense to fix it there first?
Requirements
Summary
superRefinevalidation to prevent saving a program enrollment when the completion date is before the enrollment dateScreenshots
Related Issue
O3-5432
Other