From a93c939dbc8805b7ec7e3b6da168b09115725c6a Mon Sep 17 00:00:00 2001 From: James Allen Date: Wed, 21 Sep 2016 10:11:35 +0100 Subject: [PATCH 1/4] Send invite email and notification in the background --- .../Features/Collaborators/CollaboratorsInviteHandler.coffee | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteHandler.coffee b/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteHandler.coffee index bf03fe242d..1ece15bf85 100644 --- a/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteHandler.coffee +++ b/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteHandler.coffee @@ -77,10 +77,12 @@ module.exports = CollaboratorsInviteHandler = if err? logger.err {err, projectId, sendingUserId: sendingUser._id, email}, "error saving token" return callback(err) + # Send email and notification in background CollaboratorsInviteHandler._sendMessages projectId, sendingUser, invite, (err) -> if err? logger.err {projectId, email}, "error sending messages for invite" - callback(err, invite) + callback(null, invite) + revokeInvite: (projectId, inviteId, callback=(err)->) -> logger.log {projectId, inviteId}, "removing invite" From bb7985208b27032c5fa2062f5882c6a1bffcac1b Mon Sep 17 00:00:00 2001 From: James Allen Date: Wed, 21 Sep 2016 10:48:04 +0100 Subject: [PATCH 2/4] Lower case email before considering if it's duplciated when sharing --- .../ShareProjectModalController.coffee | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/services/web/public/coffee/ide/share/controllers/ShareProjectModalController.coffee b/services/web/public/coffee/ide/share/controllers/ShareProjectModalController.coffee index 48c6440718..6f95d4e38f 100644 --- a/services/web/public/coffee/ide/share/controllers/ShareProjectModalController.coffee +++ b/services/web/public/coffee/ide/share/controllers/ShareProjectModalController.coffee @@ -79,19 +79,20 @@ define [ return member = members.shift() - if !member.type? and member.display in currentMemberEmails + if member.type == "user" + email = member.email + else # Not an auto-complete object, so email == display + email = member.display + email = email.toLowerCase() + + if email in currentMemberEmails # Skip this existing member return addNextMember() - # NOTE: groups aren't really a thing in ShareLaTeX, partially inherited from DJ - if member.display in currentInviteEmails and inviteId = _.find(($scope.project.invites || []), (invite) -> invite.email == member.display)?._id + if email in currentInviteEmails and inviteId = _.find(($scope.project.invites || []), (invite) -> invite.email == email)?._id request = projectInvites.resendInvite(inviteId) - else if member.type == "user" - request = projectInvites.sendInvite(member.email, $scope.inputs.privileges) - else if member.type == "group" - request = projectMembers.addGroup(member.id, $scope.inputs.privileges) - else # Not an auto-complete object, so email == display - request = projectInvites.sendInvite(member.display, $scope.inputs.privileges) + else + request = projectInvites.sendInvite(email, $scope.inputs.privileges) request .success (data) -> From d904e500410694988932ce792842114a6d03e1f1 Mon Sep 17 00:00:00 2001 From: James Allen Date: Wed, 21 Sep 2016 11:59:35 +0100 Subject: [PATCH 3/4] Make project invite notification dynamic and accept via ajax request Needs translations: "joining": "Joining", "notification_project_invite_message": "{{ userName }} would like you to join {{ projectName }}", "notification_project_invite_accepted_message": "You've joined {{ projectName }}", "open_project": "Open Project" --- .../CollaboratorsInviteController.coffee | 5 +++- .../app/views/project/list/notifications.jade | 27 +++++++++++++++---- .../notifications-controller.coffee | 21 +++++++++++++++ 3 files changed, 47 insertions(+), 6 deletions(-) diff --git a/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteController.coffee b/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteController.coffee index 4f8daf4bdb..051518a957 100644 --- a/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteController.coffee +++ b/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteController.coffee @@ -121,4 +121,7 @@ module.exports = CollaboratorsInviteController = return next(err) EditorRealTimeController.emitToRoom projectId, 'project:membership:changed', {invites: true, members: true} AnalyticsManger.recordEvent(currentUser._id, "project-invite-accept", {inviteId:inviteId, projectId:projectId}) - res.redirect "/project/#{projectId}" + if req.xhr + res.sendStatus 204 # Done async via project page notification + else + res.redirect "/project/#{projectId}" diff --git a/services/web/app/views/project/list/notifications.jade b/services/web/app/views/project/list/notifications.jade index 8dba0852ff..f4be585f7b 100644 --- a/services/web/app/views/project/list/notifications.jade +++ b/services/web/app/views/project/list/notifications.jade @@ -4,14 +4,31 @@ span(ng-controller="NotificationsController").userNotifications ng-cloak ) li.notification_entry( - ng-repeat="unreadNotification in notifications", + ng-repeat="notification in notifications", ) - .row(ng-hide="unreadNotification.hide") + .row(ng-hide="notification.hide") .col-xs-12 - .alert.alert-info + .alert.alert-info(ng-if="notification.templateKey == 'notification_project_invite'", ng-controller="ProjectInviteNotificationController") div.notification_inner - span(ng-bind-html="unreadNotification.html").notification_body + .notification_body(ng-show="!notification.accepted") + | !{translate("notification_project_invite_message")} + a.pull-right.btn.btn-sm.btn-info(href, ng-click="accept()", ng-disabled="notification.inflight") + span(ng-show="!notification.inflight") #{translate("join_project")} + span(ng-show="notification.inflight") + i.fa.fa-fw.fa-spinner.fa-spin + |   + | #{translate("joining")}... + .notification_body(ng-show="notification.accepted") + | !{translate("notification_project_invite_accepted_message")} + a.pull-right.btn.btn-sm.btn-info(href="/project/{{ notification.messageOpts.projectId }}") #{translate("open_project")} span().notification_close - button(ng-click="dismiss(unreadNotification)").close.pull-right + button(ng-click="dismiss(notification)").close.pull-right + span(aria-hidden="true") × + span.sr-only #{translate("close")} + .alert.alert-info(ng-if="notification.templateKey != 'notification_project_invite'") + div.notification_inner + span(ng-bind-html="notification.html").notification_body + span().notification_close + button(ng-click="dismiss(notification)").close.pull-right span(aria-hidden="true") × span.sr-only #{translate("close")} diff --git a/services/web/public/coffee/main/project-list/notifications-controller.coffee b/services/web/public/coffee/main/project-list/notifications-controller.coffee index 36a725f778..e3ec2f4c02 100644 --- a/services/web/public/coffee/main/project-list/notifications-controller.coffee +++ b/services/web/public/coffee/main/project-list/notifications-controller.coffee @@ -15,3 +15,24 @@ define [ }) .success (data) -> notification.hide = true + + App.controller "ProjectInviteNotificationController", ($scope, $http) -> + # Shortcuts for translation keys + $scope.projectName = $scope.notification.messageOpts.projectName + $scope.userName = $scope.notification.messageOpts.userName + + $scope.accept = () -> + $scope.notification.inflight = true + $http({ + url: "/project/#{$scope.notification.messageOpts.projectId}/invite/#{$scope.notification.messageOpts.token}/accept" + method: "POST" + headers: + "X-Csrf-Token": window.csrfToken + "X-Requested-With": "XMLHttpRequest" + }) + .success () -> + $scope.notification.inflight = false + $scope.notification.accepted = true + .error () -> + $scope.notification.inflight = false + $scope.notification.error = true \ No newline at end of file From e15976be21b119b6f6c369509ab9bc209574f2ad Mon Sep 17 00:00:00 2001 From: James Allen Date: Thu, 22 Sep 2016 17:24:06 +0100 Subject: [PATCH 4/4] Use token in URL to force its precense when invite and allow easy dynamic notifications --- .../CollaboratorsInviteController.coffee | 11 +++++------ .../Collaborators/CollaboratorsInviteHandler.coffee | 8 ++++---- .../Collaborators/CollaboratorsRouter.coffee | 2 +- services/web/app/views/project/invite/show.jade | 2 +- .../project-list/notifications-controller.coffee | 2 +- .../CollaboratorsInviteControllerTests.coffee | 12 ++++++------ .../CollaboratorsInviteHandlerTests.coffee | 2 +- 7 files changed, 19 insertions(+), 20 deletions(-) diff --git a/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteController.coffee b/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteController.coffee index 051518a957..7cbd09c28a 100644 --- a/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteController.coffee +++ b/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteController.coffee @@ -111,16 +111,15 @@ module.exports = CollaboratorsInviteController = acceptInvite: (req, res, next) -> projectId = req.params.Project_id - inviteId = req.params.invite_id - {token} = req.body + token = req.params.token currentUser = req.session.user - logger.log {projectId, inviteId, userId: currentUser._id}, "accepting invite" - CollaboratorsInviteHandler.acceptInvite projectId, inviteId, token, currentUser, (err) -> + logger.log {projectId, userId: currentUser._id, token}, "got request to accept invite" + CollaboratorsInviteHandler.acceptInvite projectId, token, currentUser, (err) -> if err? - logger.err {projectId, inviteId}, "error accepting invite by token" + logger.err {projectId, token}, "error accepting invite by token" return next(err) EditorRealTimeController.emitToRoom projectId, 'project:membership:changed', {invites: true, members: true} - AnalyticsManger.recordEvent(currentUser._id, "project-invite-accept", {inviteId:inviteId, projectId:projectId}) + AnalyticsManger.recordEvent(currentUser._id, "project-invite-accept", {projectId:projectId, userId:currentUser._id}) if req.xhr res.sendStatus 204 # Done async via project page notification else diff --git a/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteHandler.coffee b/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteHandler.coffee index 1ece15bf85..960bcff5c3 100644 --- a/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteHandler.coffee +++ b/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteHandler.coffee @@ -119,15 +119,15 @@ module.exports = CollaboratorsInviteHandler = return callback(null, null) callback(null, invite) - acceptInvite: (projectId, inviteId, tokenString, user, callback=(err)->) -> - logger.log {projectId, inviteId, userId: user._id}, "accepting invite" + acceptInvite: (projectId, tokenString, user, callback=(err)->) -> + logger.log {projectId, userId: user._id, tokenString}, "accepting invite" CollaboratorsInviteHandler.getInviteByToken projectId, tokenString, (err, invite) -> if err? - logger.err {err, projectId, inviteId}, "error finding invite" + logger.err {err, projectId, tokenString}, "error finding invite" return callback(err) if !invite err = new Errors.NotFoundError("no matching invite found") - logger.log {err, projectId, inviteId, tokenString}, "no matching invite found" + logger.log {err, projectId, tokenString}, "no matching invite found" return callback(err) inviteId = invite._id CollaboratorsHandler.addUserIdToProject projectId, invite.sendingUserId, user._id, invite.privileges, (err) -> diff --git a/services/web/app/coffee/Features/Collaborators/CollaboratorsRouter.coffee b/services/web/app/coffee/Features/Collaborators/CollaboratorsRouter.coffee index 7fc4722ef2..4c7cc8c76a 100644 --- a/services/web/app/coffee/Features/Collaborators/CollaboratorsRouter.coffee +++ b/services/web/app/coffee/Features/Collaborators/CollaboratorsRouter.coffee @@ -66,7 +66,7 @@ module.exports = ) webRouter.post( - '/project/:Project_id/invite/:invite_id/accept', + '/project/:Project_id/invite/token/:token/accept', AuthenticationController.requireLogin(), CollaboratorsInviteController.acceptInvite ) diff --git a/services/web/app/views/project/invite/show.jade b/services/web/app/views/project/invite/show.jade index 833e75085e..eed30d3d19 100644 --- a/services/web/app/views/project/invite/show.jade +++ b/services/web/app/views/project/invite/show.jade @@ -20,7 +20,7 @@ block content form.form( name="acceptForm", method="POST", - action="/project/#{invite.projectId}/invite/#{invite._id}/accept" + action="/project/#{invite.projectId}/invite/token/#{invite.token}/accept" ) input(name='_csrf', type='hidden', value=csrfToken) input(name='token', type='hidden', value="#{invite.token}") diff --git a/services/web/public/coffee/main/project-list/notifications-controller.coffee b/services/web/public/coffee/main/project-list/notifications-controller.coffee index e3ec2f4c02..c9f2d0c68b 100644 --- a/services/web/public/coffee/main/project-list/notifications-controller.coffee +++ b/services/web/public/coffee/main/project-list/notifications-controller.coffee @@ -24,7 +24,7 @@ define [ $scope.accept = () -> $scope.notification.inflight = true $http({ - url: "/project/#{$scope.notification.messageOpts.projectId}/invite/#{$scope.notification.messageOpts.token}/accept" + url: "/project/#{$scope.notification.messageOpts.projectId}/invite/token/#{$scope.notification.messageOpts.token}/accept" method: "POST" headers: "X-Csrf-Token": window.csrfToken diff --git a/services/web/test/UnitTests/coffee/Collaborators/CollaboratorsInviteControllerTests.coffee b/services/web/test/UnitTests/coffee/Collaborators/CollaboratorsInviteControllerTests.coffee index 0144583f05..f1e1634ca1 100644 --- a/services/web/test/UnitTests/coffee/Collaborators/CollaboratorsInviteControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Collaborators/CollaboratorsInviteControllerTests.coffee @@ -531,14 +531,12 @@ describe "CollaboratorsInviteController", -> beforeEach -> @req.params = Project_id: @project_id - invite_id: @invite_id = "thuseoautoh" + token: @token = "mock-token" @req.session = user: _id: @current_user_id = "current-user-id" - @req.body = - token: "thsueothaueotauahsuet" @res.render = sinon.stub() @res.redirect = sinon.stub() - @CollaboratorsInviteHandler.acceptInvite = sinon.stub().callsArgWith(4, null) + @CollaboratorsInviteHandler.acceptInvite = sinon.stub().callsArgWith(3, null) @callback = sinon.stub() @next = sinon.stub() @@ -552,7 +550,9 @@ describe "CollaboratorsInviteController", -> @res.redirect.calledWith("/project/#{@project_id}").should.equal true it 'should have called acceptInvite', -> - @CollaboratorsInviteHandler.acceptInvite.callCount.should.equal 1 + @CollaboratorsInviteHandler.acceptInvite + .calledWith(@project_id, @token) + .should.equal true it 'should have called emitToRoom', -> @EditorRealTimeController.emitToRoom.callCount.should.equal 1 @@ -562,7 +562,7 @@ describe "CollaboratorsInviteController", -> beforeEach -> @err = new Error('woops') - @CollaboratorsInviteHandler.acceptInvite = sinon.stub().callsArgWith(4, @err) + @CollaboratorsInviteHandler.acceptInvite = sinon.stub().callsArgWith(3, @err) @CollaboratorsInviteController.acceptInvite @req, @res, @next it 'should not redirect to project page', -> diff --git a/services/web/test/UnitTests/coffee/Collaborators/CollaboratorsInviteHandlerTests.coffee b/services/web/test/UnitTests/coffee/Collaborators/CollaboratorsInviteHandlerTests.coffee index d4ae9a4229..ac94fcf10d 100644 --- a/services/web/test/UnitTests/coffee/Collaborators/CollaboratorsInviteHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/Collaborators/CollaboratorsInviteHandlerTests.coffee @@ -404,7 +404,7 @@ describe "CollaboratorsInviteHandler", -> @CollaboratorsInviteHandler._tryCancelInviteNotification = sinon.stub().callsArgWith(1, null) @ProjectInvite.remove.callsArgWith(1, null) @call = (callback) => - @CollaboratorsInviteHandler.acceptInvite @projectId, @inviteId, @token, @user, callback + @CollaboratorsInviteHandler.acceptInvite @projectId, @token, @user, callback afterEach -> @_getInviteByToken.restore()