diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionGroupController.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionGroupController.coffee index cb87355359..55ad4703c5 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionGroupController.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionGroupController.coffee @@ -1,8 +1,6 @@ SubscriptionGroupHandler = require("./SubscriptionGroupHandler") logger = require("logger-sharelatex") SubscriptionLocator = require("./SubscriptionLocator") -ErrorsController = require("../Errors/ErrorController") -SubscriptionDomainHandler = require("./SubscriptionDomainHandler") AuthenticationController = require('../Authentication/AuthenticationController') _ = require("underscore") async = require("async") @@ -10,16 +8,14 @@ async = require("async") module.exports = removeUserFromGroup: (req, res, next)-> - adminUserId = AuthenticationController.getLoggedInUserId(req) + subscription = req.entity userToRemove_id = req.params.user_id - getManagedSubscription adminUserId, (error, subscription) -> - return next(error) if error? - logger.log adminUserId:adminUserId, userToRemove_id:userToRemove_id, "removing user from group subscription" - SubscriptionGroupHandler.removeUserFromGroup subscription._id, userToRemove_id, (err)-> - if err? - logger.err err:err, adminUserId:adminUserId, userToRemove_id:userToRemove_id, "error removing user from group" - return res.sendStatus 500 - res.send() + logger.log subscriptionId: subscription._id, userToRemove_id:userToRemove_id, "removing user from group subscription" + SubscriptionGroupHandler.removeUserFromGroup subscription._id, userToRemove_id, (err)-> + if err? + logger.err err:err, adminUserId:adminUserId, userToRemove_id:userToRemove_id, "error removing user from group" + return next(err) + res.send() removeSelfFromGroup: (req, res, next)-> adminUserId = req.query.admin_user_id @@ -33,24 +29,6 @@ module.exports = return res.sendStatus 500 res.send() - exportGroupCsv: (req, res, next)-> - user_id = AuthenticationController.getLoggedInUserId(req) - logger.log user_id: user_id, "exporting group csv" - getManagedSubscription user_id, (err, subscription)-> - return next(error) if error? - if !subscription.groupPlan - return res.redirect("/") - SubscriptionGroupHandler.getPopulatedListOfMembers subscription._id, (err, users)-> - groupCsv = "" - for user in users - groupCsv += user.email + "\n" - res.header( - "Content-Disposition", - "attachment; filename=Group.csv" - ) - res.contentType('text/csv') - res.send(groupCsv) - # legacy route redirectToSubscriptionGroupAdminPage: (req, res, next) -> user_id = AuthenticationController.getLoggedInUserId(req) diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee index 53f4c05cf8..1effdf4504 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee @@ -7,7 +7,6 @@ Subscription = require("../../models/Subscription").Subscription LimitationsManager = require("./LimitationsManager") logger = require("logger-sharelatex") OneTimeTokenHandler = require("../Security/OneTimeTokenHandler") -TeamInvitesHandler = require("./TeamInvitesHandler") EmailHandler = require("../Email/EmailHandler") settings = require("settings-sharelatex") NotificationsBuilder = require("../Notifications/NotificationsBuilder") diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee index b2f9bddfdd..e8c9b916a2 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee @@ -21,19 +21,13 @@ module.exports = webRouter.get '/subscription/group', AuthenticationController.requireLogin(), SubscriptionGroupController.redirectToSubscriptionGroupAdminPage - webRouter.get '/subscription/group/export', AuthenticationController.requireLogin(), SubscriptionGroupController.exportGroupCsv - webRouter.delete '/subscription/group/user/:user_id', AuthenticationController.requireLogin(), SubscriptionGroupController.removeUserFromGroup webRouter.delete '/subscription/group/user', AuthenticationController.requireLogin(), SubscriptionGroupController.removeSelfFromGroup # Team invites - webRouter.post '/subscription/invites', AuthenticationController.requireLogin(), - TeamInvitesController.createInvite webRouter.get '/subscription/invites/:token/', AuthenticationController.requireLogin(), TeamInvitesController.viewInvite webRouter.put '/subscription/invites/:token/', AuthenticationController.requireLogin(), TeamInvitesController.acceptInvite - webRouter.delete '/subscription/invites/:email/', AuthenticationController.requireLogin(), - TeamInvitesController.revokeInvite # Routes to join a domain licence team webRouter.get '/user/subscription/domain/join', AuthenticationController.requireLogin(), DomainLicenceController.join diff --git a/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee b/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee index 3fdd7e3e56..2ebca7682f 100644 --- a/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee +++ b/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee @@ -9,11 +9,12 @@ EmailHelper = require("../Helpers/EmailHelper") module.exports = createInvite: (req, res, next) -> teamManagerId = AuthenticationController.getLoggedInUserId(req) + subscription = req.entity email = EmailHelper.parseEmail(req.body.email) if !email? return res.sendStatus(400) - TeamInvitesHandler.createInvite teamManagerId, email, (err, invite) -> + TeamInvitesHandler.createInvite teamManagerId, subscription, email, (err, invite) -> return next(err) if err? inviteView = { user: { email: invite.email, sentAt: invite.sentAt, invite: true } @@ -48,11 +49,12 @@ module.exports = res.sendStatus 204 revokeInvite: (req, res) -> + subscription = req.entity email = EmailHelper.parseEmail(req.params.email) teamManagerId = AuthenticationController.getLoggedInUserId(req) if !email? return res.sendStatus(400) - TeamInvitesHandler.revokeInvite teamManagerId, email, (err, results) -> + TeamInvitesHandler.revokeInvite teamManagerId, subscription, email, (err, results) -> return next(err) if err? res.sendStatus 204 diff --git a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee index b089f943ce..2051a9282f 100644 --- a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee @@ -27,24 +27,21 @@ module.exports = TeamInvitesHandler = invite = subscription.teamInvites.find (i) -> i.token == token return callback(null, invite, subscription) - createInvite: (teamManagerId, email, callback) -> + createInvite: (teamManagerId, subscription, email, callback) -> email = EmailHelper.parseEmail(email) return callback(new Error('invalid email')) if !email? logger.log {teamManagerId, email}, "Creating manager team invite" UserGetter.getUser teamManagerId, (error, teamManager) -> return callback(error) if error? - SubscriptionLocator.getUsersSubscription teamManagerId, (error, subscription) -> + if teamManager.first_name and teamManager.last_name + inviterName = "#{teamManager.first_name} #{teamManager.last_name} (#{teamManager.email})" + else + inviterName = teamManager.email + + removeLegacyInvite subscription.id, email, (error) -> return callback(error) if error? - - if teamManager.first_name and teamManager.last_name - inviterName = "#{teamManager.first_name} #{teamManager.last_name} (#{teamManager.email})" - else - inviterName = teamManager.email - - removeLegacyInvite subscription.id, email, (error) -> - return callback(error) if error? - createInvite(subscription, email, inviterName, callback) + createInvite(subscription, email, inviterName, callback) createDomainInvite: (user, licence, callback) -> email = EmailHelper.parseEmail(user.email) @@ -83,14 +80,11 @@ module.exports = TeamInvitesHandler = removeInviteFromTeam(subscription.id, invite.email, callback) - revokeInvite: (teamManagerId, email, callback) -> + revokeInvite: (teamManagerId, subscription, email, callback) -> email = EmailHelper.parseEmail(email) return callback(new Error('invalid email')) if !email? logger.log {teamManagerId, email}, "Revoking invite" - SubscriptionLocator.getUsersSubscription teamManagerId, (err, teamSubscription) -> - return callback(err) if err? - - removeInviteFromTeam(teamSubscription.id, email, callback) + removeInviteFromTeam(subscription.id, email, callback) # Legacy method to allow a user to receive a confirmation email if their # email is in Subscription.invited_emails when they join. We'll remove this @@ -100,7 +94,7 @@ module.exports = TeamInvitesHandler = return callback(err) if err? async.map teams, - (team, cb) -> TeamInvitesHandler.createInvite(team.admin_id, email, cb) + (team, cb) -> TeamInvitesHandler.createInvite(team.admin_id, team, email, cb) , callback createInvite = (subscription, email, inviterName, callback) -> diff --git a/services/web/app/coffee/Features/UserMembership/UserMembershipController.coffee b/services/web/app/coffee/Features/UserMembership/UserMembershipController.coffee index 1f5ef5d42d..654d3638de 100644 --- a/services/web/app/coffee/Features/UserMembership/UserMembershipController.coffee +++ b/services/web/app/coffee/Features/UserMembership/UserMembershipController.coffee @@ -38,3 +38,18 @@ module.exports = UserMembershipHandler.removeUser entity, entityConfig, userId, (error, user)-> return next(error) if error? res.send() + + exportCsv: (req, res, next)-> + { entity, entityConfig } = req + logger.log subscriptionId: entity._id, "exporting csv" + UserMembershipHandler.getUsers entity, entityConfig, (error, users)-> + return next(error) if error? + csvOutput = "" + for user in users + csvOutput += user.email + "\n" + res.header( + "Content-Disposition", + "attachment; filename=Group.csv" + ) + res.contentType('text/csv') + res.send(csvOutput) diff --git a/services/web/app/coffee/Features/UserMembership/UserMembershipEntityConfigs.coffee b/services/web/app/coffee/Features/UserMembership/UserMembershipEntityConfigs.coffee index 03e4bdf5ef..fb843fa4d9 100644 --- a/services/web/app/coffee/Features/UserMembership/UserMembershipEntityConfigs.coffee +++ b/services/web/app/coffee/Features/UserMembership/UserMembershipEntityConfigs.coffee @@ -13,11 +13,11 @@ module.exports = translations: title: 'group_account' remove: 'remove_from_group' - pathsFor: () -> - addMember: '/subscription/invites' - removeMember: '/subscription/group/user' - removeInvite: '/subscription/invites' - exportMembers: '/subscription/group/export' + pathsFor: (id) -> + addMember: "/manage/groups/#{id}/invites" + removeMember: "/manage/groups/#{id}/user" + removeInvite: "/manage/groups/#{id}/invites" + exportMembers: "/manage/groups/#{id}/members/export" team: # for metrics only modelName: 'Subscription' diff --git a/services/web/app/coffee/Features/UserMembership/UserMembershipRouter.coffee b/services/web/app/coffee/Features/UserMembership/UserMembershipRouter.coffee index 79f9401add..c53a5af7c7 100644 --- a/services/web/app/coffee/Features/UserMembership/UserMembershipRouter.coffee +++ b/services/web/app/coffee/Features/UserMembership/UserMembershipRouter.coffee @@ -1,11 +1,25 @@ UserMembershipAuthorization = require './UserMembershipAuthorization' UserMembershipController = require './UserMembershipController' +SubscriptionGroupController = require '../Subscription/SubscriptionGroupController' +TeamInvitesController = require '../Subscription/TeamInvitesController' module.exports = apply: (webRouter) -> webRouter.get '/manage/groups/:id/members', UserMembershipAuthorization.requireEntityAccess('group'), UserMembershipController.index + webRouter.post '/manage/groups/:id/invites', + UserMembershipAuthorization.requireEntityAccess('group'), + TeamInvitesController.createInvite + webRouter.delete '/manage/groups/:id/user/:user_id', + UserMembershipAuthorization.requireEntityAccess('group'), + SubscriptionGroupController.removeUserFromGroup + webRouter.delete '/manage/groups/:id/invites/:email', + UserMembershipAuthorization.requireEntityAccess('group'), + TeamInvitesController.revokeInvite + webRouter.get '/manage/groups/:id/members/export', + UserMembershipAuthorization.requireEntityAccess('group'), + UserMembershipController.exportCsv regularEntitites = diff --git a/services/web/test/unit/coffee/Subscription/SubscriptionGroupControllerTests.coffee b/services/web/test/unit/coffee/Subscription/SubscriptionGroupControllerTests.coffee index 6c77cda1b8..d4ee92f4ea 100644 --- a/services/web/test/unit/coffee/Subscription/SubscriptionGroupControllerTests.coffee +++ b/services/web/test/unit/coffee/Subscription/SubscriptionGroupControllerTests.coffee @@ -20,13 +20,14 @@ describe "SubscriptionGroupController", -> params: subscriptionId:@subscriptionId query:{} + @subscription = { _id: @subscriptionId } + @GroupHandler = removeUserFromGroup: sinon.stub().callsArgWith(2) - isUserPartOfGroup: sinon.stub() - getPopulatedListOfMembers: sinon.stub().callsArgWith(1, null, [@user]) + @SubscriptionLocator = findManagedSubscription: sinon.stub().callsArgWith(1, null, @subscription) @@ -34,62 +35,21 @@ describe "SubscriptionGroupController", -> getLoggedInUserId: (req) -> req.session.user._id getSessionUser: (req) -> req.session.user - @SubscriptionDomainHandler = - findDomainLicenceBySubscriptionId:sinon.stub() - - @OneTimeTokenHandler = - getValueFromTokenAndExpire:sinon.stub() - - - @ErrorsController = - notFound:sinon.stub() - @Controller = SandboxedModule.require modulePath, requires: "./SubscriptionGroupHandler":@GroupHandler "logger-sharelatex": log:-> "./SubscriptionLocator": @SubscriptionLocator - "./SubscriptionDomainHandler":@SubscriptionDomainHandler - "../Errors/ErrorController":@ErrorsController '../Authentication/AuthenticationController': @AuthenticationController - @token = "super-secret-token" - - describe "removeUserFromGroup", -> it "should use the subscription id for the logged in user and take the user id from the params", (done)-> userIdToRemove = "31231" @req.params = user_id: userIdToRemove + @req.entity = @subscription res = send : => @GroupHandler.removeUserFromGroup.calledWith(@subscriptionId, userIdToRemove).should.equal true done() @Controller.removeUserFromGroup @req, res - - describe "exportGroupCsv", -> - - beforeEach -> - @subscription.groupPlan = true - @res = new MockResponse() - @res.contentType = sinon.stub() - @res.header = sinon.stub() - @res.send = sinon.stub() - @Controller.exportGroupCsv @req, @res - - it "should set the correct content type on the request", -> - @res.contentType - .calledWith("text/csv") - .should.equal true - - it "should name the exported csv file", -> - @res.header - .calledWith( - "Content-Disposition", - "attachment; filename=Group.csv") - .should.equal true - - it "should export the correct csv", -> - @res.send - .calledWith("user@email.com\n") - .should.equal true diff --git a/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee b/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee index f6b00e36d4..bfd87f2f5a 100644 --- a/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee +++ b/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee @@ -65,7 +65,6 @@ describe "SubscriptionGroupHandler", -> "logger-sharelatex": log:-> "../User/UserCreator": @UserCreator "./SubscriptionUpdater": @SubscriptionUpdater - "./TeamInvitesHandler": @TeamInvitesHandler "./SubscriptionLocator": @SubscriptionLocator "../../models/Subscription": Subscription: @Subscription "../User/UserGetter": @UserGetter diff --git a/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee b/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee index 40cb18e41f..102340af39 100644 --- a/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee +++ b/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee @@ -104,7 +104,7 @@ describe "TeamInvitesHandler", -> describe "createInvite", -> it "adds the team invite to the subscription", (done) -> - @TeamInvitesHandler.createInvite @manager.id, "John.Snow@example.com", (err, invite) => + @TeamInvitesHandler.createInvite @manager.id, @subscription, "John.Snow@example.com", (err, invite) => expect(err).to.eq(null) expect(invite.token).to.eq(@newToken) expect(invite.email).to.eq("john.snow@example.com") @@ -113,7 +113,7 @@ describe "TeamInvitesHandler", -> done() it "sends an email", (done) -> - @TeamInvitesHandler.createInvite @manager.id, "John.Snow@example.com", (err, invite) => + @TeamInvitesHandler.createInvite @manager.id, @subscription, "John.Snow@example.com", (err, invite) => @EmailHandler.sendEmail.calledWith("verifyEmailToJoinTeam", sinon.match({ to: "john.snow@example.com", @@ -126,7 +126,7 @@ describe "TeamInvitesHandler", -> it "refreshes the existing invite if the email has already been invited", (done) -> originalInvite = Object.assign({}, @teamInvite) - @TeamInvitesHandler.createInvite @manager.id, originalInvite.email, (err, invite) => + @TeamInvitesHandler.createInvite @manager.id, @subscription, originalInvite.email, (err, invite) => expect(err).to.eq(null) expect(invite).to.exist @@ -140,7 +140,7 @@ describe "TeamInvitesHandler", -> done() it "removes any legacy invite from the subscription", (done) -> - @TeamInvitesHandler.createInvite @manager.id, "John.Snow@example.com", (err, invite) => + @TeamInvitesHandler.createInvite @manager.id, @subscription, "John.Snow@example.com", (err, invite) => @Subscription.update.calledWith( { _id: new ObjectId("55153a8014829a865bbf700d") }, { '$pull': { invited_emails: "john.snow@example.com" } } @@ -245,7 +245,7 @@ describe "TeamInvitesHandler", -> describe "revokeInvite", -> it "removes the team invite from the subscription", (done) -> - @TeamInvitesHandler.revokeInvite @manager.id, "jorah@example.com", => + @TeamInvitesHandler.revokeInvite @manager.id, @subscription, "jorah@example.com", => @Subscription.update.calledWith( { _id: new ObjectId("55153a8014829a865bbf700d") }, { '$pull': { teamInvites: { email: "jorah@example.com" } } } @@ -269,6 +269,7 @@ describe "TeamInvitesHandler", -> @TeamInvitesHandler.createInvite.calledWith( @subscription.admin_id, + @subscription, "eddard@example.com" ).should.eq true @@ -279,13 +280,13 @@ describe "TeamInvitesHandler", -> describe "validation", -> it "doesn't create an invite if the team limit has been reached", (done) -> @LimitationsManager.teamHasReachedMemberLimit = sinon.stub().returns(true) - @TeamInvitesHandler.createInvite @manager.id, "John.Snow@example.com", (err, invite) => + @TeamInvitesHandler.createInvite @manager.id, @subscription, "John.Snow@example.com", (err, invite) => expect(err).to.deep.equal(limitReached: true) done() it "doesn't create an invite if the subscription is not in a group plan", (done) -> @subscription.groupPlan = false - @TeamInvitesHandler.createInvite @manager.id, "John.Snow@example.com", (err, invite) => + @TeamInvitesHandler.createInvite @manager.id, @subscription, "John.Snow@example.com", (err, invite) => expect(err).to.deep.equal(wrongPlan: true) done() @@ -299,7 +300,7 @@ describe "TeamInvitesHandler", -> @subscription.member_ids = [member.id] @UserGetter.getUserByAnyEmail.withArgs(member.email).yields(null, member) - @TeamInvitesHandler.createInvite @manager.id, "tyrion@example.com", (err, invite) => + @TeamInvitesHandler.createInvite @manager.id, @subscription, "tyrion@example.com", (err, invite) => expect(err).to.deep.equal(alreadyInTeam: true) expect(invite).not.to.exist done() diff --git a/services/web/test/unit/coffee/UserMembership/UserMembershipControllerTests.coffee b/services/web/test/unit/coffee/UserMembership/UserMembershipControllerTests.coffee index 58176fbab6..bb2012a045 100644 --- a/services/web/test/unit/coffee/UserMembership/UserMembershipControllerTests.coffee +++ b/services/web/test/unit/coffee/UserMembership/UserMembershipControllerTests.coffee @@ -20,7 +20,10 @@ describe "UserMembershipController", -> @newUser = _id: 'mock-new-user-id', email: 'new-user-email@foo.bar' @subscription = { _id: 'mock-subscription-id'} @institution = _id: 'mock-institution-id', v1Id: 123 - @users = [{ _id: 'mock-member-id-1' }, { _id: 'mock-member-id-2' }] + @users = [ + { _id: 'mock-member-id-1', email: 'mock-email-1@foo.com' } + { _id: 'mock-member-id-2', email: 'mock-email-2@foo.com' } + ] @AuthenticationController = getSessionUser: sinon.stub().returns(@user) @@ -57,7 +60,7 @@ describe "UserMembershipController", -> expect(viewParams.users).to.deep.equal @users expect(viewParams.groupSize).to.equal @subscription.membersLimit expect(viewParams.translations.title).to.equal 'group_account' - expect(viewParams.paths.addMember).to.equal '/subscription/invites' + expect(viewParams.paths.addMember).to.equal "/manage/groups/#{@subscription._id}/invites" done() it 'render group managers view', (done) -> @@ -129,3 +132,34 @@ describe "UserMembershipController", -> expect(error).to.extist expect(error).to.be.an.instanceof(Errors.NotFoundError) done() + + describe "exportCsv", -> + + beforeEach -> + @req.entity = @subscription + @req.entityConfig = EntityConfigs.groupManagers + @res = new MockResponse() + @res.contentType = sinon.stub() + @res.header = sinon.stub() + @res.send = sinon.stub() + @UserMembershipController.exportCsv @req, @res + + it 'get users', -> + sinon.assert.calledWithMatch( + @UserMembershipHandler.getUsers, + @subscription, + modelName: 'Subscription', + ) + + it "should set the correct content type on the request", -> + assertCalledWith(@res.contentType, "text/csv") + + it "should name the exported csv file", -> + assertCalledWith( + @res.header + "Content-Disposition", + "attachment; filename=Group.csv" + ) + + it "should export the correct csv", -> + assertCalledWith(@res.send, "mock-email-1@foo.com\nmock-email-2@foo.com\n")