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

Resource consolidation for resources created with POST #2217

Merged
merged 28 commits into from
Oct 26, 2023

Conversation

anchita-g
Copy link
Collaborator

@anchita-g anchita-g commented Sep 29, 2023

IMPORTANT: All PRs must be linked to an issue (except for extremely trivial and straightforward changes).

One of the PRs to fix #760

Description

  • Added resource consolidation for cases where resources are created through POST. We would need to consolidate the IDs of the newly created resources in any LocalChange referring to these created resources as well in any Resource referring to these created resources.
  • Introduced a new entity called LocalChangeResourceReferenceEntity which maintains a mapping between a LocalChange and any references to a resource the contents of the LocalChange contains. This would help us in easily looking up all the LocalChanges that need to be updated when a referred resource is assigned a new ID by the server. Similarly, we can use the mapped LocalChange to get al the resources that refer to the said resource. These resources as well as their respective references would need to be updated as well.
  • Added a migration to create the new mapping from the existing content of the LocalChange to the "references" present in the payload of the LocalChange along with a test case to test the migration.

Alternative(s) considered
Have you considered any alternatives? And if so, why have you chosen the approach in this PR?

  • An alternative to update the references in the Resource or the LocalChange of the resource referring to the created resource was to use the FHIR path that is mentioned in the LocalChangeResourceReferenceEntity. However, in cases where there are lists of references, FHIR path does not indicate the index in the list at which the reference exists. In such cases we would have to resolve to updating the reference value by either replacing string value by regex match or use JSON update strategy. Similar case holds for LocalChange where in for the UPDATE type LocalChange, updating the reference using FHIR path would not be possible.

Type
Choose one: Feature

Screenshots (if applicable)

Checklist

  • I have read and acknowledged the Code of conduct.
  • I have read the Contributing page.
  • I have signed the Google Individual CLA, or I am covered by my company's Corporate CLA.
  • I have discussed my proposed solution with code owners in the linked issue(s) and we have agreed upon the general approach.
  • I have run ./gradlew spotlessApply and ./gradlew spotlessCheck to check my code follows the style guide of this project.
  • I have run ./gradlew check and ./gradlew connectedCheck to test my changes locally.
  • I have built and run the demo app(s) to verify my change fixes the issue and/or does not break the demo app(s).

@anchita-g anchita-g changed the title Resource consolidation POST PerResource URL Request Resource consolidation Sep 29, 2023
@anchita-g anchita-g marked this pull request as draft September 29, 2023 10:17
@anchita-g anchita-g marked this pull request as ready for review October 3, 2023 09:22
@anchita-g anchita-g marked this pull request as draft October 5, 2023 12:28
@anchita-g anchita-g changed the title POST PerResource URL Request Resource consolidation POST URL Request Resource consolidation Oct 6, 2023
@anchita-g anchita-g changed the title POST URL Request Resource consolidation Resource consolidation for resources created with POST Oct 9, 2023
@anchita-g anchita-g self-assigned this Oct 9, 2023
@anchita-g anchita-g marked this pull request as ready for review October 9, 2023 07:54
Copy link
Contributor

@omarismail94 omarismail94 left a comment

Choose a reason for hiding this comment

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

Just two minor comments!

@omarismail94
Copy link
Contributor

@anchita-g can you merge the master branch and resolve any conflicts? can approve once that is done and the comments are addressed

@omarismail94 omarismail94 enabled auto-merge (squash) October 26, 2023 12:50
@omarismail94 omarismail94 dismissed jingtang10’s stale review October 26, 2023 14:44

Anchita has resolved all comments

@omarismail94 omarismail94 merged commit 0e91a64 into google:master Oct 26, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

Create new resources on the server using POST
3 participants