diff --git a/migrations/20250110102114-fix-deleted-expense-relationships.ts b/migrations/20250110102114-fix-deleted-expense-relationships.ts new file mode 100644 index 00000000000..9ec00c71dc2 --- /dev/null +++ b/migrations/20250110102114-fix-deleted-expense-relationships.ts @@ -0,0 +1,54 @@ +'use strict'; + +import logger from '../server/lib/logger'; + +/** @type {import('sequelize-cli').Migration} */ +module.exports = { + async up(queryInterface) { + const [deletedComments] = await queryInterface.sequelize.query(` + UPDATE "Comments" + SET "deletedAt" = e."deletedAt" + FROM "Expenses" e + WHERE "Comments"."ExpenseId" IS NOT NULL + AND "Comments"."ExpenseId" = e."id" + AND e."deletedAt" IS NOT NULL + AND "Comments"."deletedAt" IS NULL + RETURNING e."id" + `); + + const [deletedItems] = await queryInterface.sequelize.query(` + UPDATE "ExpenseItems" + SET "deletedAt" = e."deletedAt" + FROM "Expenses" e + WHERE "ExpenseItems"."ExpenseId" = e."id" + AND e."deletedAt" IS NOT NULL + AND "ExpenseItems"."deletedAt" IS NULL + RETURNING e."id" + `); + + logger.info(`Expenses: Fixed ${deletedComments.length} deleted comments, ${deletedItems.length} deleted items`); + + if (deletedComments.length === 0 || deletedItems.length === 0) { + await queryInterface.sequelize.query( + ` + INSERT INTO "MigrationLogs" + ("createdAt", "type", "description", "CreatedByUserId", "data") + VALUES (NOW(), 'MIGRATION', 'Fix deleted expense relationships', NULL, :data) + `, + { + type: queryInterface.sequelize.QueryTypes.INSERT, + replacements: { + data: JSON.stringify({ + deletedComments, + deletedItems, + }), + }, + }, + ); + } + }, + + async down() { + console.log(`No rollback, check the MigrationLogs table for the data`); + }, +}; diff --git a/server/graphql/v2/mutation/ExpenseMutations.ts b/server/graphql/v2/mutation/ExpenseMutations.ts index 922c135f92d..9908e8d7950 100644 --- a/server/graphql/v2/mutation/ExpenseMutations.ts +++ b/server/graphql/v2/mutation/ExpenseMutations.ts @@ -277,11 +277,6 @@ const expenseMutations = { await twoFactorAuthLib.enforceForAccountsUserIsAdminOf(req, accountsFor2FA); // Cancel recurring expense - const recurringExpense = await expense.getRecurringExpense(); - if (recurringExpense) { - await recurringExpense.destroy(); - } - await expense.destroy(); return expense; }, diff --git a/server/models/Expense.ts b/server/models/Expense.ts index 5dc6e36cfbb..7a50d61a9fe 100644 --- a/server/models/Expense.ts +++ b/server/models/Expense.ts @@ -967,7 +967,17 @@ Expense.init( tableName: 'Expenses', hooks: { async afterDestroy(expense: Expense) { - await ExpenseItem.destroy({ where: { ExpenseId: expense.id } }); + // Not considering ExpensesAttachedFiles because they don't support soft delete (they should) + const promises = [ + ExpenseItem.destroy({ where: { ExpenseId: expense.id } }), + models.Comment.destroy({ where: { ExpenseId: expense.id } }), + ]; + + if (expense.RecurringExpenseId) { + promises.push(RecurringExpense.destroy({ where: { id: expense.RecurringExpenseId } })); + } + + await Promise.all(promises); }, }, }, diff --git a/server/models/Update.ts b/server/models/Update.ts index cc4e326d23e..1985356b864 100644 --- a/server/models/Update.ts +++ b/server/models/Update.ts @@ -162,10 +162,15 @@ class Update extends Model, InferCreationAttributes { + await Comment.destroy({ where: { UpdateId: this.id }, transaction }); + await Update.update( + { deletedAt: new Date(), LastEditedByUserId: remoteUser.id }, + { where: { id: this.id }, transaction }, + ); + + return this; + }); }; // Returns the User model of the User that created this Update diff --git a/test/server/graphql/v2/mutation/ExpenseMutations-legacy.test.js b/test/server/graphql/v2/mutation/ExpenseMutations-legacy.test.js index 577bc1f3de5..23d4584806e 100644 --- a/test/server/graphql/v2/mutation/ExpenseMutations-legacy.test.js +++ b/test/server/graphql/v2/mutation/ExpenseMutations-legacy.test.js @@ -27,6 +27,7 @@ import { randEmail, randUrl } from '../../../../stores'; import { fakeAccountingCategory, fakeCollective, + fakeComment, fakeConnectedAccount, fakeExpense, fakeExpenseItem, @@ -1177,11 +1178,16 @@ describe('server/graphql/v2/mutation/ExpenseMutations', () => { describe('can delete rejected expenses', () => { it('if owner', async () => { const expense = await fakeExpense({ status: expenseStatus.REJECTED }); + const item = await fakeExpenseItem({ ExpenseId: expense.id }); + const comment = await fakeComment({ ExpenseId: expense.id }); const result = await graphqlQueryV2(deleteExpenseMutation, prepareGQLParams(expense), expense.User); expect(result.data.deleteExpense.legacyId).to.eq(expense.id); - await expense.reload({ paranoid: false }); - expect(expense.deletedAt).to.exist; + + for (const model of [expense, item, comment]) { + const deletedModel = await model.reload({ paranoid: false }); + expect(deletedModel.deletedAt).to.exist; + } }); it('if collective admin', async () => { diff --git a/test/server/models/Update.test.js b/test/server/models/Update.test.js index c47a6beb39b..aa0ba3f3b51 100644 --- a/test/server/models/Update.test.js +++ b/test/server/models/Update.test.js @@ -3,7 +3,14 @@ import { flatten, reduce, times } from 'lodash'; import models, { Op } from '../../../server/models'; import { randEmail } from '../../stores'; -import { fakeCollective, fakeMember, fakeOrganization, fakeUpdate, fakeUser } from '../../test-helpers/fake-data'; +import { + fakeCollective, + fakeComment, + fakeMember, + fakeOrganization, + fakeUpdate, + fakeUser, +} from '../../test-helpers/fake-data'; import * as utils from '../../utils'; const addRandomMemberUsers = (collective, count, role) => { @@ -120,6 +127,21 @@ describe('server/models/Update', () => { expect(uniqueSlug).to.not.be.equal(update.slug); expect(/-\d+$/.test(update.slug)).to.be.true; }); + + it('deletes related comments and record the user who deleted it', async () => { + const user = await fakeUser(); + const update = await fakeUpdate({ id: 4242 }); + const comment = await fakeComment({ UpdateId: update.id }); + + await update.delete(user); + await update.reload({ paranoid: false }); + + expect(update.LastEditedByUserId).to.equal(user.id); + expect(update.deletedAt).to.not.be.null; + + const comments = await models.Comment.findAll({ where: { UpdateId: update.id } }); + expect(comments.length).to.equal(0); + }); }); describe('Update audience', () => {