Merge pull request #2646 from overleaf/as-fix-trashing-old-archived-project

Fix error thrown when trashing project with legacy boolean archived state

GitOrigin-RevId: 0131a2294767e00d69c4dd80ed86dfd5d77339b8

services/web/app/src/Features/Project/ProjectDeleter.js

@@ -144,11 +144,17 @@ async function trashProject(projectId, userId) {
       throw new Errors.NotFoundError('project not found')
     }
 
+    const archived = ProjectHelper.calculateArchivedArray(
+      project,
+      userId,
+      'UNARCHIVE'
+    )
+
     await Project.update(
       { _id: projectId },
       {
         $addToSet: { trashed: ObjectId(userId) },
-        $pull: { archived: ObjectId(userId) }
+        $set: { archived: archived }
       }
     )
   } catch (err) {

services/web/test/acceptance/src/ProjectCRUDTests.js

@@ -1,6 +1,7 @@
 const { expect } = require('chai')
 const User = require('./helpers/User').promises
 const { Project } = require('../../../app/src/models/Project')
+const { ObjectId } = require('../../../app/src/infrastructure/mongojs')
 
 describe('Project CRUD', function() {
   beforeEach(async function() {
@@ -49,14 +50,52 @@ describe('Project CRUD', function() {
       const trashedProject = await Project.findById(this.projectId).exec()
       expectObjectIdArrayEqual(trashedProject.trashed, [this.user._id])
     })
+
+    describe('with an array archived state', function() {
+      it('should mark the project as not archived for the user', async function() {
+        await Project.update(
+          { _id: this.projectId },
+          { $set: { archived: [ObjectId(this.user._id)] } }
+        ).exec()
+
+        const { response } = await this.user.doRequest(
+          'POST',
+          `/project/${this.projectId}/trash`
+        )
+
+        expect(response.statusCode).to.equal(200)
+
+        const trashedProject = await Project.findById(this.projectId).exec()
+        expectObjectIdArrayEqual(trashedProject.archived, [])
+      })
+    })
+
+    describe('with a legacy boolean state', function() {
+      it('should mark the project as not archived for the user', async function() {
+        await Project.update(
+          { _id: this.projectId },
+          { $set: { archived: true } }
+        ).exec()
+
+        const { response } = await this.user.doRequest(
+          'POST',
+          `/project/${this.projectId}/trash`
+        )
+
+        expect(response.statusCode).to.equal(200)
+
+        const trashedProject = await Project.findById(this.projectId).exec()
+        expectObjectIdArrayEqual(trashedProject.archived, [])
+      })
+    })
   })
 
   describe('when untrashing a project', function() {
     it('should mark the project as untrashed for the user', async function() {
       await Project.update(
         { _id: this.projectId },
-        { trashed: [this.user._id] }
+        { trashed: [ObjectId(this.user._id)] }
-      )
+      ).exec()
       const { response } = await this.user.doRequest(
         'DELETE',
         `/project/${this.projectId}/trash`
@@ -70,8 +109,8 @@ describe('Project CRUD', function() {
     it('does nothing if the user has already untrashed the project', async function() {
       await Project.update(
         { _id: this.projectId },
-        { trashed: [this.user._id] }
+        { trashed: [ObjectId(this.user._id)] }
-      )
+      ).exec()
       // Mark as untrashed the first time
       await this.user.doRequest('DELETE', `/project/${this.projectId}/trash`)
 

services/web/test/unit/src/Project/ProjectDeleterTests.js

@@ -453,6 +453,18 @@ describe('ProjectDeleter', function() {
       )
       this.ProjectMock.verify()
     })
+
+    it('calculates the archived array', async function() {
+      await this.ProjectDeleter.promises.archiveProject(
+        this.project._id,
+        this.user._id
+      )
+      expect(this.ProjectHelper.calculateArchivedArray).to.have.been.calledWith(
+        this.project,
+        this.user._id,
+        'ARCHIVE'
+      )
+    })
   })
 
   describe('unarchiveProject', function() {
@@ -477,10 +489,25 @@ describe('ProjectDeleter', function() {
       )
       this.ProjectMock.verify()
     })
+
+    it('calculates the archived array', async function() {
+      await this.ProjectDeleter.promises.unarchiveProject(
+        this.project._id,
+        this.user._id
+      )
+      expect(this.ProjectHelper.calculateArchivedArray).to.have.been.calledWith(
+        this.project,
+        this.user._id,
+        'UNARCHIVE'
+      )
+    })
   })
 
   describe('trashProject', function() {
     beforeEach(function() {
+      let archived = [ObjectId(this.user._id)]
+      this.ProjectHelper.calculateArchivedArray.returns(archived)
+
       this.ProjectMock.expects('findOne')
         .withArgs({ _id: this.project._id })
         .chain('exec')
@@ -491,7 +518,7 @@ describe('ProjectDeleter', function() {
           { _id: this.project._id },
           {
             $addToSet: { trashed: ObjectId(this.user._id) },
-            $pull: { archived: ObjectId(this.user._id) }
+            $set: { archived: archived }
           }
         )
         .resolves()
@@ -504,6 +531,18 @@ describe('ProjectDeleter', function() {
       )
       this.ProjectMock.verify()
     })
+
+    it('unarchives the project', async function() {
+      await this.ProjectDeleter.promises.trashProject(
+        this.project._id,
+        this.user._id
+      )
+      expect(this.ProjectHelper.calculateArchivedArray).to.have.been.calledWith(
+        this.project,
+        this.user._id,
+        'UNARCHIVE'
+      )
+    })
   })
 
   describe('untrashProject', function() {