Merge pull request #1750 from overleaf/cmg-project-metadata

Preserve project metadata when deleting

GitOrigin-RevId: 38efb139406d6111f5b6186d82c3fbfb5ab9b2b9

services/web/app/coffee/Features/Project/ProjectController.coffee

@@ -90,18 +90,18 @@ module.exports = ProjectController =
 		project_id = req.params.Project_id
 		forever    = req.query?.forever?
 		logger.log project_id: project_id, forever: forever, "received request to archive project"
-
-		if forever
-			doDelete = projectDeleter.deleteProject
-		else
-			doDelete = projectDeleter.archiveProject
-
-		doDelete project_id, (err)->
+		user = AuthenticationController.getSessionUser(req)
+		cb = (err)->
 			if err?
 				res.sendStatus 500
 			else
 				res.sendStatus 200
 
+		if forever
+			projectDeleter.deleteProject project_id, {deleterUser: user, ipAddress:req.ip} , cb
+		else
+			projectDeleter.archiveProject project_id, cb
+
 	restoreProject: (req, res) ->
 		project_id = req.params.Project_id
 		logger.log project_id:project_id, "received request to restore project"

services/web/app/coffee/Features/Project/ProjectDeleter.coffee

@@ -1,11 +1,11 @@
 Project = require('../../models/Project').Project
+DeletedProject = require('../../models/DeletedProject').DeletedProject
 logger = require('logger-sharelatex')
 documentUpdaterHandler = require('../DocumentUpdater/DocumentUpdaterHandler')
 tagsHandler = require("../Tags/TagsHandler")
 async = require("async")
 FileStoreHandler = require("../FileStore/FileStoreHandler")
 CollaboratorsHandler = require("../Collaborators/CollaboratorsHandler")
-{db, ObjectId} = require("../../infrastructure/mongojs")
 
 module.exports = ProjectDeleter =
 
@@ -17,64 +17,67 @@ module.exports = ProjectDeleter =
 		Project.update conditions, update, {}, (err)->
 			require('../Editor/EditorController').notifyUsersProjectHasBeenDeletedOrRenamed project_id, ->
 				callback()
-				
+
 	unmarkAsDeletedByExternalSource: (project_id, callback = (error) ->) ->
 		logger.log project_id: project_id, "removing flag marking project as deleted by external data source"
 		conditions = {_id:project_id.toString()}
 		update = {deletedByExternalDataSource: false}
 		Project.update conditions, update, {}, callback
 
-	deleteUsersProjects: (user_id, callback)->
+	deleteUsersProjects: (user_id, callback) ->
 		logger.log {user_id}, "deleting users projects"
-		ProjectDeleter._deleteUsersProjectWithMethod user_id, ProjectDeleter.deleteProject, callback
 
-	softDeleteUsersProjectsForMigration: (user_id, callback)->
-		logger.log {user_id}, "soft-deleting users projects"
-		ProjectDeleter._deleteUsersProjectWithMethod user_id, ProjectDeleter.softDeleteProjectForMigration, callback
-
-	_deleteUsersProjectWithMethod: (user_id, deleteMethod, callback) ->
 		Project.find {owner_ref: user_id}, (error, projects) ->
 			return callback(error) if error?
 			async.each(
 				projects,
 				(project, cb) ->
-					deleteMethod project._id, cb
+					ProjectDeleter.deleteProject project._id, cb
 				(err) ->
 					return callback(err) if err?
 					CollaboratorsHandler.removeUserFromAllProjets user_id, callback
 			)
 
-	softDeleteProjectForMigration: (project_id, callback) ->
-		logger.log project_id: project_id, "soft-deleting project"
+	deleteProject: (project_id, options = {}, callback = (error) ->) ->
+		data = {}
+		logger.log project_id: project_id, "deleting project"
+
+		if typeof options == 'function'
+			callback = options
+			options = {}
+
 		async.waterfall [
 			(cb) ->
 				Project.findOne {_id: project_id}, (err, project) -> cb(err, project)
 			(project, cb) ->
-				return callback(new Errors.NotFoundError("project not found")) unless project?
-				project.deletedAt = new Date()
-				db.projectsDeletedByMigration.insert project, (err) -> cb(err)
-			(cb) ->
-				ProjectDeleter.deleteProject project_id, cb
-		], callback
+				deletedProject = new DeletedProject()
+				deletedProject.project = project
+				deletedProject.deleterData =
+					deletedAt: new Date()
+					deleterId: options.deleterUser?._id
+					deleterIpAddress: options.ipAddress
 
-	deleteProject: (project_id, callback = (error) ->) ->
-		logger.log project_id: project_id, "deleting project"
-		async.series [
-			(cb)->
-				documentUpdaterHandler.flushProjectToMongoAndDelete project_id, cb
-			(cb)->
+				return callback(new Errors.NotFoundError("project not found")) unless project?
+
+				deletedProject.save (err) ->
+					cb(err, deletedProject)
+			(deletedProject, cb) ->
+				documentUpdaterHandler.flushProjectToMongoAndDelete project_id, (err) ->
+					cb(err, deletedProject)
+			(deletedProject, cb) ->
 				CollaboratorsHandler.getMemberIds project_id, (error, member_ids = []) ->
 					for member_id in member_ids
 						tagsHandler.removeProjectFromAllTags member_id, project_id, (err)->
-				cb() #doesn't matter if this fails or the order it happens in
-			(cb) ->
-				Project.remove _id: project_id, cb
-		], (err) ->
+				cb(null, deletedProject) #doesn't matter if this fails or the order it happens in
+			(deletedProject, cb) ->
+				Project.remove _id: project_id, (err) ->
+					cb(err, deletedProject)
+		], (err, deletedProject) ->
 			if err?
 				logger.err err:err, "problem deleting project"
 				return callback(err)
 			logger.log project_id:project_id, "successfully deleting project from user request"
-			callback()
+			callback(null, deletedProject)
 
 	archiveProject: (project_id, callback = (error) ->)->
 		logger.log project_id:project_id, "archived project from user request"

services/web/app/coffee/Features/User/UserDeleter.coffee

@@ -23,7 +23,7 @@ module.exports = UserDeleter =
 				(cb) ->
 					UserDeleter._cleanupUser user, cb
 				(cb) ->
-					ProjectDeleter.softDeleteUsersProjectsForMigration user._id, cb
+					ProjectDeleter.deleteUsersProjects user._id, cb
 				(cb) ->
 					user.deletedAt = new Date()
 					db.usersDeletedByMigration.insert user, cb

services/web/app/coffee/models/DeletedProject.coffee

@@ -0,0 +1,27 @@
+mongoose = require('mongoose')
+Settings = require 'settings-sharelatex'
+ProjectSchema = require('./Project.js').ProjectSchema
+
+Schema = mongoose.Schema
+ObjectId = Schema.ObjectId
+
+DeleterDataSchema = new Schema
+	deleterId: {type: ObjectId, ref: 'User'}
+	deleterIpAddress: { type: String }
+	deletedAt: { type: Date }
+
+DeletedProjectSchema = new Schema({
+	deleterData : [DeleterDataSchema]
+	project: [ProjectSchema]
+}, collection: 'deletedProjects')
+
+conn = mongoose.createConnection(Settings.mongo.url, {
+	server: {poolSize: Settings.mongo.poolSize || 10},
+	config: {autoIndex: false}
+})
+
+DeletedProject = conn.model('DeletedProject', DeletedProjectSchema)
+
+mongoose.model 'DeletedProject', DeletedProjectSchema
+exports.DeletedProject = DeletedProject
+exports.DeletedProjectSchema = DeletedProjectSchema

services/web/test/unit/coffee/Project/ProjectControllerTests.coffee

@@ -31,7 +31,7 @@ describe "ProjectController", ->
 		@token = 'some-token'
 		@ProjectDeleter =
 			archiveProject: sinon.stub().callsArg(1)
-			deleteProject: sinon.stub().callsArg(1)
+			deleteProject: sinon.stub().callsArg(2)
 			restoreProject: sinon.stub().callsArg(1)
 			findArchivedProjects: sinon.stub()
 		@ProjectDuplicator =
@@ -140,6 +140,7 @@ describe "ProjectController", ->
 				projectName: @projectName
 			i18n:
 				translate:->
+			ip: "192.170.18.1"
 		@res =
 			locals:
 				jsPath:"js path here"
@@ -230,7 +231,7 @@ describe "ProjectController", ->
 		it "should tell the project deleter to delete when forever=true", (done)->
 			@req.query = forever: "true"
 			@res.sendStatus = (code)=>
-				@ProjectDeleter.deleteProject.calledWith(@project_id).should.equal true
+				@ProjectDeleter.deleteProject.calledWith(@project_id, {deleterUser: @user, ipAddress:@req.ip}).should.equal true
 				code.should.equal 200
 				done()
 			@ProjectController.deleteProject @req, @res

services/web/test/unit/coffee/Project/ProjectDeleterTests.coffee

@@ -3,7 +3,6 @@ modulePath = "../../../../app/js/Features/Project/ProjectDeleter"
 SandboxedModule = require('sandboxed-module')
 sinon = require('sinon')
 
-
 describe 'ProjectDeleter', ->
 
 	beforeEach ->
@@ -16,14 +15,10 @@ describe 'ProjectDeleter', ->
 			owner_ref:"owner ref here"
 			remove: sinon.stub().callsArg(0)
 
-		@user_id = 1234
-
-		@mongojs =
-			db:
-				deletedProjects:
-					insert: sinon.stub().callsArg(1)
-				projectsDeletedByMigration:
-					insert: sinon.stub().callsArg(1)
+		@user =
+			_id:"588f3ddae8ebc1bac07c9fa4"
+			first_name: "bjkdsjfk"
+			features: {}
 	
 		@Project = 
 			update: sinon.stub().callsArgWith(3)
@@ -31,6 +26,9 @@ describe 'ProjectDeleter', ->
 			findOne: sinon.stub().callsArgWith(1, null, @project)
 			find: sinon.stub().callsArgWith(1, null, [@project])
 			applyToAllFilesRecursivly: sinon.stub()
+		@DeletedProject = class DeletedProject
+			constructor: ->
+			save: sinon.stub().callsArgWith(0)
 		@documentUpdaterHandler = 
 			flushProjectToMongoAndDelete:sinon.stub().callsArgWith(1)
 		@editorController = notifyUsersProjectHasBeenDeletedOrRenamed : sinon.stub().callsArgWith(1)
@@ -42,11 +40,11 @@ describe 'ProjectDeleter', ->
 		@deleter = SandboxedModule.require modulePath, requires:
 			"../Editor/EditorController": @editorController
 			'../../models/Project':{Project:@Project}
+			'../../models/DeletedProject':{DeletedProject:@DeletedProject}
 			'../DocumentUpdater/DocumentUpdaterHandler': @documentUpdaterHandler
 			"../Tags/TagsHandler":@TagsHandler
 			"../FileStore/FileStoreHandler": @FileStoreHandler = {}
 			"../Collaborators/CollaboratorsHandler": @CollaboratorsHandler
-			"../../infrastructure/mongojs": @mongojs
 			'logger-sharelatex':
 				log:->
 
@@ -83,12 +81,12 @@ describe 'ProjectDeleter', ->
 			@deleter.deleteProject = sinon.stub().callsArg(1)
 
 		it "should find all the projects owned by the user_id", (done)->
-			@deleter.deleteUsersProjects @user_id, =>
-				sinon.assert.calledWith(@Project.find, owner_ref: @user_id)
+			@deleter.deleteUsersProjects @user._id, =>
+				sinon.assert.calledWith(@Project.find, owner_ref: @user._id)
 				done()
 
 		it "should call deleteProject on the found projects", (done)->
-			@deleter.deleteUsersProjects @user_id, =>
+			@deleter.deleteUsersProjects @user._id, =>
 				sinon.assert.calledWith(@deleter.deleteProject, @project._id)
 				done()
 
@@ -96,54 +94,31 @@ describe 'ProjectDeleter', ->
 			@Project.find.callsArgWith(1, null, [
 				{_id: 'potato'}, {_id: 'wombat'}
 			])
-			@deleter.deleteUsersProjects @user_id, =>
+			@deleter.deleteUsersProjects @user._id, =>
 				sinon.assert.calledTwice(@deleter.deleteProject)
 				sinon.assert.calledWith(@deleter.deleteProject, 'wombat')
 				sinon.assert.calledWith(@deleter.deleteProject, 'potato')
 				done()
 
 		it "should remove all the projects the user is a collaborator of", (done)->
-			@deleter.deleteUsersProjects @user_id, =>
-				@CollaboratorsHandler.removeUserFromAllProjets.calledWith(@user_id).should.equal true
-				done()
-
-	describe "softDeleteUsersProjectsForMigrationForMigration", ->
-		beforeEach ->
-			@deleter.softDeleteProjectForMigration = sinon.stub().callsArg(1)
-
-		it "should find all the projects owned by the user_id", (done)->
-			@deleter.softDeleteUsersProjectsForMigration @user_id, =>
-				@Project.find.calledWith(owner_ref: @user_id).should.equal true
-				done()
-
-		it "should call deleteProject on the found projects", (done)->
-			@deleter.softDeleteUsersProjectsForMigration @user_id, =>
-				sinon.assert.calledWith(@deleter.softDeleteProjectForMigration, @project._id)
-				done()
-
-		it "should call deleteProject once for each project", (done)->
-			@Project.find.callsArgWith(1, null, [
-				{_id: 'potato'}, {_id: 'wombat'}
-			])
-			@deleter.softDeleteUsersProjectsForMigration @user_id, =>
-				sinon.assert.calledTwice(@deleter.softDeleteProjectForMigration)
-				sinon.assert.calledWith(@deleter.softDeleteProjectForMigration, 'wombat')
-				sinon.assert.calledWith(@deleter.softDeleteProjectForMigration, 'potato')
-				done()
-
-		it "should remove all the projects the user is a collaborator of", (done)->
-			@deleter.softDeleteUsersProjectsForMigration @user_id, =>
-				@CollaboratorsHandler.removeUserFromAllProjets.calledWith(@user_id).should.equal true
+			@deleter.deleteUsersProjects @user._id, =>
+				@CollaboratorsHandler.removeUserFromAllProjets.calledWith(@user._id).should.equal true
 				done()
 
 	describe "deleteProject", ->
-		beforeEach (done) ->
+		beforeEach () ->
 			@project_id = "mock-project-id-123"
-			@Project.remove.callsArgWith(1)
-			done()
+			@ip = "192.170.18.1"
+
+		it "should save a DeletedProject with additional deleterData", (done) ->
+			@deleter.deleteProject @project_id, {deleterUser: @user, ipAddress: @ip}, (err, deletedProject) =>
+				@DeletedProject::save.called.should.equal true
+				deletedProject.deleterData.deleterIpAddress.should.equal(@ip)
+				deletedProject.deleterData.deleterId.should.equal(@user._id)
+				done()
 
 		it "should flushProjectToMongoAndDelete in doc updater", (done)->
-			@deleter.deleteProject @project_id, =>
+			@deleter.deleteProject @project_id, {deleterUser: @user, ipAddress: @ip}, =>
 				@documentUpdaterHandler.flushProjectToMongoAndDelete.calledWith(@project_id).should.equal true
 				done()
 
@@ -160,25 +135,6 @@ describe 'ProjectDeleter', ->
 				}).should.equal true
 				done()
 
-	describe "softDeleteProjectForMigration", ->
-		beforeEach ->
-			@deleter.deleteProject = sinon.stub().callsArg(1)
-
-		it "should set the deletedAt time", (done)->
-			@deleter.softDeleteProjectForMigration @project_id, =>
-				@project.deletedAt.should.exist
-				done()
-
-		it "should insert the project into the deleted projects collection", (done)->
-			@deleter.softDeleteProjectForMigration @project_id, =>
-				sinon.assert.calledWith(@mongojs.db.projectsDeletedByMigration.insert, @project)
-				done()
-
-		it "should delete the project", (done)->
-			@deleter.softDeleteProjectForMigration @project_id, =>
-				sinon.assert.calledWith(@deleter.deleteProject, @project_id)
-				done()
-
 	describe "archiveProject", ->
 		beforeEach ->
 			@Project.update.callsArgWith(2)

services/web/test/unit/coffee/User/UserDeleterTests.coffee

@@ -8,7 +8,7 @@ describe "UserDeleter", ->
 
 	beforeEach ->
 		@user = 
-			_id:"12390i"
+			_id: "12390i"
 			email: "bob@bob.com"
 			remove: sinon.stub().callsArgWith(0)
 
@@ -20,7 +20,6 @@ describe "UserDeleter", ->
 
 		@ProjectDeleter =
 			deleteUsersProjects: sinon.stub().callsArgWith(1)
-			softDeleteUsersProjectsForMigration: sinon.stub().callsArgWith(1)
 
 		@SubscriptionHandler = 
 			cancelSubscription: sinon.stub().callsArgWith(1)
@@ -85,9 +84,9 @@ describe "UserDeleter", ->
 				@deleteAffiliations.calledWith(@user._id).should.equal true
 				done()
 
-		it "should soft-delete all the projects of a user", (done)->
+		it "should delete all the projects of a user", (done)->
 			@UserDeleter.softDeleteUserForMigration @user._id, (err)=>
-				@ProjectDeleter.softDeleteUsersProjectsForMigration.calledWith(@user._id).should.equal true
+				@ProjectDeleter.deleteUsersProjects.calledWith(@user._id).should.equal true
 				done()
 
 		it "should remove user memberships", (done)->