Skip to content
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

OHRI-1963 Enable editing of L&D form infant details in OHRI #1689

Merged
merged 4 commits into from
Nov 28, 2023

Conversation

kajambiya
Copy link
Contributor

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 the ticket number in the format OHRI-123 My PR title.
  • My work includes tests or is validated by existing tests.

Summary

Screenshots

Related Issue

Other

@kajambiya kajambiya marked this pull request as ready for review November 24, 2023 15:53
@@ -82,6 +82,17 @@ export function getEstimatedDeliveryDate(patientUuid: string, pTrackerId: string
});
}

export function getIdentifierInfo(identifier: string){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kajambiya this function looks solid to me but the only thing missing here is HTTP request & response error handling, would be good if you can add to this function and we show this to the other devs otherwise we will keep having those errors that emerge out of the blue and the user doesn't understand. Here in case an HTTP request or response error happens I would expect to see a message like Failed to retrieve identifier information. Also shouldn't this function be marked as async ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ebambo, All the error handling regarding api calls including http error handling is done in the openmrsFetch function
We don't need to mark the function async because we do not care when it finishes processing. We would have to add async if there was need to await for the operation to first finish.

export const getIdentifierAssignee = async (identifier: string, identifierType: string) => {
return getIdentifierInfo(identifier).then((data) => {
if (data) {
for (const result of data.results) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if data has no results attribute?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment also applies to the lines below, you better explicitly check or add the question marks

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, by the time we get here, all the other possible errors were checked and the call is fine. If there's no data, it only means that there's no patient with that identifier meaning it's ok to go ahead and assign it to the current patient.

Comment on lines 24 to 25
// only do this the first time the form is entered
if (sessionMode !== 'enter') {
if (sessionMode === 'view') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update the comment or delete it to indicate the hook runs on both enter and edit

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment on lines 66 to 67
if(existingpTrackerAssignee){
if(sessionMode === 'enter'){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this include edit mode?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adjusted accordingly

} else {
//In edit mode, only throw error if the patient with the existing PTracker is not linked with the current mother
const parentRelationships = await fetchPatientRelationships(parent.id);
const isAlreadyLinked = parentRelationships.some(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙌🏿

@pirupius pirupius requested a review from ebambo November 27, 2023 14:14
@ebambo ebambo merged commit e0e3e09 into UCSF-IGHS:dev Nov 28, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants