diff --git a/edx_sga/sga.py b/edx_sga/sga.py index 0e41ecca..826f7cca 100644 --- a/edx_sga/sga.py +++ b/edx_sga/sga.py @@ -21,7 +21,7 @@ from django.conf import settings from django.template import Context, Template -from student.models import user_by_anonymous_id, anonymous_id_for_user +from student.models import user_by_anonymous_id from submissions import api as submissions_api from submissions.models import StudentItem as SubmissionsStudent @@ -263,57 +263,53 @@ def student_state(self): "upload_allowed": self.upload_allowed(), } - def staff_grading_data(self, student_user=None): + def staff_grading_data(self): """ Return student assignment information for display on the grading screen. """ - student_data = [] - - # Submissions doesn't have API for this, just use model directly. - if student_user is not None: - student_anonymous_id = anonymous_id_for_user( - user=student_user, course_id=self.course_id, save=False - ) - students = SubmissionsStudent.objects.filter( - student_id=student_anonymous_id - ) - else: + def get_student_data(): + # pylint: disable=no-member + """ + Returns a dict of student assignment information along with + annotated file name, student id and module id, this + information will be used on grading screen + """ + # Submissions doesn't have API for this, just use model directly. students = SubmissionsStudent.objects.filter( course_id=self.course_id, item_id=self.block_id) - for student in students: - submission = self.get_submission(student.student_id) - if not submission: - continue - user = user_by_anonymous_id(student.student_id) - module, created = StudentModule.objects.get_or_create( - course_id=self.course_id, - module_state_key=self.location, - student=user, - defaults={ - 'state': '{}', - 'module_type': self.category, - }) - if created: - log.info( - "Init for course:%s module:%s student:%s ", - module.course_id, - module.module_state_key, - module.student.username - ) - - state = json.loads(module.state) - score = self.get_score(student.student_id) - approved = score is not None - if score is None: - score = state.get('staff_score') - needs_approval = score is not None - else: - needs_approval = False - instructor = self.is_instructor() - student_data.append( - { + for student in students: + submission = self.get_submission(student.student_id) + if not submission: + continue + user = user_by_anonymous_id(student.student_id) + module, created = StudentModule.objects.get_or_create( + course_id=self.course_id, + module_state_key=self.location, + student=user, + defaults={ + 'state': '{}', + 'module_type': self.category, + }) + if created: + log.info( + "Init for course:%s module:%s student:%s ", + module.course_id, + module.module_state_key, + module.student.username + ) + + state = json.loads(module.state) + score = self.get_score(student.student_id) + approved = score is not None + if score is None: + score = state.get('staff_score') + needs_approval = score is not None + else: + needs_approval = False + instructor = self.is_instructor() + yield { 'module_id': module.id, 'student_id': student.student_id, 'submission_id': submission['uuid'], @@ -330,10 +326,9 @@ def staff_grading_data(self, student_user=None): 'annotated': state.get("annotated_filename"), 'comment': state.get("comment", ''), } - ) return { - 'assignments': student_data, + 'assignments': list(get_student_data()), 'max_score': self.max_score(), 'display_name': self.display_name } @@ -458,9 +453,7 @@ def staff_upload_annotated(self, request, suffix=''): module.module_state_key, module.student.username ) - return Response( - json_body=self.staff_grading_data(student_user=module.student) - ) + return Response(json_body=self.staff_grading_data()) @XBlock.handler def download_assignment(self, request, suffix=''): @@ -559,9 +552,7 @@ def get_staff_grading_data(self, request, suffix=''): Return the html for the staff grading view """ require(self.is_course_staff()) - return Response( - json_body=self.staff_grading_data() - ) + return Response(json_body=self.staff_grading_data()) @XBlock.handler def enter_grade(self, request, suffix=''): @@ -587,9 +578,8 @@ def enter_grade(self, request, suffix=''): module.module_state_key, module.student.username ) - return Response( - json_body=self.staff_grading_data(student_user=module.student) - ) + + return Response(json_body=self.staff_grading_data()) @XBlock.handler def remove_grade(self, request, suffix=''): @@ -616,9 +606,7 @@ def remove_grade(self, request, suffix=''): module.module_state_key, module.student.username ) - return Response( - json_body=self.staff_grading_data(student_user=module.student) - ) + return Response(json_body=self.staff_grading_data()) def is_course_staff(self): # pylint: disable=no-member diff --git a/edx_sga/static/js/src/edx_sga.js b/edx_sga/static/js/src/edx_sga.js index 513690cb..c0200d38 100644 --- a/edx_sga/static/js/src/edx_sga.js +++ b/edx_sga/static/js/src/edx_sga.js @@ -16,7 +16,6 @@ function StaffGradedAssignmentXBlock(runtime, element) { var removeGradeUrl = runtime.handlerUrl(element, 'remove_grade'); var template = _.template($(element).find("#sga-tmpl").text()); var gradingTemplate; - var studentAssignments; function render(state) { // Add download urls to template context @@ -105,30 +104,7 @@ function StaffGradedAssignmentXBlock(runtime, element) { updateChangeEvent(fileUpload); } - // function to replace the entire content of the global variable - // containing all the assignments - function replaceStudentsAssignments (data) { - studentAssignments = data; - renderStaffGrading(); - } - - // function to update one student assignment - function updateStudentAssignment (data) { - _.map(data.assignments, function (assignment) { - studentAssignments.assignments = studentAssignments.assignments.map( - function(obj) { - if (obj.submission_id === assignment.submission_id) { - return assignment; - } - return obj; - } - ); - }); - renderStaffGrading(); - } - - function renderStaffGrading() { - var data = studentAssignments; + function renderStaffGrading(data) { $('.grade-modal').hide(); if (data.display_name !== '') { @@ -169,7 +145,7 @@ function StaffGradedAssignmentXBlock(runtime, element) { // Add a time delay so user will notice upload finishing // for small files setTimeout( - function() { updateStudentAssignment(data.result); }, + function() { renderStaffGrading(data.result); }, 3000); } }); @@ -231,21 +207,34 @@ function StaffGradedAssignmentXBlock(runtime, element) { } else { // No errors $.post(enterGradeUrl, form.serialize()) - .success(updateStudentAssignment); + .success(renderStaffGrading); } }); - form.find('#remove-grade').off('click').on('click', function() { + form.find('#remove-grade').on('click', function() { var url = removeGradeUrl + '?module_id=' + row.data('module_id') + '&student_id=' + row.data('student_id'); - $.get(url).success(updateStudentAssignment); + $.get(url).success(renderStaffGrading); + }); + form.find('#enter-grade-cancel').on('click', function() { + /* We're kind of stretching the limits of leanModal, here, + * by nesting modals one on top of the other. One side effect + * is that when the enter grade modal is closed, it hides + * the overlay for itself and for the staff grading modal, + * so the overlay is no longer present to click on to close + * the staff grading modal. Since leanModal uses a fade out + * time of 200ms to hide the overlay, our work around is to + * wait 225ms and then just "click" the 'Grade Submissions' + * button again. It would also probably be pretty + * straightforward to submit a patch to leanModal so that it + * would work properly with nested modals. + * + * See: https://github.com/mitodl/edx-sga/issues/13 + */ + setTimeout(function() { + $('#grade-submissions-button').click(); + }, 225); }); - form.find('#enter-grade-cancel').off('click').on( - 'click', - function() { - renderStaffGrading(); - } - ); } function updateChangeEvent(fileUploadObj) { @@ -280,7 +269,7 @@ function StaffGradedAssignmentXBlock(runtime, element) { .on('click', function() { $.ajax({ url: getStaffGradingUrl, - success: replaceStudentsAssignments + success: renderStaffGrading }); }); block.find('#staff-debug-info-button') diff --git a/edx_sga/tests.py b/edx_sga/tests.py index ed129e64..6c16fa8a 100644 --- a/edx_sga/tests.py +++ b/edx_sga/tests.py @@ -564,36 +564,6 @@ def test_get_staff_grading_data(self): self.assertEqual(assignments[1]['annotated'], None) self.assertEqual(assignments[1]['comment'], u'') - def test_staff_grading_data(self): - """ - Test it can return all students or one student - """ - block = self.make_one() - barney = self.make_student( - block, "barney", - filename="foo.txt", - score=10, - annotated_filename="foo_corrected.txt", - comment="Good work!")['module'] - fred = self.make_student( - block, "fred", - filename="bar.txt")['module'] - data = block.staff_grading_data() - assignments = data.get('assignments') - self.assertIsNotNone(assignments) - self.assertEqual(len(assignments), 2) - # 50b287502d8dbf83bec2dcfb78fc4d8e - user_fred = User.objects.get(username='fred') - data = block.staff_grading_data(student_user=user_fred) - assignments = data.get('assignments') - self.assertIsNotNone(assignments) - self.assertEqual(len(assignments), 1) - self.assertEqual(assignments[0]['module_id'], fred.id) - self.assertEqual(assignments[0]['username'], 'fred') - self.assertEqual(assignments[0]['fullname'], 'fred') - self.assertEqual(assignments[0]['filename'], 'bar.txt') - - @mock.patch('edx_sga.sga.log') def test_assert_logging_when_student_module_created(self, mocked_log): """