-
Notifications
You must be signed in to change notification settings - Fork 16
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.
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
6204 name insurance join #6395
base: develop
Are you sure you want to change the base?
6204 name insurance join #6395
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good as far as I can tell. Just that failing sync due the PG Enums to sort out, plus those suggestions re the OMS schema fields.
server/repository/src/migrations/v2_06_00/add_name_insurance_join.rs
Outdated
Show resolved
Hide resolved
policy_number_person TEXT, | ||
policy_number_family TEXT, | ||
policy_number TEXT NOT NULL, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed, probably want to make "person" and "family" just boolean flags in OMS, and just translate them to OG?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've checked in mSupply, it looks like if both fields are filled in, they're combined in to the full
field.
E.g. 123-321
So I think we probably want to keep it how it is...
policy_number_family TEXT, | ||
policy_number TEXT NOT NULL, | ||
policy_type {policy_type} NOT NULL, | ||
discount_percentage INTEGER NOT NULL, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can policy_type be null
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't look like it in OG, so I think it's ok to keep it as NOT NULL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of quick things before approve:
- The "discount percentage" in mSupply accepts non-integer values, so we should probably make our field float rather than integer?
- The syncing only seems to be going one way (from OMS-> to OG) -- if I add a policy to a user in mSupply it doesn't sync back. Is this what we're expecting at the moment?
Fixes #6204
π©π»βπ» What does this PR do?
Adds sync for
nameInsuranceJoin
to open-mSupplyπ Any notes for the reviewer?
π§ͺ Testing
For dev testing, we can do something like this...
Run an insert record something like this in the database
INSERT INTO "main"."name_insurance_join" ("id", "name_link_id", "insurance_provider_id", "policy_number", "policy_type", "discount_percentage", "expiry_date", "is_active") VALUES ('TEST_NAME_INSURANCE_JOIN', 'PATIENT_NAME_ID', 'INSURANCE_PROVIDER_ID', '2312', 'business', 10, '2027-01-01', 'true');
Then insert the changelog
INSERT INTO changelog (cursor, table_name, record_id, row_action) VALUES ((SELECT max(cursor)+1 FROM changelog), 'name_insurance_join', 'TEST_NAME_INSURANCE_JOIN' ,'UPSERT')
π Documentation
1.
2.