Просмотр исходного кода

Merge pull request #26175 from overleaf/bg-fix-logging-in-project-deletion

test logging in user and project deletion

GitOrigin-RevId: ea51082aeada81f8e7ae356966cda0f57f7cd072
Brian Gough 1 год назад
Родитель
Сommit
f0c63b6ccd

+ 1 - 1
services/web/app/src/Features/Project/ProjectDeleter.js

@@ -385,7 +385,7 @@ async function expireDeletedProject(projectId) {
       )
       )
       return
       return
     }
     }
-    const userId = deletedProject.deletedProjectOwnerId
+    const userId = deletedProject.deleterData?.deletedProjectOwnerId?.toString()
     const historyId =
     const historyId =
       deletedProject.project.overleaf &&
       deletedProject.project.overleaf &&
       deletedProject.project.overleaf.history &&
       deletedProject.project.overleaf.history &&

+ 36 - 0
services/web/test/acceptance/src/DeletionTests.mjs

@@ -1,3 +1,5 @@
+import logger from '@overleaf/logger'
+import sinon from 'sinon'
 import User from './helpers/User.mjs'
 import User from './helpers/User.mjs'
 import Subscription from './helpers/Subscription.mjs'
 import Subscription from './helpers/Subscription.mjs'
 import request from './helpers/request.js'
 import request from './helpers/request.js'
@@ -18,6 +20,8 @@ let MockDocstoreApi,
   MockGitBridgeApi,
   MockGitBridgeApi,
   MockHistoryBackupDeletionApi
   MockHistoryBackupDeletionApi
 
 
+let spy
+
 before(function () {
 before(function () {
   MockDocstoreApi = MockDocstoreApiClass.instance()
   MockDocstoreApi = MockDocstoreApiClass.instance()
   MockFilestoreApi = MockFilestoreApiClass.instance()
   MockFilestoreApi = MockFilestoreApiClass.instance()
@@ -28,6 +32,7 @@ before(function () {
 
 
 describe('Deleting a user', function () {
 describe('Deleting a user', function () {
   beforeEach(function (done) {
   beforeEach(function (done) {
+    spy = sinon.spy(logger, 'info')
     async.auto(
     async.auto(
       {
       {
         user: cb => {
         user: cb => {
@@ -64,6 +69,10 @@ describe('Deleting a user', function () {
     )
     )
   })
   })
 
 
+  afterEach(function () {
+    spy.restore()
+  })
+
   it('Should remove the user from active users', function (done) {
   it('Should remove the user from active users', function (done) {
     this.user.get((error, user) => {
     this.user.get((error, user) => {
       expect(error).not.to.exist
       expect(error).not.to.exist
@@ -183,6 +192,7 @@ describe('Deleting a user', function () {
 
 
 describe('Deleting a project', function () {
 describe('Deleting a project', function () {
   beforeEach(function (done) {
   beforeEach(function (done) {
+    spy = sinon.spy(logger, 'info')
     this.user = new User()
     this.user = new User()
     this.projectName = 'wombat'
     this.projectName = 'wombat'
     this.user.ensureUserExists(() => {
     this.user.ensureUserExists(() => {
@@ -195,6 +205,10 @@ describe('Deleting a project', function () {
     })
     })
   })
   })
 
 
+  afterEach(function () {
+    logger.info.restore()
+  })
+
   it('Should remove the project from active projects', function (done) {
   it('Should remove the project from active projects', function (done) {
     this.user.getProject(this.projectId, (error, project) => {
     this.user.getProject(this.projectId, (error, project) => {
       expect(error).not.to.exist
       expect(error).not.to.exist
@@ -292,6 +306,28 @@ describe('Deleting a project', function () {
         })
         })
       })
       })
 
 
+      it('Should log a successful deletion', function (done) {
+        request.post(
+          `/internal/project/${this.projectId}/expire-deleted-project`,
+          {
+            auth: {
+              user: settings.apis.web.user,
+              pass: settings.apis.web.pass,
+              sendImmediately: true,
+            },
+          },
+          (error, res) => {
+            expect(error).not.to.exist
+            expect(res.statusCode).to.equal(200)
+            expect(spy).to.have.been.calledWithMatch(
+              { projectId: this.projectId, userId: this.user._id },
+              'expired deleted project successfully'
+            )
+            done()
+          }
+        )
+      })
+
       it('Should destroy the docs', function (done) {
       it('Should destroy the docs', function (done) {
         expect(
         expect(
           MockDocstoreApi.docs[this.projectId.toString()][this.docId.toString()]
           MockDocstoreApi.docs[this.projectId.toString()][this.docId.toString()]

+ 11 - 0
services/web/test/unit/src/Project/ProjectDeleterTests.js

@@ -36,6 +36,7 @@ describe('ProjectDeleter', function () {
           deleterId: '588f3ddae8ebc1bac07c9fa4',
           deleterId: '588f3ddae8ebc1bac07c9fa4',
           deleterIpAddress: '172.19.0.1',
           deleterIpAddress: '172.19.0.1',
           deletedProjectId: '5cf9270b4eff6e186cf8b05e',
           deletedProjectId: '5cf9270b4eff6e186cf8b05e',
+          deletedProjectOwnerId: this.user._id,
         },
         },
         project: {
         project: {
           _id: '5cf9270b4eff6e186cf8b05e',
           _id: '5cf9270b4eff6e186cf8b05e',
@@ -501,6 +502,16 @@ describe('ProjectDeleter', function () {
           projectId: this.deletedProjects[0].project._id,
           projectId: this.deletedProjects[0].project._id,
         })
         })
       })
       })
+
+      it('should log a completed deletion', async function () {
+        expect(this.logger.info).to.have.been.calledWith(
+          {
+            projectId: this.deletedProjects[0].project._id,
+            userId: this.user._id,
+          },
+          'expired deleted project successfully'
+        )
+      })
     })
     })
 
 
     describe('on an active project (from an incomplete delete)', function () {
     describe('on an active project (from an incomplete delete)', function () {