-
Notifications
You must be signed in to change notification settings - Fork 24
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
feat: [DHIS2-16123] Add inheritable TEAs to Relationships #3464
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.
Review service time! 😄
The big thing I noticed is that there is still missing inheritance from non-TET attributes.
In addition you'll find that I've added the usual assorted code comments below.
...ure-core/components/Pages/common/TEIRelationshipsWidget/RegisterTei/RegisterTei.container.js
Outdated
Show resolved
Hide resolved
...s/capture-core/components/Pages/common/TEIRelationshipsWidget/useInheritedAttributeValues.js
Outdated
Show resolved
Hide resolved
...ure-core/components/FormFields/New/Fields/OrgUnitField/SingleOrgUnitSelectField.component.js
Show resolved
Hide resolved
@@ -152,7 +150,7 @@ export const useFormValues = ({ program, trackedEntityInstanceAttributes, orgUni | |||
formFoundation && | |||
Object.entries(formFoundation).length > 0 && | |||
formValuesReadyRef.current === false && | |||
areAttributesWithSubvaluesReady | |||
!!clientAttributesWithSubvalues |
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.
Nice! NIT: Can omit !!
...s/capture-core/components/Pages/common/TEIRelationshipsWidget/useInheritedAttributeValues.js
Outdated
Show resolved
Hide resolved
...pture-core/components/DataEntries/TeiRegistrationEntry/hooks/useFormValuesFromSearchTerms.js
Outdated
Show resolved
Hide resolved
...pture-core/components/DataEntries/TeiRegistrationEntry/hooks/useFormValuesFromSearchTerms.js
Show resolved
Hide resolved
Thanks for the review @superskip - fixed all of the things mentioned above! |
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.
@eirikhaugstulen Good job getting through the comments so quickly 🙂
Clarification: TEAs not associated with the TET (for example phone number in TB program).
...s/capture-core/components/Pages/common/TEIRelationshipsWidget/useInheritedAttributeValues.js
Outdated
Show resolved
Hide resolved
.../common/TEIRelationshipsWidget/RegisterTei/DataEntry/Enrollment/dataEntryEnrollment.types.js
Outdated
Show resolved
Hide resolved
All right, @superskip - now I get what you mean. Should be better now! |
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.
LGTM 🎉
🚀 Deployed on https://deploy-preview-3464--dhis2-capture.netlify.app |
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.
Tested successfully on 2.41,2.40.3,2.39.4,2.38.6 versions
🎉 This PR is included in version 100.47.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Tech summary: