Remove User Memberships on Deletion (#1688)

Remove User Memberships on Deletion

GitOrigin-RevId: d6e6a4cc01e31288f660c2fd36a0cee174ee5dd1

services/web/app/coffee/Features/Institutions/InstitutionsGetter.coffee

@@ -1,5 +1,5 @@
 UserGetter = require '../User/UserGetter'
-UserMembershipHandler = require "../UserMembership/UserMembershipHandler"
+UserMembershipsHandler = require "../UserMembership/UserMembershipsHandler"
 UserMembershipEntityConfigs = require "../UserMembership/UserMembershipEntityConfigs"
 logger = require 'logger-sharelatex'
 
@@ -16,4 +16,4 @@ module.exports = InstitutionsGetter =
 			callback(null, confirmedInstitutions)
 
 	getManagedInstitutions: (user_id, callback = (error, managedInstitutions) ->) ->
-		UserMembershipHandler.getEntitiesByUser UserMembershipEntityConfigs.institution, user_id, callback
+		UserMembershipsHandler.getEntitiesByUser UserMembershipEntityConfigs.institution, user_id, callback

services/web/app/coffee/Features/Publishers/PublishersGetter.coffee

@@ -1,9 +1,9 @@
-UserMembershipHandler = require "../UserMembership/UserMembershipHandler"
+UserMembershipsHandler = require "../UserMembership/UserMembershipsHandler"
 UserMembershipEntityConfigs = require "../UserMembership/UserMembershipEntityConfigs"
 logger = require 'logger-sharelatex'
 _ = require 'underscore'
 
 module.exports = PublishersGetter =
 	getManagedPublishers: (user_id, callback = (error, managedPublishers) ->) ->
-		UserMembershipHandler.getEntitiesByUser UserMembershipEntityConfigs.publisher, user_id, (error, managedPublishers) ->
+		UserMembershipsHandler.getEntitiesByUser UserMembershipEntityConfigs.publisher, user_id, (error, managedPublishers) ->
 			callback(error, managedPublishers)

services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee

@@ -48,20 +48,28 @@ module.exports = SubscriptionUpdater =
 
 		Subscription.findAndModify searchOps, insertOperation, callback
 
-	removeUserFromGroup: (subscriptionId, user_id, callback)->
-		searchOps =
-			_id: subscriptionId
+	removeUserFromGroups: (filter, user_id, callback)->
 		removeOperation =
 			"$pull": {member_ids:user_id}
-		Subscription.update searchOps, removeOperation, (err)->
+		Subscription.updateMany filter, removeOperation, (err)->
 			if err?
-				logger.err err:err, searchOps:searchOps, removeOperation:removeOperation, "error removing user from group"
+				logger.err err:err, searchOps:searchOps, removeOperation:removeOperation, "error removing user from groups"
 				return callback(err)
 			UserGetter.getUserOrUserStubById user_id, {}, (error, user, isStub) ->
 				return callback(error) if error
 				return callback() if isStub
 				FeaturesUpdater.refreshFeatures user_id, callback
 
+	removeUserFromGroup: (subscriptionId, user_id, callback)->
+		SubscriptionUpdater.removeUserFromGroups { _id: subscriptionId }, user_id, callback
+
+	removeUserFromAllGroups: (user_id, callback) ->
+		SubscriptionLocator.getMemberSubscriptions user_id, (error, subscriptions) ->
+			return callback(error) if error
+			return callback() unless subscriptions
+			subscriptionIds = subscriptions.map (sub) -> sub._id
+			SubscriptionUpdater.removeUserFromGroups { _id: subscriptionIds }, user_id, callback
+
 	deleteWithV1Id: (v1TeamId, callback)->
 		Subscription.deleteOne { "overleaf.id": v1TeamId }, callback
 

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

@@ -3,6 +3,8 @@ NewsletterManager = require "../Newsletter/NewsletterManager"
 ProjectDeleter = require("../Project/ProjectDeleter")
 logger = require("logger-sharelatex")
 SubscriptionHandler = require("../Subscription/SubscriptionHandler")
+SubscriptionUpdater = require("../Subscription/SubscriptionUpdater")
+UserMembershipsHandler = require("../UserMembership/UserMembershipsHandler")
 async = require("async")
 InstitutionsAPI = require("../Institutions/InstitutionsAPI")
 Errors = require("../Errors/Errors")
@@ -58,4 +60,8 @@ module.exports = UserDeleter =
 				SubscriptionHandler.cancelSubscription user, cb
 			(cb)->
 				InstitutionsAPI.deleteAffiliations user._id, cb
+			(cb)->
+				SubscriptionUpdater.removeUserFromAllGroups user._id, cb
+			(cb)->
+				UserMembershipsHandler.removeUserFromAllEntities user._id, cb
 		], callback)

services/web/app/coffee/Features/UserMembership/UserMembershipHandler.coffee

@@ -47,15 +47,6 @@ module.exports =
 			return callback(isAdmin: true)
 		removeUserFromEntity entity, attribute, userId, callback
 
-	getEntitiesByUser: (entityConfig, userId, callback = (error, entities) ->) ->
-		query = Object.assign({}, entityConfig.baseQuery)
-		query[entityConfig.fields.access] = userId
-		EntityModels[entityConfig.modelName].find query, (error, entities = []) ->
-			return callback(error) if error?
-			async.mapSeries entities,
-				(entity, cb) -> entity.fetchV1Data(cb),
-				callback
-
 getPopulatedListOfMembers = (entity, attributes, callback = (error, users)->)->
 		userObjects = []
 

services/web/app/coffee/Features/UserMembership/UserMembershipsHandler.coffee

@@ -0,0 +1,32 @@
+async = require("async")
+EntityModels =
+	Institution: require('../../models/Institution').Institution
+	Subscription: require('../../models/Subscription').Subscription
+	Publisher: require('../../models/Publisher').Publisher
+UserMembershipEntityConfigs = require "./UserMembershipEntityConfigs"
+
+module.exports = UserMembershipsHandler =
+	removeUserFromAllEntities: (userId, callback = (error) ->) ->
+		# get all writable entity types
+		entityConfigs = []
+		for key, entityConfig of UserMembershipEntityConfigs
+			entityConfigs.push(entityConfig) if entityConfig.fields.write?
+
+		# remove the user from all entities types
+		async.map entityConfigs, ((entityConfig, innerCallback) ->
+			UserMembershipsHandler.removeUserFromEntities entityConfig, userId, innerCallback
+		), callback
+
+	removeUserFromEntities: (entityConfig, userId, callback = (error) ->) ->
+		removeOperation = "$pull": {}
+		removeOperation["$pull"][entityConfig.fields.write] = userId
+		EntityModels[entityConfig.modelName].updateMany {}, removeOperation, callback
+
+	getEntitiesByUser: (entityConfig, userId, callback = (error, entities) ->) ->
+		query = Object.assign({}, entityConfig.baseQuery)
+		query[entityConfig.fields.access] = userId
+		EntityModels[entityConfig.modelName].find query, (error, entities = []) ->
+			return callback(error) if error?
+			async.mapSeries entities,
+				(entity, cb) -> entity.fetchV1Data(cb),
+				callback

services/web/test/unit/coffee/Institutions/InstitutionsGetterTests.coffee

@@ -9,7 +9,7 @@ describe 'InstitutionsGetter', ->
 		@UserGetter = getUserFullEmails: sinon.stub()
 		@InstitutionsGetter = SandboxedModule.require modulePath, requires:
 			'../User/UserGetter': @UserGetter
-			"../UserMembership/UserMembershipHandler": @UserMembershipHandler = {}
+			"../UserMembership/UserMembershipsHandler": @UserMembershipsHandler = {}
 			"../UserMembership/UserMembershipEntityConfigs": @UserMembershipEntityConfigs = {}
 			'logger-sharelatex':
 				log:-> console.log(arguments)

services/web/test/unit/coffee/Publishers/PublishersGetterTests.coffee

@@ -13,7 +13,7 @@ describe 'PublishersGetter', ->
 
 		@PublishersGetter = SandboxedModule.require modulePath, requires:
 			'../User/UserGetter': @UserGetter
-			"../UserMembership/UserMembershipHandler": @UserMembershipHandler = {
+			"../UserMembership/UserMembershipsHandler": @UserMembershipsHandler = {
 				getEntitiesByUser: sinon.stub().callsArgWith(2, null, [@publisher])
 			}
 			"../UserMembership/UserMembershipEntityConfigs": @UserMembershipEntityConfigs = {

services/web/test/unit/coffee/Subscription/SubscriptionUpdaterTests.coffee

@@ -37,6 +37,7 @@ describe "SubscriptionUpdater", ->
 
 
 		@updateStub = sinon.stub().callsArgWith(2, null)
+		@updateManyStub = sinon.stub().callsArgWith(2, null)
 		@findAndModifyStub = sinon.stub().callsArgWith(2, null, @subscription)
 		@SubscriptionModel = class
 			constructor: (opts)->
@@ -45,11 +46,13 @@ describe "SubscriptionUpdater", ->
 				return subscription
 			@remove: sinon.stub().yields()
 		@SubscriptionModel.update = @updateStub
+		@SubscriptionModel.updateMany = @updateManyStub
 		@SubscriptionModel.findAndModify = @findAndModifyStub
 
 		@SubscriptionLocator =
 			getUsersSubscription: sinon.stub()
 			getGroupSubscriptionMemberOf:sinon.stub()
+			getMemberSubscriptions: sinon.stub().yields(null, [])
 
 		@Settings =
 			defaultPlanCode: "personal"
@@ -181,10 +184,12 @@ describe "SubscriptionUpdater", ->
 				@FeaturesUpdater.refreshFeatures.calledWith(@otherUserId).should.equal true
 				done()
 
-	describe "removeUserFromGroup", ->
+	describe "removeUserFromGroups", ->
 		beforeEach ->
 			@FeaturesUpdater.refreshFeatures = sinon.stub().callsArgWith(1)
 			@UserGetter.getUserOrUserStubById.yields(null, {}, false)
+			@fakeSubscriptions = [{ _id: 'fake-id-1' }, { _id: 'fake-id-2' }]
+			@SubscriptionLocator.getMemberSubscriptions.yields(null, @fakeSubscriptions)
 
 		it "should pull the users id from the group", (done)->
 			@SubscriptionUpdater.removeUserFromGroup @subscription._id, @otherUserId, =>
@@ -192,7 +197,16 @@ describe "SubscriptionUpdater", ->
 					_id: @subscription._id
 				removeOperation =
 					"$pull": {member_ids:@otherUserId}
-				@updateStub.calledWith(searchOps, removeOperation).should.equal true
+				@updateManyStub.calledWith(searchOps, removeOperation).should.equal true
+				done()
+
+		it "should pull the users id from all groups", (done)->
+			@SubscriptionUpdater.removeUserFromAllGroups @otherUserId, =>
+				filter =
+					_id: ['fake-id-1', 'fake-id-2']
+				removeOperation =
+					"$pull": {member_ids:@otherUserId}
+				sinon.assert.calledWith(@updateManyStub, filter, removeOperation)
 				done()
 
 		it "should update the users features", (done)->

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

@@ -25,6 +25,12 @@ describe "UserDeleter", ->
 		@SubscriptionHandler = 
 			cancelSubscription: sinon.stub().callsArgWith(1)
 
+		@SubscriptionUpdater =
+			removeUserFromAllGroups: sinon.stub().callsArgWith(1)
+
+		@UserMembershipsHandler =
+			removeUserFromAllEntities: sinon.stub().callsArgWith(1)
+
 		@deleteAffiliations = sinon.stub().callsArgWith(1)
 
 		@mongojs =
@@ -36,6 +42,8 @@ describe "UserDeleter", ->
 			"../../models/User": User: @User
 			"../Newsletter/NewsletterManager":  @NewsletterManager
 			"../Subscription/SubscriptionHandler": @SubscriptionHandler
+			"../Subscription/SubscriptionUpdater": @SubscriptionUpdater
+			"../UserMembership/UserMembershipsHandler": @UserMembershipsHandler
 			"../Project/ProjectDeleter": @ProjectDeleter
 			"../Institutions/InstitutionsAPI":
 				deleteAffiliations: @deleteAffiliations
@@ -80,6 +88,11 @@ describe "UserDeleter", ->
 				@ProjectDeleter.softDeleteUsersProjects.calledWith(@user._id).should.equal true
 				done()
 
+		it "should remove user memberships", (done)->
+			@UserDeleter.softDeleteUser @user._id, (err)=>
+				@UserMembershipsHandler.removeUserFromAllEntities.calledWith(@user._id).should.equal true
+				done()
+
 	describe "deleteUser", ->
 
 		it "should delete the user in mongo", (done)->
@@ -107,3 +120,13 @@ describe "UserDeleter", ->
 			@UserDeleter.deleteUser @user._id, (err)=>
 				@deleteAffiliations.calledWith(@user._id).should.equal true
 				done()
+
+		it "should remove user from group subscriptions", (done)->
+			@UserDeleter.deleteUser @user._id, (err)=>
+				@SubscriptionUpdater.removeUserFromAllGroups.calledWith(@user._id).should.equal true
+				done()
+
+		it "should remove user memberships", (done)->
+			@UserDeleter.deleteUser @user._id, (err)=>
+				@UserMembershipsHandler.removeUserFromAllEntities.calledWith(@user._id).should.equal true
+				done()

services/web/test/unit/coffee/UserMembership/UserMembershipsHandlerTests.coffee

@@ -0,0 +1,40 @@
+sinon = require('sinon')
+assertCalledWith = sinon.assert.calledWith
+ObjectId = require("../../../../app/js/infrastructure/mongojs").ObjectId
+modulePath = "../../../../app/js/Features/UserMembership/UserMembershipsHandler"
+SandboxedModule = require("sandboxed-module")
+
+describe 'UserMembershipsHandler', ->
+	beforeEach ->
+		@user = _id: ObjectId()
+
+		@Institution =
+			updateMany: sinon.stub().yields(null)
+		@Subscription =
+			updateMany: sinon.stub().yields(null)
+		@Publisher =
+			updateMany: sinon.stub().yields(null)
+		@UserMembershipsHandler = SandboxedModule.require modulePath, requires:
+			'../../models/Institution': Institution: @Institution
+			'../../models/Subscription': Subscription: @Subscription
+			'../../models/Publisher': Publisher: @Publisher
+
+	describe 'remove user', ->
+		it 'remove user from all entities', (done) ->
+			@UserMembershipsHandler.removeUserFromAllEntities @user._id, (error) =>
+				assertCalledWith(
+					@Institution.updateMany,
+					{},
+					{ "$pull": { managerIds: @user._id } }
+				)
+				assertCalledWith(
+					@Subscription.updateMany,
+					{},
+					{ "$pull": { manager_ids: @user._id } }
+				)
+				assertCalledWith(
+					@Publisher.updateMany,
+					{},
+					{ "$pull": { managerIds: @user._id } }
+				)
+				done()