fix: persist group assignment when editing a non-group expense

src/server/api/routers/expense.ts

@@ -13,7 +13,12 @@ import {
   getBatchCurrencyRatesSchema,
   getCurrencyRateSchema,
 } from '~/types/expense.types';
-import { createExpense, deleteExpense, editExpense } from '../services/splitService';
+import {
+  assertExpenseGroupReassignment,
+  createExpense,
+  deleteExpense,
+  editExpense,
+} from '../services/splitService';
 import { currencyRateProvider } from '../services/currencyRateService';
 import { isCurrencyCode } from '~/lib/currency';
 import { SplitType } from '@prisma/client';
@@ -144,6 +149,16 @@ export const expenseRouter = createTRPCRouter({
           throw new TRPCError({ code: 'BAD_REQUEST', message: 'Invalid split type' });
         }
 
+        if (input.expenseId) {
+          await assertExpenseGroupReassignment(
+            input.expenseId,
+            input.groupId,
+            ctx.session.user.id,
+            input.paidBy,
+            input.participants.map((participant) => participant.userId),
+          );
+        }
+
         try {
           const expense = input.expenseId
             ? await editExpense(input, ctx.session.user.id)
@@ -209,6 +224,13 @@ export const expenseRouter = createTRPCRouter({
       if (!expenseId) {
         res = await createExpense(conversionTo, ctx.session.user.id, conversionFrom);
       } else {
+        await assertExpenseGroupReassignment(
+          expenseId,
+          conversionFrom.groupId,
+          ctx.session.user.id,
+          conversionFrom.paidBy,
+          conversionFrom.participants.map((participant) => participant.userId),
+        );
         res = await editExpense(conversionFrom, ctx.session.user.id, conversionTo);
       }
 
@@ -659,7 +681,8 @@ const validateExpenseWritePermission = async (
       where: { id: expenseId },
       select: { groupId: true },
     });
-    if (expense.groupId !== groupId) {
+    // An ungrouped expense may be assigned to a group; a grouped expense keeps its group.
+    if (null !== expense.groupId && expense.groupId !== groupId) {
       throw new TRPCError({
         code: 'BAD_REQUEST',
         message: 'Cannot move an expense to another group',

src/server/api/services/splitService.ts

@@ -1,4 +1,5 @@
 import { SplitType, type User } from '@prisma/client';
+import { TRPCError } from '@trpc/server';
 import { nanoid } from 'nanoid';
 
 import { db } from '~/server/db';
@@ -192,10 +193,81 @@ export async function deleteExpense(expenseId: string, deletedBy: number) {
   sendExpensePushNotification(expenseId).catch(console.error);
 }
 
+export async function assertExpenseGroupReassignment(
+  expenseId: string,
+  groupId: number | null,
+  actorUserId: number,
+  paidBy: number,
+  participantUserIds: number[],
+) {
+  const expense = await db.expense.findUnique({
+    where: { id: expenseId },
+    select: { groupId: true },
+  });
+
+  if (!expense) {
+    throw new TRPCError({ code: 'BAD_REQUEST', message: 'Expense not found' });
+  }
+
+  const storedGroupId = expense.groupId;
+
+  if (null !== storedGroupId) {
+    if (storedGroupId !== groupId) {
+      throw new TRPCError({ code: 'BAD_REQUEST', message: 'Cannot change group while editing' });
+    }
+
+    return;
+  }
+
+  if (null === groupId) {
+    return;
+  }
+
+  const group = await db.group.findUnique({
+    where: { id: groupId },
+    select: { archivedAt: true },
+  });
+
+  if (!group) {
+    throw new TRPCError({ code: 'BAD_REQUEST', message: 'Group not found' });
+  }
+
+  if (group.archivedAt) {
+    throw new TRPCError({ code: 'BAD_REQUEST', message: 'Group is archived' });
+  }
+
+  const members = await db.groupUser.findMany({
+    where: {
+      groupId,
+      userId: {
+        in: [actorUserId, paidBy, ...participantUserIds],
+      },
+    },
+    select: { userId: true },
+  });
+  const memberIds = new Set(members.map((member) => member.userId));
+
+  if (!memberIds.has(actorUserId)) {
+    throw new TRPCError({ code: 'FORBIDDEN', message: 'Not a group member' });
+  }
+
+  if (!memberIds.has(paidBy)) {
+    throw new TRPCError({ code: 'BAD_REQUEST', message: 'Payer is not a member of the group' });
+  }
+
+  if (participantUserIds.some((userId) => !memberIds.has(userId))) {
+    throw new TRPCError({
+      code: 'BAD_REQUEST',
+      message: 'Participant is not a member of the group',
+    });
+  }
+}
+
 export async function editExpense(
   {
     expenseId,
     paidBy,
+    groupId,
     name,
     category,
     amount,
@@ -253,6 +325,7 @@ export async function editExpense(
       where: { id: expenseId },
       data: {
         paidBy,
+        groupId,
         name,
         category,
         amount,

src/tests/integration/expenseGroupReassignment.integration.test.ts

@@ -0,0 +1,131 @@
+import { db } from './database';
+import { accountingSnapshot, expenseInput, testGroup, testScenario, testUser } from './factories';
+import { callerFor } from './trpc';
+
+describe('assigning an ungrouped expense to a group', () => {
+  it('persists the group when an ungrouped expense is edited into a group', async () => {
+    const { owner, member, group } = await testScenario();
+    const caller = callerFor(owner.id);
+    const input = expenseInput(owner.id, member.id);
+    const [expense] = await caller.expense.addOrEditExpense(input);
+
+    await caller.expense.addOrEditExpense({ ...input, expenseId: expense!.id, groupId: group.id });
+
+    expect(await db.expense.findUnique({ where: { id: expense!.id } })).toMatchObject({
+      groupId: group.id,
+    });
+    expect(await caller.expense.getGroupExpenses({ groupId: group.id })).toEqual([
+      expect.objectContaining({ id: expense!.id }),
+    ]);
+    expect(
+      await db.balanceView.findMany({ where: { groupId: group.id }, orderBy: { userId: 'asc' } }),
+    ).toEqual([
+      expect.objectContaining({ userId: owner.id, friendId: member.id, amount: 1000n }),
+      expect.objectContaining({ userId: member.id, friendId: owner.id, amount: -1000n }),
+    ]);
+  });
+
+  it('assigns both linked records of an ungrouped currency conversion', async () => {
+    const { owner, member, group } = await testScenario();
+    const conversion = {
+      amount: 1000n,
+      rate: 0.9,
+      from: 'USD',
+      to: 'EUR',
+      senderId: owner.id,
+      receiverId: member.id,
+      groupId: null,
+    };
+    await callerFor(owner.id).expense.addOrEditCurrencyConversion(conversion);
+    const expense = await db.expense.findFirstOrThrow({ where: { conversionToId: { not: null } } });
+
+    await callerFor(owner.id).expense.addOrEditCurrencyConversion({
+      ...conversion,
+      expenseId: expense.id,
+      groupId: group.id,
+    });
+
+    expect(
+      (await db.expense.findMany({ select: { groupId: true } })).map(({ groupId }) => groupId),
+    ).toEqual([group.id, group.id]);
+  });
+
+  it('rejects a caller who neither added nor participates in the expense', async () => {
+    const { owner, member, outsider } = await testScenario();
+    const outsiderGroup = await testGroup(outsider.id, [owner.id, member.id]);
+    const input = expenseInput(owner.id, member.id);
+    const [expense] = await callerFor(owner.id).expense.addOrEditExpense(input);
+    const before = await accountingSnapshot();
+
+    await expect(
+      callerFor(outsider.id).expense.addOrEditExpense({
+        ...input,
+        expenseId: expense!.id,
+        groupId: outsiderGroup.id,
+      }),
+    ).rejects.toMatchObject({ code: 'UNAUTHORIZED' });
+    expect(await accountingSnapshot()).toEqual(before);
+  });
+
+  it.each([
+    {
+      label: 'the caller is not a member',
+      payer: 'member',
+      groupMembers: ['other', 'member'],
+      code: 'FORBIDDEN',
+      message: 'Not a group member',
+    },
+    {
+      label: 'the payer is not a member',
+      payer: 'member',
+      groupMembers: ['owner'],
+      code: 'BAD_REQUEST',
+      message: 'Payer is not a member of the group',
+    },
+    {
+      label: 'a participant is not a member',
+      payer: 'owner',
+      groupMembers: ['owner'],
+      code: 'BAD_REQUEST',
+      message: 'Participant is not a member of the group',
+    },
+  ] as const)(
+    'rejects the assignment when $label',
+    async ({ payer, groupMembers, code, message }) => {
+      const users = {
+        owner: await testUser('Owner'),
+        member: await testUser('Member'),
+        other: await testUser('Other'),
+      };
+      const otherParticipant = 'owner' === payer ? users.member : users.owner;
+      const input = expenseInput(users[payer].id, otherParticipant.id);
+      const [expense] = await callerFor(users.owner.id).expense.addOrEditExpense(input);
+      const [groupOwner, ...rest] = groupMembers.map((name) => users[name].id);
+      const group = await testGroup(groupOwner!, rest);
+      const before = await accountingSnapshot();
+
+      await expect(
+        callerFor(users.owner.id).expense.addOrEditExpense({
+          ...input,
+          expenseId: expense!.id,
+          groupId: group.id,
+        }),
+      ).rejects.toMatchObject({ code, message });
+      expect(await accountingSnapshot()).toEqual(before);
+    },
+  );
+
+  it('rejects assigning an expense to an archived group', async () => {
+    const { owner, member, group } = await testScenario();
+    const caller = callerFor(owner.id);
+    const input = expenseInput(owner.id, member.id);
+    const [expense] = await caller.expense.addOrEditExpense(input);
+    await db.group.update({ where: { id: group.id }, data: { archivedAt: new Date() } });
+    const before = await accountingSnapshot();
+
+    await expect(
+      caller.expense.addOrEditExpense({ ...input, expenseId: expense!.id, groupId: group.id }),
+    ).rejects.toMatchObject({ code: 'BAD_REQUEST', message: 'Group is archived' });
+    expect(await accountingSnapshot()).toEqual(before);
+  });
+});