Repository navigation
Keep expenses when removing a participant from a group - #671
Open
Harshil-Jani wants to merge 2 commits into
Open
Harshil-Jani wants to merge 2 commits into
Harshil-Jani wants to merge 2 commits into
Conversation
scastiel
reviewed
Oct 10, 2026
scastiel
left a comment
Member
There was a problem hiding this comment.
Thanks @Harshil-Jani, this is a clean fix for a nasty data-loss bug, and the before/after plus the e2e test that fails on the old constraints make it easy to trust. I left a few inline comments.
Known limitations, OK to leave for later
- Agreed on the reloaded form losing other unsaved edits. Acceptable for now given how rare the race is.
- Related but pre-existing:
deleteManyinupdateGroupis computed from a freshgetGroup()rather than from the participants the form loaded. So a participant added in another tab after the settings page opened is deleted on save (now refused only if they already have an expense). Worth a separate issue rather than expanding this PR. - I'll open an issue on spliit-ios so the app handles the new 409.
Removing a participant deleted every expense they paid and their share of every other expense, through ON DELETE CASCADE. The settings page hides the remove button for participants in expenses, but only for the expenses it knew of when it loaded, and the server never checked. The two foreign keys are now NO ACTION, DEFERRABLE INITIALLY DEFERRED, so the database refuses the removal and deleting a whole group still cascades. The refusal is a CONFLICT the settings page explains, reloading the group.
updateGroup now looks up the removed participants' expenses inside a transaction and refuses with their names, so the client no longer re-derives them. The update and its activity log are atomic. The foreign key refusal stays as a backstop, matched by constraint name only, with a generic message when no names are known. A jest test keeps the two constraints DEFERRABLE in later migrations. Co-Authored-By: Claude <noreply@anthropic.com>
Harshil-Jani
force-pushed
the
fix/670-participant-removal-deletes-expenses
branch
from
October 18, 2026 08:17
2abf94f to
ca47ca6
Compare
Author
|
@scastiel rebased onto main and resolved the conflicts, so this is mergeable again.
CI needs a maintainer approval to run on this fork PR. Thanks for the review! |
This branch has not been deployed
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 #670.
Removing a participant cascaded (
ON DELETE CASCADE) to every expense they paid and their share of the others. The settings page only protects participants who had expenses when it loaded; the server never checked.Fix
Expense.paidByandExpensePaidFor.participantbecomeON DELETE NO ACTION,DEFERRABLE INITIALLY DEFERRED. Deferred because deleting a whole group cascades to participants and expenses as separate statements; a per-statement check refused it. Prisma can't express this and ignores it (prisma migrate diffis empty afterwards).updateGroupruns in a transaction. It first checks whether any removed participant is part of an expense, and if so refuses with their names (CONFLICT, names inerror.data.participantsWithExpenses). The settings page shows them in a toast and reloads the group, so the participant comes back protected. The update and its activity log are atomic, so a refused save changes and logs nothing.CONFLICT(generic toast, since no names are known).Before / after
Same steps as #670 (remove Bob after an expense was added for him in another tab):
Tests
group-management.spec.tsfor the steps above. With the oldCASCADEconstraints it fails (no toast, group left with 0 expenses).prisma/migrations.test.tsfails if a later migration recreates either foreign key withoutDEFERRABLE INITIALLY DEFERRED.DELETE FROM "Group"still cascades on a database migrated from scratch, with the deferred checks forced.check-types,lint(0 errors),check-formatting;group-management.spec.tse2e 7/7 againstnext dev(rebased on main, 18 Oct).group.deleteinpurgeGroup's transaction) cascades cleanly on a fresh database; deleting a participant who paid or shares an expense is still refused.Not covered: a refused save reloads the form, so other unsaved edits in it are lost. The iOS app now gets a 409 here, and I haven't checked how it shows it.