Skip to content

Commit

Permalink
More code review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
differentmatt committed Apr 20, 2017
1 parent 0fd1008 commit abcc1b9
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 41 deletions.
4 changes: 2 additions & 2 deletions app/collections/Payments.coffee
Expand Up @@ -5,8 +5,8 @@ module.exports = class Payments extends CocoCollection
model: Payment
url: '/db/payment'

fetchByCreator: (creatorID, opts) ->
fetchByRecipient: (recipientId, opts) ->
opts ?= {}
opts.data ?= {}
opts.data.creator = creatorID
opts.data.recipient = recipientId
@fetch opts
7 changes: 2 additions & 5 deletions app/templates/account/payments-view.jade
Expand Up @@ -27,10 +27,7 @@ block content
when 'subscription'
td(data-i18n="subscribe.stripe_description")
when 'terminal_subscription'
if view.prepaidMap[prepaidID].get('description')
td= view.prepaidMap[prepaidID].get('description')
else
td(data-i18n="subscribe.stripe_description")
td(data-i18n="subscribe.stripe_description")
when 'course'
td(data-i18n="special_offer.course_prefix")
when 'starter_license'
Expand All @@ -41,7 +38,7 @@ block content
td= payment.get('description')
else if productID
td(data-i18n="account.gems")
else if parseInt(payment.get('gems')) === 42000
else if me.get('stripe').free != null
if typeof(me.get('stripe').free) === 'boolean'
if me.get('stripe').customerID
td(data-i18n="subscribe.lifetime")
Expand Down
4 changes: 2 additions & 2 deletions app/views/account/PaymentsView.coffee
Expand Up @@ -10,9 +10,9 @@ module.exports = class PaymentsView extends RootView

initialize: ->
@payments = new Payments()
@supermodel.trackRequest @payments.fetchByCreator(me.id)
@supermodel.trackRequest @payments.fetchByRecipient(me.id)
@prepaids = new Prepaids()
@supermodel.trackRequest @prepaids.fetchByCreator(me.id)
@supermodel.trackRequest @prepaids.fetchByCreator(me.id, {data: {allTypes: true}})

onLoaded: ->
@prepaidMap = _.zipObject(_.map(@prepaids.models, (m) => m.id), @prepaids.models)
Expand Down
13 changes: 6 additions & 7 deletions server/middleware/prepaids.coffee
Expand Up @@ -68,14 +68,14 @@ module.exports =
return res.status(200).send(prepaid.toObject({req: req}))

yield prepaid.redeem(user)

# return prepaid with new redeemer added locally
redeemers = _.clone(prepaid.get('redeemers') or [])
redeemers.push({ date: new Date(), userID: user._id })
prepaid.set('redeemers', redeemers)
res.status(201).send(prepaid.toObject({req: req}))


revoke: wrap (req, res) ->
if not req.user?.isTeacher()
throw new errors.Forbidden('Must be a teacher to use enrollments')
Expand Down Expand Up @@ -116,7 +116,6 @@ module.exports =
prepaid.set('redeemers', _.filter(prepaid.get('redeemers') or [], (obj) -> not obj.userID.equals(user._id)))
res.status(200).send(prepaid.toObject({req: req}))


fetchByCreator: wrap (req, res, next) ->
creator = req.query.creator
return next() if not creator
Expand All @@ -129,8 +128,8 @@ module.exports =
q = {
_id: { $gt: cutoffID }
creator: mongoose.Types.ObjectId(creator)
type: { $in: ['course', 'starter_license'] }
}
q.type = { $in: ['course', 'starter_license'] } unless req.query.allTypes

prepaids = yield Prepaid.find(q)
res.send((prepaid.toObject({req: req}) for prepaid in prepaids))
Expand Down Expand Up @@ -240,7 +239,7 @@ module.exports =
schoolPrepaidsMap[school].push prepaid

res.send({prepaidActivityMap, schoolPrepaidsMap})

# Separate endpoint from legacy prepaid purchase handler
purchaseStarterLicenses: wrap (req, res) ->
if req.body.type not in ['starter_license']
Expand All @@ -263,7 +262,7 @@ module.exports =

if maxRedeemers + alreadyOwnedStarterLicenseCount > Prepaid.MAX_STARTER_LICENSES
throw new errors.Forbidden('You cannot own more than 75 starter licenses.')

if not (token or creator.isAdmin())
throw new errors.UnprocessableEntity('Missing required Stripe token')

Expand Down
74 changes: 49 additions & 25 deletions spec/server/functional/prepaid.spec.coffee
Expand Up @@ -19,7 +19,7 @@ describe 'POST /db/prepaid', ->
admin = yield utils.initAdmin()
yield utils.loginUser(admin)
done()

it 'creates a new prepaid for type "course"', utils.wrap (done) ->
user = yield utils.initUser()
[res, body] = yield request.postAsync({url: getURL('/db/prepaid'), json: {
Expand All @@ -32,7 +32,7 @@ describe 'POST /db/prepaid', ->
expect(prepaid.get('creator').equals(user._id)).toBe(true)
expect(prepaid.get('code')).toBeDefined()
done()

it 'does not work for non-admins', utils.wrap (done) ->
user = yield utils.initUser()
yield utils.loginUser(user)
Expand All @@ -42,7 +42,7 @@ describe 'POST /db/prepaid', ->
}})
expect(res.statusCode).toBe(403)
done()

it 'accepts start and end dates', utils.wrap (done) ->
user = yield utils.initUser()
[res, body] = yield request.postAsync({url: getURL('/db/prepaid'), json: {
Expand All @@ -57,8 +57,33 @@ describe 'POST /db/prepaid', ->
expect(prepaid.get('startDate')).toBeDefined()
expect(prepaid.get('endDate')).toBeDefined()
done()



describe 'GET /db/prepaid', ->
beforeEach utils.wrap (done) ->
@user = yield utils.initUser()
yield utils.loginUser(@user)
prepaid = new Prepaid({creator: @user.id, type: 'course'})
yield prepaid.save()
prepaid = new Prepaid({creator: @user.id, type: 'starter_license'})
yield prepaid.save()
prepaid = new Prepaid({creator: @user.id, type: 'terminal_subscription'})
yield prepaid.save()
prepaid = new Prepaid({creator: @user.id, type: 'subscription'})
yield prepaid.save()
done()

describe 'when creator param', ->
it 'returns only course and starter_license prepaids for creator', utils.wrap (done) ->
[res, body] = yield request.getAsync({url: getURL("/db/prepaid?creator=#{@user.id}"), json: true})
expect(body.length).toEqual(2)
done()

describe 'when creator and allTypes=true', ->
it 'returns all for creator', utils.wrap (done) ->
[res, body] = yield request.getAsync({url: getURL("/db/prepaid?creator=#{@user.id}&allTypes=true"), json: true})
expect(body.length).toEqual(4)
done()

describe 'GET /db/prepaid/:handle', ->
it 'populates startDate and endDate with default values', utils.wrap (done) ->
prepaid = new Prepaid({type: 'course' })
Expand All @@ -67,10 +92,9 @@ describe 'GET /db/prepaid/:handle', ->
expect(body.endDate).toBe(Prepaid.DEFAULT_END_DATE)
expect(body.startDate).toBe(Prepaid.DEFAULT_START_DATE)
done()


describe 'POST /db/prepaid/:handle/redeemers', ->

beforeEach utils.wrap (done) ->
yield utils.clearModels([Course, CourseInstance, Payment, Prepaid, User])
@teacher = yield utils.initUser({role: 'teacher'})
Expand Down Expand Up @@ -142,7 +166,7 @@ describe 'POST /db/prepaid/:handle/redeemers', ->
student = yield User.findById(@student.id)
expect(student.get('coursePrepaid')._id.equals(@prepaid._id)).toBe(true)
done()

it 'updates the user if their license is expired', utils.wrap (done) ->
yield utils.loginUser(@admin)
prepaid = yield utils.makePrepaid({
Expand Down Expand Up @@ -269,7 +293,7 @@ describe 'DELETE /db/prepaid/:handle/redeemers', ->
expect(student.get('coursePrepaid')).toBeUndefined()
expect(student.get('coursePrepaidID')).toBeUndefined()
done()

it 'returns 403 unless the user is the "creator"', utils.wrap (done) ->
otherTeacher = yield utils.initUser({role: 'teacher'})
yield utils.loginUser(otherTeacher)
Expand All @@ -282,7 +306,7 @@ describe 'DELETE /db/prepaid/:handle/redeemers', ->
[res, body] = yield request.delAsync {uri: @url, json: { userID: otherStudent.id } }
expect(res.statusCode).toBe(422)
done()

it 'returns 403 if the prepaid is a starter license', utils.wrap ->
yield @prepaid.update({$set: {type: 'starter_license'}})
[res, body] = yield request.delAsync {uri: @url, json: { userID: @student.id } }
Expand All @@ -301,7 +325,7 @@ describe 'GET /db/prepaid?creator=:id', ->
yield @unmigratedPrepaid.update({$unset: { endDate: '', startDate: '' }})
yield utils.loginUser(@teacher)
done()

it 'return all prepaids for the creator', utils.wrap (done) ->
url = getURL("/db/prepaid?creator=#{@teacher.id}")
[res, body] = yield request.getAsync({uri: url, json: true})
Expand All @@ -314,20 +338,20 @@ describe 'GET /db/prepaid?creator=:id', ->
fail('All prepaids should have start and end dates')
expect(res.body[0]._id).toBe(@prepaid.id)
done()

it 'returns 403 if the user tries to view another user\'s prepaids', utils.wrap (done) ->
anotherUser = yield utils.initUser()
anotherUser = yield utils.initUser()
url = getURL("/db/prepaid?creator=#{anotherUser.id}")
[res, body] = yield request.getAsync({uri: url, json: true})
expect(res.statusCode).toBe(403)
done()


describe '/db/prepaid', ->
beforeEach utils.wrap (done) ->
yield utils.populateProducts()
done()

prepaidURL = getURL('/db/prepaid')

headers = {'X-Change-Plan': 'true'}
Expand Down Expand Up @@ -514,7 +538,7 @@ describe '/db/prepaid', ->

describe 'Purchase terminal_subscription', ->
afterEach nockUtils.teardownNock

it 'Anonymous submits a prepaid purchase', (done) ->
nockUtils.setupNock 'db-prepaid-purchase-term-sub-test-1.json', (err, nockDone) ->
stripe.tokens.create {
Expand Down Expand Up @@ -697,7 +721,7 @@ describe '/db/prepaid', ->
# TODO: Move redeem subscription prepaid code tests to subscription tests file
describe 'Subscription redeem tests', ->
afterEach nockUtils.teardownNock

it 'Creator can redeeem a prepaid code', (done) ->
nockUtils.setupNock 'db-sub-redeem-test-1.json', (err, nockDone) ->
loginJoe (joe) ->
Expand All @@ -709,7 +733,7 @@ describe '/db/prepaid', ->
# joe has a stripe subscription, so test if the months are added to the end of it.
stripe.customers.retrieve joeData.stripe.customerID, (err, customer) =>
expect(err).toBeNull()

findStripeSubscription customer.id, subscriptionID: joeData.stripe?.subscriptionID, (err, subscription) =>
if subscription
stripeSubscriptionPeriodEndDate = new moment(subscription.current_period_end * 1000)
Expand Down Expand Up @@ -808,14 +832,14 @@ describe '/db/prepaid', ->

user = yield utils.initUser()
yield utils.loginUser(user)

codeRedeemers = 50
codeMonths = 3
redeemers = 51

purchasePrepaidAsync = Promise.promisify(purchasePrepaid, {multiArgs: true})
[res, prepaid] = yield purchasePrepaidAsync('terminal_subscription', months: codeMonths, codeRedeemers, token.id)

expect(prepaid).toBeDefined()
expect(prepaid.code).toBeDefined()

Expand All @@ -828,17 +852,17 @@ describe '/db/prepaid', ->
thread.user = yield utils.initUser()
yield utils.loginUser(thread.user, {request: thread.request})
threads.push(thread)

# Spawn all requests at once!
requests = []
options = {
options = {
url: getURL('/db/subscription/-/subscribe_prepaid')
json: { ppc: prepaid.code }
}
for thread in threads
requests.push(thread.request.postAsync(options))
# Wait until all requests finish, make sure all but one succeeded

# Wait until all requests finish, make sure all but one succeeded
responses = yield requests
redeemed = _.size(_.where(responses, {statusCode: 200}))
errors = _.size(_.where(responses, {statusCode: 403}))
Expand Down

0 comments on commit abcc1b9

Please sign in to comment.