Skip to content

Commit

Permalink
[LTI Provider] Grade passback for non-leaf blocks.
Browse files Browse the repository at this point in the history
This change allows graded assignments to be added to a campus LMS
regardless of the granularity at which the problem sits. Previously
a grade could only be returned if the usage ID for the problem itself
was specified in the LTI launch.

The code assumes that courses taking advantage of this functionality
are arranged in a hiearchy (with sections being parents to verticals,
and verticals being parents to problems). When a grading event occurs
it traverses the parent hiearchy to identify any previous graded LTI
launches for which the new scoring event should generate a grade
update. It then calculates and sends scores to each of those outcome
services.

Since grade calculation is an expensive operation, the code optimizes
the case where a problem has been added only once as a leaf unit. In
that case it is able to behave as before, just taking the grade from
the signal without having to calculate grades for the whole course.
  • Loading branch information
mcgachey authored and amir-qayyum-khan committed Mar 1, 2016
1 parent 01a6b0d commit 0c49f2c
Show file tree
Hide file tree
Showing 11 changed files with 785 additions and 152 deletions.
102 changes: 85 additions & 17 deletions lms/djangoapps/courseware/grades.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,51 @@ def get(self, location):
return max_score


class ProgressSummary(object):
"""
Wrapper class for the computation of a user's scores across a course.
Attributes
chapters: a summary of all sections with problems in the course. It is
organized as an array of chapters, each containing an array of sections,
each containing an array of scores. This contains information for graded
and ungraded problems, and is good for displaying a course summary with
due dates, etc.
weighted_scores: a dictionary mapping module locations to weighted Score
objects.
locations_to_children: a dictionary mapping module locations to their
direct descendants.
"""
def __init__(self, chapters, weighted_scores, locations_to_children):
self.chapters = chapters
self.weighted_scores = weighted_scores
self.locations_to_children = locations_to_children

def score_for_module(self, location):
"""
Calculate the aggregate weighted score for any location in the course.
This method returns a tuple containing (earned_score, possible_score).
If the location is of 'problem' type, this method will return the
possible and earned scores for that problem. If the location refers to a
composite module (a vertical or section ) the scores will be the sums of
all scored problems that are children of the chosen location.
"""
if location in self.weighted_scores:
score = self.weighted_scores[location]
return score.earned, score.possible
children = self.locations_to_children[location]
earned = 0.0
possible = 0.0
for child in children:
child_earned, child_possible = self.score_for_module(child)
earned += child_earned
possible += child_possible
return earned, possible


def descriptor_affects_grading(block_types_affecting_grading, descriptor):
"""
Returns True if the descriptor could have any impact on grading, else False.
Expand Down Expand Up @@ -452,6 +497,21 @@ def progress_summary(student, request, course, field_data_cache=None, scores_cli
in case there are unanticipated errors.
"""
with manual_transaction():
progress = _progress_summary(student, request, course, field_data_cache, scores_client)
if progress:
return progress.chapters
else:
return None


@transaction.commit_manually
def get_weighted_scores(student, course, field_data_cache=None, scores_client=None):
"""
Uses the _progress_summary method to return a ProgressSummmary object
containing details of a students weighted scores for the course.
"""
with manual_transaction():
request = _get_mock_request(student)
return _progress_summary(student, request, course, field_data_cache, scores_client)


Expand Down Expand Up @@ -502,14 +562,15 @@ def _progress_summary(student, request, course, field_data_cache=None, scores_cl
max_scores_cache.fetch_from_remote(field_data_cache.scorable_locations)

chapters = []
locations_to_children = defaultdict(list)
locations_to_weighted_scores = {}
# Don't include chapters that aren't displayable (e.g. due to error)
for chapter_module in course_module.get_display_items():
# Skip if the chapter is hidden
if chapter_module.hide_from_toc:
continue

sections = []

for section_module in chapter_module.get_display_items():
# Skip if the section is hidden
with manual_transaction():
Expand All @@ -524,7 +585,7 @@ def _progress_summary(student, request, course, field_data_cache=None, scores_cl
for module_descriptor in yield_dynamic_descriptor_descendants(
section_module, student.id, module_creator
):
course_id = course.id
locations_to_children[module_descriptor.parent].append(module_descriptor.location)
(correct, total) = get_score(
student,
module_descriptor,
Expand All @@ -536,16 +597,17 @@ def _progress_summary(student, request, course, field_data_cache=None, scores_cl
if correct is None and total is None:
continue

scores.append(
Score(
correct,
total,
graded,
module_descriptor.display_name_with_default,
module_descriptor.location
)
weighted_location_score = Score(
correct,
total,
graded,
module_descriptor.display_name_with_default,
module_descriptor.location
)

scores.append(weighted_location_score)
locations_to_weighted_scores[module_descriptor.location] = weighted_location_score

scores.reverse()
section_total, _ = graders.aggregate_scores(
scores, section_module.display_name_with_default)
Expand All @@ -570,7 +632,7 @@ def _progress_summary(student, request, course, field_data_cache=None, scores_cl

max_scores_cache.push_to_remote()

return chapters
return ProgressSummary(chapters, locations_to_weighted_scores, locations_to_children)


def weighted_score(raw_correct, raw_total, weight):
Expand Down Expand Up @@ -698,15 +760,10 @@ def iterate_grades_for(course_or_id, students, keep_raw_scores=False):
else:
course = course_or_id

# We make a fake request because grading code expects to be able to look at
# the request. We have to attach the correct user to the request before
# grading that student.
request = RequestFactory().get('/')

for student in students:
with dog_stats_api.timer('lms.grades.iterate_grades_for', tags=[u'action:{}'.format(course.id)]):
try:
request.user = student
request = _get_mock_request(student)
# Grading calls problem rendering, which calls masquerading,
# which checks session vars -- thus the empty session dict below.
# It's not pretty, but untangling that is currently beyond the
Expand All @@ -725,3 +782,14 @@ def iterate_grades_for(course_or_id, students, keep_raw_scores=False):
exc.message
)
yield student, {}, exc.message


def _get_mock_request(student):
"""
Make a fake request because grading code expects to be able to look at
the request. We have to attach the correct user to the request before
grading that student.
"""
request = RequestFactory().get('/')
request.user = student
return request
128 changes: 126 additions & 2 deletions lms/djangoapps/courseware/tests/test_grades.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@
Test grade calculation.
"""
from django.http import Http404
from django.test import TestCase
from django.test.client import RequestFactory

from mock import patch
from mock import patch, MagicMock
from nose.plugins.attrib import attr
from opaque_keys.edx.locations import SlashSeparatedCourseKey
from opaque_keys.edx.locator import CourseLocator, BlockUsageLocator

from courseware.grades import field_data_cache_for_grading, grade, iterate_grades_for, MaxScoresCache
from courseware.grades import field_data_cache_for_grading, grade, iterate_grades_for, MaxScoresCache, ProgressSummary
from student.tests.factories import UserFactory
from student.models import CourseEnrollment
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
Expand Down Expand Up @@ -194,3 +196,125 @@ def test_field_data_cache_scorable_locations(self):
self.assertNotIn('html', block_types)
self.assertNotIn('discussion', block_types)
self.assertIn('problem', block_types)


class TestProgressSummary(TestCase):
"""
Test the method that calculates the score for a given block based on the
cumulative scores of its children. This test class uses a hard-coded block
hierarchy with scores as follows:
a
+--------+--------+
b c
+--------------+-----------+ |
d e f g
+-----+ +-----+-----+ | |
h i j k l m n
(2/5) (3/5) (0/1) - (1/3) - (3/10)
"""
def setUp(self):
super(TestProgressSummary, self).setUp()
self.course_key = CourseLocator(
org='some_org',
course='some_course',
run='some_run'
)
self.loc_a = self.create_location('chapter', 'a')
self.loc_b = self.create_location('section', 'b')
self.loc_c = self.create_location('section', 'c')
self.loc_d = self.create_location('vertical', 'd')
self.loc_e = self.create_location('vertical', 'e')
self.loc_f = self.create_location('vertical', 'f')
self.loc_g = self.create_location('vertical', 'g')
self.loc_h = self.create_location('problem', 'h')
self.loc_i = self.create_location('problem', 'i')
self.loc_j = self.create_location('problem', 'j')
self.loc_k = self.create_location('html', 'k')
self.loc_l = self.create_location('problem', 'l')
self.loc_m = self.create_location('html', 'm')
self.loc_n = self.create_location('problem', 'n')

weighted_scores = {
self.loc_h: self.create_score(2, 5),
self.loc_i: self.create_score(3, 5),
self.loc_j: self.create_score(0, 1),
self.loc_l: self.create_score(1, 3),
self.loc_n: self.create_score(3, 10),
}
locations_to_scored_children = {
self.loc_a: [self.loc_h, self.loc_i, self.loc_j, self.loc_l, self.loc_n],
self.loc_b: [self.loc_h, self.loc_i, self.loc_j, self.loc_l],
self.loc_c: [self.loc_n],
self.loc_d: [self.loc_h, self.loc_i],
self.loc_e: [self.loc_j, self.loc_l],
self.loc_f: [],
self.loc_g: [self.loc_n],
self.loc_k: [],
self.loc_m: [],
}
self.progress_summary = ProgressSummary(
None, weighted_scores, locations_to_scored_children
)

def create_score(self, earned, possible):
"""
Create a new mock Score object with specified earned and possible values
"""
score = MagicMock()
score.possible = possible
score.earned = earned
return score

def create_location(self, block_type, block_id):
"""
Create a new BlockUsageLocation with the given type and ID.
"""
return BlockUsageLocator(
course_key=self.course_key, block_type=block_type, block_id=block_id
)

def test_score_chapter(self):
earned, possible = self.progress_summary.score_for_module(self.loc_a)
self.assertEqual(earned, 9)
self.assertEqual(possible, 24)

def test_score_section_many_leaves(self):
earned, possible = self.progress_summary.score_for_module(self.loc_b)
self.assertEqual(earned, 6)
self.assertEqual(possible, 14)

def test_score_section_one_leaf(self):
earned, possible = self.progress_summary.score_for_module(self.loc_c)
self.assertEqual(earned, 3)
self.assertEqual(possible, 10)

def test_score_vertical_two_leaves(self):
earned, possible = self.progress_summary.score_for_module(self.loc_d)
self.assertEqual(earned, 5)
self.assertEqual(possible, 10)

def test_score_vertical_two_leaves_one_unscored(self):
earned, possible = self.progress_summary.score_for_module(self.loc_e)
self.assertEqual(earned, 1)
self.assertEqual(possible, 4)

def test_score_vertical_no_score(self):
earned, possible = self.progress_summary.score_for_module(self.loc_f)
self.assertEqual(earned, 0)
self.assertEqual(possible, 0)

def test_score_vertical_one_leaf(self):
earned, possible = self.progress_summary.score_for_module(self.loc_g)
self.assertEqual(earned, 3)
self.assertEqual(possible, 10)

def test_score_leaf(self):
earned, possible = self.progress_summary.score_for_module(self.loc_h)
self.assertEqual(earned, 2)
self.assertEqual(possible, 5)

def test_score_leaf_no_score(self):
earned, possible = self.progress_summary.score_for_module(self.loc_m)
self.assertEqual(earned, 0)
self.assertEqual(possible, 0)
30 changes: 30 additions & 0 deletions lms/djangoapps/courseware/tests/test_submitting_problems.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,36 @@ def show_question_answer(self, problem_url_name):
resp = self.client.post(modx_url)
return resp


class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase, ProblemSubmissionTestMixin):
"""
Check that a course gets graded properly.
"""

# arbitrary constant
COURSE_SLUG = "100"
COURSE_NAME = "test_course"

def setUp(self):

super(TestSubmittingProblems, self).setUp(create_user=False)
# Create course
self.course = CourseFactory.create(display_name=self.COURSE_NAME, number=self.COURSE_SLUG)
assert self.course, "Couldn't load course %r" % self.COURSE_NAME

# create a test student
self.student = 'view@test.com'
self.password = 'foo'
self.create_account('u1', self.student, self.password)
self.activate_user(self.student)
self.enroll(self.course)
self.student_user = User.objects.get(email=self.student)
self.factory = RequestFactory()
# Disable the score change signal to prevent other components from being pulled into tests.
signal_patch = patch('courseware.module_render.SCORE_CHANGED.send')
signal_patch.start()
self.addCleanup(signal_patch.stop)

def add_dropdown_to_section(self, section_location, name, num_inputs=2):
"""
Create and return a dropdown problem.
Expand Down
5 changes: 4 additions & 1 deletion lms/djangoapps/instructor/tests/test_enrollment.py
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,10 @@ def test_delete_student_attempts(self):
module_state_key=msk
).count(), 0)

def test_delete_submission_scores(self):
# Disable the score change signal to prevent other components from being
# pulled into tests.
@mock.patch('courseware.module_render.SCORE_CHANGED.send')
def test_delete_submission_scores(self, _lti_mock):
user = UserFactory()
problem_location = self.course_key.make_usage_key('dummy', 'module')

Expand Down
Loading

0 comments on commit 0c49f2c

Please sign in to comment.