Add support for generating problem response reports for multiple blocks, or filtered block types.

This change adds support for specifying multiple root blocks while generating
problem response reports. It also allows specifying a block type filter so that
only blocks of the filtered types will be included in the report.

Finally, this change also consistenly uses absolute path for the location in the
report instead of relative paths.
This commit is contained in:
Kshitij Sobti
2019-02-11 13:07:56 +05:30
parent aec8c0057f
commit 98af7496d7
6 changed files with 180 additions and 68 deletions

View File

@@ -1023,22 +1023,23 @@ def get_problem_responses(request, course_id):
Responds with BadRequest if problem location is faulty.
"""
course_key = CourseKey.from_string(course_id)
problem_location = request.POST.get('problem_location', '')
# A comma-separated list of problem locations
# The name of the POST parameter is `problem_location` (not pluralised) in
# order to preserve backwards compatibility with existing third-party
# scripts.
problem_locations = request.POST.get('problem_location', '')
# A comma-separated list of block types
problem_types_filter = request.POST.get('problem_types_filter', '')
report_type = _('problem responses')
try:
problem_key = UsageKey.from_string(problem_location)
# Are we dealing with an "old-style" problem location?
run = problem_key.run
if not run:
for problem_location in problem_locations.split(','):
problem_key = UsageKey.from_string(problem_location).map_into_course(course_key)
if problem_key.course_key != course_key:
raise InvalidKeyError(type(problem_key), problem_key)
except InvalidKeyError:
return JsonResponseBadRequest(_("Could not find problem with this location."))
task = task_api.submit_calculate_problem_responses_csv(
request, course_key, problem_location
request, course_key, problem_locations, problem_types_filter,
)
success_status = SUCCESS_MESSAGE_TEMPLATE.format(report_type=report_type)

View File

@@ -324,7 +324,9 @@ def submit_bulk_course_email(request, course_key, email_id):
return submit_task(request, task_type, task_class, course_key, task_input, task_key)
def submit_calculate_problem_responses_csv(request, course_key, problem_location):
def submit_calculate_problem_responses_csv(
request, course_key, problem_locations, problem_types_filter=None,
):
"""
Submits a task to generate a CSV file containing all student
answers to a given problem.
@@ -333,7 +335,11 @@ def submit_calculate_problem_responses_csv(request, course_key, problem_location
"""
task_type = 'problem_responses_csv'
task_class = calculate_problem_responses_csv
task_input = {'problem_location': problem_location, 'user_id': request.user.pk}
task_input = {
'problem_location': problem_locations,
'problem_types_filter': problem_types_filter,
'user_id': request.user.pk,
}
task_key = ""
return submit_task(request, task_type, task_class, course_key, task_input, task_key)

View File

@@ -161,7 +161,11 @@ def send_bulk_course_email(entry_id, _xmodule_instance_args):
return run_main_task(entry_id, visit_fcn, action_name)
@task(base=BaseInstructorTask, routing_key=settings.GRADES_DOWNLOAD_ROUTING_KEY)
@task(
name='lms.djangoapps.instructor_task.tasks.calculate_problem_responses_csv.v2',
base=BaseInstructorTask,
routing_key=settings.GRADES_DOWNLOAD_ROUTING_KEY,
)
def calculate_problem_responses_csv(entry_id, xmodule_instance_args):
"""
Compute student answers to a given problem and upload the CSV to

View File

@@ -604,6 +604,23 @@ class ProblemGradeReport(object):
class ProblemResponses(object):
@staticmethod
def _build_block_base_path(block):
"""
Return the display names of the blocks that lie above the supplied block in hierarchy.
Arguments:
block: a single block
Returns:
List[str]: a list of display names of blocks starting from the root block (Course)
"""
path = []
while block.parent:
block = block.get_parent()
path.append(block.display_name)
return list(reversed(path))
@classmethod
def _build_problem_list(cls, course_blocks, root, path=None):
"""
@@ -632,7 +649,9 @@ class ProblemResponses(object):
yield result
@classmethod
def _build_student_data(cls, user_id, course_key, usage_key_str):
def _build_student_data(
cls, user_id, course_key, usage_key_str_list, filter_types=None,
):
"""
Generate a list of problem responses for all problem under the
``problem_location`` root.
@@ -641,17 +660,21 @@ class ProblemResponses(object):
user_id (int): The user id for the user generating the report
course_key (CourseKey): The ``CourseKey`` for the course whose report
is being generated
usage_key_str (str): The generated report will include this
block and it child blocks.
usage_key_str_list (List[str]): The generated report will include these
blocks and their child blocks.
filter_types (List[str]): The report generator will only include data for
block types in this list.
Returns:
Tuple[List[Dict], List[str]]: Returns a list of dictionaries
containing the student data which will be included in the
final csv, and the features/keys to include in that CSV.
"""
usage_key = UsageKey.from_string(usage_key_str).map_into_course(course_key)
usage_keys = [
UsageKey.from_string(usage_key_str).map_into_course(course_key)
for usage_key_str in usage_key_str_list
]
user = get_user_model().objects.get(pk=user_id)
course_blocks = get_course_blocks(user, usage_key)
student_data = []
max_count = settings.FEATURES.get('MAX_PROBLEM_RESPONSES_COUNT')
@@ -662,53 +685,61 @@ class ProblemResponses(object):
student_data_keys = set()
with store.bulk_operations(course_key):
for title, path, block_key in cls._build_problem_list(course_blocks, usage_key):
# Chapter and sequential blocks are filtered out since they include state
# which isn't useful for this report.
if block_key.block_type in ('sequential', 'chapter'):
continue
for usage_key in usage_keys:
if max_count is not None and max_count <= 0:
break
course_blocks = get_course_blocks(user, usage_key)
base_path = cls._build_block_base_path(store.get_item(usage_key))
for title, path, block_key in cls._build_problem_list(course_blocks, usage_key):
# Chapter and sequential blocks are filtered out since they include state
# which isn't useful for this report.
if block_key.block_type in ('sequential', 'chapter'):
continue
block = store.get_item(block_key)
generated_report_data = defaultdict(list)
if filter_types is not None and block_key.block_type not in filter_types:
continue
# Blocks can implement the generate_report_data method to provide their own
# human-readable formatting for user state.
if hasattr(block, 'generate_report_data'):
try:
user_state_iterator = user_state_client.iter_all_for_block(block_key)
for username, state in block.generate_report_data(user_state_iterator, max_count):
generated_report_data[username].append(state)
except NotImplementedError:
pass
block = store.get_item(block_key)
generated_report_data = defaultdict(list)
responses = []
# Blocks can implement the generate_report_data method to provide their own
# human-readable formatting for user state.
if hasattr(block, 'generate_report_data'):
try:
user_state_iterator = user_state_client.iter_all_for_block(block_key)
for username, state in block.generate_report_data(user_state_iterator, max_count):
generated_report_data[username].append(state)
except NotImplementedError:
pass
for response in list_problem_responses(course_key, block_key, max_count):
response['title'] = title
# A human-readable location for the current block
response['location'] = ' > '.join(path)
# A machine-friendly location for the current block
response['block_key'] = str(block_key)
# A block that has a single state per user can contain multiple responses
# within the same state.
user_states = generated_report_data.get(response['username'], [])
if user_states:
# For each response in the block, copy over the basic data like the
# title, location, block_key and state, and add in the responses
for user_state in user_states:
user_response = response.copy()
user_response.update(user_state)
student_data_keys = student_data_keys.union(list(user_state.keys()))
responses.append(user_response)
else:
responses.append(response)
responses = []
student_data += responses
for response in list_problem_responses(course_key, block_key, max_count):
response['title'] = title
# A human-readable location for the current block
response['location'] = ' > '.join(base_path + path)
# A machine-friendly location for the current block
response['block_key'] = str(block_key)
# A block that has a single state per user can contain multiple responses
# within the same state.
user_states = generated_report_data.get(response['username'])
if user_states:
# For each response in the block, copy over the basic data like the
# title, location, block_key and state, and add in the responses
for user_state in user_states:
user_response = response.copy()
user_response.update(user_state)
student_data_keys = student_data_keys.union(list(user_state.keys()))
responses.append(user_response)
else:
responses.append(response)
if max_count is not None:
max_count -= len(responses)
if max_count <= 0:
break
student_data += responses
if max_count is not None:
max_count -= len(responses)
if max_count <= 0:
break
# Keep the keys in a useful order, starting with username, title and location,
# then the columns returned by the xblock report generator in sorted order and
@@ -733,13 +764,19 @@ class ProblemResponses(object):
task_progress = TaskProgress(action_name, num_reports, start_time)
current_step = {'step': 'Calculating students answers to problem'}
task_progress.update_task_state(extra_meta=current_step)
problem_location = task_input.get('problem_location')
problem_locations = task_input.get('problem_locations')
problem_types_filter = task_input.get('problem_types_filter')
filter_types = None
if problem_types_filter:
filter_types = problem_types_filter.split(',')
# Compute result table and format it
student_data, student_data_keys = cls._build_student_data(
user_id=task_input.get('user_id'),
course_key=course_id,
usage_key_str=problem_location
usage_key_str_list=problem_locations.split(','),
filter_types=filter_types,
)
for data in student_data:
@@ -757,7 +794,9 @@ class ProblemResponses(object):
task_progress.update_task_state(extra_meta=current_step)
# Perform the upload
problem_location = re.sub(r'[:/]', '_', problem_location)
# Limit problem locations string to 200 characters in case a large number of
# problem locations are selected.
problem_location = re.sub(r'[:/]', '_', problem_locations)[:200]
csv_name = 'student_state_from_{}'.format(problem_location)
report_name = upload_csv_to_report_store(rows, csv_name, course_id, start_date)
current_step = {'step': 'CSV uploaded', 'report_name': report_name}

View File

@@ -262,7 +262,7 @@ class InstructorTaskCourseSubmitTest(TestReportMixin, InstructorTaskCourseTestCa
api_call = lambda: submit_calculate_problem_responses_csv(
self.create_task_request(self.instructor),
self.course.id,
problem_location=''
problem_locations='',
)
self._test_resubmission(api_call)

View File

@@ -32,6 +32,7 @@ import openedx.core.djangoapps.user_api.course_tag.api as course_tag_api
from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory
from course_modes.models import CourseMode
from course_modes.tests.factories import CourseModeFactory
from courseware.models import StudentModule
from courseware.tests.factories import InstructorFactory
from instructor_analytics.basic import UNAVAILABLE, list_problem_responses
from lms.djangoapps.certificates.models import CertificateStatuses, GeneratedCertificate
@@ -478,6 +479,7 @@ class TestTeamGradeReport(InstructorGradeReportTestCase):
# pylint: disable=protected-access
@ddt.ddt
class TestProblemResponsesReport(TestReportMixin, InstructorTaskModuleTestCase):
"""
Tests that generation of CSV files listing student answers to a
@@ -518,7 +520,7 @@ class TestProblemResponsesReport(TestReportMixin, InstructorTaskModuleTestCase):
student_data, _ = ProblemResponses._build_student_data(
user_id=self.instructor.id,
course_key=self.course.id,
usage_key_str=str(self.course.location),
usage_key_str_list=[str(self.course.location)],
)
self.assertEquals(len(student_data), 4)
@@ -538,12 +540,12 @@ class TestProblemResponsesReport(TestReportMixin, InstructorTaskModuleTestCase):
student_data, _ = ProblemResponses._build_student_data(
user_id=self.instructor.id,
course_key=self.course.id,
usage_key_str=str(problem.location),
usage_key_str_list=[str(problem.location)],
)
self.assertEquals(len(student_data), 1)
self.assertDictContainsSubset({
'username': 'student',
'location': 'Problem1',
'location': 'test_course > Section > Subsection > Problem1',
'block_key': 'i4x://edx/1.23x/problem/Problem1',
'title': 'Problem1',
}, student_data[0])
@@ -567,7 +569,7 @@ class TestProblemResponsesReport(TestReportMixin, InstructorTaskModuleTestCase):
student_data, _ = ProblemResponses._build_student_data(
user_id=self.instructor.id,
course_key=self.course.id,
usage_key_str=str(self.course.location),
usage_key_str_list=[str(self.course.location)],
)
self.assertEquals(len(student_data), 2)
self.assertDictContainsSubset({
@@ -598,7 +600,7 @@ class TestProblemResponsesReport(TestReportMixin, InstructorTaskModuleTestCase):
student_data, _ = ProblemResponses._build_student_data(
user_id=self.instructor.id,
course_key=self.course.id,
usage_key_str=str(self.course.location),
usage_key_str_list=[str(self.course.location)],
)
self.assertEquals(len(student_data), 1)
self.assertDictContainsSubset({
@@ -613,6 +615,66 @@ class TestProblemResponsesReport(TestReportMixin, InstructorTaskModuleTestCase):
}, student_data[0])
self.assertIn('state', student_data[0])
def test_build_student_data_for_multiple_problems(self):
"""
Ensure that building student data works when supplied multiple usage keys.
"""
problem1 = self.define_option_problem(u'Problem1')
problem2 = self.define_option_problem(u'Problem2')
self.submit_student_answer(self.student.username, u'Problem1', ['Option 1'])
self.submit_student_answer(self.student.username, u'Problem2', ['Option 1'])
student_data, _ = ProblemResponses._build_student_data(
user_id=self.instructor.id,
course_key=self.course.id,
usage_key_str_list=[str(problem1.location), str(problem2.location)],
)
self.assertEquals(len(student_data), 2)
for idx in range(1, 3):
self.assertDictContainsSubset({
'username': 'student',
'location': u'test_course > Section > Subsection > Problem{}'.format(idx),
'block_key': 'i4x://edx/1.23x/problem/Problem{}'.format(idx),
'title': u'Problem{}'.format(idx),
'Answer ID': 'i4x-edx-1_23x-problem-Problem{}_2_1'.format(idx),
'Answer': u'Option 1',
'Correct Answer': u'Option 1',
'Question': u'The correct answer is Option 1',
}, student_data[idx - 1])
self.assertIn('state', student_data[idx - 1])
@ddt.data(
(['problem'], 5),
(['other'], 0),
(['problem', 'test-category'], 10),
(None, 10),
)
@ddt.unpack
def test_build_student_data_with_filter(self, filters, filtered_count):
"""
Ensure that building student data works when supplied multiple usage keys.
"""
for idx in range(1, 6):
self.define_option_problem(u'Problem{}'.format(idx))
item = ItemFactory.create(
parent_location=self.problem_section.location,
parent=self.problem_section,
category="test-category",
display_name=u"Item{}".format(idx),
data=''
)
StudentModule.save_state(self.student, self.course.id, item.location, {})
for idx in range(1, 6):
self.submit_student_answer(self.student.username, u'Problem{}'.format(idx), ['Option 1'])
student_data, _ = ProblemResponses._build_student_data(
user_id=self.instructor.id,
course_key=self.course.id,
usage_key_str_list=[str(self.course.location)],
filter_types=filters,
)
self.assertEquals(len(student_data), filtered_count)
@patch('lms.djangoapps.instructor_task.tasks_helper.grades.list_problem_responses')
@patch('xmodule.capa_module.ProblemBlock.generate_report_data', create=True)
def test_build_student_data_for_block_with_generate_report_data_not_implemented(
@@ -629,14 +691,14 @@ class TestProblemResponsesReport(TestReportMixin, InstructorTaskModuleTestCase):
ProblemResponses._build_student_data(
user_id=self.instructor.id,
course_key=self.course.id,
usage_key_str=str(problem.location),
usage_key_str_list=[str(problem.location)],
)
mock_generate_report_data.assert_called_with(ANY, ANY)
mock_list_problem_responses.assert_called_with(self.course.id, ANY, ANY)
def test_success(self):
task_input = {
'problem_location': str(self.course.location),
'problem_locations': str(self.course.location),
'user_id': self.instructor.id
}
with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'):