(fix) O3-3982: Validate against starting a visit at a future time #2013

njiddasalifu · 2024-09-17T12:00:45Z

Requirements

This PR has a title that briefly describes the work done including the ticket number. If there is a ticket, make sure your PR title includes a conventional commit label. See existing PR titles for inspiration.
My work conforms to the OpenMRS 3.0 Styleguide and design documentation.
My work includes tests or is validated by existing tests.

Summary

This PR adds validation logic that ensures a visit cannot be started at a time in the future. It does so by adding a refinement to the visitFormSchema zod schema that compares the value of the visitStartTime field with the current timestamp. If the visitStartTime is in the future, a validation error message will be shown under the field, and the form will not be submitted. The error message shown reads "Visit start time cannot be in the future".

Screenshots

visit-start-time.mp4

Related Issue

https://openmrs.atlassian.net/browse/O3-3982

Other

njiddasalifu · 2024-09-17T12:03:07Z

Hi @denniskigen here is the PR. we can follow from

denniskigen · 2024-09-24T18:07:57Z

packages/esm-patient-chart-app/src/visit/visit-form/visit-form.component.tsx

@@ -464,70 +445,66 @@ const StartVisitForm: React.FC<StartVisitFormProps> = ({
          .pipe(first())
          .subscribe({
            next: (response) => {
-              if (response.status === 201) {


openmrsFetch handles checking the response headers for us, so explicitly checking the status as we're doing here isn't necessary.

njiddasalifu

LGTM!
thanks for comments and addons @denniskigen

denniskigen · 2024-09-25T19:52:24Z

packages/esm-patient-chart-app/src/visit/visit-form/visit-form.component.tsx

@@ -121,59 +121,120 @@ const StartVisitForm: React.FC<StartVisitFormProps> = ({
  });

  const displayVisitStopDateTimeFields = useMemo(
-    () => visitToEdit?.stopDatetime || showVisitEndDateTimeFields,
+    () => Boolean(visitToEdit?.stopDatetime) || showVisitEndDateTimeFields,


This ensures displayVisitStopDateTimeFields always returns a boolean.

denniskigen · 2024-09-27T07:54:10Z

packages/esm-patient-chart-app/src/visit/visit-form/visit-date-time.component.tsx

                onChange={([date]) => onChange(date)}
                value={value ? dayjs(value).format('DD/MM/YYYY') : null}
              >
                <DatePickerInput
                  id={`${dateFieldName}Input`}
+                  invalid={Boolean(errors[dateFieldName])}


The invalid prop expects a boolean.

denniskigen · 2024-09-27T07:54:20Z

packages/esm-patient-chart-app/src/visit/visit-form/visit-date-time.component.tsx

@@ -78,26 +78,24 @@ const VisitDateTimeField: React.FC<VisitDateTimeFieldProps> = ({
            render={({ field: { onBlur, onChange, value } }) => (
              <TimePicker
                id={timeFieldName}
+                invalid={Boolean(errors[timeFieldName])}


Likewise here.

denniskigen · 2024-09-27T07:54:47Z

packages/esm-patient-chart-app/src/visit/visit-form/visit-form.component.tsx

@@ -121,58 +121,73 @@ const StartVisitForm: React.FC<StartVisitFormProps> = ({
  });

  const displayVisitStopDateTimeFields = useMemo(
-    () => visitToEdit?.stopDatetime || showVisitEndDateTimeFields,
+    () => Boolean(visitToEdit?.stopDatetime || showVisitEndDateTimeFields),


We want to ensure that this variable is always a boolean.

denniskigen · 2024-09-27T07:55:11Z

packages/esm-patient-chart-app/src/visit/visit-form/visit-form.component.tsx

+      .refine((data) => validateStartTime(data), {
+        message: t('futureStartTime', 'Visit start time cannot be in the future'),
+        path: ['visitStartTime'],
+      });


This is the key change. It runs whatever value gets filled into the Visit start time input against validateStartTime. If the start time is in the future, the validation error message above is rendered.

denniskigen · 2024-09-27T07:59:10Z

packages/esm-patient-chart-app/src/visit/visit-form/visit-form.test.tsx

+  // TODO: Figure out why this test is failing
+  xit('displays an error message when the visit start time is in the future', async () => {


I don't know why this test is failing. It mimics the user interaction of filling in the field with a value that ought to trigger the validation (similar to the test just above).

This PR adds validation logic that ensures a visit cannot be started at a time in the future. It does so by adding a refinement to the visitFormSchema zod schema that compares the value of the visitStartTime field with the current timestamp. If the visitStartTime is in the future, a validation error message will be shown under the field, and the form will not be submitted. The error message shown reads "Visit start time cannot be in the future". Additionally, this PR refactors the zod schema, centralizing some of the validation logic into reusable helper functions that are hopefully more easier to maintain. It also adds some test coverage for the new validation logic.

packages/esm-patient-chart-app/src/visit/visit-form/visit-date-time.component.tsx

packages/esm-patient-chart-app/src/visit/visit-form/visit-form.component.tsx

denniskigen · 2024-10-02T07:26:37Z

Thanks for the review, @vasharma05!

…enmrs#2013) * (fix) O3-3982: Validate against starting a visit at a future time This PR adds validation logic that ensures a visit cannot be started at a time in the future. It does so by adding a refinement to the visitFormSchema zod schema that compares the value of the visitStartTime field with the current timestamp. If the visitStartTime is in the future, a validation error message will be shown under the field, and the form will not be submitted. The error message shown reads "Visit start time cannot be in the future". Additionally, this PR refactors the zod schema, centralizing some of the validation logic into reusable helper functions that are hopefully more easier to maintain. It also adds some test coverage for the new validation logic. * Review feedback --------- Co-authored-by: Dennis Kigen <[email protected]>

njiddasalifu marked this pull request as draft September 17, 2024 12:01

denniskigen self-requested a review September 17, 2024 12:03

denniskigen changed the title ~~(fix): O3-3982 Prevent future time for start visits~~ (fix) O3-3982: Validate against starting a visit at a future time Sep 24, 2024

denniskigen force-pushed the O3-3982 branch from d1566bc to 7361790 Compare September 24, 2024 17:47

denniskigen reviewed Sep 24, 2024

View reviewed changes

denniskigen marked this pull request as ready for review September 24, 2024 18:20

denniskigen requested review from ibacher, vasharma05 and donaldkibet September 24, 2024 18:52

denniskigen marked this pull request as draft September 25, 2024 10:00

denniskigen force-pushed the O3-3982 branch from a4ea943 to aa0d778 Compare September 25, 2024 13:23

njiddasalifu commented Sep 25, 2024

View reviewed changes

njiddasalifu marked this pull request as ready for review September 25, 2024 13:32

denniskigen marked this pull request as draft September 25, 2024 20:38

denniskigen force-pushed the O3-3982 branch 2 times, most recently from 6ff6226 to 000bd97 Compare September 27, 2024 07:52

denniskigen reviewed Sep 27, 2024

View reviewed changes

denniskigen marked this pull request as ready for review September 27, 2024 08:00

denniskigen requested review from NethmiRodrigo, jayasanka-sack and mogoodrich September 27, 2024 08:47

denniskigen force-pushed the O3-3982 branch from 8c6cbcf to ab0ec0f Compare October 1, 2024 22:05

vasharma05 approved these changes Oct 2, 2024

View reviewed changes

Review feedback

4fe6fad

denniskigen merged commit fd714ea into openmrs:main Oct 2, 2024
6 checks passed

denniskigen mentioned this pull request Dec 17, 2024

(chore) Release v9.0.0 #2162

Merged

3 tasks

njiddasalifu deleted the O3-3982 branch January 8, 2025 12:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(fix) O3-3982: Validate against starting a visit at a future time #2013

(fix) O3-3982: Validate against starting a visit at a future time #2013

njiddasalifu commented Sep 17, 2024 •

edited by denniskigen

Loading

njiddasalifu commented Sep 17, 2024

denniskigen Sep 24, 2024

njiddasalifu left a comment

denniskigen Sep 25, 2024 •

edited

Loading

denniskigen Sep 27, 2024

denniskigen Sep 27, 2024

denniskigen Sep 27, 2024

denniskigen Sep 27, 2024

denniskigen Sep 27, 2024

denniskigen commented Oct 2, 2024

		// TODO: Figure out why this test is failing
		xit('displays an error message when the visit start time is in the future', async () => {

(fix) O3-3982: Validate against starting a visit at a future time #2013

(fix) O3-3982: Validate against starting a visit at a future time #2013

Conversation

njiddasalifu commented Sep 17, 2024 • edited by denniskigen Loading

Requirements

Summary

Screenshots

Related Issue

Other

njiddasalifu commented Sep 17, 2024

denniskigen Sep 24, 2024

Choose a reason for hiding this comment

njiddasalifu left a comment

Choose a reason for hiding this comment

denniskigen Sep 25, 2024 • edited Loading

Choose a reason for hiding this comment

denniskigen Sep 27, 2024

Choose a reason for hiding this comment

denniskigen Sep 27, 2024

Choose a reason for hiding this comment

denniskigen Sep 27, 2024

Choose a reason for hiding this comment

denniskigen Sep 27, 2024

Choose a reason for hiding this comment

denniskigen Sep 27, 2024

Choose a reason for hiding this comment

denniskigen commented Oct 2, 2024

njiddasalifu commented Sep 17, 2024 •

edited by denniskigen

Loading

denniskigen Sep 25, 2024 •

edited

Loading