Remove SubscriptionLocator.getManagedSubscription

It was used as a kind of access control check, but it's clearer
if the check is in the only controller that actually needs it.
This commit is contained in:
Alberto Fernández Capel 2018-07-13 11:47:26 +01:00
parent 46a1cdc510
commit 0bf807fa9f
4 changed files with 17 additions and 16 deletions

View file

@ -13,7 +13,7 @@ module.exports =
adminUserId = AuthenticationController.getLoggedInUserId(req)
newEmail = req.body?.email?.toLowerCase()?.trim()
SubscriptionLocator.getManagedSubscription adminUserId, (error, subscription) ->
getManagedSubscription adminUserId, (error, subscription) ->
return next(error) if error?
logger.log adminUserId:adminUserId, newEmail:newEmail, "adding user to group subscription"
@ -31,7 +31,7 @@ module.exports =
removeUserFromGroup: (req, res, next)->
adminUserId = AuthenticationController.getLoggedInUserId(req)
userToRemove_id = req.params.user_id
SubscriptionLocator.getManagedSubscription adminUserId, (error, subscription) ->
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)->
@ -43,7 +43,7 @@ module.exports =
removeSelfFromGroup: (req, res, next)->
adminUserId = req.query.admin_user_id
userToRemove_id = AuthenticationController.getLoggedInUserId(req)
SubscriptionLocator.getManagedSubscription adminUserId, (error, subscription) ->
getManagedSubscription adminUserId, (error, subscription) ->
return next(error) if error?
logger.log adminUserId:adminUserId, userToRemove_id:userToRemove_id, "removing user from group subscription after self request"
SubscriptionGroupHandler.removeUserFromGroup subscription._id, userToRemove_id, (err)->
@ -54,7 +54,7 @@ module.exports =
renderSubscriptionGroupAdminPage: (req, res, next)->
user_id = AuthenticationController.getLoggedInUserId(req)
SubscriptionLocator.getManagedSubscription user_id, (error, subscription)->
getManagedSubscription user_id, (error, subscription)->
return next(error) if error?
if !subscription?.groupPlan
return res.redirect("/user/subscription")
@ -67,7 +67,7 @@ module.exports =
exportGroupCsv: (req, res)->
user_id = AuthenticationController.getLoggedInUserId(req)
logger.log user_id: user_id, "exporting group csv"
SubscriptionLocator.getManagedSubscription user_id, (err, subscription)->
getManagedSubscription user_id, (err, subscription)->
return next(error) if error?
if !subscription.groupPlan
return res.redirect("/")
@ -81,3 +81,13 @@ module.exports =
)
res.contentType('text/csv')
res.send(groupCsv)
getManagedSubscription = (managerId, callback) ->
SubscriptionLocator.findManagedSubscription managerId, (err, subscription)->
if subscription?
logger.log managerId: managerId, "got managed subscription"
else
err ||= new Error("No subscription found managed by user #{managerId}")
return callback(err, subscription)

View file

@ -14,15 +14,6 @@ module.exports = SubscriptionLocator =
logger.log user_id:user_id, "got users subscription"
callback(err, subscription)
getManagedSubscription: (managerId, callback)->
SubscriptionLocator.findManagedSubscription managerId, (err, subscription)->
if subscription?
logger.log managerId: managerId, "got managed subscription"
else
err ||= new Error("No subscription found managed by user #{managerId}")
callback(err, subscription)
findManagedSubscription: (managerId, callback)->
logger.log managerId: managerId, "finding managed subscription"
Subscription.findOne manager_ids: managerId, callback

View file

@ -29,7 +29,7 @@ describe "SubscriptionGroupController", ->
isUserPartOfGroup: sinon.stub()
getPopulatedListOfMembers: sinon.stub().callsArgWith(1, null, [@user])
@SubscriptionLocator =
getManagedSubscription: sinon.stub().callsArgWith(1, null, @subscription)
findManagedSubscription: sinon.stub().callsArgWith(1, null, @subscription)
@AuthenticationController =
getLoggedInUserId: (req) -> req.session.user._id

View file

@ -45,7 +45,7 @@ describe "Subscription Locator Tests", ->
it "should query the database", (done)->
@Subscription.findOne.callsArgWith(1, null, @subscription)
@SubscriptionLocator.getManagedSubscription @user._id, (err, subscription)=>
@SubscriptionLocator.findManagedSubscription @user._id, (err, subscription)=>
@Subscription.findOne.calledWith({"manager_ids":@user._id}).should.equal true
subscription.should.equal @subscription
done()