Merge pull request #1572 from mauriceboe/fix/budget-split-removed-member-1553

fix(budget): re-split expenses when a member is removed

client/src/components/Trips/TripMembersModal.tsx

@@ -290,6 +290,7 @@ export default function TripMembersModal({ isOpen, onClose, tripId, tripTitle, o
   const { t } = useTranslation()
   const can = useCanDo()
   const trip = useTripStore((s) => s.trip)
+  const loadBudgetItems = useTripStore((s) => s.loadBudgetItems)
   const canManageMembers = can('member_manage', trip)
   const canManageShare = can('share_manage', trip)
 
@@ -387,6 +388,7 @@ export default function TripMembersModal({ isOpen, onClose, tripId, tripTitle, o
     try {
       await tripsApi.deleteGuest(tripId, userId)
       await loadMembers(true)
+      await loadBudgetItems(tripId)
       toast.success(t('members.guestRemoved'))
     } catch {
       toast.error(t('members.removeError'))

server/src/services/budgetService.ts

@@ -30,13 +30,23 @@ function loadItemPayers(itemId: number | string) {
   return rows.map(p => ({ ...p, avatar_url: avatarUrl(p) }));
 }
 
+function knownUserIds(userIds: number[]): Set<number> {
+  const unique = [...new Set(userIds)];
+  if (unique.length === 0) return new Set();
+  const rows = db.prepare(
+    `SELECT id FROM users WHERE id IN (${unique.map(() => '?').join(',')})`
+  ).all(...unique) as { id: number }[];
+  return new Set(rows.map(r => r.id));
+}
+
 /** Replace the payer rows of an item and keep total_price = sum of payer amounts. */
 function writeItemPayers(itemId: number | string, payers: { user_id: number; amount: number }[]) {
   db.prepare('DELETE FROM budget_item_payers WHERE budget_item_id = ?').run(itemId);
   const insert = db.prepare('INSERT OR IGNORE INTO budget_item_payers (budget_item_id, user_id, amount) VALUES (?, ?, ?)');
+  const known = knownUserIds(payers.map(p => p.user_id));
   let total = 0;
   for (const p of payers) {
-    if (!(p.amount > 0)) continue;
+    if (!(p.amount > 0) || !known.has(p.user_id)) continue;
     insert.run(itemId, p.user_id, p.amount);
     total += p.amount;
   }
@@ -242,6 +252,11 @@ export function createBudgetItem(
   const payerTotal = (data.payers || []).reduce((a, p) => a + (p.amount > 0 ? p.amount : 0), 0);
   const total = data.payers && data.payers.length > 0 ? payerTotal : (data.total_price || 0);
 
+  const knownMembers = data.members ? knownUserIds(data.members.map(m => m.user_id)) : null;
+  const members = data.members && knownMembers ? data.members.filter(m => knownMembers.has(m.user_id)) : undefined;
+  const knownIds = data.member_ids ? knownUserIds(data.member_ids) : null;
+  const memberIds = data.member_ids && knownIds ? data.member_ids.filter(uid => knownIds.has(uid)) : undefined;
+
   const result = db.prepare(
     'INSERT INTO budget_items (trip_id, category, name, total_price, currency, exchange_rate, persons, days, note, sort_order, expense_date, reservation_id) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)'
   ).run(
@@ -251,7 +266,7 @@ export function createBudgetItem(
     total,
     data.currency || null,
     data.exchange_rate != null ? data.exchange_rate : 1,
-    data.member_ids ? data.member_ids.length : (data.persons != null ? data.persons : null),
+    memberIds ? memberIds.length : (data.persons != null ? data.persons : null),
     data.days !== undefined && data.days !== null ? data.days : null,
     data.note || null,
     sortOrder,
@@ -261,12 +276,12 @@ export function createBudgetItem(
 
   const itemId = result.lastInsertRowid as number;
   if (data.payers && data.payers.length > 0) writeItemPayers(itemId, data.payers);
-  if (data.members && data.members.length > 0) {
+  if (members && members.length > 0) {
     const insert = db.prepare('INSERT OR IGNORE INTO budget_item_members (budget_item_id, user_id, paid, amount) VALUES (?, ?, 0, ?)');
-    for (const m of data.members) insert.run(itemId, m.user_id, m.amount !== undefined && m.amount !== null ? m.amount : null);
-  } else if (data.member_ids && data.member_ids.length > 0) {
+    for (const m of members) insert.run(itemId, m.user_id, m.amount !== undefined && m.amount !== null ? m.amount : null);
+  } else if (memberIds && memberIds.length > 0) {
     const insert = db.prepare('INSERT OR IGNORE INTO budget_item_members (budget_item_id, user_id, paid, amount) VALUES (?, ?, 0, NULL)');
-    for (const uid of data.member_ids) insert.run(itemId, uid);
+    for (const uid of memberIds) insert.run(itemId, uid);
   }
 
   const item = db.prepare('SELECT * FROM budget_items WHERE id = ?').get(itemId) as BudgetItem;
@@ -347,15 +362,19 @@ export function updateBudgetItem(
     }
   }
   if (data.members !== undefined) {
+    const known = knownUserIds(data.members.map(m => m.user_id));
+    const members = data.members.filter(m => known.has(m.user_id));
     db.prepare('DELETE FROM budget_item_members WHERE budget_item_id = ?').run(id);
     const insert = db.prepare('INSERT OR IGNORE INTO budget_item_members (budget_item_id, user_id, paid, amount) VALUES (?, ?, 0, ?)');
-    for (const m of data.members) insert.run(id, m.user_id, m.amount !== undefined && m.amount !== null ? m.amount : null);
-    db.prepare('UPDATE budget_items SET persons = ? WHERE id = ?').run(data.members.length || null, id);
+    for (const m of members) insert.run(id, m.user_id, m.amount !== undefined && m.amount !== null ? m.amount : null);
+    db.prepare('UPDATE budget_items SET persons = ? WHERE id = ?').run(members.length || null, id);
   } else if (data.member_ids !== undefined) {
+    const known = knownUserIds(data.member_ids);
+    const memberIds = data.member_ids.filter(uid => known.has(uid));
     db.prepare('DELETE FROM budget_item_members WHERE budget_item_id = ?').run(id);
     const insert = db.prepare('INSERT OR IGNORE INTO budget_item_members (budget_item_id, user_id, paid, amount) VALUES (?, ?, 0, NULL)');
-    for (const uid of data.member_ids) insert.run(id, uid);
-    db.prepare('UPDATE budget_items SET persons = ? WHERE id = ?').run(data.member_ids.length || null, id);
+    for (const uid of memberIds) insert.run(id, uid);
+    db.prepare('UPDATE budget_items SET persons = ? WHERE id = ?').run(memberIds.length || null, id);
   }
 
   // If category changed, update category order table
@@ -409,10 +428,12 @@ export function updateMembers(id: string | number, tripId: string | number, user
 
   db.prepare('DELETE FROM budget_item_members WHERE budget_item_id = ?').run(id);
 
-  if (userIds.length > 0) {
+  const known = knownUserIds(userIds);
+  const memberIds = userIds.filter(uid => known.has(uid));
+  if (memberIds.length > 0) {
     const insert = db.prepare('INSERT OR IGNORE INTO budget_item_members (budget_item_id, user_id, paid) VALUES (?, ?, ?)');
-    for (const userId of userIds) insert.run(id, userId, existingPaid[userId] || 0);
-    db.prepare('UPDATE budget_items SET persons = ? WHERE id = ?').run(userIds.length, id);
+    for (const userId of memberIds) insert.run(id, userId, existingPaid[userId] || 0);
+    db.prepare('UPDATE budget_items SET persons = ? WHERE id = ?').run(memberIds.length, id);
   } else {
     db.prepare('UPDATE budget_items SET persons = NULL WHERE id = ?').run(id);
   }
@@ -422,6 +443,23 @@ export function updateMembers(id: string | number, tripId: string | number, user
   return { members, item: updated };
 }
 
+export function removeUserFromBudgetItems(userId: number): void {
+  const itemIds = (db.prepare('SELECT DISTINCT budget_item_id FROM budget_item_members WHERE user_id = ?')
+    .all(userId) as { budget_item_id: number }[]).map(r => r.budget_item_id);
+  if (itemIds.length === 0) {
+    return;
+  }
+
+  db.prepare('DELETE FROM budget_item_members WHERE user_id = ?').run(userId);
+
+  const remaining = db.prepare('SELECT COUNT(*) AS count FROM budget_item_members WHERE budget_item_id = ?');
+  const setPersons = db.prepare('UPDATE budget_items SET persons = ? WHERE id = ?');
+  for (const itemId of itemIds) {
+    const { count } = remaining.get(itemId) as { count: number };
+    setPersons.run(count || null, itemId);
+  }
+}
+
 export function toggleMemberPaid(id: string | number, tripId: string | number, userId: string | number, paid: boolean) {
   // Resolve the item within the caller's trip before updating.
   const item = db.prepare('SELECT id FROM budget_items WHERE id = ? AND trip_id = ?').get(id, tripId);

server/src/services/tripService.ts

@@ -7,7 +7,7 @@ import { erasePluginUserData } from './userCleanupService';
 import { emitUserDeleted } from '../plugin-user-lifecycle';
 import { Trip, User } from '../types';
 import { listDays, listAccommodations, addDays } from './dayService';
-import { listBudgetItems } from './budgetService';
+import { listBudgetItems, removeUserFromBudgetItems } from './budgetService';
 import { listItems as listPackingItems } from './packingService';
 import { listReservations, loadEndpointsByTrip, resyncReservationDays } from './reservationService';
 import { listNotes as listCollabNotes } from './collabService';
@@ -525,6 +525,9 @@ export function deleteGuest(tripId: string | number, guestUserId: number): boole
   // host-side per-user tables + a durable own-db erasure per granted plugin — exactly
   // like a full account deletion (otherwise a deleted guest's plugin data lingers).
   erasePluginUserData(guestUserId);
+  // Re-split the expenses they were part of before the cascade takes their member
+  // rows away — the divisor is denormalized and cannot follow a foreign key (#1553).
+  removeUserFromBudgetItems(guestUserId);
   // Deleting the guest's users row cascades its membership and every assignment join
   // (trip_members, budget/packing/assignment links) via the ON DELETE foreign keys.
   db.prepare('DELETE FROM users WHERE id = ? AND is_guest = 1').run(guestUserId);

server/src/services/userCleanupService.ts

@@ -1,6 +1,7 @@
 import fs from 'node:fs';
 import { db } from '../db/database';
 import { pluginsDataRoot } from '../nest/plugins/paths';
+import { removeUserFromBudgetItems } from './budgetService';
 
 /**
  * Erase a user's PLUGIN-held data on account deletion. Two parts:
@@ -44,6 +45,7 @@ export function erasePluginUserData(userId: number): void {
 function cleanupUserReferences(userId: number): void {
   db.prepare('UPDATE trip_members SET invited_by = NULL WHERE invited_by = ?').run(userId);
   db.prepare('UPDATE budget_items SET paid_by_user_id = NULL WHERE paid_by_user_id = ?').run(userId);
+  removeUserFromBudgetItems(userId);
   db.prepare('DELETE FROM share_tokens WHERE created_by = ?').run(userId);
   db.prepare('DELETE FROM journey_share_tokens WHERE created_by = ?').run(userId);
   // Owned journeys cascade-delete their entries/contributors/share_tokens/photos via journey_id FKs

server/tests/unit/services/budgetServiceDb.test.ts

@@ -41,7 +41,8 @@ import { createTables } from '../../../src/db/schema';
 import { runMigrations } from '../../../src/db/migrations';
 import { resetTestDb } from '../../helpers/test-db';
 import { createUser, createTrip } from '../../helpers/factories';
-import { createBudgetItem, updateMembers, toggleMemberPaid, calculateSettlement, rebaseTripCurrency } from '../../../src/services/budgetService';
+import { createBudgetItem, updateBudgetItem, updateMembers, toggleMemberPaid, calculateSettlement, rebaseTripCurrency } from '../../../src/services/budgetService';
+import { createGuest, deleteGuest } from '../../../src/services/tripService';
 
 beforeAll(() => {
   createTables(testDb);
@@ -63,6 +64,87 @@ function paidFlag(itemId: number, memberId: number): number | undefined {
   return row?.paid;
 }
 
+describe('deleting a member re-splits their expenses (#1553)', () => {
+  function personsOf(itemId: number): number | null {
+    return (testDb.prepare('SELECT persons FROM budget_items WHERE id = ?').get(itemId) as { persons: number | null }).persons;
+  }
+  function memberCount(itemId: number): number {
+    return (testDb.prepare('SELECT COUNT(*) AS count FROM budget_item_members WHERE budget_item_id = ?')
+      .get(itemId) as { count: number }).count;
+  }
+
+  it('BUDGET-SVC-DB-010: re-derives the persons divisor when a guest in the split is deleted', () => {
+    const { user: owner } = createUser(testDb);
+    const trip = createTrip(testDb, owner.id);
+    const guests = ['G1', 'G2', 'G3'].map(n => createGuest(trip.id, n, owner.id).member);
+    const item = createBudgetItem(trip.id, {
+      name: 'Dinner', total_price: 400,
+      member_ids: [owner.id, ...guests.map(g => g.id)],
+    });
+    expect(personsOf(item.id)).toBe(4);
+
+    deleteGuest(trip.id, guests[0].id);
+    deleteGuest(trip.id, guests[1].id);
+
+    // The member rows cascade with the users row; `persons` is denormalized and has to
+    // be re-derived, or the per-person column keeps dividing by the departed.
+    expect(memberCount(item.id)).toBe(2);
+    expect(personsOf(item.id)).toBe(2);
+  });
+
+  it('BUDGET-SVC-DB-011: leaves a manually entered persons count alone', () => {
+    const { user: owner } = createUser(testDb);
+    const trip = createTrip(testDb, owner.id);
+    const guest = createGuest(trip.id, 'G1', owner.id).member;
+    // No member rows — `persons` is just a number someone typed.
+    const item = createBudgetItem(trip.id, { name: 'Rental', total_price: 300, persons: 6 });
+
+    deleteGuest(trip.id, guest.id);
+
+    expect(personsOf(item.id)).toBe(6);
+  });
+
+  it('BUDGET-SVC-DB-012: drops the last member to a null divisor rather than zero', () => {
+    const { user: owner } = createUser(testDb);
+    const trip = createTrip(testDb, owner.id);
+    const guest = createGuest(trip.id, 'G1', owner.id).member;
+    const item = createBudgetItem(trip.id, { name: 'Taxi', total_price: 50, member_ids: [guest.id] });
+
+    deleteGuest(trip.id, guest.id);
+
+    expect(memberCount(item.id)).toBe(0);
+    expect(personsOf(item.id)).toBeNull();
+  });
+
+  it('BUDGET-SVC-DB-013: saves a split from a stale client instead of failing on the users FK', () => {
+    const { user: owner } = createUser(testDb);
+    const trip = createTrip(testDb, owner.id);
+    const guest = createGuest(trip.id, 'G1', owner.id).member;
+    const item = createBudgetItem(trip.id, { name: 'Dinner', total_price: 200, member_ids: [owner.id, guest.id] });
+
+    deleteGuest(trip.id, guest.id);
+
+    // A client that loaded before the deletion still sends the guest back (#1553).
+    const updated = updateBudgetItem(item.id, trip.id, { member_ids: [owner.id, guest.id] });
+
+    expect(updated!.members.map(m => m.user_id)).toEqual([owner.id]);
+    expect(personsOf(item.id)).toBe(1);
+  });
+
+  it('BUDGET-SVC-DB-014: ignores a deleted member arriving through updateMembers', () => {
+    const { user: owner } = createUser(testDb);
+    const trip = createTrip(testDb, owner.id);
+    const guest = createGuest(trip.id, 'G1', owner.id).member;
+    const item = createBudgetItem(trip.id, { name: 'Drinks', total_price: 60, member_ids: [owner.id, guest.id] });
+
+    deleteGuest(trip.id, guest.id);
+    const result = updateMembers(item.id, trip.id, [owner.id, guest.id]);
+
+    expect(result!.members.map(m => m.user_id)).toEqual([owner.id]);
+    expect(personsOf(item.id)).toBe(1);
+  });
+});
+
 describe('toggleMemberPaid trip-scoping', () => {
   it('BUDGET-SVC-DB-001: toggles paid for an item that belongs to the given trip', () => {
     const { user } = createUser(testDb);