Allow teachers to apply full licenses over starter licenses

Generalize prepaid redeem handling

Fix test

Add more tests, fix bugs

Address CR feedback

Move Prepaid unit tests

app/views/courses/TeacherClassView.coffee

@@ -443,24 +443,23 @@ module.exports = class TeacherClassView extends RootView
 
     # Automatically apply licenses to students if necessary
     .then =>
+      # Find the prepaids and users we're acting on (for both starter and full license cases)
       availablePrepaids = @prepaids.filter((prepaid) -> prepaid.status() is 'available' and prepaid.includesCourse(courseID))
       unenrolledStudents = _(members)
         .map((userID) => @students.get(userID))
-        .filter((user) => user.prepaidStatus() isnt 'enrolled')
+        .filter((user) => not user.isEnrolled() or not user.prepaidIncludesCourse(courseID))
         .value()
       totalSpotsAvailable = _.reduce(prepaid.openSpots() for prepaid in availablePrepaids, (val, total) -> val + total) or 0
 
-      availableFullLicenses = @prepaids.filter((prepaid) -> prepaid.status() is 'available' and prepaid.get('type') is 'course')
-      numStudentsWithoutFullLicenses = _(members)
-        .map((userID) => @students.get(userID))
-        .filter((user) => user.prepaidType() isnt 'course' or user.prepaidStatus() isnt 'enrolled')
-        .size()
-      numFullLicensesAvailable = _.reduce(prepaid.openSpots() for prepaid in availableFullLicenses, (val, total) -> val + total) or 0
-      if courseID not in STARTER_LICENSE_COURSE_IDS
-        canAssignCourses = numFullLicensesAvailable >= numStudentsWithoutFullLicenses
-      else
-        canAssignCourses = totalSpotsAvailable >= _.size(unenrolledStudents)
+      canAssignCourses = totalSpotsAvailable >= _.size(unenrolledStudents)
       if not canAssignCourses
+        # These ones just matter for display
+        availableFullLicenses = @prepaids.filter((prepaid) -> prepaid.status() is 'available' and prepaid.get('type') is 'course')
+        numStudentsWithoutFullLicenses = _(members)
+          .map((userID) => @students.get(userID))
+          .filter((user) => user.prepaidType() isnt 'course' or not user.isEnrolled())
+          .size()
+        numFullLicensesAvailable = _.reduce(prepaid.openSpots() for prepaid in availableFullLicenses, (val, total) -> val + total) or 0
         modal = new CoursesNotAssignedModal({
           selected: members.length
           numStudentsWithoutFullLicenses
@@ -476,6 +475,7 @@ module.exports = class TeacherClassView extends RootView
       remainingSpots = totalSpotsAvailable - numberEnrolled
 
       requests = []
+
       for prepaid in availablePrepaids
         for i in _.range(prepaid.openSpots())
           break unless _.size(unenrolledStudents) > 0

server/commons/Handler.coffee

@@ -104,7 +104,7 @@ module.exports = class Handler
     res.end()
 
   sendNoContent: (res) ->
-    res.send 204
+    res.sendStatus 204
     res.end()
 
   # generic handlers

server/middleware/prepaids.coffee

@@ -24,7 +24,7 @@ cutoffID = mongoose.Types.ObjectId(Math.floor(cutoffDate/1000).toString(16)+'000
 module.exports =
   # Create a prepaid manually (as an admin)
   post: wrap (req, res) ->
-    validTypes = ['course']
+    validTypes = ['course', 'starter_license']
     unless req.body.type in validTypes
       throw new errors.UnprocessableEntity("Prepaid type must be one of: #{validTypes}.")
       # TODO: deprecate or refactor other prepaid types
@@ -60,7 +60,7 @@ module.exports =
       throw new errors.Forbidden('You may not redeem licenses from this prepaid')
     unless prepaid.get('type') in ['course', 'starter_license']
       throw new errors.Forbidden('This prepaid is not of type "course" or "starter_license"')
-    if user.isEnrolled()
+    unless prepaid.canReplaceUserPrepaid(user.get('coursePrepaid'))
       return res.status(200).send(prepaid.toObject({req: req}))
 
     yield prepaid.redeem(user)

server/models/Prepaid.coffee

@@ -35,6 +35,14 @@ PrepaidSchema.statics.generateNewCodeAsync = co.wrap (done) ->
     break if not prepaid
   return code
 
+PrepaidSchema.methods.canReplaceUserPrepaid = (otherPrepaid) ->
+  return true if !otherPrepaid
+  otherPrepaidObj = otherPrepaid.toJSON?() or otherPrepaid
+  return true if otherPrepaidObj.endDate and otherPrepaidObj.endDate <= new Date().toISOString()
+  if otherPrepaidObj.type is 'starter_license' and @get('type') is 'course'
+    return true
+  return false
+
 PrepaidSchema.pre('save', (next) ->
   @set('exhausted', @get('maxRedeemers') <= _.size(@get('redeemers')))
   if not @get('code')

server/models/User.coffee

@@ -372,6 +372,12 @@ UserSchema.methods.isEnrolled = ->
   return true unless coursePrepaid.endDate
   return coursePrepaid.endDate > new Date().toISOString()
 
+UserSchema.methods.prepaidType = ->
+  # TODO: remove once legacy prepaidIDs are migrated to objects
+  return undefined unless @get('coursePrepaid') or @get('coursePrepaidID')
+  # NOTE: Default type is 'course' if no type is marked on the user's copy
+  return @get('coursePrepaid')?.type or 'course'
+
 UserSchema.methods.prepaidIncludesCourse = (course) ->
   # TODO: Migrate legacy prepaids that just use coursePrepaidID
   return false if not (@get('coursePrepaid') or @get('coursePrepaidID'))

spec/server/functional/prepaid.spec.coffee

@@ -159,7 +159,58 @@ describe 'POST /db/prepaid/:handle/redeemers', ->
     student = yield User.findById(@student.id)
     expect(student.get('coursePrepaid')._id.equals(@prepaid._id)).toBe(true)
     done()
-
+
+  it 'replaces a starter license with a full license', utils.wrap (done) ->
+    yield utils.loginUser(@admin)
+    oldPrepaid = yield utils.makePrepaid({
+      creator: @teacher.id
+      startDate: moment().subtract(2, 'month').toISOString()
+      endDate: moment().add(4, 'month').toISOString()
+      type: 'starter_license'
+    })
+    @student.set('coursePrepaid', _.pick(oldPrepaid.toObject(), '_id', 'startDate', 'endDate', 'type'))
+    yield @student.save()
+    yield utils.loginUser(@teacher)
+    [res, body] = yield request.postAsync {uri: @url, json: { userID: @student.id } }
+    expect(body.redeemers.length).toBe(1)
+    expect(res.statusCode).toBe(201)
+    prepaid = yield Prepaid.findById(@prepaid._id)
+    expect(prepaid.get('redeemers').length).toBe(1)
+    student = yield User.findById(@student.id)
+    expect(student.get('coursePrepaid')._id.equals(@prepaid._id)).toBe(true)
+    done()
+
+  it 'does NOT replace a full license with a starter license', utils.wrap (done) ->
+    yield utils.loginUser(@admin)
+    @prepaid.set({
+      creator: @teacher.id
+      startDate: moment().subtract(2, 'month').toISOString()
+      endDate: moment().add(4, 'month').toISOString()
+      type: 'starter_license'
+    })
+    yield @prepaid.save()
+    oldPrepaid = yield utils.makePrepaid({
+      creator: @teacher.id
+      startDate: moment().subtract(2, 'month').toISOString()
+      endDate: moment().add(10, 'month').toISOString()
+      type: 'course'
+    })
+    yield oldPrepaid.redeem(@student)
+    yield utils.loginUser(@teacher)
+
+    student = yield User.findById(@student.id)
+    expect(student.get('coursePrepaid')._id.equals(oldPrepaid._id)).toBe(true)
+    expect(student.get('coursePrepaid')._id.toString()).toBe(oldPrepaid._id.toString())
+
+    [res, body] = yield request.postAsync {uri: @url, json: { userID: @student.id } }
+    expect(body.redeemers.length).toBe(0)
+    expect(res.statusCode).toBe(200)
+    student = yield User.findById(@student.id)
+    expect(student.get('coursePrepaid')._id.equals(oldPrepaid._id)).toBe(true)
+    expect(student.get('coursePrepaid')._id.toString()).toBe(oldPrepaid._id.toString())
+    expect((yield Prepaid.findById(oldPrepaid._id)).get('redeemers').length).toBe(1)
+    done()
+
   it 'adds includedCourseIDs to the user when redeeming', utils.wrap (done) ->
     yield utils.loginUser(@admin)
     @prepaid.set({

spec/server/unit/Prepaid.spec.coffee

@@ -0,0 +1,76 @@
+utils = require '../utils'
+Prepaid = require '../../../server/models/Prepaid'
+Course = require '../../../server/models/Course'
+CourseInstance = require '../../../server/models/CourseInstance'
+User = require '../../../server/models/User'
+moment = require 'moment'
+
+describe 'POST /db/prepaid/:handle/redeemers', ->
+  beforeEach utils.wrap (done) ->
+    yield utils.clearModels([Course, CourseInstance, Prepaid, User])
+    @teacher = yield utils.initUser({role: 'teacher'})
+    @admin = yield utils.initAdmin()
+    yield utils.loginUser(@admin)
+    @prepaid = yield utils.makePrepaid({ creator: @teacher.id })
+    yield utils.loginUser(@teacher)
+    @student = yield utils.initUser()
+    @url = getURL("/db/prepaid/#{@prepaid.id}/redeemers")
+    done()
+
+  describe '.canReplaceUserPrepaid', ->
+    beforeEach utils.wrap (done) ->
+      yield utils.clearModels([Prepaid])
+      yield utils.loginUser(@admin)
+      @starter = yield utils.makePrepaid({
+        creator: @teacher.id
+        startDate: moment().subtract(2, 'month').toISOString()
+        endDate: moment().add(4, 'month').toISOString()
+        type: 'starter_license'
+      })
+      @course = yield utils.makePrepaid({
+        creator: @teacher.id
+        startDate: moment().subtract(2, 'month').toISOString()
+        endDate: moment().add(10, 'month').toISOString()
+        type: 'course'
+      })
+      @courseExpired = yield utils.makePrepaid({
+        creator: @teacher.id
+        startDate: moment().subtract(16, 'month').toISOString()
+        endDate: moment().subtract(4, 'month').toISOString()
+        type: 'course'
+      })
+      done()
+
+    describe 'when the user has a starter license,', ->
+      describe 'and we are assigning a full license', ->
+        it 'returns true', ->
+          expect(@course.canReplaceUserPrepaid(@starter)).toBe(true)
+
+      describe 'and we are assigning a starter license', ->
+        it 'returns false', ->
+          expect(@starter.canReplaceUserPrepaid(@starter)).toBe(false)
+
+    describe 'when the user has a full license,', ->
+      describe 'that is NOT expired,', ->
+        describe 'and we are assigning a full license', ->
+          it 'returns false', ->
+            expect(@course.canReplaceUserPrepaid(@course)).toBe(false)
+            expect(@course.canReplaceUserPrepaid(@course.toObject())).toBe(false)
+
+        describe 'and we are assigning a starter license', ->
+          it 'returns false', ->
+            expect(@starter.canReplaceUserPrepaid(@course)).toBe(false)
+
+      describe 'that is expired,', ->
+        describe 'and we are assigning a full license', ->
+          it 'returns true', ->
+            expect(@course.canReplaceUserPrepaid(@courseExpired)).toBe(true)
+
+        describe 'and we are assigning a starter license', ->
+          it 'returns true', ->
+            expect(@starter.canReplaceUserPrepaid(@courseExpired)).toBe(true)
+
+    describe 'when the user has no license', ->
+      it 'returns true', ->
+        expect(@course.canReplaceUserPrepaid(undefined)).toBe(true)
+        expect(@starter.canReplaceUserPrepaid(undefined)).toBe(true)

spec/server/utils.coffee

@@ -243,7 +243,7 @@ module.exports = mw =
 
     unless mw.lastLogin?.isAdmin()
       # TODO: Make this function transparently turn into an admin if necessary
-      done("Must be logged in as an admin to create a campaign") 
+      done("Must be logged in as an admin to create a campaign")
 
     data = _.extend({}, {
       name: _.uniqueId('Campaign ')

test/app/factories.coffee

@@ -73,7 +73,7 @@ module.exports = {
     }, attrs)
 
     if sources.prepaid and not attrs.coursePrepaid
-      attrs.coursePrepaid = sources.prepaid.pick('_id', 'startDate', 'endDate')
+      attrs.coursePrepaid = sources.prepaid.pick('_id', 'startDate', 'endDate', 'type', 'includedCourseIDs')
 
     return new User(attrs)
 

test/app/views/teachers/TeacherClassView.spec.coffee

@@ -32,14 +32,14 @@ describe 'TeacherClassView', ->
       ])
       @releasedCourses = new Courses(@courses.where({ releasePhase: 'released' }))
       @available1 = factories.makePrepaid({maxRedeemers: 1})
-      @available2 = factories.makePrepaid({maxRedeemers: 1})
+      @available2 = factories.makePrepaid({maxRedeemers: 1, type: 'starter_license', includedCourseIDs: [@courses.at(0).id]})
       expired = factories.makePrepaid({endDate: moment().subtract(1, 'day').toISOString()})
       @prepaids = new Prepaids([@available1, @available2, expired])
       @students = new Users([
         factories.makeUser({name: 'Abner'})
         factories.makeUser({name: 'Abigail'})
         factories.makeUser({name: 'Abby'}, {prepaid: @available1})
-        factories.makeUser({name: 'Ben'}, {prepaid: @available1})
+        factories.makeUser({name: 'Ben'}, {prepaid: @available2})
         factories.makeUser({name: 'Ned'}, {prepaid: expired})
         factories.makeUser({name: 'Ebner'}, {prepaid: expired})
       ])
@@ -216,6 +216,53 @@ describe 'TeacherClassView', ->
           @view.$el.find('.export-student-progress-btn').click()
           expect(window.open).toHaveBeenCalled()
 
+    describe '.assignCourse(courseID, members)', ->
+      beforeEach (done) ->
+        @classroom = factories.makeClassroom({ aceConfig: { language: 'javascript' }}, { courses: @releasedCourses, members: @students, levels: [@levels, new Levels()]})
+        @courseInstances = new CourseInstances([
+          factories.makeCourseInstance({}, { course: @releasedCourses.first(), @classroom, members: new Users() })
+          factories.makeCourseInstance({}, { course: @releasedCourses.last(), @classroom, members: new Users() })
+        ])
+
+        sessions = []
+        @finishedStudent = @students.first()
+        @unfinishedStudent = @students.last()
+        classLanguage = @classroom.get('aceConfig')?.language
+        for level in @levels.models
+          continue if classLanguage and classLanguage is level.get('primerLanguage')
+          sessions.push(factories.makeLevelSession(
+              {state: {complete: true}, playtime: 60},
+              {level, creator: @finishedStudent})
+          )
+        sessions.push(factories.makeLevelSession(
+            {state: {complete: true}, playtime: 60},
+            {level: @levels.first(), creator: @unfinishedStudent})
+        )
+        @levelSessions = new LevelSessions(sessions)
+
+        @view = new TeacherClassView({}, @courseInstances.first().id)
+        @view.classroom.fakeRequests[0].respondWith({ status: 200, responseText: @classroom.stringify() })
+        @view.courses.fakeRequests[0].respondWith({ status: 200, responseText: @courses.stringify() })
+        @view.courseInstances.fakeRequests[0].respondWith({ status: 200, responseText: @courseInstances.stringify() })
+        @view.students.fakeRequests[0].respondWith({ status: 200, responseText: @students.stringify() })
+        @view.classroom.sessions.fakeRequests[0].respondWith({ status: 200, responseText: @levelSessions.stringify() })
+        @view.levels.fakeRequests[0].respondWith({ status: 200, responseText: @levels.stringify() })
+        @view.prepaids.fakeRequests[0].respondWith({ status: 200, responseText: @prepaids.stringify() })
+
+        jasmine.demoEl(@view.$el)
+        _.defer done
+
+      describe 'when the student has a starter license', ->
+        describe 'and the course is NOT covered by starter licenses', ->
+          beforeEach (done) ->
+            spyOn(@view.prepaids.at(1), 'redeem')
+            @starterStudent = @students.find (s) -> s.prepaidType() is 'starter_license'
+            @view.assignCourse(@courses.at(1).id, [@starterStudent.id])
+            @view.wait('begin-redeem-for-assign-course').then(done)
+
+          it 'replaces their license with a full license', (done) ->
+            expect(@view.prepaids.at(1).redeem).toHaveBeenCalled()
+            done()
 
     describe '.assignCourse(courseID, members)', ->
       beforeEach (done) ->