Fix category deletion and stale group tally after expense changes

README.md

@@ -156,6 +156,7 @@ The insert policy on `group_members` only lets a group's creator add their own f
 * `add_guest_member(group_id, name)` verifies the caller's membership before inserting a guest.
 * `update_expense(...)` rewrites an expense and its splits in one transaction, re-checking membership and re-verifying that the splits sum to the amount **server-side**, so client validation is a convenience rather than the enforcement point.
 * `delete_expense(id)` performs the soft delete. A plain `update` that sets `deleted_at` is rejected, because the resulting row no longer satisfies the `deleted_at is null` select policy.
+* `delete_category(id)` detaches every expense / recurring template / tag pointing at a category and then deletes it. A direct `delete from categories` silently removes zero rows when RLS hides the target (a global default, another group's row) and fails on a foreign-key violation wherever the `0012` `on delete set null` cascade isn't in place; the RPC raises a real error for the former and doesn't depend on the latter.
 * `update_group_photo(group_id, url)` writes the photo URL after the storage upload.
 * `import_expenses(group_id, items)` promotes reviewed statement rows into expenses and splits in one transaction, re-checks the splits-sum-to-amount invariant per row, and returns `{ inserted, skipped, skipped_fingerprints }` so a row someone else already imported is reported rather than raised as an error.
 

docs/categories-and-tags.md

@@ -22,13 +22,22 @@ a category never blocks or cascades, the rows that pointed at it just become
 uncategorized.
 
 RLS: `"members manage group categories"` (`for all`) lets a member
-insert/update/delete rows scoped to their own group; a null `group_id` makes
+insert/update rows scoped to their own group; a null `group_id` makes
 `is_group_member(group_id)` evaluate to null (not true), so global defaults
 are correctly read-only from the client. `CategoriesRepository`
 (`lib/features/expenses/data/categories_repository.dart`) only ever writes
 with an explicit `group_id` for this reason — there's no path to insert a
 global row from the app.
 
+**Deletion** goes through the `delete_category(id)` RPC
+(`0013_delete_category_rpc.sql`), not a direct `delete`. A plain
+`delete from categories` removes zero rows — silently, with no error — when
+RLS hides the target (a global default, or another group's category), and
+fails outright with a foreign-key violation anywhere the `0012` `on delete
+set null` cascade isn't in place. The `SECURITY DEFINER` RPC authorizes the
+caller, raises a real error for the default / wrong-group cases, and detaches
+every reference before deleting — the same pattern as `delete_expense`.
+
 ## Tags are `merchant_rules`, not a separate table
 
 A tag — *"COSTCO always means Groceries"* — is the exact shape the statement
@@ -89,7 +98,8 @@ lib/features/labels/ui/
   labels_screen.dart     standalone route (old /import/rules redirects here)
   category_dialog.dart   name + icon picker
   tag_dialog.dart         keyword + category, match type behind "Advanced"
-supabase/migrations/0012_categories.sql
+supabase/migrations/0012_categories.sql   FKs -> on delete set null, unique names
+supabase/migrations/0013_delete_category_rpc.sql   delete_category() RPC
 test/features/import/merchant_rules_test.dart   matchMerchantRule, matchTagCategory
 test/features/labels/labels_tab_test.dart
 ```

lib/features/balances/ui/balances_tab.dart

@@ -4,7 +4,6 @@ import 'package:tally/core/money.dart';
 import 'package:tally/features/balances/data/balances_repository.dart';
 import 'package:tally/features/balances/logic/settle.dart';
 import 'package:tally/features/balances/providers/balances_provider.dart';
-import 'package:tally/features/expenses/providers/expenses_provider.dart';
 import 'package:tally/features/groups/providers/groups_provider.dart';
 import 'package:tally/models/group_member.dart';
 import 'package:flutter/material.dart';
@@ -156,8 +155,7 @@ class _SettlementRowState extends ConsumerState<_SettlementRow> {
         toMemberId: t.toMemberId,
         amount: t.amount.toString(),
       );
-      ref.invalidate(balancesProvider(widget.groupId));
-      ref.invalidate(expensesProvider(widget.groupId));
+      invalidateGroupMoney(ref, widget.groupId);
     } on Exception catch (e) {
       if (mounted) {
         ScaffoldMessenger.of(context)

lib/features/expenses/data/categories_repository.dart

@@ -39,10 +39,12 @@ class CategoriesRepository {
         .update({'name': name.trim(), 'icon': icon}).eq('id', id);
   }
 
-  // Any expense, recurring template or tag pointing at this category falls
-  // back to uncategorized (on delete set null, migration 0012) rather than
-  // blocking the delete.
+  // Routed through the delete_category RPC (migration 0013), not a direct
+  // delete. The RPC detaches any expense/recurring/tag pointing at the
+  // category before removing it, and — unlike a plain delete, which just
+  // removes zero rows and reports nothing — raises a real error when the
+  // target is a global default or lives in another group.
   Future<void> deleteCategory({required String id}) async {
-    await supabase.from('categories').delete().eq('id', id);
+    await supabase.rpc('delete_category', params: {'p_category_id': id});
   }
 }

lib/features/groups/providers/groups_provider.dart

@@ -1,3 +1,5 @@
+import 'package:tally/features/balances/providers/balances_provider.dart';
+import 'package:tally/features/expenses/providers/expenses_provider.dart';
 import 'package:tally/features/groups/data/groups_repository.dart';
 import 'package:tally/models/group.dart';
 import 'package:tally/models/group_member.dart';
@@ -27,3 +29,15 @@ final groupProvider = FutureProvider.family<Group, String>((ref, groupId) {
 final membersProvider = FutureProvider.family<List<GroupMember>, String>(
   (ref, groupId) => GroupsRepository().fetchMembers(groupId: groupId),
 );
+
+// Call after anything changes a group's money — adding, editing, deleting or
+// importing an expense, or recording a settlement. Refreshes the expense list
+// and the in-group balances, and — easy to forget — groupSummariesProvider,
+// the separate view behind the "you owe / you're owed" tally on the groups
+// list. Nothing else invalidates it, so skipping it leaves that tally stale
+// until a manual pull-to-refresh.
+void invalidateGroupMoney(WidgetRef ref, String groupId) {
+  ref.invalidate(expensesProvider(groupId));
+  ref.invalidate(balancesProvider(groupId));
+  ref.invalidate(groupSummariesProvider);
+}

lib/features/groups/ui/group_detail_screen.dart

@@ -2,7 +2,6 @@ import 'package:tally/core/icons.dart';
 import 'package:tally/core/money.dart';
 import 'package:tally/core/supabase_client.dart';
 import 'package:tally/core/widgets/page_body.dart';
-import 'package:tally/features/balances/providers/balances_provider.dart';
 import 'package:tally/features/balances/ui/balances_tab.dart';
 import 'package:tally/features/expenses/data/expenses_repository.dart';
 import 'package:tally/features/expenses/providers/categories_provider.dart';
@@ -42,14 +41,12 @@ class _GroupDetailScreenState extends ConsumerState<GroupDetailScreen>
 
   Future<void> _addExpense() async {
     await context.push('/groups/${widget.groupId}/expenses/new');
-    ref.invalidate(expensesProvider(widget.groupId));
-    ref.invalidate(balancesProvider(widget.groupId));
+    invalidateGroupMoney(ref, widget.groupId);
   }
 
   Future<void> _importStatement() async {
     await context.push('/groups/${widget.groupId}/import');
-    ref.invalidate(expensesProvider(widget.groupId));
-    ref.invalidate(balancesProvider(widget.groupId));
+    invalidateGroupMoney(ref, widget.groupId);
   }
 
   Future<void> _addGuest() async {
@@ -182,10 +179,7 @@ class _ExpenseCard extends ConsumerWidget {
   final String groupId;
   final Category? category;
 
-  void _refresh(WidgetRef ref) {
-    ref.invalidate(expensesProvider(groupId));
-    ref.invalidate(balancesProvider(groupId));
-  }
+  void _refresh(WidgetRef ref) => invalidateGroupMoney(ref, groupId);
 
   Future<void> _edit(BuildContext context, WidgetRef ref) async {
     await context.push('/groups/$groupId/expenses/${expense.id}/edit');

supabase/migrations/0013_delete_category_rpc.sql

@@ -0,0 +1,55 @@
+-- TALLY — deletable categories, take two
+--
+-- 0012 switched the category foreign keys to ON DELETE SET NULL so a category
+-- still referenced by an expense could be removed without orphaning rows. That
+-- fixes the database, but the client still deletes categories with a plain
+--   delete from categories where id = ?
+-- and that has two failure modes the user sees as "can't delete categories":
+--
+--   1. It silently deletes zero rows whenever RLS hides the target — a global
+--      default (group_id is null), or a category owned by another group. No
+--      error comes back, the list refetches, the category is still there.
+--   2. Where the SET NULL cascade from 0012 isn't in place (an environment
+--      still on <= 0011), the delete fails outright with a foreign-key
+--      violation from expenses / recurring_expenses / merchant_rules.
+--
+-- Route deletion through a SECURITY DEFINER RPC instead — the same pattern
+-- delete_expense, update_expense and update_group_photo already use. It
+-- authorizes the caller as a member of the category's group, raises a clear
+-- error for the cases that should fail, detaches every reference explicitly
+-- (so it works regardless of the FK action), then deletes.
+
+create or replace function delete_category(p_category_id uuid)
+returns void
+language plpgsql
+security definer
+set search_path = public
+as $$
+declare
+  v_group_id uuid;
+begin
+  select group_id into v_group_id
+  from   categories
+  where  id = p_category_id;
+
+  if not found then
+    raise exception 'Category not found';
+  end if;
+  -- Global defaults belong to every group; a single group can't delete them.
+  if v_group_id is null then
+    raise exception 'Default categories cannot be deleted';
+  end if;
+  if not is_group_member(v_group_id) then
+    raise exception 'Not authorized';
+  end if;
+
+  -- Detach references explicitly so the delete succeeds even where the
+  -- ON DELETE SET NULL cascade from 0012 never ran. Only this group's own
+  -- rows can point at the category, and SECURITY DEFINER clears RLS.
+  update expenses          set category_id = null where category_id = p_category_id;
+  update recurring_expenses set category_id = null where category_id = p_category_id;
+  update merchant_rules     set category_id = null where category_id = p_category_id;
+
+  delete from categories where id = p_category_id;
+end;
+$$;

test/features/groups/group_money_invalidation_test.dart

@@ -0,0 +1,66 @@
+import 'package:tally/features/balances/providers/balances_provider.dart';
+import 'package:tally/features/expenses/providers/expenses_provider.dart';
+import 'package:tally/features/groups/providers/groups_provider.dart';
+import 'package:tally/models/expense.dart';
+import 'package:tally/models/member_balance.dart';
+import 'package:tally/models/group_summary.dart';
+import 'package:flutter/material.dart';
+import 'package:flutter_riverpod/flutter_riverpod.dart';
+import 'package:flutter_test/flutter_test.dart';
+
+// Regression: deleting (or adding/editing/importing) an expense used to refresh
+// only the in-group providers, leaving groupSummariesProvider — the "you owe /
+// you're owed" tally on the groups list — stale until a manual refresh.
+// invalidateGroupMoney must hit all three.
+void main() {
+  testWidgets('invalidateGroupMoney refetches the groups-list tally too',
+      (tester) async {
+    var expenseFetches = 0;
+    var balanceFetches = 0;
+    var summaryFetches = 0;
+
+    late WidgetRef ref;
+    await tester.pumpWidget(
+      ProviderScope(
+        overrides: [
+          expensesProvider('g1').overrideWith((ref) async {
+            expenseFetches++;
+            return <Expense>[];
+          }),
+          balancesProvider('g1').overrideWith((ref) async {
+            balanceFetches++;
+            return <MemberBalance>[];
+          }),
+          groupSummariesProvider.overrideWith((ref) async {
+            summaryFetches++;
+            return <GroupSummary>[];
+          }),
+        ],
+        child: Consumer(
+          builder: (_, r, __) {
+            ref = r;
+            r.watch(expensesProvider('g1'));
+            r.watch(balancesProvider('g1'));
+            r.watch(groupSummariesProvider);
+            return const SizedBox();
+          },
+        ),
+      ),
+    );
+    await tester.pumpAndSettle();
+    expect(
+      [expenseFetches, balanceFetches, summaryFetches],
+      [1, 1, 1],
+      reason: 'each provider fetched once on first watch',
+    );
+
+    invalidateGroupMoney(ref, 'g1');
+    await tester.pumpAndSettle();
+
+    expect(
+      [expenseFetches, balanceFetches, summaryFetches],
+      [2, 2, 2],
+      reason: 'all three refetch, including the groups-list summary',
+    );
+  });
+}