From 1da929fcdbb395b1fb1f0ec68b31f0addca72933 Mon Sep 17 00:00:00 2001 From: Eric Mc Sween Date: Thu, 27 Feb 2020 07:46:37 -0500 Subject: [PATCH] Merge pull request #2618 from overleaf/ew-clear-root-doc-on-delete Clear root doc on delete GitOrigin-RevId: 4121d198f5253417bca2284c5f750c088debcb8c --- .../src/Features/Compile/CompileController.js | 70 +++++------- .../ProjectEntityMongoUpdateHandler.js | 38 ++++--- .../acceptance/src/ProjectStructureTests.js | 100 ++++++++++++++++++ .../src/Compile/CompileControllerTests.js | 6 +- 4 files changed, 154 insertions(+), 60 deletions(-) diff --git a/services/web/app/src/Features/Compile/CompileController.js b/services/web/app/src/Features/Compile/CompileController.js index 9802ef0eef..ea48c74413 100644 --- a/services/web/app/src/Features/Compile/CompileController.js +++ b/services/web/app/src/Features/Compile/CompileController.js @@ -10,7 +10,6 @@ /* * decaffeinate suggestions: * DS102: Remove unnecessary code created because of implicit returns - * DS103: Rewrite code to no longer use __guard__ * DS207: Consider shorter variations of null checks * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md */ @@ -35,59 +34,52 @@ const COMPILE_TIMEOUT_MS = 10 * 60 * 1000 module.exports = CompileController = { compile(req, res, next) { - if (next == null) { - next = function(error) {} - } res.setTimeout(COMPILE_TIMEOUT_MS) const project_id = req.params.Project_id - const isAutoCompile = !!(req.query != null - ? req.query.auto_compile - : undefined) + const isAutoCompile = !!req.query.auto_compile const user_id = AuthenticationController.getLoggedInUserId(req) const options = { isAutoCompile } - if ((req.body != null ? req.body.rootDoc_id : undefined) != null) { + + if (req.body.rootDoc_id) { options.rootDoc_id = req.body.rootDoc_id } else if ( - __guard__( - req.body != null ? req.body.settingsOverride : undefined, - x => x.rootDoc_id - ) != null + req.body.settingsOverride && + req.body.settingsOverride.rootDoc_id ) { // Can be removed after deploy options.rootDoc_id = req.body.settingsOverride.rootDoc_id } - if (req.body != null ? req.body.compiler : undefined) { + if (req.body.compiler) { options.compiler = req.body.compiler } - if (req.body != null ? req.body.draft : undefined) { + if (req.body.draft) { options.draft = req.body.draft } - if ( - ['validate', 'error', 'silent'].includes( - req.body != null ? req.body.check : undefined - ) - ) { + if (['validate', 'error', 'silent'].includes(req.body.check)) { options.check = req.body.check } - if (req.body != null ? req.body.incrementalCompilesEnabled : undefined) { + if (req.body.incrementalCompilesEnabled) { options.incrementalCompilesEnabled = true } - return CompileManager.compile(project_id, user_id, options, function( - error, - status, - outputFiles, - clsiServerId, - limits, - validationProblems - ) { - if (error != null) { - return next(error) - } - res.contentType('application/json') - return res.status(200).send( - JSON.stringify({ + + CompileManager.compile( + project_id, + user_id, + options, + ( + error, + status, + outputFiles, + clsiServerId, + limits, + validationProblems + ) => { + if (error) { + return next(error) + } + res.json({ status, outputFiles, compileGroup: limits != null ? limits.compileGroup : undefined, @@ -95,8 +87,8 @@ module.exports = CompileController = { validationProblems, pdfDownloadDomain: Settings.pdfDownloadDomain }) - ) - }) + } + ) }, stopCompile(req, res, next) { @@ -540,9 +532,3 @@ module.exports = CompileController = { }) } } - -function __guard__(value, transform) { - return typeof value !== 'undefined' && value !== null - ? transform(value) - : undefined -} diff --git a/services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.js b/services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.js index 20c958c9bc..b380dd8b1a 100644 --- a/services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.js +++ b/services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.js @@ -314,8 +314,12 @@ async function moveEntity(projectId, entityId, destFolderId, entityType) { async function deleteEntity(projectId, entityId, entityType, callback) { const project = await ProjectGetter.promises.getProjectWithoutLock( projectId, - { name: true, rootFolder: true, overleaf: true } + { name: true, rootFolder: true, overleaf: true, rootDoc_id: true } ) + const deleteRootDoc = + project.rootDoc_id && + entityId && + project.rootDoc_id.toString() === entityId.toString() const { element: entity, path } = await ProjectLocator.promises.findElement({ project, element_id: entityId, @@ -325,7 +329,8 @@ async function deleteEntity(projectId, entityId, entityType, callback) { Project, projectId, path.mongo, - entityId + entityId, + deleteRootDoc ) return { entity, path, projectBeforeDeletion: project, newProject } } @@ -414,19 +419,24 @@ async function _insertDeletedFileReference(projectId, fileRef) { ).exec() } -async function _removeElementFromMongoArray(model, modelId, path, elementId) { +async function _removeElementFromMongoArray( + model, + modelId, + path, + elementId, + deleteRootDoc = false +) { const nonArrayPath = path.slice(0, path.lastIndexOf('.')) - const newDoc = model - .findOneAndUpdate( - { _id: modelId }, - { - $pull: { [nonArrayPath]: { _id: elementId } }, - $inc: { version: 1 } - }, - { new: true } - ) - .exec() - return newDoc + const options = { new: true } + const query = { _id: modelId } + const update = { + $pull: { [nonArrayPath]: { _id: elementId } }, + $inc: { version: 1 } + } + if (deleteRootDoc) { + update.$unset = { rootDoc_id: 1 } + } + return model.findOneAndUpdate(query, update, options).exec() } function _countElements(project) { diff --git a/services/web/test/acceptance/src/ProjectStructureTests.js b/services/web/test/acceptance/src/ProjectStructureTests.js index 17f15ac240..2ef62558a4 100644 --- a/services/web/test/acceptance/src/ProjectStructureTests.js +++ b/services/web/test/acceptance/src/ProjectStructureTests.js @@ -6,6 +6,7 @@ const fs = require('fs') const Settings = require('settings-sharelatex') const _ = require('underscore') +const { Project } = require('../../../app/src/models/Project') const ProjectGetter = require('../../../app/src/Features/Project/ProjectGetter.js') const MockDocUpdaterApi = require('./helpers/MockDocUpdaterApi') @@ -1098,6 +1099,105 @@ describe('ProjectStructureChanges', function() { }) }) + describe('deleting docs', function() { + beforeEach(function(done) { + createExampleProject(owner, (err, projectId) => { + if (err) { + return done(err) + } + this.exampleProjectId = projectId + createExampleFolder(owner, projectId, (err, folderId) => { + if (err) { + return done(err) + } + this.exampleFolderId = folderId + createExampleDoc(owner, projectId, (err, docId) => { + if (err) { + return done(err) + } + this.exampleDocId = docId + MockDocUpdaterApi.clearProjectStructureUpdates() + ProjectGetter.getProject( + this.exampleProjectId, + (error, project) => { + if (error) { + throw error + } + this.project0 = project + done() + } + ) + }) + }) + }) + }) + + describe('when rootDoc_id matches doc being deleted', function() { + beforeEach(function(done) { + Project.update( + { _id: this.exampleProjectId }, + { $set: { rootDoc_id: this.exampleDocId } }, + done + ) + }) + + it('should clear rootDoc_id', function(done) { + deleteItem( + owner, + this.exampleProjectId, + 'doc', + this.exampleDocId, + () => { + ProjectGetter.getProject( + this.exampleProjectId, + (error, project) => { + if (error) { + throw error + } + expect(project.rootDoc_id).to.be.undefined + done() + } + ) + } + ) + }) + }) + + describe('when rootDoc_id does not match doc being deleted', function() { + beforeEach(function(done) { + this.exampleRootDocId = new ObjectId() + Project.update( + { _id: this.exampleProjectId }, + { $set: { rootDoc_id: this.exampleRootDocId } }, + done + ) + }) + + it('should not clear rootDoc_id', function(done) { + deleteItem( + owner, + this.exampleProjectId, + 'doc', + this.exampleDocId, + () => { + ProjectGetter.getProject( + this.exampleProjectId, + (error, project) => { + if (error) { + throw error + } + expect(project.rootDoc_id.toString()).to.equal( + this.exampleRootDocId.toString() + ) + done() + } + ) + } + ) + }) + }) + }) + describe('tpds', function() { let projectName, exampleProjectId, oldVersion, rootFolderId diff --git a/services/web/test/unit/src/Compile/CompileControllerTests.js b/services/web/test/unit/src/Compile/CompileControllerTests.js index 944a7d06d5..4120eb0eaf 100644 --- a/services/web/test/unit/src/Compile/CompileControllerTests.js +++ b/services/web/test/unit/src/Compile/CompileControllerTests.js @@ -120,14 +120,12 @@ describe('CompileController', function() { }) it('should set the content-type of the response to application/json', function() { - return this.res.contentType - .calledWith('application/json') - .should.equal(true) + this.res.type.should.equal('application/json') }) it('should send a successful response reporting the status and files', function() { this.res.statusCode.should.equal(200) - return this.res.body.should.equal( + this.res.body.should.equal( JSON.stringify({ status: this.status, outputFiles: this.outputFiles