Skip to content

Commit

Permalink
Merge pull request #129 from mitodl/bug/gdm/111_performance
Browse files Browse the repository at this point in the history 
Improved performances for assignment listing

fixes #111 
[delivers #101927046]
  • Loading branch information
@pdpinch
pdpinch committed Sep 4, 2015
2 parents 54e4bd1 + 0bbf948 commit 2a37338
Show file tree
Hide file tree
Showing 3 changed files with 125 additions and 72 deletions.
106 changes: 59 additions & 47 deletions edx_sga/sga.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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'],
Expand All @@ -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
}
Expand Down Expand Up @@ -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=''):
Expand Down Expand Up @@ -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=''):
Expand All @@ -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=''):
Expand All @@ -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
Expand Down
61 changes: 36 additions & 25 deletions edx_sga/static/js/src/edx_sga.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 !== '') {
Expand Down Expand Up @@ -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);
}
});
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -269,7 +280,7 @@ function StaffGradedAssignmentXBlock(runtime, element) {
.on('click', function() {
$.ajax({
url: getStaffGradingUrl,
success: renderStaffGrading
success: replaceStudentsAssignments
});
});
block.find('#staff-debug-info-button')
Expand Down
30 changes: 30 additions & 0 deletions edx_sga/tests.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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):
"""
Expand Down

0 comments on commit 2a37338

Please sign in to comment.