Skip to content

Keep expenses when removing a participant from a group - #671

Open
Harshil-Jani wants to merge 2 commits into
spliit-app:mainfrom
Harshil-Jani:fix/670-participant-removal-deletes-expenses
Open

Harshil-Jani wants to merge 2 commits into
spliit-app:mainfrom
Harshil-Jani:fix/670-participant-removal-deletes-expenses

Conversation

@Harshil-Jani

@Harshil-Jani Harshil-Jani commented Oct 9, 2026 •

Copy link
Copy Markdown

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

  • Migration: Expense.paidBy and ExpensePaidFor.participant become ON 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 diff is empty afterwards).
  • updateGroup runs in a transaction. It first checks whether any removed participant is part of an expense, and if so refuses with their names (CONFLICT, names in error.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.
  • The foreign keys stay as a backstop for an expense added between that check and the commit. Only those two constraints map to 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):

Before After
Hotel deleted Save refused, Bob and Hotel kept

Tests

  • New e2e test in group-management.spec.ts for the steps above. With the old CASCADE constraints it fails (no toast, group left with 0 expenses).
  • prisma/migrations.test.ts fails if a later migration recreates either foreign key without DEFERRABLE INITIALLY DEFERRED.
  • DELETE FROM "Group" still cascades on a database migrated from scratch, with the deferred checks forced.
  • jest 502/502, check-types, lint (0 errors), check-formatting; group-management.spec.ts e2e 7/7 against next dev (rebased on main, 18 Oct).
  • Group deletion (feat: schedule group for deletion #420): purging a group with expenses (group.delete in purgeGroup'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.

@scastiel scastiel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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: deleteMany in updateGroup is computed from a fresh getGroup() 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.

Comment thread src/trpc/routers/groups/update.procedure.ts Outdated
Comment thread src/app/groups/[groupId]/edit/edit-group.tsx Outdated
Comment thread src/lib/api.ts Outdated
Comment thread src/lib/api.ts Outdated
Harshil-Jani and others added 2 commits October 18, 2026 13:46
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
Harshil-Jani force-pushed the fix/670-participant-removal-deletes-expenses branch from 2abf94f to ca47ca6 Compare October 18, 2026 08:17
@Harshil-Jani

Copy link
Copy Markdown
Author

@scastiel rebased onto main and resolved the conflicts, so this is mergeable again.

  • Renamed the migration to 20261018000000_keep_expenses_when_removing_participant so it sorts after feat: schedule group for deletion #420's migration and the recurring anchor one. Instances that already applied those still get it last.
  • With feat: schedule group for deletion #420 merged the app can now delete groups, so the deferred FKs matter in practice. My earlier note that the app had no way to delete a group is out of date. On a fresh database, purging a group with expenses (group.delete inside purgeGroup's transaction) cascades cleanly, and deleting a participant who paid or shares an expense is still refused.
  • jest 502/502, check-types, lint (0 errors), check-formatting, and group-management.spec.ts e2e 7/7 against next dev.

CI needs a maintainer approval to run on this fork PR. Thanks for the review!

This branch has not been deployed

No deployments
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.

Removing a person from a group can silently delete their expenses

2 participants