-
Notifications
You must be signed in to change notification settings - Fork 295
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
Add API to support saving local only resources #2178
base: master
Are you sure you want to change the base?
Conversation
- With unmerged PR #1 - With unmerged PR google#1669 - With unmerged PR google#2178 - With unmerged PR #9 - With unmerged PR #10
- With unmerged PR #1 - With unmerged PR google#1669 - With unmerged PR google#2178 - With unmerged PR #9 - With unmerged PR #10
This reverts commit 7a45e18. The changes will be introduced by the PR google#2178
- With unmerged PR google#1669 - Wup google#2178 - Wup #9 - Wup #10 - Wup google#2076
- With unmerged PR #11 - Wup google#1669 - Wup google#2076 - Wup #9 - Wup google#2178 - Wup #10
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.
IMO, adding a default value to FhirEngine.create
to specify whether the resources are meant to be local only is a better design choice.
Additionally, we would want all the updates to the resource to be local only as well. Therefore, we should mark the resource as local only in the db itself.
suspend fun create(vararg resource: Resource): List<String> | ||
|
||
/** | ||
* Creates one or more remote FHIR [resource]s in the local storage without flagging as local | ||
* changes. | ||
*/ | ||
suspend fun createRemote(vararg resource: 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.
suspend fun create(vararg resource: Resource): List<String> | |
/** | |
* Creates one or more remote FHIR [resource]s in the local storage without flagging as local | |
* changes. | |
*/ | |
suspend fun createRemote(vararg resource: Resource) | |
/** | |
* Creates one or more FHIR [resource]s in the local storage. | |
* | |
* @param isLocalOnly - Setting the value to [true] instructs engine that the resource and its subsequent updates should never be synced to the server. | |
* @return the logical IDs of the newly created resources. | |
*/ | |
suspend fun create(vararg resource: Resource, isLocalOnly: Boolean = false): List<String> |
The default value can be provided in the interface i.e FhirEngine.kt and the FhirEngineImpl.kt would be called with that value if no value is provided by the caller.
override suspend fun createRemote(vararg resource: Resource) { | ||
return database.insertRemote(*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.
override suspend fun createRemote(vararg resource: Resource) { | |
return database.insertRemote(*resource) | |
} |
insertRemote
would just add the resource in the db without creating a LocalChange
counterintuitively marking the change as local only.
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.
Looking at the test, createRemote is doing what it is supposed to do. But, the name createRemote
is counterintuitive and suggests that the resource has something to do with the remote server. A more intutive name could have be createLocal
?
@aditya-07 could you have a second look to see if this is what you had in mind? |
@jingtang10 you had a concern with the updates for the related resources processed by the implementation here, could you share that here or on the ticket #2123 ? |
- With unmerged PR #11 - Wup google#1669 - Wup #9 - Wup google#2178 - Wup #10 - Wup google#2230 - Wup google#2262
- With unmerged PR #11 - Wup google#1669 - Wup #9 - Wup google#2178
- With unmerged PR #11 - Wup google#1669 - Wup #9 - Wup google#2178
- With unmerged PR #9 - WUP PR google#2178 - WUP PR google#2511
- With unmerged PR #9 - WUP PR google#2178 - WUP PR google#2511 - WUP PR google#2537
- With unmerged PR #9 - WUP PR google#2178 - WUP PR google#2511 - WUP PR google#2464
- With unmerged PR #9 - WUP PR google#2178 - WUP PR google#2511
- With unmerged PR #9 - WUP PR google#2178 - WUP PR google#2511 - WUP PR google#2537 - WUP PR google#2511
- With unmerged PR #9 - WUP PR google#2178 - WUP PR google#2511 - WUP PR google#2511
- With unmerged PR #9 - WUP PR google#2178 - WUP PR google#2511 - WUP PR google#2511 - WUP PR google#2544
- With unmerged PR #9 - WUP PR google#2178 - WUP PR google#2544 - WUP PR google#2607
- With unmerged PR #9 - WUP PR google#2178 - WUP google#2652 - WUP google#2521 - WUP google#2645 - WUP google#2648 - WUP google#2650
This is blocked by/Superseded by this discussion - #2625 |
- With unmerged PR #9 - WUP PR google#2178 - WUP google#2650
FORK - With unmerged PR #9 - WUP #13 SDK - WUP google#2178 - WUP google#2650 - WUP google#2663 PERF - WUP google#2669 - WUP google#2565 - WUP google#2561 - WUP google#2535
- With unmerged PR #9 - WUP #13 - WUP google#2178 - WUP google#2650 - WUP google#2663
why is this blocked by custom resources? |
FORK - With unmerged PR #9 - WUP #13 SDK - WUP google#2178 - WUP google#2650 - WUP google#2663 PERF - WUP google#2669 - WUP google#2565 - WUP google#2561 - WUP google#2535
FORK - With unmerged PR #9 - WUP #13 SDK - WUP google#2178 - WUP google#2650 - WUP google#2663 - WUP google#2689
FORK - With unmerged PR #9 - WUP #13 SDK - WUP google#2178 - WUP google#2650 - WUP google#2663 - WUP google#2689 - WUP google#2645
FORK - With unmerged PR #9 - WUP #13 SDK - WUP google#2178 - WUP google#2650 - WUP google#2663 - WUP google#2645
FORK - With unmerged PR #9 - WUP #13 SDK - WUP google#2178 - WUP google#2650 - WUP google#2663 - WUP google#2645 - WUP google#2715
FORK - With unmerged PR #9 - WUP #13 SDK - WUP google#2178 - WUP google#2650 - WUP google#2645
FORK - With unmerged PR #9 - WUP #13 SDK - WUP google#2178 - WUP google#2650 - WUP google#2645
FORK - With unmerged PR #9 - WUP #13 SDK - WUP google#2178 - WUP google#2650 - WUP google#2645 - WUP google#2678 - WUP google#2755
IMPORTANT: All PRs must be linked to an issue (except for extremely trivial and straightforward changes).
Fixes #2123
Description
Clear and concise code change description.
See #2123
Alternative(s) considered
Have you considered any alternatives? And if so, why have you chosen the approach in this PR?
See #2123
Type
Choose one: (Bug fix | Feature | Documentation | Testing | Code health | Builds | Releases | Other)
Feature
Screenshots (if applicable)
N/A
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.