Merge pull request #3652 from overleaf/jpa-tell-docstore-doc-name-on-delete

[DocStoreManager] pass doc name as part of DELETE request

GitOrigin-RevId: cdb5fb21580a95aa8a78e0a44ca1e4db4da7f934

services/web/app/src/Features/Docstore/DocstoreManager.js

@@ -22,16 +22,14 @@ const { promisifyAll } = require('../../util/promises')
 const TIMEOUT = 30 * 1000 // request timeout
 
 const DocstoreManager = {
-  deleteDoc(project_id, doc_id, callback) {
+  deleteDoc(project_id, doc_id, name, callback) {
     if (callback == null) {
       callback = function(error) {}
     }
     const url = `${settings.apis.docstore.url}/project/${project_id}/doc/${doc_id}`
-    return request.del({ url: url, timeout: TIMEOUT }, function(
-      error,
-      res,
-      body
-    ) {
+    const docMetaData = { deleted: true, deletedAt: new Date(), name }
+    const options = { url, json: docMetaData, timeout: TIMEOUT }
+    request.patch(options, function(error, res) {
       if (error != null) {
         return callback(error)
       }

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

@@ -1518,7 +1518,7 @@ const ProjectEntityUpdateHandler = {
             if (error != null) {
               return callback(error)
             }
-            DocstoreManager.deleteDoc(projectId, docId, callback)
+            DocstoreManager.deleteDoc(projectId, docId, doc.name, callback)
           })
         }
       )

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

@@ -10,12 +10,14 @@ const { Project } = require('../../../app/src/models/Project')
 const ProjectGetter = require('../../../app/src/Features/Project/ProjectGetter.js')
 
 const User = require('./helpers/User')
+const MockDocStoreApiClass = require('./mocks/MockDocstoreApi')
 const MockDocUpdaterApiClass = require('./mocks/MockDocUpdaterApi')
 
-let MockDocUpdaterApi
+let MockDocStoreApi, MockDocUpdaterApi
 
 before(function() {
   MockDocUpdaterApi = MockDocUpdaterApiClass.instance()
+  MockDocStoreApi = MockDocStoreApiClass.instance()
 })
 
 describe('ProjectStructureChanges', function() {
@@ -1030,6 +1032,22 @@ describe('ProjectStructureChanges', function() {
       })
     })
 
+    it('should pass the doc name to docstore', function(done) {
+      deleteItem(
+        owner,
+        this.exampleProjectId,
+        'doc',
+        this.exampleDocId,
+        error => {
+          if (error) return done(error)
+          expect(
+            MockDocStoreApi.getDeletedDocs(this.exampleProjectId)
+          ).to.deep.equal([{ _id: this.exampleDocId, name: 'new.tex' }])
+          done()
+        }
+      )
+    })
+
     describe('when rootDoc_id matches doc being deleted', function() {
       beforeEach(function(done) {
         Project.updateOne(

services/web/test/acceptance/src/mocks/MockDocstoreApi.js

@@ -5,6 +5,14 @@ class MockDocstoreApi extends AbstractMockApi {
     this.docs = {}
   }
 
+  getDeletedDocs(projectId) {
+    return Object.entries(this.docs[projectId] || {})
+      .filter(([_, doc]) => doc.deleted)
+      .map(([docId, doc]) => {
+        return { _id: docId, name: doc.name }
+      })
+  }
+
   applyRoutes() {
     this.app.post('/project/:projectId/doc/:docId', (req, res) => {
       const { projectId, docId } = req.params
@@ -51,14 +59,14 @@ class MockDocstoreApi extends AbstractMockApi {
       }
     })
 
-    this.app.delete('/project/:projectId/doc/:docId', (req, res) => {
+    this.app.patch('/project/:projectId/doc/:docId', (req, res) => {
       const { projectId, docId } = req.params
       if (!this.docs[projectId]) {
         res.sendStatus(404)
       } else if (!this.docs[projectId][docId]) {
         res.sendStatus(404)
       } else {
-        this.docs[projectId][docId].deleted = true
+        Object.assign(this.docs[projectId][docId], req.body)
         res.sendStatus(204)
       }
     })

services/web/test/unit/src/Docstore/DocstoreManagerTests.js

@@ -13,6 +13,7 @@ const sinon = require('sinon')
 const modulePath = '../../../../app/src/Features/Docstore/DocstoreManager'
 const SandboxedModule = require('sandboxed-module')
 const Errors = require('../../../../app/src/Features/Errors/Errors.js')
+const tk = require('timekeeper')
 
 describe('DocstoreManager', function() {
   beforeEach(function() {
@@ -41,21 +42,31 @@ describe('DocstoreManager', function() {
 
   describe('deleteDoc', function() {
     describe('with a successful response code', function() {
+      // for assertions on the deletedAt timestamp, we need to freeze the clock.
+      before(function() {
+        tk.freeze(Date.now())
+      })
+      after(function() {
+        tk.reset()
+      })
+
       beforeEach(function() {
-        this.request.del = sinon
+        this.request.patch = sinon
           .stub()
           .callsArgWith(1, null, { statusCode: 204 }, '')
         return this.DocstoreManager.deleteDoc(
           this.project_id,
           this.doc_id,
+          'wombat.tex',
           this.callback
         )
       })
 
       it('should delete the doc in the docstore api', function() {
-        return this.request.del
+        return this.request.patch
           .calledWith({
             url: `${this.settings.apis.docstore.url}/project/${this.project_id}/doc/${this.doc_id}`,
+            json: { deleted: true, deletedAt: new Date(), name: 'wombat.tex' },
             timeout: 30 * 1000
           })
           .should.equal(true)
@@ -68,12 +79,13 @@ describe('DocstoreManager', function() {
 
     describe('with a failed response code', function() {
       beforeEach(function() {
-        this.request.del = sinon
+        this.request.patch = sinon
           .stub()
           .callsArgWith(1, null, { statusCode: 500 }, '')
         return this.DocstoreManager.deleteDoc(
           this.project_id,
           this.doc_id,
+          'main.tex',
           this.callback
         )
       })
@@ -96,12 +108,13 @@ describe('DocstoreManager', function() {
 
     describe('with a missing (404) response code', function() {
       beforeEach(function() {
-        this.request.del = sinon
+        this.request.patch = sinon
           .stub()
           .callsArgWith(1, null, { statusCode: 404 }, '')
         return this.DocstoreManager.deleteDoc(
           this.project_id,
           this.doc_id,
+          'main.tex',
           this.callback
         )
       })

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

@@ -2180,10 +2180,9 @@ describe('ProjectEntityUpdateHandler', function() {
       })
 
       it('should delete the doc in the doc updater', function() {
-        this.DocumentUpdaterHandler.deleteDoc.calledWith(
-          projectId,
-          this.doc._id.toString()
-        )
+        this.DocumentUpdaterHandler.deleteDoc
+          .calledWith(projectId, this.doc._id.toString())
+          .should.equal(true)
       })
 
       it('should insert the doc into the deletedDocs array', function() {
@@ -2194,7 +2193,7 @@ describe('ProjectEntityUpdateHandler', function() {
 
       it('should delete the doc in the doc store', function() {
         this.DocstoreManager.deleteDoc
-          .calledWith(projectId, this.doc._id.toString())
+          .calledWith(projectId, this.doc._id.toString(), 'test.tex')
           .should.equal(true)
       })