Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved performances for assignment listing #129

Merged
merged 1 commit into from
Sep 4, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 59 additions & 47 deletions edx_sga/sga.py
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
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
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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this for?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left that behind: it's the anonymous student ID for that test

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