-
Notifications
You must be signed in to change notification settings - Fork 258
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
Activity flow api changes. #2476
base: master
Are you sure you want to change the base?
Conversation
… perform is called
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 looks great!
Requesting few small changes .
workflow/src/main/java/com/google/android/fhir/workflow/activity/ActivityFlow.kt
Outdated
Show resolved
Hide resolved
workflow/src/main/java/com/google/android/fhir/workflow/activity/request/CPGRequestResource.kt
Outdated
Show resolved
Hide resolved
workflow/src/main/java/com/google/android/fhir/workflow/activity/event/CPGEventResource.kt
Outdated
Show resolved
Hide resolved
...n/java/com/google/android/fhir/workflow/activity/event/CPGEventResourceForOrderMedication.kt
Outdated
Show resolved
Hide resolved
workflow/src/main/java/com/google/android/fhir/workflow/activity/event/CPGCaseEvent.kt
Outdated
Show resolved
Hide resolved
|
||
import org.hl7.fhir.r4.model.Task | ||
|
||
class CPGProposeDiagnosisTask(override val resource: Task) : CPGTaskRequest(resource) { |
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.
Similarly for other TaskRequest resources
class CPGProposeDiagnosisTask(override val resource: Task) : CPGTaskRequest(resource) { | |
class CPGProposeDiagnosisTaskRequest(override val resource: Task) : CPGTaskRequest(resource) { |
workflow/src/main/java/com/google/android/fhir/workflow/activity/ActivityFlow.kt
Outdated
Show resolved
Hide resolved
workflow/src/main/java/com/google/android/fhir/workflow/activity/ActivityFlowPhases.kt
Outdated
Show resolved
Hide resolved
workflow/src/main/java/com/google/android/fhir/workflow/activity/ActivityFlowPhases.kt
Outdated
Show resolved
Hide resolved
workflow/src/main/java/com/google/android/fhir/workflow/activity/ActivityFlowPhases.kt
Outdated
Show resolved
Hide resolved
enum class EventStatus(val code: String) { | ||
PREPARATION("preparation"), | ||
INPROGRESS("in-progress"), | ||
CANCELLED("not-done"), |
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.
CANCELLED("not-done"), | |
CANCELLED("cancelled"), |
COMPLETED("completed"), | ||
ENTEREDINERROR("entered-in-error"), | ||
STOPPED("stopped"), | ||
DECLINED("decline"), |
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.
DECLINED("decline"), | |
DECLINED("declined"), |
when (code) { | ||
"preparation" -> PREPARATION | ||
"in-progress" -> INPROGRESS | ||
"not-done" -> CANCELLED |
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-done" -> CANCELLED | |
"cancelled" -> CANCELLED |
"completed" -> COMPLETED | ||
"entered-in-error" -> ENTEREDINERROR | ||
"stopped" -> STOPPED | ||
"decline" -> DECLINED |
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.
"decline" -> DECLINED | |
"declined" -> DECLINED |
|
||
companion object { | ||
|
||
fun of(code: String) = |
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 case is missing
fun of(code: String) = | |
"not-done" -> NOTDONE |
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.
can you add a page in the doc folder?
workflow/src/test/java/com/google/android/fhir/workflow/activity/ActivityFlowTest.kt
Outdated
Show resolved
Hide resolved
...c/main/java/com/google/android/fhir/workflow/activity/resource/request/CPGRequestResource.kt
Outdated
Show resolved
Hide resolved
workflow/src/main/java/com/google/android/fhir/workflow/activity/ActivityFlow.kt
Outdated
Show resolved
Hide resolved
workflow/src/main/java/com/google/android/fhir/workflow/activity/ActivityFlow.kt
Outdated
Show resolved
Hide resolved
workflow/src/main/java/com/google/android/fhir/workflow/activity/ActivityFlow.kt
Outdated
Show resolved
Hide resolved
workflow/src/main/java/com/google/android/fhir/workflow/activity/ActivityFlow.kt
Show resolved
Hide resolved
workflow/src/main/java/com/google/android/fhir/workflow/activity/phase/request/PlanPhase.kt
Outdated
Show resolved
Hide resolved
workflow/src/main/java/com/google/android/fhir/workflow/activity/phase/request/PlanPhase.kt
Outdated
Show resolved
Hide resolved
workflow/src/main/java/com/google/android/fhir/workflow/activity/resource/event/CPGCaseEvent.kt
Outdated
Show resolved
Hide resolved
workflow/src/main/java/com/google/android/fhir/workflow/activity/ActivityFlow.kt
Outdated
Show resolved
Hide resolved
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.
just noting comments from bryn:
- Can we change "draft" to "prepare"
- Can we change "start" to "submit"
- Can we change "resume" in the test cases to "continue"?
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.
Thanks Aditya for the changes.
2 main concerns:-
- Should we consider making the operations asynchronous ? Since we are dealing with Repository it only makes sense to do the operations in a IO coroutine. Suggesting to make the initiate and update methods suspend functions.
- Do you think we should add a rollback api ? If there are errors in later phases one might want to rollback. To do this we could maintain a stack pf phases instead of only the currentPhase.
/** | ||
* Creates a draft plan resource based on the state of the [currentPhase]. | ||
* | ||
* @return [R] if the action is successful, error otherwise. |
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.
Correct the return kdoc.
* @return [R] if the action is successful, error otherwise. | |
* @return [Result]<R> containing the draft plan resource if the action is successful, [Result.failure] otherwise. |
/** | ||
* Creates a draft order resource based on the state of the [currentPhase]. | ||
* | ||
* @return [R] if the action is successful, error otherwise. |
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.
Correct the return kdoc like above and other places.
@Suppress( | ||
"UnstableApiUsage", /* Repository is marked @Beta */ | ||
) | ||
class ActivityFlow<R : CPGRequestResource<*>, E : CPGEventResource<*>> |
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.
Can you comment on thread-safety of this class ?
} | ||
|
||
@Test | ||
fun `order medication flow for medication dispense no test `(): Unit = runBlockingOnWorkerThread { |
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 this the same as order medication flow for medication dispense
?
I see a lot of duplicate codes. Can we reuse them ?
} | ||
|
||
@Test | ||
fun `draftPlan is success when flow is in proposal phase`(): Unit = runBlockingOnWorkerThread { |
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.
Maybe this is better ?
fun `draftPlan is success when flow is in proposal phase`(): Unit = runBlockingOnWorkerThread { | |
fun `preparePlan should succeed when in proposal phase`(): Unit = runBlockingOnWorkerThread { |
} | ||
|
||
@Test | ||
fun `draftPlan is failure when flow is in plan phase`(): Unit = runBlockingOnWorkerThread { |
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.
Similarly others ?
fun `draftPlan is failure when flow is in plan phase`(): Unit = runBlockingOnWorkerThread { | |
fun `preparePlan should fail when in plan phase`(): Unit = runBlockingOnWorkerThread { |
64d62ed
to
ec9fbcd
Compare
…supported activities
ec9fbcd
to
0e8bcc7
Compare
IMPORTANT: All PRs must be linked to an issue (except for extremely trivial and straightforward changes).
Fixes #2469
Description
Api for creating a flow to move a request through various phases of an ActivityFlow.
The class
ActivityFlow
has two main functionalities:e.g.
ActivityFlow.draftPlan
corresponds to beginPlan andActivityFlow.startPlan
corresponds to endPlan .The
Phase
class has two distinct hierarchiesRequestPhase
andEventPhase
each with unique apis to allow valid state transitions for Request based phases (Proposal, Plan and Order) and Event based phases (Perform) respectively.ActivityFlow
provides variousActivityFlow.of
static api to help user createActivityFlow
for particular activities with particular request or event types.Alternative(s) considered
Have you considered any alternatives? And if so, why have you chosen the approach in this PR?
Type
Choose one: (Bug fix | Feature | Documentation | Testing | Code health | Builds | Releases | Other)
Screenshots (if applicable)
Checklist
./gradlew spotlessApply
and./gradlew spotlessCheck
to check my code follows the style guide of this project../gradlew check
and./gradlew connectedCheck
to test my changes locally.