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

(feat): include openmrs id on payment history #486

Merged

Conversation

amosmachora
Copy link
Collaborator

Requirements

  • This PR has a title that briefly describes the work done, including the ticket number if there is a ticket.
  • My work conforms to the OpenMRS 3.0 Styleguide.
  • I checked for feature overlap with existing widgets.

Summary

This PR adds the patient's openmrs id on the payment history page. The current implementation works but the slight downside is for every patient we have to fetch their openMRS ID from the backend therefore there is a loading state on the patient's name which is not the best user experience, if the bill could come prepopulated with the openMRS ID that would be ideal.

Screenshots

image

Related Issue

None.

Other

None.

@ojwanganto
Copy link
Contributor

Could we adopt the standard approach of having the identifier in its own column?

@amosmachora
Copy link
Collaborator Author

Could we adopt the standard approach of having the identifier in its own column?

technically harder to implement but I will try see if its possible

@ojwanganto
Copy link
Contributor

ojwanganto commented Nov 26, 2024

This is what I mean:
image

Comment on lines 117 to 123
const { openMRSIDUUID } = useConfig<BillingConfig>();
const { patient, isLoading } = usePatient(patientUUID);

const openMRSId = patient?.identifier
?.filter((identifier) => identifier)
.filter((identifier) => identifier.type.coding.some((coding) => coding.code === openMRSIDUUID))
.at(0)?.value;
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @amosmachora, we already provide the OpenMRSID by default using the bill endpoint. IMO the current implementation adds a call to fetch patient for every row which is additional load on the server and might cause degraded performance.

See section of /cashier/bill endpoint
Screenshot 2024-11-26 at 11 30 32

From the above we could add function

const extractPatientIdentifier = patient => patient?.display?.split('-')[0]?.trim() || '';

that extracts the identifier. In the event more identifier are required we could ask support from backend to include full patient object

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wish I had thought of that. I agree the current approach does make a request for every row. Will update ASAP.

@amosmachora
Copy link
Collaborator Author

@donaldkibet turns out there is a property for that I just did not look.
image

donaldkibet
donaldkibet previously approved these changes Nov 28, 2024
Copy link
Contributor

@donaldkibet donaldkibet left a comment

Choose a reason for hiding this comment

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

Thanks @amosmachora

…/payment-history-table.component.tsx

Co-authored-by: Donald Kibet <[email protected]>
@donaldkibet donaldkibet merged commit 4ea2ffa into palladiumkenya:main Nov 28, 2024
Ogollah pushed a commit that referenced this pull request Dec 1, 2024
* feat: include openmrs id on bill history

* refactor: use saved id

* Update packages/esm-billing-app/src/billable-services/payment-history/payment-history-table.component.tsx

Co-authored-by: Donald Kibet <[email protected]>

---------

Co-authored-by: Donald Kibet <[email protected]>
@amosmachora amosmachora deleted the feat/openmrs-id-on-bill-history branch December 2, 2024 15:02
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