From 8dbc86ca04574e868c11c6c6624586aa511fc695 Mon Sep 17 00:00:00 2001 From: Scott Erickson Date: Mon, 23 May 2016 10:26:34 -0700 Subject: [PATCH] Fix bugquest bugs Hide TeachersContactModal after sending message Fix GET /db/level/:handle/session, more extensively test Fix EnrollmentView number of students input to stop losing focus on input Fix EnrollmentsView syntax Fix ActivateLicensesModal "Get Enrollments" button when already in the enrollments page Update EnrollmentsView with new credit numbers when ActivateLicensesModal closes Hide search box in TeacherClassView "Enrollment Status" tab Tweak EnrollmentsView styling Fix EnrollmentsView tests Fix AnalyticsView Make EnrollmentsView more explicitly handle undefined and empty array prepaid groups Remove log CoursesView handles JoinClassModal cancel Re-enable EditStudentModal set password button when the form changes Fix course instance tests, next level endpoint bug Fix EditStudentModal tests --- app/styles/courses/enrollments-view.sass | 4 +- app/templates/courses/enrollments-view.jade | 4 +- app/templates/courses/teacher-class-view.jade | 15 ++++--- .../teachers/teachers-contact-modal.jade | 10 ++--- app/views/admin/AnalyticsView.coffee | 2 +- .../courses/ActivateLicensesModal.coffee | 4 ++ app/views/courses/CoursesView.coffee | 3 ++ app/views/courses/EnrollmentsView.coffee | 11 ++++- app/views/courses/TeacherClassView.coffee | 1 - app/views/teachers/EditStudentModal.coffee | 7 ++- .../teachers/TeachersContactModal.coffee | 12 ++--- server/middleware/course-instances.coffee | 4 +- server/middleware/levels.coffee | 5 ++- server/routes/index.coffee | 2 +- .../functional/course_instance.spec.coffee | 12 ++--- spec/server/functional/level.spec.coffee | 44 ++++++++++++++++++- .../views/courses/EnrollmentsView.spec.coffee | 4 +- .../courses/TeachersContactModal.spec.coffee | 7 +-- .../teachers/EditStudentModal.spec.coffee | 6 ++- 19 files changed, 111 insertions(+), 46 deletions(-) diff --git a/app/styles/courses/enrollments-view.sass b/app/styles/courses/enrollments-view.sass index 40f0bf8db..e3179a5d2 100644 --- a/app/styles/courses/enrollments-view.sass +++ b/app/styles/courses/enrollments-view.sass @@ -32,9 +32,9 @@ border-radius: 5px #students-input - width: 180px + width: 220px height: 80px - font-size: 60px + font-size: 50px &::-webkit-inner-spin-button, &::-webkit-outer-spin-button -webkit-appearance: none diff --git a/app/templates/courses/enrollments-view.jade b/app/templates/courses/enrollments-view.jade index 79711b891..15e77b330 100644 --- a/app/templates/courses/enrollments-view.jade +++ b/app/templates/courses/enrollments-view.jade @@ -48,14 +48,14 @@ block content .row.m-t-3 if anyPrepaids #prepaids-col.col-md-9 - if available + if _.size(available) > 0 h5.m-b-1(data-i18n="teacher.available_credits") .row for prepaid in available .col-sm-4.col-xs-6 +prepaidCard(prepaid) - if pending + if _.size(pending) > 0 h5.m-b-1.m-t-3(data-i18n="teacher.pending_credits") .row for prepaid in pending diff --git a/app/templates/courses/teacher-class-view.jade b/app/templates/courses/teacher-class-view.jade index a55696881..002f89ae6 100644 --- a/app/templates/courses/teacher-class-view.jade +++ b/app/templates/courses/teacher-class-view.jade @@ -378,14 +378,17 @@ mixin bulkAssignControls span(data-i18n='teacher.enroll_selected_students') mixin enrollmentStatusTab - form.form-inline.text-center.m-t-3 - #search-form-group.form-group - label(for="student-search") Search for student: - input#student-search.form-control.m-l-1(type="search") - span.glyphicon.glyphicon-search.form-control-feedback + // TODO: Have search input in all tabs + + //form.form-inline.text-center.m-t-3 + // #search-form-group.form-group + // label(for="student-search") Search for student: + // input#student-search.form-control.m-l-1(type="search") + // span.glyphicon.glyphicon-search.form-control-feedback - table.table#enrollment-status-table.table-condensed + table.table#enrollment-status-table.table-condensed.m-t-3 thead + // Checkbox code works, but don't need it yet. //th.checkbox-col.select-all .checkbox-flat input(type='checkbox' id='checkbox-all-students') diff --git a/app/templates/teachers/teachers-contact-modal.jade b/app/templates/teachers/teachers-contact-modal.jade index 2b7079d88..de3826063 100644 --- a/app/templates/teachers/teachers-contact-modal.jade +++ b/app/templates/teachers/teachers-contact-modal.jade @@ -7,28 +7,28 @@ block modal-header-content block modal-body-content p Send us a message and our classroom success team will be in touch to help find the best solution for your students' needs! form - - var sending = view.state.get('sendingState') === 'sending'; + - var sending = view.state.get('sendingState') === 'sending' + - var sent = view.state.get('sendingState') === 'sent'; - var values = view.state.get('formValues'); - var errors = view.state.get('formErrors'); .form-group(class=errors.email ? 'has-error' : '') label.control-label(for="email" data-i18n="general.email") +formErrors(errors.email) - input.form-control(name="email", type="email", value=values.email || '', tabindex=1, disabled=sending) + input.form-control(name="email", type="email", value=values.email || '', tabindex=1, disabled=sending || sent) .form-group(class=errors.message ? 'has-error' : '') label.control-label(for="message" data-i18n="general.message") +formErrors(errors.message) - textarea.form-control(name="message", tabindex=1 disabled=sending)= values.message + textarea.form-control(name="message", tabindex=1 disabled=sending || sent)= values.message if view.state.get('sendingState') === 'error' .alert.alert-danger Could not send message. - if view.state.get('sendingState') === 'sent' + if sent .alert.alert-success Message sent! .text-right - - var sent = view.state.get('sendingState') === 'sent'; button#submit-btn.btn.btn-navy.btn-lg(type='submit' disabled=sending || sent) Submit block modal-footer diff --git a/app/views/admin/AnalyticsView.coffee b/app/views/admin/AnalyticsView.coffee index a0a1183b7..f23c0e58a 100644 --- a/app/views/admin/AnalyticsView.coffee +++ b/app/views/admin/AnalyticsView.coffee @@ -290,7 +290,7 @@ module.exports = class AnalyticsView extends RootView prepaidUserMap = {} for user in data.students continue unless studentPaidStatusMap[user._id] - if prepaidID = user.coursePrepaidID or user.course.coursePrepaid?._id # TODO: make sure this works for coursePrepaid + if prepaidID = user.coursePrepaidID or user.coursePrepaid?._id studentPaidStatusMap[user._id] = 'paid' prepaidUserMap[prepaidID] ?= [] prepaidUserMap[prepaidID].push(user._id) diff --git a/app/views/courses/ActivateLicensesModal.coffee b/app/views/courses/ActivateLicensesModal.coffee index 530087f59..28345d1a7 100644 --- a/app/views/courses/ActivateLicensesModal.coffee +++ b/app/views/courses/ActivateLicensesModal.coffee @@ -16,6 +16,7 @@ module.exports = class ActivateLicensesModal extends ModalView 'change input[type="checkbox"][name="user"]': 'updateSelectedStudents' 'change select.classroom-select': 'replaceStudentList' 'submit form': 'onSubmitForm' + 'click #get-more-licenses-btn': 'onClickGetMoreLicensesButton' getInitialState: (options) -> selectedUsers = options.selectedUsers or options.users @@ -113,3 +114,6 @@ module.exports = class ActivateLicensesModal extends ModalView finishRedeemUsers: -> @trigger 'redeem-users', @state.get('selectedUsers') + + onClickGetMoreLicensesButton: -> + @hide?() # In case this is opened in /teachers/enrollments itself, otherwise the button does nothing diff --git a/app/views/courses/CoursesView.coffee b/app/views/courses/CoursesView.coffee index 5343d6163..749318cf7 100644 --- a/app/views/courses/CoursesView.coffee +++ b/app/views/courses/CoursesView.coffee @@ -102,6 +102,9 @@ module.exports = class CoursesView extends RootView @openModalView modal @listenTo modal, 'join:success', @onJoinClassroomSuccess @listenTo modal, 'join:error', @onJoinClassroomError + @listenTo modal, 'hidden', -> + @state = null + @renderSelectors '#join-class-form' onJoinClassroomError: (classroom, jqxhr, options) -> @state = null diff --git a/app/views/courses/EnrollmentsView.coffee b/app/views/courses/EnrollmentsView.coffee index bc61e1015..b79d51886 100644 --- a/app/views/courses/EnrollmentsView.coffee +++ b/app/views/courses/EnrollmentsView.coffee @@ -44,8 +44,9 @@ module.exports = class EnrollmentsView extends RootView @prepaids.comparator = '_id' @supermodel.trackRequest @prepaids.fetchByCreator(me.id) @debouncedRender = _.debounce @render, 0 - @listenTo @prepaids, 'all', -> @state.set('prepaidGroups', @prepaids.groupBy((p) -> p.status())) + @listenTo @prepaids, 'sync', @updatePrepaidGroups @listenTo(@state, 'all', @debouncedRender) + @listenTo(me, 'change:enrollmentRequestSent', @debouncedRender) onceClassroomsSync: -> for classroom in @classrooms.models @@ -55,6 +56,9 @@ module.exports = class EnrollmentsView extends RootView @calculateEnrollmentStats() @state.set('totalCourses', @courses.size()) super() + + updatePrepaidGroups: -> + @state.set('prepaidGroups', @prepaids.groupBy((p) -> p.status())) calculateEnrollmentStats: -> @removeDeletedStudents() @@ -95,10 +99,13 @@ module.exports = class EnrollmentsView extends RootView if input isnt "" and (parseFloat(input) isnt parseInt(input) or _.isNaN parseInt(input)) @$('#students-input').val(@state.get('numberOfStudents')) else - @state.set('numberOfStudents', Math.max(parseInt(@$('#students-input').val()) or 0, 0)) + @state.set({'numberOfStudents': Math.max(parseInt(@$('#students-input').val()) or 0, 0)}, {silent: true}) # do not re-render numberOfStudentsIsValid: -> 0 < @get('numberOfStudents') < 100000 onClickEnrollStudentsButton: -> modal = new ActivateLicensesModal({ selectedUsers: @notEnrolledUsers, users: @members }) @openModalView(modal) + modal.once 'hidden', => + @prepaids.add(modal.prepaids.models, { merge: true }) + @debouncedRender() # Because one changed model does not a collection update make diff --git a/app/views/courses/TeacherClassView.coffee b/app/views/courses/TeacherClassView.coffee index 4a086837c..ad1a3b0de 100644 --- a/app/views/courses/TeacherClassView.coffee +++ b/app/views/courses/TeacherClassView.coffee @@ -272,7 +272,6 @@ module.exports = class TeacherClassView extends RootView @students.sort() onKeyPressStudentSearch: (e) -> - console.log 'emit event' @state.set('searchTerm', $(e.target).val()) getSelectedStudentIDs: -> diff --git a/app/views/teachers/EditStudentModal.coffee b/app/views/teachers/EditStudentModal.coffee index 5ff377852..7f7feae65 100644 --- a/app/views/teachers/EditStudentModal.coffee +++ b/app/views/teachers/EditStudentModal.coffee @@ -37,4 +37,9 @@ module.exports = class EditStudentModal extends ModalView @classroom.setStudentPassword(@user, @state.get('newPassword')) onChangeNewPasswordInput: (e) -> - @state.set { newPassword: $(e.currentTarget).val() }, { silent: true } + @state.set { + newPassword: $(e.currentTarget).val() + emailSent: false + passwordChanged: false + }, { silent: true } + @renderSelectors('.change-password-btn') diff --git a/app/views/teachers/TeachersContactModal.coffee b/app/views/teachers/TeachersContactModal.coffee index 260a3bf4e..5e2c1a183 100644 --- a/app/views/teachers/TeachersContactModal.coffee +++ b/app/views/teachers/TeachersContactModal.coffee @@ -10,7 +10,6 @@ module.exports = class TeachersContactModal extends ModalView events: 'submit form': 'onSubmitForm' - 'change form': 'onChangeForm' initialize: (options={}) -> @state = new State({ @@ -40,10 +39,6 @@ module.exports = class TeachersContactModal extends ModalView @state.set('formValues', { email, message }) super() - onChangeForm: -> - # Want to re-render without losing form focus. TODO: figure out how in state system. - @$('#submit-btn').attr('disabled', false) - onSubmitForm: (e) -> e.preventDefault() return if @state.get('sendingState') is 'sending' @@ -64,7 +59,12 @@ module.exports = class TeachersContactModal extends ModalView contact.send({ data context: @ - success: -> @state.set({ sendingState: 'sent' }) + success: -> + @state.set({ sendingState: 'sent' }) + me.set('enrollmentRequestSent', true) + setTimeout(=> + @hide?() + , 3000) error: -> @state.set({ sendingState: 'error' }) }) diff --git a/server/middleware/course-instances.coffee b/server/middleware/course-instances.coffee index 86f66b01c..b7cee43f4 100644 --- a/server/middleware/course-instances.coffee +++ b/server/middleware/course-instances.coffee @@ -98,7 +98,7 @@ module.exports = throw new errors.NotFound('Level original ObjectId not found in Classroom courses') if not nextLevelOriginal - res.status(200).send({}) + return res.status(200).send({}) dbq = Level.findOne({original: mongoose.Types.ObjectId(nextLevelOriginal)}) dbq.sort({ 'version.major': -1, 'version.minor': -1 }) @@ -137,7 +137,7 @@ module.exports = for courseInstance in courseInstances if members = courseInstance.get('members') userIDs.push(userID) for userID in members - users = yield User.find({_id: {$in: userIDs}}, {coursePrepaid: 1}) + users = yield User.find({_id: {$in: userIDs}}, {coursePrepaid: 1, coursePrepaidID: 1}) prepaidIDs = [] for user in users diff --git a/server/middleware/levels.coffee b/server/middleware/levels.coffee index 814c7add0..0d04b1c81 100644 --- a/server/middleware/levels.coffee +++ b/server/middleware/levels.coffee @@ -50,12 +50,13 @@ module.exports = courseID = null classroomMap = {} classroomMap[classroom.id] = classroom for classroom in classrooms + levelOriginal = level.get('original') for courseInstance in courseInstances classroom = classroomMap[courseInstance.get('classroomID').toString()] courseID = courseInstance.get('courseID') classroomCourse = _.find(classroom.get('courses'), (c) -> c._id.equals(courseID)) for courseLevel in classroomCourse.levels - if courseLevel.original.equals(level._id) + if courseLevel.original.equals(levelOriginal) classroomWithLevel = classroom break break if classroomWithLevel @@ -67,7 +68,7 @@ module.exports = unless course.get('free') or req.user.isEnrolled() throw new errors.PaymentRequired('You must be enrolled to access this content') - lang = classroomWithLevel.get('aceConfig').language + lang = classroomWithLevel.get('aceConfig')?.language attrs.codeLanguage = lang if lang else diff --git a/server/routes/index.coffee b/server/routes/index.coffee index 7dafff5e8..1a66bd12b 100644 --- a/server/routes/index.coffee +++ b/server/routes/index.coffee @@ -73,7 +73,7 @@ module.exports.setup = (app) -> app.get('/db/course/:handle', mw.rest.getByHandle(Course)) app.get('/db/course/:handle/levels/:levelOriginal/next', mw.courses.fetchNextLevel) - app.get('/db/course_instance/-/recent', mw.auth.checkHasPermission(['admin']), mw.courseInstances.fetchRecent) + app.post('/db/course_instance/-/recent', mw.auth.checkHasPermission(['admin']), mw.courseInstances.fetchRecent) app.get('/db/course_instance/:handle/levels/:levelOriginal/next', mw.courseInstances.fetchNextLevel) app.post('/db/course_instance/:handle/members', mw.auth.checkLoggedIn(), mw.courseInstances.addMembers) app.get('/db/course_instance/:handle/classroom', mw.auth.checkLoggedIn(), mw.courseInstances.fetchClassroom) diff --git a/spec/server/functional/course_instance.spec.coffee b/spec/server/functional/course_instance.spec.coffee index 751c22db1..da89aa2b1 100644 --- a/spec/server/functional/course_instance.spec.coffee +++ b/spec/server/functional/course_instance.spec.coffee @@ -361,7 +361,7 @@ describe 'GET /db/course_instance/:handle/classroom', -> expect(res.statusCode).toBe(403) done() -describe 'GET /db/course_instance/-/recent', -> +describe 'POST /db/course_instance/-/recent', -> url = getURL('/db/course_instance/-/recent') @@ -383,7 +383,7 @@ describe 'GET /db/course_instance/-/recent', -> done() it 'returns all non-HoC course instances and their related users and prepaids', utils.wrap (done) -> - [res, body] = yield request.getAsync(url, { json: true }) + [res, body] = yield request.postAsync(url, { json: true }) expect(res.statusCode).toBe(200) expect(res.body.courseInstances[0]._id).toBe(@courseInstance.id) expect(res.body.students[0]._id).toBe(@student.id) @@ -393,23 +393,23 @@ describe 'GET /db/course_instance/-/recent', -> it 'returns course instances within a specified range', utils.wrap (done) -> startDay = moment().subtract(1, 'day').format('YYYY-MM-DD') endDay = moment().add(1, 'day').format('YYYY-MM-DD') - [res, body] = yield request.getAsync(url, { json: { startDay, endDay } }) + [res, body] = yield request.postAsync(url, { json: { startDay, endDay } }) expect(res.body.courseInstances.length).toBe(1) startDay = moment().add(1, 'day').format('YYYY-MM-DD') endDay = moment().add(2, 'day').format('YYYY-MM-DD') - [res, body] = yield request.getAsync(url, { json: { startDay, endDay } }) + [res, body] = yield request.postAsync(url, { json: { startDay, endDay } }) expect(res.body.courseInstances.length).toBe(0) startDay = moment().subtract(2, 'day').format('YYYY-MM-DD') endDay = moment().subtract(1, 'day').format('YYYY-MM-DD') - [res, body] = yield request.getAsync(url, { json: { startDay, endDay } }) + [res, body] = yield request.postAsync(url, { json: { startDay, endDay } }) expect(res.body.courseInstances.length).toBe(0) done() it 'returns 403 if not an admin', utils.wrap (done) -> yield utils.loginUser(@teacher) - [res, body] = yield request.getAsync(url, { json: true }) + [res, body] = yield request.postAsync(url, { json: true }) expect(res.statusCode).toBe(403) done() diff --git a/spec/server/functional/level.spec.coffee b/spec/server/functional/level.spec.coffee index 7898fdaf4..d97a68919 100644 --- a/spec/server/functional/level.spec.coffee +++ b/spec/server/functional/level.spec.coffee @@ -8,6 +8,7 @@ User = require '../../../server/models/User' request = require '../request' utils = require '../utils' moment = require 'moment' +mongoose = require 'mongoose' describe 'Level', -> @@ -42,13 +43,18 @@ describe 'Level', -> describe 'GET /db/level/:handle/session', -> - describe 'when level is a course level', -> + describe 'when level IS a course level', -> beforeEach utils.wrap (done) -> yield utils.clearModels([Campaign, Course, CourseInstance, Level, User]) admin = yield utils.initAdmin() yield utils.loginUser(admin) @level = yield utils.makeLevel({type: 'course'}) + + # To ensure test compares original, not id, make them different. TODO: Make factories do this normally? + @level.set('original', new mongoose.Types.ObjectId()) + @level.save() + @campaign = yield utils.makeCampaign({}, {levels: [@level]}) @course = yield utils.makeCourse({free: true}, {campaign: @campaign}) @student = yield utils.initUser({role: 'student'}) @@ -66,6 +72,14 @@ describe 'GET /db/level/:handle/session', -> expect(res.statusCode).toBe(200) expect(body.codeLanguage).toBe('javascript') done() + + it 'works if the classroom has no aceConfig', utils.wrap (done) -> + @classroom.set('aceConfig', undefined) + yield @classroom.save() + [res, body] = yield request.getAsync { uri: @url, json: true } + expect(res.statusCode).toBe(200) + expect(body.codeLanguage).toBe('python') + done() it 'returns 402 if the user is not in a course with that level', utils.wrap (done) -> otherStudent = yield utils.initUser({role: 'student'}) @@ -136,3 +150,31 @@ describe 'GET /db/level/:handle/session', -> [res, body] = yield request.getAsync { uri: @url, json: true } expect(body._id).toBe(sessionID) done() + + describe 'when the level is not free', -> + beforeEach utils.wrap (done) -> + yield @level.update({$set: {requiresSubscription: true}}) + done() + + it 'returns 402 for normal users', utils.wrap (done) -> + [res, body] = yield request.getAsync { uri: @url, json: true } + expect(res.statusCode).toBe(402) + done() + + it 'returns 200 for admins', utils.wrap (done) -> + yield @player.update({$set: {permissions: ['admin']}}) + [res, body] = yield request.getAsync { uri: @url, json: true } + expect(res.statusCode).toBe(200) + done() + + it 'returns 200 for adventurer levels', utils.wrap (done) -> + yield @level.update({$set: {adventurer: true}}) + [res, body] = yield request.getAsync { uri: @url, json: true } + expect(res.statusCode).toBe(200) + done() + + it 'returns 200 for subscribed users', utils.wrap (done) -> + yield @player.update({$set: {stripe: {free: true}}}) + [res, body] = yield request.getAsync { uri: @url, json: true } + expect(res.statusCode).toBe(200) + done() diff --git a/test/app/views/courses/EnrollmentsView.spec.coffee b/test/app/views/courses/EnrollmentsView.spec.coffee index 99b880d33..eb6c59a27 100644 --- a/test/app/views/courses/EnrollmentsView.spec.coffee +++ b/test/app/views/courses/EnrollmentsView.spec.coffee @@ -11,6 +11,7 @@ describe 'EnrollmentsView', -> beforeEach (done) -> me.set('anonymous', false) me.set('role', 'teacher') + me.set('enrollmentRequestSent', false) @view = new EnrollmentsView() # Make three classrooms, sharing users from a pool of 10, 5 of which are enrolled @@ -92,7 +93,8 @@ describe 'EnrollmentsView', -> describe 'when there are no prepaids to show', -> beforeEach (done) -> - @view.prepaids.reset() + @view.prepaids.reset([]) + @view.updatePrepaidGroups() _.defer(done) it 'fills the void with the rest of the page content', -> diff --git a/test/app/views/courses/TeachersContactModal.spec.coffee b/test/app/views/courses/TeachersContactModal.spec.coffee index 82d62cea1..69552323e 100644 --- a/test/app/views/courses/TeachersContactModal.spec.coffee +++ b/test/app/views/courses/TeachersContactModal.spec.coffee @@ -45,9 +45,6 @@ describe 'TeachersContactModal', -> it 'shows a success message', -> expect(@modal.$('.alert-success').length).toBe(1) - describe 'submit button', -> - it 'is disabled until one of the inputs changes', -> - expect(@modal.$('#submit-btn').is(':disabled')).toBe(true) - @modal.$('input[name="email"]').trigger('change') - expect(@modal.$('#submit-btn').is(':disabled')).toBe(false) + it 'disables the submit button', -> + expect(@modal.$('#submit-btn').is(':disabled')).toBe(true) diff --git a/test/app/views/teachers/EditStudentModal.spec.coffee b/test/app/views/teachers/EditStudentModal.spec.coffee index cb268911a..3535f9db8 100644 --- a/test/app/views/teachers/EditStudentModal.spec.coffee +++ b/test/app/views/teachers/EditStudentModal.spec.coffee @@ -12,7 +12,8 @@ describe 'EditStudentModal', -> describe 'for a verified user', -> beforeEach (done) -> user = factories.makeUser({ email, emailVerified: true }) - modal = new EditStudentModal({ user }) + classroom = factories.makeClassroom() + modal = new EditStudentModal({ user, classroom }) request = jasmine.Ajax.requests.mostRecent() request.respondWith({ status: 200, responseText: JSON.stringify(user) }) jasmine.demoModal(modal) @@ -37,7 +38,8 @@ describe 'EditStudentModal', -> describe 'for an unverified user', -> beforeEach (done) -> user = factories.makeUser({ email , emailVerified: false }) - modal = new EditStudentModal({ user }) + classroom = factories.makeClassroom() + modal = new EditStudentModal({ user, classroom }) request = jasmine.Ajax.requests.mostRecent() request.respondWith({ status: 200, responseText: JSON.stringify(user) }) jasmine.demoModal(modal)