From 0bbf948859173cc5e674f5d5e61659aa48a6128b Mon Sep 17 00:00:00 2001 From: Giovanni Di Milia Date: Tue, 1 Sep 2015 12:01:21 -0400 Subject: [PATCH] Improved performances for assignment listing - Modified "staff_grading_data" function to be able to pull infos for many students or a specific one - Fixed bug with multiple identical incremental ajax requests: - Now the cancel button of the "Enter Grade" modal does not trigger an Ajax get request --- edx_sga/sga.py | 106 +++++++++++++++++-------------- edx_sga/static/js/src/edx_sga.js | 61 ++++++++++-------- edx_sga/tests.py | 30 +++++++++ 3 files changed, 125 insertions(+), 72 deletions(-) diff --git a/edx_sga/sga.py b/edx_sga/sga.py index 826f7cca..0e41ecca 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 +from student.models import user_by_anonymous_id, anonymous_id_for_user from submissions import api as submissions_api from submissions.models import StudentItem as SubmissionsStudent @@ -263,53 +263,57 @@ def student_state(self): "upload_allowed": self.upload_allowed(), } - def staff_grading_data(self): + def staff_grading_data(self, student_user=None): """ Return student assignment information for display on the grading screen. """ - 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. + 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: 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() - yield { + 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( + { 'module_id': module.id, 'student_id': student.student_id, 'submission_id': submission['uuid'], @@ -326,9 +330,10 @@ def get_student_data(): 'annotated': state.get("annotated_filename"), 'comment': state.get("comment", ''), } + ) return { - 'assignments': list(get_student_data()), + 'assignments': student_data, 'max_score': self.max_score(), 'display_name': self.display_name } @@ -453,7 +458,9 @@ def staff_upload_annotated(self, request, suffix=''): module.module_state_key, module.student.username ) - return Response(json_body=self.staff_grading_data()) + return Response( + json_body=self.staff_grading_data(student_user=module.student) + ) @XBlock.handler def download_assignment(self, request, suffix=''): @@ -552,7 +559,9 @@ 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=''): @@ -578,8 +587,9 @@ def enter_grade(self, request, suffix=''): module.module_state_key, module.student.username ) - - return Response(json_body=self.staff_grading_data()) + return Response( + json_body=self.staff_grading_data(student_user=module.student) + ) @XBlock.handler def remove_grade(self, request, suffix=''): @@ -606,7 +616,9 @@ def remove_grade(self, request, suffix=''): module.module_state_key, module.student.username ) - return Response(json_body=self.staff_grading_data()) + return Response( + json_body=self.staff_grading_data(student_user=module.student) + ) 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 c0200d38..513690cb 100644 --- a/edx_sga/static/js/src/edx_sga.js +++ b/edx_sga/static/js/src/edx_sga.js @@ -16,6 +16,7 @@ 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 @@ -104,7 +105,30 @@ function StaffGradedAssignmentXBlock(runtime, element) { updateChangeEvent(fileUpload); } - function renderStaffGrading(data) { + // 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; $('.grade-modal').hide(); if (data.display_name !== '') { @@ -145,7 +169,7 @@ function StaffGradedAssignmentXBlock(runtime, element) { // Add a time delay so user will notice upload finishing // for small files setTimeout( - function() { renderStaffGrading(data.result); }, + function() { updateStudentAssignment(data.result); }, 3000); } }); @@ -207,34 +231,21 @@ function StaffGradedAssignmentXBlock(runtime, element) { } else { // No errors $.post(enterGradeUrl, form.serialize()) - .success(renderStaffGrading); + .success(updateStudentAssignment); } }); - form.find('#remove-grade').on('click', function() { + form.find('#remove-grade').off('click').on('click', function() { var url = removeGradeUrl + '?module_id=' + row.data('module_id') + '&student_id=' + row.data('student_id'); - $.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); + $.get(url).success(updateStudentAssignment); }); + form.find('#enter-grade-cancel').off('click').on( + 'click', + function() { + renderStaffGrading(); + } + ); } function updateChangeEvent(fileUploadObj) { @@ -269,7 +280,7 @@ function StaffGradedAssignmentXBlock(runtime, element) { .on('click', function() { $.ajax({ url: getStaffGradingUrl, - success: renderStaffGrading + success: replaceStudentsAssignments }); }); block.find('#staff-debug-info-button') diff --git a/edx_sga/tests.py b/edx_sga/tests.py index 6c16fa8a..ed129e64 100644 --- a/edx_sga/tests.py +++ b/edx_sga/tests.py @@ -564,6 +564,36 @@ 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): """