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

Child artifact delete #94

Merged
merged 3 commits into from
Apr 24, 2024
Merged

Child artifact delete #94

merged 3 commits into from
Apr 24, 2024

Conversation

elsaperelli
Copy link
Contributor

Summary

This PR continues our work of ensuring that child artifacts in the measure-repository do not have a lifecycle outside of their parent artifact. This PR prevents child artifacts from being deleted from the draft-repository and implements recursive deletion of a parent artifact and all of its children in a batch transaction.

New behavior

  • The "delete" button is disabled in the draft-repository (authoring side of the app) for artifacts that are child artifacts (meaning they have the isOwned extension).
  • Artifact deletion of the draft-repository is now batched so that the artifact and any of its child artifacts are also deleted.

Code changes

  • app/src/components/ResourceInfoCard.tsx - deleteParent trpc procedure is now used instead of deleteDraft, delete button is now disabled for child artifacts
  • app/src/server/db/dbOperations.ts - new batchDeleteDraft function to delete a parent artifact and any of its children in a batch transaction. I had to also change how batchCreateDraft was written because I would randomly get errors along the lines of "Given transaction number 1 does not match any in-progress transactions" and I saw that a solution was to use withTransaction instead.
  • app/src/server/trpc/routers/draft.ts - new deleteParent procedure
  • app/src/util/modifyResourceFields.ts - fixes a bug I found- basically if a parent artifact was drafted and the version had to be incremented more than once, then we had to make sure the same happened to the version in the reference in relatedArtifacts
  • app/src/util/resourceCardUtils.ts / app/src/util/types/fhir.ts - new isChild property to be used in the ResourceInfoCard.tsx component to determine whether to disable the delete button or not

Testing guidance

  • npm run check:all
  • npm run start:all
  • cd service
  • npm run db:reset
  • npm run db:loadBundle <path-to-bundle> (use the edited ColorectalCancerScreeningsFHIR bundle I made for Fix recursion #90)
  • In the root: npm run start:all
  • In the app, try various combinations of drafting, releasing, and deleting artifacts and making sure the same happens to their children. Make sure you cannot delete any of the child artifacts from the draft-repository. There should hopefully be a clearer lifecycle now of drafting, deleting, and releasing parent artifacts and their children, but try your best to break it.

@elsaperelli elsaperelli marked this pull request as ready for review April 18, 2024 16:46
@elsaperelli elsaperelli requested a review from lmd59 April 19, 2024 14:23
Copy link
Contributor

@lmd59 lmd59 left a comment

Choose a reason for hiding this comment

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

Looks great and nice bug catch too!
One small suggestion:
Since this is a destructive action, it feels important to make sure the user knows that this will also delete child artifacts in the initial delete modal:
Screenshot 2024-04-24 at 12 22 06 PM
Can we just add a bit about the children, i.e. "This will delete draft Measure "ColorectalCancerScreeningsFHIR" and any child artifacts permanently." or something like that?

@elsaperelli elsaperelli requested a review from lmd59 April 24, 2024 17:46
@lmd59 lmd59 merged commit 5d5bfe8 into main Apr 24, 2024
2 checks passed
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.

2 participants