-
Notifications
You must be signed in to change notification settings - Fork 59
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) Introduce generic program enrollment #153
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.
@kajambiya thank you for this but please have a look at my comments. Happy to discuss them
@@ -0,0 +1,71 @@ | |||
export function isPostSubmissionEnabled(expression: string, encounters: any[]): boolean { |
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.
this function is doing more than checking if a postSubmissionAction
is enabled... please rename this and also check if there is no function in the form engine that already does this... how hideWhenExpression
is processed ?
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.
There's an existing expression-runner
but after studying it, I realised it couldn't be reused for this scenario:
The expression runner has access to all fields so expressions are evaluated in relation to all dependent fields regardless of whether they have data or not. The post submission action
on the other hand has no access to all fields but has access to the submitted encounter and the submitted encounter eliminates fields/obs that don't have values, so it's just a few fields that get submitted.
The expression-runner also uses the encounter context
which is not present at post-submission.
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.
@kajambiya but the name of this function is not inline with what it does.
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.
Great job 🚀
patient: patient.id, | ||
program: programUuid, | ||
dateEnrolled: enrollmentDate ? dayjs(enrollmentDate).format() : null, | ||
dateCompleted: completionDate ? dayjs(enrollmentDate).format() : null, |
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.
is dateCompleted required for enrollments?
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.
So the api works in such a way that when the completion date is supplied, the program status changes fromActive
to Completed
. So to answer your question, a user is disenrolled from a program the moment a completion date is supplied.
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.
I think I get what you meant. I was passing enrollmentDate
as the value of dateCompleted
. This is now fixed.
}, | ||
); | ||
} else { | ||
const patientEnrolledPrograms = await getPatientEnrolledPrograms(patient.id); |
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.
if (hasActiveEnrollment) { throw new Error('Cannot enroll patient to program. Patient already has an active enrollment'); }
Will this section get executed because you are checking if someone is already enrolled ?
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.
Not sure I fully get the question but let me briefly explain what is happening here;
So there's an existing module through which patients are being enrolled into programs. However we are also adding provision for enrolling patients into programs after successful submission of some encounters using post submission actions. Now when the post submission action is in enter mode
, the assumption is that someone is being enrolled into a program for the 1st time, however if they already has an active program of the same type, they can't be enrolled again. This is being handled by the backend but saw it wise to handle it on the frontend as well. And yah, the section will get executed.
src/api/api.ts
Outdated
}); | ||
} | ||
|
||
export function updateProgramEnrollment(programEnrollmentUuid: string, payload, abortController) { |
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.
The typings have to be added here as well
src/ohri-form.component.tsx
Outdated
}, | ||
config, | ||
); | ||
const isActionEnabled = enabled ? isPostSubmissionEnabled(enabled, encounterData) : true; |
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.
Was logic meant to take care of backwards compatibility? if am reading this correctly if translates to if enabled (true)
then do an action otherwise return true
. So in both cases it resolves to true
. If the condition enabled
only applies to programs, shouldn't we consider adding it within the that handler to skip this step?
The main approach here assuming it's to either enroll or dis-enroll
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.
Hi @pirupius, if you remember that call we had with @ebambo, the post submission action
is by default enabled, however it may be disabled given some conditions. See example below:
"postSubmissionActions": [
{
"actionId": "ProgramEnrollmentSubmissionAction",
"enabled":"tbProgramType === '160541AAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'",
"config": {
"enrollmentDate": "tbRegDate",
"programUuid": "58005eb2-4560-4ada-b7bb-67a5cffa0a27",
"completionDate": "outcomeTBRx"
}
},
{
"actionId": "ProgramEnrollmentSubmissionAction",
"enabled":"tbProgramType === '160052AAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'",
"config": {
"enrollmentDate": "tbRegDate",
"programUuid": "00f37871-0578-4ebc-af1d-e4b3ce75310d",
"completionDate": "outcomeTBRx"
}
}
],
We have 2 post submission actions
but only one can run depending on the program type selected. The other will return false
for the enabled flag
src/ohri-form.component.test.tsx
Outdated
|
||
// Render the form | ||
await act(async () => renderForm(null, postSubmission_test_form)); | ||
//const getRegisteredPostSubmissionActionSpy = jest.spyOn(registry, 'getRegisteredPostSubmissionAction'); |
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.
Do we need this? If not then let's clean this up
src/ohri-form.component.test.tsx
Outdated
//expect(postSubmissionSpy).toHaveBeenCalled(); | ||
|
||
//expect(api.createProgramEnrollment).toHaveBeenCalled(); |
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.
same applies to this
Requirements
Summary
This PR introduces generic program enrolment to the form engine. If an encounter is supposed to trigger a program enrolment, the user shall specify the same in the schema using
ProgramEnrollmentSubmissionAction
and specifying the program details through the config parameter. The user shall also use the enabled flag to specify whether the post submission is enabled or not. See sample below:Screenshots
Screen.Recording.2023-12-12.at.22.17.56.mov
Related Issue
Other