Repository navigation
fix(vendor): return 409 instead of 500 when deleting a vendor with linked records - #625
Open
prince-shakyaa wants to merge 1 commit into
Open
prince-shakyaa wants to merge 1 commit into
prince-shakyaa wants to merge 1 commit into
Conversation
…nked records Deleting a vendor that still had invoices, files or emails hit a database constraint and came back as an uncaught 500. Catch it, roll back and return a 409 so nothing is lost and the caller gets a clear answer.
Author
|
Hi @saikishu @e2hln, this fixes #624. Deleting a vendor that still has invoices, files or emails now returns a 409 instead of a 500, so nothing gets deleted and the caller knows why. It's the same approach as #614. Tests are in tests/unit/vendor/test_vendor_delete.py. If you'd prefer a soft delete instead, I can switch it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #624
What was wrong
DELETE /vendor/api/v1/vendors/{id}returned a 500 for any vendor that had an invoice, a FinDrive file or an email. Those rows still point at the vendor, so the database refused the delete and theIntegrityErrorwasn't caught. Only brand-new vendors with nothing attached could be deleted.What changed
IntegrityError, rolls back and returns a 409 saying the vendor has linked records and can't be deleted. This matches how fix(vendor): block invoice edits after approval or payment #614 handles edits to approved or paid invoices, and keeps the invoice and payment history intact.I went with the 409 because it's the smallest change and doesn't lose any data. If you'd rather have a soft delete (mark the vendor inactive), I'm happy to switch to that.
Testing
tests/unit/vendor/test_vendor_delete.py: vendor with an invoice, a file or an email gets a 409 and stays in the DB; an unused vendor is deleted; another vendor's delete is a 403.IntegrityErroras in the issue.tests/unit/vendor: all pass.tests/unitoverall: everything passes except the 3 tests intests/unit/agents/test_specialized_agents.py, which fail the same way onmain.