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

feat: Allow soft-deletion of orphaned relation rows using orphanedRow… #8414

Merged

Conversation

oxeye-yuvalk
Copy link
Contributor

@oxeye-yuvalk oxeye-yuvalk commented Dec 1, 2021

Allow soft-deletion of orphaned relation rows using orphanedRowAction
related to - #8408

Description of change

The Problem
orphanedRowAction supports nullifying the relation rows or deleting them but not soft-deleting them.

The Solution
Add a soft-delete option for orphanedRowAction.

Pull-Request Checklist

  • Code is up-to-date with the master branch
  • npm run lint passes with this change
  • npm run test passes with this change
  • This pull request links relevant issues as Fixes #0000
  • There are new or updated unit tests validating the change
  • Documentation has been updated to reflect this change
  • The new commits follow conventions explained in CONTRIBUTING.md

@pleerock
Copy link
Member

pleerock commented Dec 1, 2021

@jbjhjm what do you think about this feature request?

@jbjhjm
Copy link
Contributor

jbjhjm commented Dec 6, 2021

@pleerock Makes sense to me. Adds another facet to making the orphanedRowAction feature complete.
Implementation is really easy so not much worrying it might break anything.

Tests look fine, but I'm missing a check validating that there actually is a soft-deleted Post in database.
As far as I can see current tests would pass if the row was hard-deleted from database too.
So I think the "should delete the orphaned Post from the database" test should become "should mark orphaned Post as soft-deleted" @oxeye-yuvalk -- do you agree?

@oxeye-yuvalk
Copy link
Contributor Author

@pleerock @jbjhjm I just changed the test file and all check have passed!

@pleerock pleerock merged commit cefddd9 into typeorm:master Dec 11, 2021
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.

None yet

3 participants