Skip to content

Commit 9e204be

Browse files
znarfclaude
andcommitted
fix(moveExpenses): clear accounting category when moving to different host
When expenses are moved to an account under a different host, the AccountingCategoryId is now cleared since accounting categories belong to specific hosts and would be invalid after the move. The migration log now also captures the previous AccountingCategoryId for audit purposes. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 878a16a commit 9e204be

File tree

2 files changed

+65
-5
lines changed

2 files changed

+65
-5
lines changed

server/graphql/common/expenses.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4152,6 +4152,12 @@ export const moveExpenses = async (req: express.Request, expenses: Expense[], de
41524152
// -- Move expenses --
41534153
const expenseIds: number[] = uniq(expenses.map(expense => expense.id));
41544154
const recurringExpenseIds: number[] = uniq(expenses.map(expense => expense.RecurringExpenseId).filter(Boolean));
4155+
4156+
// Check if moving to a different host - if so, we need to clear accounting categories
4157+
const isMovingToDifferentHost = expenses.some(
4158+
e => e.collective.HostCollectiveId !== destinationAccount.HostCollectiveId,
4159+
);
4160+
41554161
const result = await sequelize.transaction(async dbTransaction => {
41564162
const associatedTransactionsCount = await models.Transaction.count({
41574163
where: { ExpenseId: expenseIds },
@@ -4162,9 +4168,12 @@ export const moveExpenses = async (req: express.Request, expenses: Expense[], de
41624168
throw new ValidationFailed('Cannot move expenses with associated transactions');
41634169
}
41644170

4165-
// Moving associated models
4171+
// Moving expenses - clear AccountingCategoryId if moving to different host (category belongs to old host)
41664172
const [, updatedExpenses] = await models.Expense.update(
4167-
{ CollectiveId: destinationAccount.id },
4173+
{
4174+
CollectiveId: destinationAccount.id,
4175+
...(isMovingToDifferentHost && { AccountingCategoryId: null }),
4176+
},
41684177
{
41694178
transaction: dbTransaction,
41704179
returning: true,
@@ -4231,7 +4240,7 @@ export const moveExpenses = async (req: express.Request, expenses: Expense[], de
42314240
},
42324241
);
42334242

4234-
// Record the migration log
4243+
// Record the migration log (previousExpenseValues includes AccountingCategoryId for audit)
42354244
await models.MigrationLog.create(
42364245
{
42374246
type: MigrationLogType.MOVE_EXPENSES,
@@ -4243,7 +4252,9 @@ export const moveExpenses = async (req: express.Request, expenses: Expense[], de
42434252
comments: updatedComments.map(c => c.id),
42444253
activities: updatedActivities.map(a => a.id),
42454254
destinationAccount: destinationAccount.id,
4246-
previousExpenseValues: mapValues(keyBy(expenses, 'id'), expense => pick(expense, ['CollectiveId'])),
4255+
previousExpenseValues: mapValues(keyBy(expenses, 'id'), expense =>
4256+
pick(expense, ['CollectiveId', 'AccountingCategoryId']),
4257+
),
42474258
},
42484259
},
42494260
{ transaction: dbTransaction },

test/server/graphql/v2/mutation/RootMutations.test.ts

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,12 @@ import roles from '../../../../../server/constants/roles';
77
import { TwoFactorAuthenticationHeader } from '../../../../../server/lib/two-factor-authentication/lib';
88
import models from '../../../../../server/models';
99
import {
10+
fakeAccountingCategory,
1011
fakeActivity,
1112
fakeCollective,
1213
fakeComment,
1314
fakeExpense,
15+
fakeHost,
1416
fakeMember,
1517
fakeRecurringExpense,
1618
fakeTransaction,
@@ -171,7 +173,9 @@ describe('server/graphql/v2/mutation/RootMutations', () => {
171173
expenses: [testExpense.id],
172174
recurringExpenses: [],
173175
destinationAccount: testCollective.id,
174-
previousExpenseValues: { [testExpense.id]: { CollectiveId: previousCollective.id } },
176+
previousExpenseValues: {
177+
[testExpense.id]: { CollectiveId: previousCollective.id, AccountingCategoryId: null },
178+
},
175179
activities: [],
176180
comments: [],
177181
});
@@ -263,6 +267,51 @@ describe('server/graphql/v2/mutation/RootMutations', () => {
263267
await recurringExpense.reload();
264268
expect(recurringExpense.CollectiveId).to.eq(collective.id);
265269
});
270+
271+
it('clears accounting category when moving expense to a different host', async () => {
272+
const sourceHost = await fakeHost();
273+
const sourceCollective = await fakeCollective({ HostCollectiveId: sourceHost.id });
274+
const accountingCategory = await fakeAccountingCategory({ CollectiveId: sourceHost.id, kind: 'EXPENSE' });
275+
const destinationHost = await fakeHost();
276+
const destinationCollective = await fakeCollective({ HostCollectiveId: destinationHost.id });
277+
278+
const expense = await fakeExpense({
279+
CollectiveId: sourceCollective.id,
280+
AccountingCategoryId: accountingCategory.id,
281+
});
282+
283+
const result = await callMoveExpenseMutation(
284+
{ destinationAccount: { legacyId: destinationCollective.id }, expenses: [{ legacyId: expense.id }] },
285+
rootUser,
286+
);
287+
288+
result.errors && console.error(result.errors);
289+
expect(result.errors).to.not.exist;
290+
await expense.reload();
291+
expect(expense.AccountingCategoryId).to.be.null;
292+
});
293+
294+
it('preserves accounting category when moving expense within the same host', async () => {
295+
const host = await fakeHost();
296+
const sourceCollective = await fakeCollective({ HostCollectiveId: host.id });
297+
const destinationCollective = await fakeCollective({ HostCollectiveId: host.id });
298+
const accountingCategory = await fakeAccountingCategory({ CollectiveId: host.id, kind: 'EXPENSE' });
299+
300+
const expense = await fakeExpense({
301+
CollectiveId: sourceCollective.id,
302+
AccountingCategoryId: accountingCategory.id,
303+
});
304+
305+
const result = await callMoveExpenseMutation(
306+
{ destinationAccount: { legacyId: destinationCollective.id }, expenses: [{ legacyId: expense.id }] },
307+
rootUser,
308+
);
309+
310+
result.errors && console.error(result.errors);
311+
expect(result.errors).to.not.exist;
312+
await expense.reload();
313+
expect(expense.AccountingCategoryId).to.equal(accountingCategory.id);
314+
});
266315
});
267316

268317
describe('editAccountType', () => {

0 commit comments

Comments
 (0)