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.

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

OHRI-2295 Update OHRI endpoints to use SWR #1902

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

OHRI-2295 Update OHRI endpoints to use SWR #1902

wants to merge 1 commit into from

Conversation

CynthiaKamau
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

Update OHRI endpoints to use SWR

Screenshots

Related Issue

Other

@CynthiaKamau CynthiaKamau force-pushed the swr-migration branch 2 times, most recently from 73e4092 to 673fcc2 Compare July 12, 2024 12:15
hadijahkyampeire
hadijahkyampeire reviewed Jul 15, 2024
View reviewed changes
Copy link
Contributor

@hadijahkyampeire hadijahkyampeire left a comment

Choose a reason for hiding this comment

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

Thanks @CynthiaKamau just a few comments

packages/esm-commons-lib/src/api.resource.ts Outdated
@@ -145,7 +137,7 @@ export async function getPatientListsForPatient(patientUuid: string) {

export function fetchPatientsFinalHIVStatus(patientUUID: string) {
return openmrsFetch(
`/ws/fhir2/R4/Observation?code=${finalHIVCodeConcept}&value-concept=${finalPositiveHIVValueConcept}&patient=${patientUUID}&_sort=-date&_count=1`,
`/ws/fhir2/R4/Observation?code=${configSchema.obsConcepts._default.finalHIVCodeConcept}&value-concept=${configSchema.obsConcepts._default.finalPositiveHIVValueConcept}&patient=${patientUUID}&_sort=-date&_count=1`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Getting configs like this seems stressful, since we are using SWR, you can make this a hook and then you will be able to use the useConfig and that way it will be easy getting the configs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For some reason, if i useConfig none of the endpoints load, i guess its because im not using a hook as the endpoint is called in a Promise.all

packages/esm-commons-lib/src/constants.ts Outdated
export const FirstName_UUID = '166102AAAAAAAAAAAAAAAAAAAAAAAAAAAAAA';
export const Relationship_UUID = '85d3b4fe-c1a9-4e27-a86b-dcc1e30c8a93';
export const BASE_WS_API_URL = '/ws/rest/v1/';
export const BASE_FHIR_API_URL = '/ws/fhir2/R4/';
Copy link
Contributor

Choose a reason for hiding this comment

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

new line please, also these baseUrls might be available in the framework we don't need to redefine them

Copy link
Contributor Author

@CynthiaKamau CynthiaKamau Jul 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Let me switch to using the openmrs ones

packages/esm-commons-lib/src/api.resource.ts Outdated
@@ -191,7 +183,9 @@ export function fetchPatientLastEncounter(patientUuid: string, encounterType) {
}

export function fetchPatientCovidOutcome() {
return openmrsFetch(`/ws/rest/v1/reportingrest/cohort/${covidOutcomesCohortUUID}`).then(({ data }) => {
return openmrsFetch(
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to also turn this into a hook

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, this endpoint is not in use anymore

@CynthiaKamau CynthiaKamau force-pushed the swr-migration branch 2 times, most recently from 6249d39 to 8f37be0 Compare July 15, 2024 15:57
hadijahkyampeire
hadijahkyampeire reviewed Jul 17, 2024
View reviewed changes
packages/esm-commons-lib/src/api.resource.ts
@@ -177,7 +167,7 @@ export function fetchPatientComputedConcept_HIV_Status(patientUUID: string) {

export function fetchPatientLastEncounter(patientUuid: string, encounterType) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@CynthiaKamau I still see fetch functions, I think most of them can be transformed to use SWR and make them hooks. The ones I think my be hard might be the ones that are using promise.all but we can get a work around on them.

hadijahkyampeire
hadijahkyampeire reviewed Jul 17, 2024
View reviewed changes
packages/esm-commons-lib/src/api.resource.ts
export function fetchConceptNameByUuid(conceptUuid: string) {
return openmrsFetch(`/ws/rest/v1/concept/${conceptUuid}/name?limit=1`).then(({ data }) => {
return openmrsFetch(`${restBaseUrl}/concept/${conceptUuid}/name?limit=1`).then(({ data }) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

All these should use SWR and can be hooks since they are GET requests

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

@hadijahkyampeire hadijahkyampeire hadijahkyampeire left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@CynthiaKamau @hadijahkyampeire