New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

(feat) O3-2814: Add field validation to the lab order form #1649

Merged

denniskigen merged 11 commits into openmrs:main from usamaidrsk:ft-add-validations-to-lab-order-form

Feb 9, 2024

Member

usamaidrsk commented Feb 8, 2024 •

edited

Loading

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 form validation to the lab orders from

Screenshots

Related Issue

Other

denniskigen requested changes

View reviewed changes

Member

denniskigen left a comment •

edited

Loading

Some good work here, @usamaidrsk, thanks! Just a few things to clean up before this is ready to go.

packages/esm-patient-labs-app/src/lab-orders/add-lab-order/add-lab-order.test.tsx Outdated Show resolved Hide resolved

packages/esm-patient-labs-app/src/lab-orders/add-lab-order/add-lab-order.test.tsx Outdated Show resolved Hide resolved

packages/esm-patient-labs-app/src/lab-orders/add-lab-order/add-lab-order.test.tsx Outdated Show resolved Hide resolved

packages/esm-patient-labs-app/src/lab-orders/add-lab-order/add-lab-order.test.tsx Show resolved Hide resolved

packages/esm-patient-labs-app/src/lab-orders/add-lab-order/add-lab-order.test.tsx Outdated Show resolved Hide resolved

packages/esm-patient-labs-app/src/lab-orders/add-lab-order/add-lab-order.test.tsx Outdated Show resolved Hide resolved

packages/esm-patient-labs-app/src/lab-orders/add-lab-order/add-lab-order.test.tsx Outdated Show resolved Hide resolved

packages/esm-patient-labs-app/src/lab-orders/add-lab-order/lab-order-form.component.tsx Outdated Show resolved Hide resolved

packages/esm-patient-labs-app/src/lab-orders/add-lab-order/lab-order-form.component.tsx Show resolved Hide resolved

packages/esm-patient-labs-app/src/lab-orders/add-lab-order/lab-order-form.component.tsx Outdated

Comment on lines 34 to 40

+                labReferenceNumber: z.string({
+                  required_error: 'Lab reference number is required',
+                  invalid_type_error: 'Lab reference number is required',
+                }),
+                testType: z.object(
+                  { label: z.string(), conceptUuid: z.string() },
+                  { required_error: 'Test type is required', invalid_type_error: 'Test type is required' },

Member

denniskigen Feb 9, 2024

AFAICT, invalid_type_error is used to specify a custom error message that will be displayed when the type of a value does not match the expected type defined in the schema. So, in this case, it will complain if the values provided in the validated fields are not strings. The error message provided should be reflective of that. Or if we're not validating type mismatches, we could remove it entirely.

Member

vasharma05 Feb 9, 2024

Hi, @denniskigen!
We should use invalid_type_error for objects, because the error thrown by Zod in case the input is removed says "Expected object, recieved null".

@usamaidrsk, we don't need need invalid_type_error messages for string, required_error should handle that for the strings. Please see if removing invalid_type_error for z.object doesn't throw the error I shared above, i.e. required_error takes care of it or not.
Thanks!

Member Author

usamaidrsk Feb 9, 2024

Thanks @denniskigen
Here I used the same error message as of required, just because Expected string, received null would not give clear meaning to the user on the form.

If there is a suggestion, I would like to know, then I can replace with that.

vasharma05 reviewed

View reviewed changes

packages/esm-patient-labs-app/src/lab-orders/add-lab-order/add-lab-order.test.tsx Outdated Show resolved Hide resolved

packages/esm-patient-labs-app/src/lab-orders/add-lab-order/lab-order-form.component.tsx Outdated Show resolved Hide resolved

packages/esm-patient-labs-app/src/lab-orders/add-lab-order/lab-order-form.component.tsx Outdated

Comment on lines 34 to 40

+                labReferenceNumber: z.string({
+                  required_error: 'Lab reference number is required',
+                  invalid_type_error: 'Lab reference number is required',
+                }),
+                testType: z.object(
+                  { label: z.string(), conceptUuid: z.string() },
+                  { required_error: 'Test type is required', invalid_type_error: 'Test type is required' },

Member

vasharma05 Feb 9, 2024

Hi, @denniskigen!
We should use invalid_type_error for objects, because the error thrown by Zod in case the input is removed says "Expected object, recieved null".

@usamaidrsk, we don't need need invalid_type_error messages for string, required_error should handle that for the strings. Please see if removing invalid_type_error for z.object doesn't throw the error I shared above, i.e. required_error takes care of it or not.
Thanks!

packages/esm-patient-labs-app/src/lab-orders/add-lab-order/lab-order-form.component.tsx Show resolved Hide resolved

packages/esm-patient-labs-app/src/lab-orders/add-lab-order/lab-order-form.component.tsx Outdated Show resolved Hide resolved

packages/esm-patient-labs-app/src/lab-orders/add-lab-order/lab-order-form.component.tsx Outdated

                 );
                 const cancelOrder = useCallback(() => {
-                  setOrders(orders.filter((order) => order.testType.conceptUuid !== inProgressLabOrder.testType.conceptUuid));
+                  setOrders(orders.filter((order) => order.testType.conceptUuid !== getValues().testType.conceptUuid));

Member

vasharma05 Feb 9, 2024

getValues expect an arg 🤔

Member Author

usamaidrsk Feb 9, 2024

If no arguments, it returns the entire form values. which I used here
Thanks @vasharma05

packages/esm-patient-labs-app/src/lab-orders/add-lab-order/lab-order-form.component.tsx Outdated Show resolved Hide resolved

usamaidrsk requested review from vasharma05 and denniskigen

February 9, 2024 09:23

denniskigen reviewed

View reviewed changes

packages/esm-patient-labs-app/src/lab-orders/add-lab-order/lab-order-form.component.tsx Outdated Show resolved Hide resolved

packages/esm-patient-labs-app/src/lab-orders/add-lab-order/lab-order-form.component.tsx Show resolved Hide resolved

packages/esm-patient-labs-app/src/lab-orders/add-lab-order/lab-order-form.component.tsx Outdated Show resolved Hide resolved

packages/esm-patient-labs-app/src/lab-orders/add-lab-order/add-lab-order.test.tsx Show resolved Hide resolved

usamaidrsk requested a review from denniskigen

February 9, 2024 09:49

Member

denniskigen commented Feb 9, 2024 •

edited

Loading

Just me, or does it appear as though the Priority ComboBox component have a double red outline when validation fails?

Removing this override should fix it, I think:

  :global(.cds--list-box) {
    border: none;
  }

Thanks for being patient, it's very close!

denniskigen changed the title ~~(feat) O3-2814 Add form validation to lab order form~~ (feat) O3-2814: Add field validation to the lab order form

Member Author

usamaidrsk commented Feb 9, 2024 •

edited

Loading

 :global(.cds--list-box) {
    border: none;
  }

removing this in thelab-order-form.scss resolved the issue.

denniskigen approved these changes

View reviewed changes

Member

denniskigen left a comment

Thanks, @usamaidrsk!

usamaidrsk added 10 commits

February 9, 2024 23:49


          refactor: add validations on laborderform

0f15cd5


          add controlled fields

3e20ab0


          refactor: and error messages

8ac2e63


          fix: fix tests

aaf0065


          fix: add lab refrence to be required

72baa9d


          add field validations

8a66048


          use enableCounter on additional instructions text area

71b8e63


          remove invalid_type_error on urgency field schema

0f5678c


          remove counterMode on instructions textarea

2b306e0


          fix double border on combo box when field is invalid

938c84e

denniskigen force-pushed the ft-add-validations-to-lab-order-form branch from 6861ede to 860a17d Compare

February 9, 2024 20:50


          Tweak form CSS and error notification placement

4592b26

denniskigen force-pushed the ft-add-validations-to-lab-order-form branch from 860a17d to 4592b26 Compare

February 9, 2024 20:53

Member

denniskigen commented Feb 9, 2024

Following my fixup commit, the error message container is now pinned to the bottom of the page:

Desktop

Tablet

denniskigen merged commit b9046f1 into openmrs:main

6 checks passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet