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

(refactor) Add workflows to pmtct summary in patient chart #1868

Merged
merged 1 commit into from
Jun 20, 2024

Conversation

CynthiaKamau
Copy link
Contributor

@CynthiaKamau CynthiaKamau commented Jun 13, 2024

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

-This pr refactors the code in the pmtct patient summary to use the new workflows approach

Screenshots

Screenshot 2024-06-13 at 15 31 38

Related Issue

Other

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.

@CynthiaKamau we should also convert the Summary cards, @lucyjemutai already made a function that you can reuse for summary cards, that way, we shall get rid of all that columns code and move it to schema configs.

@hadijahkyampeire
Copy link
Contributor

Also I don't know if it is just on on my end but I see double navigations for pmtct and I can't open the summary
Screenshot 2024-06-13 at 22 40 53

return encounter.patient.age;
}

if (actionOptions?.formName === 'selectMchForm') {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be put in the builder file where we are constructing form actions because in this getObsFromEncounter function, only other columns other than actions use that function, so actions can be handled in their own function. Check something I did about conditional actions in the covid lab results table, you might need something like that.

Copy link
Contributor

@lucyjemutai lucyjemutai Jun 14, 2024

Choose a reason for hiding this comment

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

@CynthiaKamau @hadijahkyampeire I realised the error is there in in cacx and Tb modues as well on dev..

@CynthiaKamau
Copy link
Contributor Author

Also I don't know if it is just on on my end but I see double navigations for pmtct and I can't open the summary Screenshot 2024-06-13 at 22 40 53

Which hackend are you using?

@hadijahkyampeire
Copy link
Contributor

Also I don't know if it is just on on my end but I see double navigations for pmtct and I can't open the summary Screenshot 2024-06-13 at 22 40 53

Which hackend are you using?

I just started the app so I think it was using the namibia backend.

@CynthiaKamau CynthiaKamau force-pushed the tb-summary-conversion branch from e16bd75 to 2fc6d94 Compare June 14, 2024 15:18
@CynthiaKamau
Copy link
Contributor Author

Also I don't know if it is just on on my end but I see double navigations for pmtct and I can't open the summary Screenshot 2024-06-13 at 22 40 53

Which hackend are you using?

I just started the app so I think it was using the namibia backend.

use the openmrs-dev backend, namibia doesnt have the new app name, if you use it you will see both the old and new app

@hadijahkyampeire
Copy link
Contributor

@CynthiaKamau sorry about the conflicts, I think they are from the Eslint rule PR that just got merged.

@CynthiaKamau CynthiaKamau force-pushed the tb-summary-conversion branch from 2fc6d94 to 1502c5b Compare June 18, 2024 06:53
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.

This is getting closer but I just noticed some other visual issues, let's have a call about them.

@@ -109,6 +109,20 @@ export const getTabColumns = (columnsDefinition: Array<ColumnDefinition>) => {
}
});
}
if (column?.conditionalActionOptions?.length) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This condition is similar to the one up, why can't you move that logic up or if anything is different you might wanna add it to the condition.

@@ -85,6 +85,7 @@ export function getObsFromEncounter(
type?: string,
fallbackConcepts?: Array<string>,
secondaryConcept?: string,
actionOptions?: any,
Copy link
Contributor

Choose a reason for hiding this comment

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

you forgot to remove this.

if (column.isConditionalConcept) {
return getConditionalConceptValue(encounter, column.conditionalConceptMappings, column.isDate);
return getConditionalConceptValue(encounter, column.conditionalconditionalConceptMappings, column.isDate);
Copy link
Contributor

Choose a reason for hiding this comment

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

This naming seems wrong, why change it to this?

}

if (column.id == 'artInitiation' && column.encounterTypes?.length > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We agreed not to use name specific conditions like art, because we want some of these functions to be reused, you might wanna do something like what I did for ART date and Reason.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also instead of using column.id, we can use the type field because ids might be different but types can be similar.

@@ -24,15 +53,45 @@ export const getSummaryCardProps = (schemaConfig, config = null) => {
},
getObsSummary: async (encounters) => {
const summaryValues = encounters.map((encounter) => {
if (encounter && encounter.observation && encounter.observation.value) {
if (column.encounterType === 'antenatal' && column.id === 'nextAppointmentDate') {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, prefer using type to column id.

"isDate": true,
"title": "Enrollment Date",
"encounterTypes": ["motherPostnatal", "labourAndDelivery", "antenatal"],
"concept": "161552AAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

"encounterTypes": [
"af1f1b24-d2e8-4282-b308-0bf79b365584"
],
"concept": "1148AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, let's not hardcode the concept uuids

"encounterTypes": [
"af1f1b24-d2e8-4282-b308-0bf79b365584"
],
"concept": "1151AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

"encounterTypes": [
"af1f1b24-d2e8-4282-b308-0bf79b365584"
],
"concept": "164460AAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same

"encounterTypes": [
"af1f1b24-d2e8-4282-b308-0bf79b365584"
],
"concept": "160433AAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same

@CynthiaKamau CynthiaKamau force-pushed the tb-summary-conversion branch 2 times, most recently from d784f0a to acdb03b Compare June 19, 2024 13:12
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.

This looks good thanks @CynthiaKamau

@hadijahkyampeire hadijahkyampeire merged commit 42759cd into dev Jun 20, 2024
5 checks passed
@hadijahkyampeire hadijahkyampeire deleted the tb-summary-conversion branch June 20, 2024 14:55
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