Fix track-changes with token-access

services/web/app/coffee/Features/Collaborators/CollaboratorsHandler.coffee

@@ -102,6 +102,13 @@ module.exports = CollaboratorsHandler =
 			CollaboratorsHandler._loadMembers members, (error, users) ->
 				callback error, users
 
+	getTokenMembersWithPrivilegeLevels: (project_id, callback = (error, members) ->) ->
+		CollaboratorsHandler.getMemberIdsWithPrivilegeLevels project_id, (error, members = []) ->
+			return callback(error) if error?
+			members = members.filter((m) -> m.source == Sources.TOKEN)
+			CollaboratorsHandler._loadMembers members, (error, users) ->
+				callback error, users
+
 	getMemberIdPrivilegeLevel: (user_id, project_id, callback = (error, privilegeLevel) ->) ->
 		# In future if the schema changes and getting all member ids is more expensive (multiple documents)
 		# then optimise this.

services/web/app/coffee/Features/Editor/EditorHttpController.coffee

@@ -41,21 +41,23 @@ module.exports = EditorHttpController =
 			return callback(new Error("not found")) if !project?
 			CollaboratorsHandler.getInvitedMembersWithPrivilegeLevels project, (error, members) ->
 				return callback(error) if error?
-				UserGetter.getUser user_id, { isAdmin: true }, (error, user) ->
+				CollaboratorsHandler.getTokenMembersWithPrivilegeLevels project, (error, tokenMembers) ->
 					return callback(error) if error?
-					token = TokenAccessHandler.getRequestToken(req, project_id)
-					AuthorizationManager.getPrivilegeLevelForProject user_id, project_id, token, (error, privilegeLevel) ->
+					UserGetter.getUser user_id, { isAdmin: true }, (error, user) ->
 						return callback(error) if error?
-						if !privilegeLevel? or privilegeLevel == PrivilegeLevels.NONE
-							logger.log {project_id, user_id, privilegeLevel}, "not an acceptable privilege level, returning null"
-							return callback null, null, false
-						CollaboratorsInviteHandler.getAllInvites project_id, (error, invites) ->
+						token = TokenAccessHandler.getRequestToken(req, project_id)
+						AuthorizationManager.getPrivilegeLevelForProject user_id, project_id, token, (error, privilegeLevel) ->
 							return callback(error) if error?
-							logger.log {project_id, user_id, memberCount: members.length, inviteCount: invites.length, privilegeLevel}, "returning project model view"
-							callback(null,
-								ProjectEditorHandler.buildProjectModelView(project, members, invites),
-								privilegeLevel
-							)
+							if !privilegeLevel? or privilegeLevel == PrivilegeLevels.NONE
+								logger.log {project_id, user_id, privilegeLevel}, "not an acceptable privilege level, returning null"
+								return callback null, null, false
+							CollaboratorsInviteHandler.getAllInvites project_id, (error, invites) ->
+								return callback(error) if error?
+								logger.log {project_id, user_id, memberCount: members.length, inviteCount: invites.length, privilegeLevel}, "returning project model view"
+								callback(null,
+									ProjectEditorHandler.buildProjectModelView(project, members, invites, tokenMembers),
+									privilegeLevel
+								)
 
 	restoreDoc: (req, res, next) ->
 		project_id = req.params.Project_id

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

@@ -3,7 +3,7 @@ _ = require("underscore")
 module.exports = ProjectEditorHandler =
 	trackChangesAvailable: false
 
-	buildProjectModelView: (project, members, invites) ->
+	buildProjectModelView: (project, members, invites, tokenMembers=[]) ->
 		result =
 			_id        : project._id
 			name       : project.name
@@ -17,15 +17,20 @@ module.exports = ProjectEditorHandler =
 			deletedByExternalDataSource : project.deletedByExternalDataSource || false
 			deletedDocs: project.deletedDocs
 			members:     []
+			tokenMembers: []
 			invites:     invites
 			tokens:      project.tokens
 
 		if !result.invites?
 			result.invites = []
-			
-		{owner, ownerFeatures, members} = @buildOwnerAndMembersViews(members)
+
+		{owner, ownerFeatures, members, tokenMembers} = @buildOwnerAndMembersViews(
+			members,
+			tokenMembers
+		)
 		result.owner = owner
 		result.members = members
+		result.tokenMembers = tokenMembers
 
 		result.features = _.defaults(ownerFeatures or {}, {
 			collaborators: -1 # Infinite
@@ -41,17 +46,25 @@ module.exports = ProjectEditorHandler =
 
 		return result
 
-	buildOwnerAndMembersViews: (members) ->
+	buildOwnerAndMembersViews: (members, tokenMembers) ->
 		owner = null
 		ownerFeatures = null
 		filteredMembers = []
+		filteredTokenMembers = []
 		for member in members
 			if member.privilegeLevel == "owner"
 				ownerFeatures = member.user.features
 				owner = @buildUserModelView member.user, "owner"
 			else
 				filteredMembers.push @buildUserModelView member.user, member.privilegeLevel
-		{owner: owner, ownerFeatures: ownerFeatures, members: filteredMembers}
+		for tokenMember in tokenMembers
+			filteredTokenMembers.push @buildUserModelView tokenMember.user, tokenMember.privilegeLevel
+		return {
+			owner: owner,
+			ownerFeatures: ownerFeatures,
+			members: filteredMembers,
+			tokenMembers: filteredTokenMembers
+		}
 
 	buildUserModelView: (user, privileges) ->
 		_id        : user._id

services/web/app/views/project/editor/review-panel.pug

@@ -80,6 +80,19 @@
 					on-toggle="toggleTrackChangesForUser(isOn, member.id);"
 					disabled="reviewPanel.trackChangesOnForEveryone || !project.features.trackChanges || !permissions.write"
 				)
+			li.rp-tc-state-separator
+			li.rp-tc-state-item(
+				ng-repeat="member in reviewPanel.formattedProjectTokenMembers"
+			)
+				span.rp-tc-state-item-name(
+					ng-class="{ 'rp-tc-state-item-name-disabled' : reviewPanel.trackChangesOnForEveryone}"
+					style="color: hsl({{ member.hue }}, 70%, 40%);"
+				) {{ member.name }}
+				review-panel-toggle(
+					ng-model="reviewPanel.trackChangesState[member.id].value"
+					on-toggle="toggleTrackChangesForUser(isOn, member.id);"
+					disabled="reviewPanel.trackChangesOnForEveryone || !project.features.trackChanges || !permissions.write"
+				)
 
 	.rp-entry-list(
 		review-panel-sorted

services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee

@@ -32,6 +32,7 @@ define [
 			resolvedThreadIds: {}
 			rendererData: {}
 			formattedProjectMembers: {}
+			formattedProjectTokenMembers: {}
 			fullTCStateCollapsed: true
 			loadingThreads: false
 			# All selected changes. If a aggregated change (insertion + deletion) is selection, the two ids
@@ -77,6 +78,13 @@ define [
 					if member.privileges == "readAndWrite"
 						$scope.reviewPanel.formattedProjectMembers[member._id] = formatUser(member)
 
+		$scope.$watch "project.tokenMembers", (tokenMembers) ->
+			$scope.reviewPanel.formattedProjectTokenMembers = {}
+			if $scope.project?.tokenMembers?
+				for member in tokenMembers
+					if member.privileges == "readAndWrite"
+						$scope.reviewPanel.formattedProjectTokenMembers[member._id] = formatUser(member)
+
 		$scope.commentState =
 			adding: false
 			content: ""
@@ -612,9 +620,10 @@ define [
 
 		_setEveryoneTCState = (newValue, isLocal = false) ->
 			$scope.reviewPanel.trackChangesOnForEveryone = newValue
-			for member in $scope.project.members
+			project = $scope.project
+			for member in project.members.concat(project.tokenMembers)
 				_setUserTCState(member._id, newValue, isLocal)
-			_setUserTCState($scope.project.owner._id, newValue, isLocal)
+			_setUserTCState(project.owner._id, newValue, isLocal)
 
 		applyClientTrackChangesStateToServer = () ->
 			if $scope.reviewPanel.trackChangesOnForEveryone
@@ -630,8 +639,9 @@ define [
 			if typeof state is "boolean"
 				_setEveryoneTCState state
 			else
+				project = $scope.project
 				$scope.reviewPanel.trackChangesOnForEveryone = false
-				for member in $scope.project.members
+				for member in project.members.concat(project.tokenMembers)
 					_setUserTCState(member._id, state[member._id] ? false)
 				_setUserTCState($scope.project.owner._id, state[$scope.project.owner._id] ? false)
 		

services/web/public/stylesheets/app/editor/review-panel.less

@@ -188,6 +188,11 @@
 				padding-bottom: 5px;
 			}
 		}
+		.rp-tc-state-separator {
+			border-bottom: 1px solid @rp-border-grey;
+			// margin-top: 4px;
+			// margin-bottom: 4px;
+		}
 		.rp-tc-state-item-everyone {
 			border-bottom: 1px solid @rp-border-grey;
 			color: @red;

services/web/test/UnitTests/coffee/Collaborators/CollaboratorsHandlerTests.coffee

@@ -141,6 +141,32 @@ describe "CollaboratorsHandler", ->
 				])
 				.should.equal true
 
+	describe "getTokenMembersWithPrivilegeLevels", ->
+		beforeEach ->
+			@CollaboratorHandler.getMemberIdsWithPrivilegeLevels = sinon.stub()
+			@CollaboratorHandler.getMemberIdsWithPrivilegeLevels.withArgs(@project_id).yields(null, [
+				{ id: "read-only-ref-1", privilegeLevel: "readOnly", source: 'token' }
+				{ id: "read-only-ref-2", privilegeLevel: "readOnly", source: 'invite' }
+				{ id: "read-write-ref-1", privilegeLevel: "readAndWrite", source: 'token' }
+				{ id: "read-write-ref-2", privilegeLevel: "readAndWrite", source: 'invite' }
+				{ id: "doesnt-exist", privilegeLevel: "readAndWrite", source: 'invite' }
+			])
+			@UserGetter.getUserOrUserStubById = sinon.stub()
+			@UserGetter.getUserOrUserStubById.withArgs("read-only-ref-1").yields(null, { _id: "read-only-ref-1" })
+			@UserGetter.getUserOrUserStubById.withArgs("read-only-ref-2").yields(null, { _id: "read-only-ref-2" })
+			@UserGetter.getUserOrUserStubById.withArgs("read-write-ref-1").yields(null, { _id: "read-write-ref-1" })
+			@UserGetter.getUserOrUserStubById.withArgs("read-write-ref-2").yields(null, { _id: "read-write-ref-2" })
+			@UserGetter.getUserOrUserStubById.withArgs("doesnt-exist").yields(null, null)
+			@CollaboratorHandler.getTokenMembersWithPrivilegeLevels @project_id, @callback
+
+		it "should return an array of token members with their privilege levels", ->
+			@callback
+				.calledWith(null, [
+					{ user: { _id: "read-only-ref-1" }, privilegeLevel: "readOnly" }
+					{ user: { _id: "read-write-ref-1" }, privilegeLevel: "readAndWrite"}
+				])
+				.should.equal true
+
 	describe "getMemberIdPrivilegeLevel", ->
 		beforeEach ->
 			@CollaboratorHandler.getMemberIdsWithPrivilegeLevels = sinon.stub()

services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee

@@ -101,6 +101,7 @@ describe "EditorHttpController", ->
 				_id: @user_id = "user-id"
 				projects: {}
 			@members = ["members", "mock"]
+			@tokenMembers = ['one', 'two']
 			@projectModelView = 
 				_id: @project_id
 				owner:{_id:"something"}
@@ -112,6 +113,7 @@ describe "EditorHttpController", ->
 			@ProjectEditorHandler.buildProjectModelView = sinon.stub().returns(@projectModelView)
 			@ProjectGetter.getProjectWithoutDocLines = sinon.stub().callsArgWith(1, null, @project)
 			@CollaboratorsHandler.getInvitedMembersWithPrivilegeLevels = sinon.stub().callsArgWith(1, null, @members)
+			@CollaboratorsHandler.getTokenMembersWithPrivilegeLevels = sinon.stub().callsArgWith(1, null, @tokenMembers)
 			@CollaboratorsInviteHandler.getAllInvites = sinon.stub().callsArgWith(1, null, @invites)
 			@UserGetter.getUser = sinon.stub().callsArgWith(2, null, @user)
 				
@@ -131,6 +133,11 @@ describe "EditorHttpController", ->
 					.calledWith(@project)
 					.should.equal true
 			
+			it "should get the list of users who access the project via token links", ->
+				@CollaboratorsHandler.getTokenMembersWithPrivilegeLevels
+					.calledWith(@project)
+					.should.equal true
+
 			it "should look up the user", ->
 				@UserGetter.getUser
 					.calledWith(@user_id, { isAdmin: true })

services/web/test/UnitTests/coffee/Project/ProjectEditorHandlerTests.coffee

@@ -68,6 +68,15 @@ describe "ProjectEditorHandler", ->
 			},
 			privilegeLevel: "readAndWrite"
 		}]
+		@tokenMembers = [{
+			user: {
+				_id: "token-read-only-id"
+				first_name : "TRead"
+				last_name  : "TOnly"
+				email      : "token-read-only@sharelatex.com"
+			},
+			privilegeLevel: "readOnly"
+		}]
 		@invites = [
 			{_id: "invite_one", email: "user-one@example.com", privileges: "readOnly", projectId: @project._id}
 			{_id: "invite_two", email: "user-two@example.com", privileges: "readOnly", projectId: @project._id}
@@ -77,7 +86,7 @@ describe "ProjectEditorHandler", ->
 	describe "buildProjectModelView", ->
 		describe "with owner and members included", ->
 			beforeEach ->
-				@result = @handler.buildProjectModelView @project, @members, @invites
+				@result = @handler.buildProjectModelView @project, @members, @invites, @tokenMembers
 
 			it "should include the id", ->
 				should.exist @result._id
@@ -127,6 +136,10 @@ describe "ProjectEditorHandler", ->
 				findMember("read-write-id").last_name.should.equal "Write"
 				findMember("read-write-id").email.should.equal "read-write@sharelatex.com"
 
+			it 'should include a list of tokenMembers', ->
+				@result.tokenMembers.length.should.equal 1
+				@result.tokenMembers[0]._id.should.equal @tokenMembers[0].user._id
+
 			it "should include folders in the project", ->
 				@result.rootFolder[0]._id.should.equal "root-folder-id"
 				@result.rootFolder[0].name.should.equal ""
@@ -157,16 +170,16 @@ describe "ProjectEditorHandler", ->
 
 			it "should set the deletedByExternalDataSource flag to false when it is not there", ->
 				delete @project.deletedByExternalDataSource
-				result = @handler.buildProjectModelView @project, @members
+				result = @handler.buildProjectModelView @project, @members, [], []
 				result.deletedByExternalDataSource.should.equal false
 
 			it "should set the deletedByExternalDataSource flag to false when it is false", ->
-				result = @handler.buildProjectModelView @project, @members
+				result = @handler.buildProjectModelView @project, @members, [], []
 				result.deletedByExternalDataSource.should.equal false
 
 			it "should set the deletedByExternalDataSource flag to true when it is true", ->
 				@project.deletedByExternalDataSource = true
-				result = @handler.buildProjectModelView @project, @members
+				result = @handler.buildProjectModelView @project, @members, [], []
 				result.deletedByExternalDataSource.should.equal true
 
 		describe "features", ->
@@ -176,7 +189,7 @@ describe "ProjectEditorHandler", ->
 					collaborators: 3
 					compileGroup:"priority"
 					compileTimeout: 96
-				@result = @handler.buildProjectModelView @project, @members
+				@result = @handler.buildProjectModelView @project, @members, [], []
 
 			it "should copy the owner features to the project", ->
 				@result.features.versioning.should.equal @owner.features.versioning
@@ -191,10 +204,10 @@ describe "ProjectEditorHandler", ->
 				collaborators: 3
 				compileGroup:"priority"
 				compileTimeout: 22
-			@result = @handler.buildOwnerAndMembersViews @members
+			@result = @handler.buildOwnerAndMembersViews @members, @tokenMembers
 
-		it 'should produce an object with owner, ownerFeatures and members keys', ->
-			expect(@result).to.have.all.keys ['owner', 'ownerFeatures', 'members']
+		it 'should produce an object with the right keys', ->
+			expect(@result).to.have.all.keys ['owner', 'ownerFeatures', 'members', 'tokenMembers']
 
 		it 'should separate the owner from the members', ->
 			@result.members.length.should.equal(@members.length-1)
@@ -202,6 +215,10 @@ describe "ProjectEditorHandler", ->
 			expect(@result.owner.email).to.equal @owner.email
 			expect(@result.members.filter((m) => m._id == @owner._id).length).to.equal 0
 
+		it 'should include a list of tokenMembers', ->
+			@result.tokenMembers.length.should.equal 1
+			@result.tokenMembers[0]._id.should.equal @tokenMembers[0].user._id
+
 		it 'should extract the ownerFeatures from the owner object', ->
 			expect(@result.ownerFeatures).to.deep.equal @owner.features
 
@@ -209,10 +226,10 @@ describe "ProjectEditorHandler", ->
 			beforeEach ->
 				# remove the owner from members list
 				@membersWithoutOwner = @members.filter((m) => m.user._id != @owner._id)
-				@result = @handler.buildOwnerAndMembersViews @membersWithoutOwner
+				@result = @handler.buildOwnerAndMembersViews @membersWithoutOwner, @tokenMembers
 
-			it 'should produce an object with owner, ownerFeatures and members keys', ->
-				expect(@result).to.have.all.keys ['owner', 'ownerFeatures', 'members']
+			it 'should produce an object with the right keys', ->
+				expect(@result).to.have.all.keys ['owner', 'ownerFeatures', 'members', 'tokenMembers']
 
 			it 'should not separate out an owner', ->
 				@result.members.length.should.equal @membersWithoutOwner.length