From b84c24520cc474df3a34d35c32a658c9e338bd8b Mon Sep 17 00:00:00 2001
From: Alasdair Smith <ali@alasdairsmith.co.uk>
Date: Tue, 28 May 2019 12:09:03 +0100
Subject: [PATCH] Merge pull request #1804 from
 overleaf/msm-removed-collaborator-as-first-step-transfer-ownership

Removing user as collaborator as a pre-step for project ownership transfer

GitOrigin-RevId: dbfe1553ae7277863908341fdd2af709dc019692
---
 .../Project/ProjectDetailsHandler.coffee      | 31 +++++++++----------
 .../Project/ProjectDetailsHandlerTests.coffee | 16 ++++++++++
 2 files changed, 31 insertions(+), 16 deletions(-)

diff --git a/services/web/app/coffee/Features/Project/ProjectDetailsHandler.coffee b/services/web/app/coffee/Features/Project/ProjectDetailsHandler.coffee
index 6d3ad46395..172b925425 100644
--- a/services/web/app/coffee/Features/Project/ProjectDetailsHandler.coffee
+++ b/services/web/app/coffee/Features/Project/ProjectDetailsHandler.coffee
@@ -10,6 +10,7 @@ Errors = require("../Errors/Errors")
 ProjectTokenGenerator = require('./ProjectTokenGenerator')
 ProjectEntityHandler = require('./ProjectEntityHandler')
 ProjectHelper = require('./ProjectHelper')
+CollaboratorsHandler = require('../Collaborators/CollaboratorsHandler')
 settings = require('settings-sharelatex')
 
 
@@ -60,23 +61,21 @@ module.exports = ProjectDetailsHandler =
 				return callback(err) if err?
 				return callback(new Errors.NotFoundError("user not found")) unless user?
 
-				ProjectDetailsHandler.generateUniqueName user_id, project.name + suffix, (err, name) ->
+				#	we make sure the user to which the project is transferred is not a collaborator for the project,
+				#	this prevents any conflict during unique name generation
+				CollaboratorsHandler.removeUserFromProject project_id, user_id, (err) ->
 					return callback(err) if err?
-
-					Project.update {_id: project_id},
-						{
-							$set: {
-								owner_ref: user_id,
-								name: name
-							},
-							$pull: {
-								readOnly_refs: user_id,
-								collaberator_refs: user_id,
-								tokenAccessReadAndWrite_refs: user_id,
-								tokenAccessReadOnly_refs: user_id
-						}}, (err) ->
-							return callback(err) if err?
-							ProjectEntityHandler.flushProjectToThirdPartyDataStore project_id, callback
+					ProjectDetailsHandler.generateUniqueName user_id, project.name + suffix, (err, name) ->
+						return callback(err) if err?
+						Project.update {_id: project_id},
+							{
+								$set: {
+									owner_ref: user_id,
+									name: name
+								}
+							}, (err) ->
+								return callback(err) if err?
+								ProjectEntityHandler.flushProjectToThirdPartyDataStore project_id, callback
 
 	renameProject: (project_id, newName, callback = ->)->
 		ProjectDetailsHandler.validateProjectName newName, (error) ->
diff --git a/services/web/test/unit/coffee/Project/ProjectDetailsHandlerTests.coffee b/services/web/test/unit/coffee/Project/ProjectDetailsHandlerTests.coffee
index 4ede2d1021..41e352eab6 100644
--- a/services/web/test/unit/coffee/Project/ProjectDetailsHandlerTests.coffee
+++ b/services/web/test/unit/coffee/Project/ProjectDetailsHandlerTests.coffee
@@ -32,12 +32,15 @@ describe 'ProjectDetailsHandler', ->
 			moveEntity:sinon.stub().callsArgWith 1
 		@ProjectEntityHandler =
 			flushProjectToThirdPartyDataStore: sinon.stub().callsArg(1)
+		@CollaboratorsHandler =
+			removeUserFromProject: sinon.stub().callsArg(2)
 		@handler = SandboxedModule.require modulePath, requires:
 			"./ProjectGetter":@ProjectGetter
 			'../../models/Project': Project:@ProjectModel
 			"../User/UserGetter": @UserGetter
 			'../ThirdPartyDataStore/TpdsUpdateSender':@tpdsUpdateSender
 			"./ProjectEntityHandler": @ProjectEntityHandler
+			"../Collaborators/CollaboratorsHandler": @CollaboratorsHandler
 			'logger-sharelatex':
 				log:->
 				err:->
@@ -102,11 +105,24 @@ describe 'ProjectDetailsHandler', ->
 				err.name.should.equal "NotFoundError"
 				done()
 
+		it "should return an error if user cannot be removed as collaborator ", (done) ->
+			errorMessage = "user-cannot-be-removed"
+			@CollaboratorsHandler.removeUserFromProject.callsArgWith(2, errorMessage)
+			@handler.transferOwnership 'abc', '123', (err) ->
+				err.should.exist
+				err.should.equal errorMessage
+				done()
+
 		it "should transfer ownership of the project", (done) ->
 			@handler.transferOwnership 'abc', '123', () =>
 				sinon.assert.calledWith(@ProjectModel.update, {_id: 'abc'}, sinon.match({$set: {name: 'teapot'}}))
 				done()
 
+		it "should remove the user from the project's collaborators", (done) ->
+			@handler.transferOwnership 'abc', '123', () =>
+				sinon.assert.calledWith(@CollaboratorsHandler.removeUserFromProject, 'abc', '123')
+				done()
+
 		it "should flush the project to tpds", (done) ->
 			@handler.transferOwnership 'abc', '123', () =>
 				sinon.assert.calledWith(@ProjectEntityHandler.flushProjectToThirdPartyDataStore, 'abc')