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

fix: Added Validation for Insurance input fields in patient registration issue (#8672) #8681

Closed

syedfardeenjeelani wants to merge 31 commits into ohcnetwork:develop from syedfardeenjeelani:issues/8672/fix-issue

Contributor

syedfardeenjeelani commented Oct 1, 2024 •

edited

Loading

Proposed Changes

Fixes Add Validation for Insurance input fields in patient registration form #8672
Added frontend validation for insurance input fields (Member ID, Policy ID, and Insurer Name) in the patient registration form.

All.fields.are.mandatory.1.mp4

@ohcnetwork/care-fe-code-reviewers

Merge Checklist

Prep screenshot or demo video for changelog entry, and attach it to the issue.


          fix: Add Validation for Insurance input fields in patient registratio…

d1c1cd5

…n form (#8672)

syedfardeenjeelani requested a review from a team as a code owner

October 1, 2024 05:53

netlify bot commented Oct 1, 2024 •

edited

Loading

✅ Deploy Preview for care-ohc ready!

Name	Link
🔨 Latest commit	`d903c88`
🔍 Latest deploy log	https://app.netlify.com/sites/care-ohc/deploys/671530c3cae7a4000807439d
😎 Deploy Preview	https://deploy-preview-8681--care-ohc.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Member

rithviknishad commented Oct 1, 2024

@syedfardeenjeelani is this ready for review?

Contributor Author

syedfardeenjeelani commented Oct 1, 2024

@syedfardeenjeelani is this ready for review?

we are getting the error from the validator.ts which is something like this
if (
!value.policy_id.trim() ||
!value.subscriber_id.trim() ||
(enable_hcx && (!value.insurer_id?.trim() || !value.insurer_name?.trim()))
)
return "All fields are mandatory";

so should i use field_required from the i18n locale here instead of "All fields are mandatory"

Member

rithviknishad commented Oct 1, 2024

yes

Contributor Author

syedfardeenjeelani commented Oct 1, 2024

yes

Thanks I'll do the changes and push

Contributor Author

syedfardeenjeelani commented Oct 1, 2024

@rithviknishad if we do it like this
if (
!value.policy_id.trim() ||
!value.subscriber_id.trim() ||
(enable_hcx && (!value.insurer_id?.trim() || !value.insurer_name?.trim()))
)
return t("field_required");
};

then it will also show it right here

so instead should a make a key which says "all_fields_mandatory": "All fields are mandatory"

or should we handle both cases seprately

Member

rithviknishad commented Oct 1, 2024 •

edited

Loading

Let's keep the error texts same as before itself instead of showing the error on ALL fields, just that ensure ALL fields required validation happens as mentioned in the issue.


          fix: Add Validation for Insurance input fields in patient registratio…

96b810f

…n form (#8672)

Contributor Author

syedfardeenjeelani commented Oct 1, 2024

@rithviknishad I've made some changes based on your feedback, but I'm not sure if I got it right. Could you take a look at my PR when you get a chance and let me know if i am headed in the right direction or not ?

rithviknishad requested changes

View reviewed changes

Member

rithviknishad left a comment •

edited

Loading

Only validation logic in this file needs to be modified: https://github.com/ohcnetwork/care_fe/blob/develop/src/Components/HCX/validators.ts

src/Components/HCX/InsuranceDetailsBuilder.tsx Outdated

		label={t("policy__insurer_name")}
		error={error}

Member

rithviknishad Oct 3, 2024

Suggested change

error={error}

src/Components/HCX/InsuranceDetailsBuilder.tsx Outdated

@@ @@ -133,6 +138,7 @@ const InsuranceDetailEditCard = ({ @@
                       <TextFormField
                         required
                         name="policy_id"
+                        error={error}

Member

rithviknishad Oct 3, 2024

Suggested change

error={error}

src/Components/HCX/InsuranceDetailsBuilder.tsx Outdated

@@ @@ -125,6 +129,7 @@ const InsuranceDetailEditCard = ({ @@
                       <TextFormField
                         required
                         name="subscriber_id"
+                        error={error}

Member

rithviknishad Oct 3, 2024

Suggested change

error={error}

src/Components/HCX/InsuranceDetailsBuilder.tsx Outdated

               }: {
                 policy: HCXPolicyModel;
                 handleUpdate: (event: FieldChangeEvent<unknown>) => void;
                 handleUpdates: (diffs: object) => void;
                 handleRemove: () => void;
                 gridView?: boolean;
+                error: FieldError;

Member

rithviknishad Oct 3, 2024

Suggested change

error: FieldError;

src/Components/HCX/InsuranceDetailsBuilder.tsx Outdated

@@ @@ -91,12 +93,14 @@ const InsuranceDetailEditCard = ({ @@
                 handleUpdates,
                 handleRemove,
                 gridView,
+                error,

Member

rithviknishad Oct 3, 2024

Suggested change

error,

src/Components/HCX/InsuranceDetailsBuilder.tsx Outdated

                         >
                           <InsuranceDetailEditCard
                             policy={policy}
+                            error={props.error}

Member

rithviknishad Oct 3, 2024

Suggested change

error={props.error}

src/Components/HCX/InsuranceDetailsBuilder.tsx Outdated

@@ @@ -15,6 +15,7 @@ import request from "../../Utils/request/request"; @@
               import routes from "../../Redux/api";
               import { useTranslation } from "react-i18next";
               import careConfig from "@careConfig";
+              import { FieldError } from "../Form/FieldValidators";

Member

rithviknishad Oct 3, 2024

Suggested change

import { FieldError } from "../Form/FieldValidators";

src/Redux/fireRequest.tsx Outdated

Comment on lines 145 to 149

+                          console.log(error.response.data.detaillocal_ip_address);
+                          // Notification.BadRequest({
+                          //   errs: error.response.data,
+                          // });
+                          // return error.response;

Member

rithviknishad Oct 3, 2024

Why was this changed?

Contributor Author

syedfardeenjeelani commented Oct 3, 2024

Only validation logic in this file needs to be modified: https://github.com/ohcnetwork/care_fe/blob/develop/src/Components/HCX/validators.ts

hey @rithviknishad should it be something like this
if (!value.policy_id.trim()) {
return "some err message";
} else if (!value.subscriber_id.trim()) {
return "some err message";
}
if (enable_hcx) {
if (!value.insurer_id?.trim()) {
return "some err message";
} else if (!value.insurer_name?.trim()) {
return "some err message";
}
}

also do we only have to change this particular file and nothing else

Member

rithviknishad commented Oct 3, 2024

also do we only have to change this particular file and nothing else

yes

syedfardeenjeelani and others added 4 commits

October 4, 2024 20:33


          fix: fixed the validation logic of the insurance form

90dcac7


          fix: fixed the validation logic of the insurance form

6bc35c4


          fix: fixed the validator.ts

8768f3b


          Update fireRequest.tsx

nihal467 added the needs testing label

Member

nihal467 commented Oct 4, 2024

@rithviknishad can you verify is it ready for testing

nihal467 added needs review and removed needs testing labels

rithviknishad requested changes

View reviewed changes

Member

rithviknishad left a comment

texts to be in i18n locale files

https://github.com/ohcnetwork/care_fe#translations

Contributor Author

syedfardeenjeelani commented Oct 5, 2024

texts to be in i18n locale files

https://github.com/ohcnetwork/care_fe#translations

@rithviknishad hello should i put them in common.json

syedfardeenjeelani added 2 commits

October 6, 2024 13:08


          fix: Move validation messages to i18n locale files and refactor HCXPo…

64963bf

…licyValidator


          Merge branch 'issues/8672/fix-issue' of https://github.com/syedfardee…

0ce16b3

…njeelani/care_fe into issues/8672/fix-issue

github-actions bot added the merge conflict label

github-actions bot commented Oct 7, 2024

👋 Hi, @syedfardeenjeelani,
Conflicts have been detected against the base branch. Please rebase your branch against the base branch.

This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there.

Member

rithviknishad commented Oct 8, 2024

Put in en.json

Contributor Author

syedfardeenjeelani commented Oct 8, 2024

Put in en.json

okay


          Merge branch 'develop' into issues/8672/fix-issue

a33a94a

Member

nihal467 commented Oct 16, 2024 •

edited

Loading

Even though the validation is added, it's not blocking the user from submitting a patient registration form when the insurance fields are kept blank

nihal467 added test failed and removed needs testing Deploy-Failed labels

Contributor

AdityaJ2305 commented Oct 16, 2024

Hey @syedfardeenjeelani , Please check by adding this line errors["insurance_details"] = insuranceDetailsError; in validateForm func in PatientRegister.tsx

github-actions bot added the merge conflict label

github-actions bot commented Oct 16, 2024

👋 Hi, @syedfardeenjeelani,
Conflicts have been detected against the base branch. Please rebase your branch against the base branch.

This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there.

Member

rithviknishad commented Oct 17, 2024

@syedfardeenjeelani could you resolve the merge conflicts?

Member

rithviknishad commented Oct 17, 2024

ensure the en.json file has keys sorted. pre-commit should do it for you if setup properly.


          fixed validation

f5a2651

syedfardeenjeelani force-pushed the issues/8672/fix-issue branch from 968340c to f5a2651 Compare

October 17, 2024 04:48

syedfardeenjeelani and others added 2 commits

October 17, 2024 10:29


          sorted en.json alphabetically

9bdfdc6


          Merge branch 'develop' into issues/8672/fix-issue

2f75935

github-actions bot added Deploy-Failed and removed merge conflict labels

Contributor Author

syedfardeenjeelani commented Oct 19, 2024

@nihal467 please test this

rithviknishad requested changes

View reviewed changes

src/Locale/en.json Outdated

Member

rithviknishad Oct 19, 2024

why is the entire file having a change?

Contributor Author

syedfardeenjeelani Oct 19, 2024

pre-commit was not sorting keys for me so i used https://codeshack.io/json-sorter/

Member

rithviknishad Oct 19, 2024

use npm run sort-locales instead; also why is pre-commit not sorting?

rithviknishad added changes required and removed test failed labels

syedfardeenjeelani and others added 6 commits

October 19, 2024 12:15


          Merge branch 'develop' into issues/8672/fix-issue

d7da9f8

wip

d39b125


          Merge branch 'issues/8672/fix-issue' of https://github.com/syedfardee…

a06d0f7

…njeelani/care_fe into issues/8672/fix-issue

wip

2b5dd16


          Merge branch 'develop' into issues/8672/fix-issue

54c08f1


          fix: sorted the json files and fixed the validation error

d903c88

rithviknishad requested changes

View reviewed changes

src/Locale/en.json

Member

rithviknishad Oct 20, 2024

Why is so many i18n keys deleted?

package-lock.json

Member

rithviknishad Oct 20, 2024

Could you discard changes to the lock file?

syedfardeenjeelani closed this

syedfardeenjeelani deleted the issues/8672/fix-issue branch

October 20, 2024 16:52

Contributor Author

syedfardeenjeelani commented Oct 20, 2024

@rithviknishad i will setup the repo again and raise a new pr

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

Labels

changes required Deploy-Failed