From 6486ef3e1ef89feefc4e0b2be8bb74f8c71bb275 Mon Sep 17 00:00:00 2001 From: Domagoj Kriskovic Date: Fri, 20 Mar 2026 13:08:26 +0100 Subject: [PATCH] [web] Add deletedReason parameter to project deletion methods (#32221) * [web] Add deletedReason parameter to project deletion methods * revert sinon.match.any in ProjectDuplicator negative assertion Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> GitOrigin-RevId: d1595eefe0e36150231ee9646fe5eba0786fd1f5 --- .../web/app/src/Features/Editor/EditorController.mjs | 7 ++++++- .../app/src/Features/Project/DeletedProjectReasons.mjs | 8 ++++++++ .../web/app/src/Features/Project/ProjectController.mjs | 2 ++ services/web/app/src/Features/Project/ProjectDeleter.mjs | 8 +++++++- .../web/app/src/Features/Project/ProjectDuplicator.mjs | 5 ++++- .../app/src/Features/Uploads/ProjectUploadManager.mjs | 9 +++++++-- services/web/app/src/models/DeletedProject.mjs | 1 + services/web/scripts/soft_delete_project.mjs | 5 ++++- .../web/test/unit/src/Editor/EditorController.test.mjs | 6 ++++-- .../web/test/unit/src/Project/ProjectController.test.mjs | 1 + .../web/test/unit/src/Project/ProjectDeleter.test.mjs | 4 +++- .../web/test/unit/src/Project/ProjectDuplicator.test.mjs | 3 ++- .../test/unit/src/Uploads/ProjectUploadManager.test.mjs | 6 ++++-- 13 files changed, 53 insertions(+), 12 deletions(-) create mode 100644 services/web/app/src/Features/Project/DeletedProjectReasons.mjs diff --git a/services/web/app/src/Features/Editor/EditorController.mjs b/services/web/app/src/Features/Editor/EditorController.mjs index f1b1c32617..57ddc2cf59 100644 --- a/services/web/app/src/Features/Editor/EditorController.mjs +++ b/services/web/app/src/Features/Editor/EditorController.mjs @@ -5,6 +5,7 @@ import ProjectEntityUpdateHandler from '../Project/ProjectEntityUpdateHandler.mj import ProjectOptionsHandler from '../Project/ProjectOptionsHandler.mjs' import ProjectDetailsHandler from '../Project/ProjectDetailsHandler.mjs' import ProjectDeleter from '../Project/ProjectDeleter.mjs' +import { DeletedProjectReasons } from '../Project/DeletedProjectReasons.mjs' import EditorRealTimeController from './EditorRealTimeController.mjs' import async from 'async' import PublicAccessLevels from '../Authorization/PublicAccessLevels.mjs' @@ -445,7 +446,11 @@ const EditorController = { deleteProject(projectId, callback) { Metrics.inc('editor.delete-project') - ProjectDeleter.deleteProject(projectId, callback) + ProjectDeleter.deleteProject( + projectId, + { deletedReason: DeletedProjectReasons.USER }, + callback + ) }, renameEntity( diff --git a/services/web/app/src/Features/Project/DeletedProjectReasons.mjs b/services/web/app/src/Features/Project/DeletedProjectReasons.mjs new file mode 100644 index 0000000000..9eb48a76cf --- /dev/null +++ b/services/web/app/src/Features/Project/DeletedProjectReasons.mjs @@ -0,0 +1,8 @@ +export const DeletedProjectReasons = /** @type {const} */ ({ + USER: 'user', + ACCOUNT_DELETION: 'account-deletion', + ZIP_IMPORT_FAILURE: 'zip-import-failure', + CLONE_FAILURE: 'clone-failure', + GITHUB_IMPORT_FAILURE: 'github-import-failure', + SCRIPT: 'script', +}) diff --git a/services/web/app/src/Features/Project/ProjectController.mjs b/services/web/app/src/Features/Project/ProjectController.mjs index 79d0ef7cc6..204c00a3a4 100644 --- a/services/web/app/src/Features/Project/ProjectController.mjs +++ b/services/web/app/src/Features/Project/ProjectController.mjs @@ -7,6 +7,7 @@ import logger from '@overleaf/logger' import { expressify } from '@overleaf/promise-utils' import mongodb from 'mongodb-legacy' import ProjectDeleter from './ProjectDeleter.mjs' +import { DeletedProjectReasons } from './DeletedProjectReasons.mjs' import ProjectDuplicator from './ProjectDuplicator.mjs' import ProjectCreationHandler from './ProjectCreationHandler.mjs' import EditorController from '../Editor/EditorController.mjs' @@ -171,6 +172,7 @@ const _ProjectController = { await ProjectDeleter.promises.deleteProject(projectId, { deleterUser: user, ipAddress: req.ip, + deletedReason: DeletedProjectReasons.USER, }) ProjectAuditLogHandler.addEntryIfManagedInBackground( projectId, diff --git a/services/web/app/src/Features/Project/ProjectDeleter.mjs b/services/web/app/src/Features/Project/ProjectDeleter.mjs index cf8f035516..0183b3d752 100644 --- a/services/web/app/src/Features/Project/ProjectDeleter.mjs +++ b/services/web/app/src/Features/Project/ProjectDeleter.mjs @@ -22,6 +22,7 @@ import EditorRealTimeController from '../Editor/EditorRealTimeController.mjs' import HistoryManager from '../History/HistoryManager.mjs' import ChatApiHandler from '../Chat/ChatApiHandler.mjs' import { promiseMapWithLimit } from '@overleaf/promise-utils' +import { DeletedProjectReasons } from './DeletedProjectReasons.mjs' const PROJECT_EXPIRATION_BATCH_SIZE = 10000 @@ -84,7 +85,11 @@ async function deleteUsersProjects(userId) { { userId, projectCount: projects.length }, 'found user projects to delete' ) - await promiseMapWithLimit(5, projects, project => deleteProject(project._id)) + await promiseMapWithLimit(5, projects, project => + deleteProject(project._id, { + deletedReason: DeletedProjectReasons.ACCOUNT_DELETION, + }) + ) logger.info({ userId }, 'deleted all user projects') await CollaboratorsHandler.promises.removeUserFromAllProjects(userId) } @@ -214,6 +219,7 @@ async function deleteProject(projectId, options = {}) { deleterId: options.deleterUser != null ? options.deleterUser._id : undefined, deleterIpAddress: options.ipAddress, + deletedReason: options.deletedReason, deletedProjectId: project._id, deletedProjectOwnerId: project.owner_ref, deletedProjectCollaboratorIds: project.collaberator_refs, diff --git a/services/web/app/src/Features/Project/ProjectDuplicator.mjs b/services/web/app/src/Features/Project/ProjectDuplicator.mjs index fdada472b0..b496bda293 100644 --- a/services/web/app/src/Features/Project/ProjectDuplicator.mjs +++ b/services/web/app/src/Features/Project/ProjectDuplicator.mjs @@ -10,6 +10,7 @@ import DocumentUpdaterHandler from '../DocumentUpdater/DocumentUpdaterHandler.mj import HistoryManager from '../History/HistoryManager.mjs' import ProjectCreationHandler from './ProjectCreationHandler.mjs' import ProjectDeleter from './ProjectDeleter.mjs' +import { DeletedProjectReasons } from './DeletedProjectReasons.mjs' import ProjectEntityMongoUpdateHandler from './ProjectEntityMongoUpdateHandler.mjs' import ProjectEntityUpdateHandler from './ProjectEntityUpdateHandler.mjs' import ProjectGetter from './ProjectGetter.mjs' @@ -161,7 +162,9 @@ async function duplicate( } catch (err) { // Clean up broken clone on error. // Make sure we delete the new failed project, not the original one! - await ProjectDeleter.promises.deleteProject(newProject._id) + await ProjectDeleter.promises.deleteProject(newProject._id, { + deletedReason: DeletedProjectReasons.CLONE_FAILURE, + }) throw OError.tag(err, 'error cloning project, broken clone deleted', { originalProjectId, newProjectName, diff --git a/services/web/app/src/Features/Uploads/ProjectUploadManager.mjs b/services/web/app/src/Features/Uploads/ProjectUploadManager.mjs index c91c67c142..2d64ca3fd3 100644 --- a/services/web/app/src/Features/Uploads/ProjectUploadManager.mjs +++ b/services/web/app/src/Features/Uploads/ProjectUploadManager.mjs @@ -13,6 +13,7 @@ import ProjectEntityMongoUpdateHandler from '../Project/ProjectEntityMongoUpdate import ProjectRootDocManager from '../Project/ProjectRootDocManager.mjs' import ProjectDetailsHandler from '../Project/ProjectDetailsHandler.mjs' import ProjectDeleter from '../Project/ProjectDeleter.mjs' +import { DeletedProjectReasons } from '../Project/DeletedProjectReasons.mjs' import TpdsProjectFlusher from '../ThirdPartyDataStore/TpdsProjectFlusher.mjs' import logger from '@overleaf/logger' import OError from '@overleaf/o-error' @@ -55,7 +56,9 @@ async function createProjectFromZipArchive(ownerId, defaultName, zipPath) { } catch (err) { // no need to wait for the cleanup here ProjectDeleter.promises - .deleteProject(project._id) + .deleteProject(project._id, { + deletedReason: DeletedProjectReasons.ZIP_IMPORT_FAILURE, + }) .catch(err => logger.error( { err, projectId: project._id }, @@ -97,7 +100,9 @@ async function createProjectFromZipArchiveWithName( } catch (err) { // no need to wait for the cleanup here ProjectDeleter.promises - .deleteProject(project._id) + .deleteProject(project._id, { + deletedReason: DeletedProjectReasons.ZIP_IMPORT_FAILURE, + }) .catch(err => logger.error( { err, projectId: project._id }, diff --git a/services/web/app/src/models/DeletedProject.mjs b/services/web/app/src/models/DeletedProject.mjs index 9cfeca0ab2..5d02773e57 100644 --- a/services/web/app/src/models/DeletedProject.mjs +++ b/services/web/app/src/models/DeletedProject.mjs @@ -20,6 +20,7 @@ export const DeleterDataSchema = new Schema({ deletedProjectLastUpdatedAt: { type: Date }, deletedProjectOverleafId: { type: Number }, deletedProjectOverleafHistoryId: { type: Schema.Types.Mixed }, + deletedReason: { type: String }, }) const DeletedProjectSchema = new Schema( diff --git a/services/web/scripts/soft_delete_project.mjs b/services/web/scripts/soft_delete_project.mjs index 517eb7c655..299be49a51 100644 --- a/services/web/scripts/soft_delete_project.mjs +++ b/services/web/scripts/soft_delete_project.mjs @@ -1,5 +1,6 @@ import minimist from 'minimist' import ProjectDeleter from '../app/src/Features/Project/ProjectDeleter.mjs' +import { DeletedProjectReasons } from '../app/src/Features/Project/DeletedProjectReasons.mjs' import { scriptRunner } from './lib/ScriptRunner.mjs' async function main() { @@ -11,7 +12,9 @@ async function main() { } console.log(`Soft deleting project ${projectId}`) // soft delete, project will be permanently deleted after 90 days - await ProjectDeleter.promises.deleteProject(projectId) + await ProjectDeleter.promises.deleteProject(projectId, { + deletedReason: DeletedProjectReasons.SCRIPT, + }) } try { diff --git a/services/web/test/unit/src/Editor/EditorController.test.mjs b/services/web/test/unit/src/Editor/EditorController.test.mjs index 383c462ca2..6528726792 100644 --- a/services/web/test/unit/src/Editor/EditorController.test.mjs +++ b/services/web/test/unit/src/Editor/EditorController.test.mjs @@ -684,7 +684,7 @@ describe('EditorController', function () { describe('deleteProject', function () { beforeEach(function (ctx) { ctx.err = 'errro' - ctx.ProjectDeleter.deleteProject.callsArgWith(1, ctx.err) + ctx.ProjectDeleter.deleteProject.callsArgWith(2, ctx.err) }) it('should call the project handler', async function (ctx) { @@ -692,7 +692,9 @@ describe('EditorController', function () { ctx.EditorController.deleteProject(ctx.project_id, err => { err.should.equal(ctx.err) ctx.ProjectDeleter.deleteProject - .calledWith(ctx.project_id) + .calledWith(ctx.project_id, { + deletedReason: 'user', + }) .should.equal(true) resolve() }) diff --git a/services/web/test/unit/src/Project/ProjectController.test.mjs b/services/web/test/unit/src/Project/ProjectController.test.mjs index d849dfd390..ca6f2f84df 100644 --- a/services/web/test/unit/src/Project/ProjectController.test.mjs +++ b/services/web/test/unit/src/Project/ProjectController.test.mjs @@ -710,6 +710,7 @@ describe('ProjectController', function () { .calledWith(ctx.project_id, { deleterUser: ctx.user, ipAddress: ctx.req.ip, + deletedReason: 'user', }) .should.equal(true) code.should.equal(200) diff --git a/services/web/test/unit/src/Project/ProjectDeleter.test.mjs b/services/web/test/unit/src/Project/ProjectDeleter.test.mjs index 0b58751221..2cc3863586 100644 --- a/services/web/test/unit/src/Project/ProjectDeleter.test.mjs +++ b/services/web/test/unit/src/Project/ProjectDeleter.test.mjs @@ -288,7 +288,7 @@ describe('ProjectDeleter', function () { { 'deleterData.deletedProjectId': project._id }, { project, - deleterData: sinon.match.object, + deleterData: sinon.match.has('deletedReason', 'account-deletion'), }, { upsert: true } ) @@ -343,6 +343,7 @@ describe('ProjectDeleter', function () { it('should save a DeletedProject with additional deleterData', async function (ctx) { ctx.deleterData.deleterIpAddress = ctx.ip ctx.deleterData.deleterId = ctx.user._id + ctx.deleterData.deletedReason = 'user' ctx.ProjectMock.expects('deleteOne').chain('exec').resolves() ctx.DeletedProjectMock.expects('updateOne') @@ -359,6 +360,7 @@ describe('ProjectDeleter', function () { await ctx.ProjectDeleter.promises.deleteProject(ctx.project._id, { deleterUser: ctx.user, ipAddress: ctx.ip, + deletedReason: 'user', }) ctx.DeletedProjectMock.verify() }) diff --git a/services/web/test/unit/src/Project/ProjectDuplicator.test.mjs b/services/web/test/unit/src/Project/ProjectDuplicator.test.mjs index 1184706903..d8f3a66758 100644 --- a/services/web/test/unit/src/Project/ProjectDuplicator.test.mjs +++ b/services/web/test/unit/src/Project/ProjectDuplicator.test.mjs @@ -459,7 +459,8 @@ describe('ProjectDuplicator', function () { it('should delete the broken cloned project', function (ctx) { ctx.ProjectDeleter.promises.deleteProject.should.have.been.calledWith( - ctx.newBlankProject._id + ctx.newBlankProject._id, + { deletedReason: 'clone-failure' } ) }) diff --git a/services/web/test/unit/src/Uploads/ProjectUploadManager.test.mjs b/services/web/test/unit/src/Uploads/ProjectUploadManager.test.mjs index 696b629405..2938184da3 100644 --- a/services/web/test/unit/src/Uploads/ProjectUploadManager.test.mjs +++ b/services/web/test/unit/src/Uploads/ProjectUploadManager.test.mjs @@ -445,7 +445,8 @@ describe('ProjectUploadManager', function () { it('should cleanup the blank project created', async function (ctx) { ctx.ProjectDeleter.promises.deleteProject.should.have.been.calledWith( - ctx.project._id + ctx.project._id, + { deletedReason: 'zip-import-failure' } ) }) @@ -471,7 +472,8 @@ describe('ProjectUploadManager', function () { it('should cleanup the blank project created', function (ctx) { ctx.ProjectDeleter.promises.deleteProject.should.have.been.calledWith( - ctx.project._id + ctx.project._id, + { deletedReason: 'zip-import-failure' } ) })