Merge pull request #21413 from open-craft/kshtij/problem-response-multi-root-and-fitler

[BB-873] Support for filters, and multiple roots in problem response reports
This commit is contained in:
David Ormsbee
2020-08-19 13:07:24 -04:00
committed by GitHub
6 changed files with 275 additions and 86 deletions

View File

@@ -1003,30 +1003,62 @@ def get_problem_responses(request, course_id):
Initiate generation of a CSV file containing all student answers
to a given problem.
Responds with JSON
{"status": "... status message ...", "task_id": created_task_UUID}
**Example requests**
if initiation is successful (or generation task is already running).
POST /courses/{course_id}/instructor/api/get_problem_responses {
"problem_location": "{usage_key1},{usage_key2},{usage_key3}""
}
POST /courses/{course_id}/instructor/api/get_problem_responses {
"problem_location": "{usage_key}",
"problem_types_filter": "problem"
}
Responds with BadRequest if problem location is faulty.
**POST Parameters**
A POST request can include the following parameters:
* problem_location: A comma-separated list of usage keys for the blocks
to include in the report. If the location is a block that contains
other blocks, (such as the course, section, subsection, or unit blocks)
then all blocks under that block will be included in the report.
* problem_types_filter: Optional. A comma-separated list of block types
to include in the repot. If set, only blocks of the specified types will
be included in the report.
To get data on all the poll and survey blocks in a course, you could
POST the usage key of the course for `problem_location`, and
"poll, survey" as the value for `problem_types_filter`.
**Example Response:**
If initiation is successful (or generation task is already running):
```json
{
"status": "The problem responses report is being created. ...",
"task_id": "4e49522f-31d9-431a-9cff-dd2a2bf4c85a"
}
```
Responds with BadRequest if any of the provided problem locations are 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

@@ -322,7 +322,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.
@@ -331,7 +333,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_locations': 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

@@ -159,7 +159,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

@@ -3,13 +3,14 @@ Functionality for generating grade reports.
"""
import logging
import re
from collections import OrderedDict, defaultdict
from datetime import datetime
from itertools import chain
from time import time
import re
import six
from course_blocks.api import get_course_blocks
from django.conf import settings
from django.contrib.auth import get_user_model
from lazy import lazy
@@ -18,33 +19,33 @@ from pytz import UTC
from six import text_type
from six.moves import zip, zip_longest
from course_blocks.api import get_course_blocks
from course_modes.models import CourseMode
from lms.djangoapps.certificates.models import CertificateWhitelist, GeneratedCertificate, certificate_info_for_user
from lms.djangoapps.courseware.courses import get_course_by_id
from lms.djangoapps.courseware.user_state_client import DjangoXBlockUserStateClient
from lms.djangoapps.grades.api import CourseGradeFactory
from lms.djangoapps.grades.api import context as grades_context
from lms.djangoapps.grades.api import prefetch_course_and_subsection_grades
from lms.djangoapps.grades.api import (
CourseGradeFactory,
context as grades_context,
prefetch_course_and_subsection_grades,
)
from lms.djangoapps.instructor_analytics.basic import list_problem_responses
from lms.djangoapps.instructor_analytics.csvs import format_dictlist
from lms.djangoapps.instructor_task.config.waffle import (
course_grade_report_verified_only,
optimize_get_learners_switch_enabled,
problem_grade_report_verified_only
problem_grade_report_verified_only,
)
from lms.djangoapps.teams.models import CourseTeamMembership
from lms.djangoapps.verify_student.services import IDVerificationService
from openedx.core.lib.cache_utils import get_cache
from openedx.core.djangoapps.content.block_structure.api import get_course_in_cache
from openedx.core.djangoapps.course_groups.cohorts import bulk_cache_cohorts, get_cohort, is_course_cohorted
from openedx.core.djangoapps.user_api.course_tag.api import BulkCourseTags
from openedx.core.lib.cache_utils import get_cache
from student.models import CourseEnrollment
from student.roles import BulkRoleCache
from xmodule.modulestore.django import modulestore
from xmodule.partitions.partitions_service import PartitionService
from xmodule.split_test_module import get_split_user_partitions
from .runner import TaskProgress
from .utils import upload_csv_to_report_store
@@ -822,6 +823,23 @@ class ProblemResponses(object):
Class to encapsulate functionality related to generating Problem Responses Reports.
"""
@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):
"""
@@ -836,19 +854,21 @@ class ProblemResponses(object):
Tuple[str, List[str], UsageKey]: tuple of a block's display name, path, and
usage key
"""
name = course_blocks.get_xblock_field(root, 'display_name') or root.category
name = course_blocks.get_xblock_field(root, 'display_name') or root.block_type
if path is None:
path = [name]
yield name, path, root
for block in course_blocks.get_children(root):
name = course_blocks.get_xblock_field(block, 'display_name') or block.category
name = course_blocks.get_xblock_field(block, 'display_name') or block.block_type
for result in cls._build_problem_list(course_blocks, block, path + [name]):
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.
@@ -856,16 +876,20 @@ 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')
@@ -876,53 +900,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
@@ -947,13 +979,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').split(',')
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,
filter_types=filter_types,
)
for data in student_data:
@@ -971,8 +1009,7 @@ class ProblemResponses(object):
task_progress.update_task_state(extra_meta=current_step)
# Perform the upload
problem_location = re.sub(r'[:/]', '_', problem_location)
csv_name = 'student_state_from_{}'.format(problem_location)
csv_name = cls._generate_upload_file_name(problem_locations, filter_types)
report_name, report_path = upload_csv_to_report_store(rows, csv_name, course_id, start_date)
current_step = {
'step': 'CSV uploaded',
@@ -981,3 +1018,17 @@ class ProblemResponses(object):
}
return task_progress.update_task_state(extra_meta=current_step)
@staticmethod
def _generate_upload_file_name(problem_locations, filters):
"""Generate a concise file name based on the report generation parameters."""
multiple_problems = len(problem_locations) > 1
csv_name = 'student_state'
if multiple_problems:
csv_name += '_from_multiple_blocks'
else:
problem_location = re.sub(r'[:/]', '_', problem_locations[0])
csv_name += '_from_' + problem_location
if filters:
csv_name += '_for_' + ','.join(filters)
return csv_name

View File

@@ -238,7 +238,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

@@ -33,6 +33,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 lms.djangoapps.courseware.models import StudentModule
from lms.djangoapps.certificates.models import CertificateStatuses, GeneratedCertificate
from lms.djangoapps.certificates.tests.factories import CertificateWhitelistFactory, GeneratedCertificateFactory
from lms.djangoapps.courseware.tests.factories import InstructorFactory
@@ -472,6 +473,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
@@ -512,7 +514,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.assertEqual(len(student_data), 4)
@@ -532,12 +534,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.assertEqual(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])
@@ -561,7 +563,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.assertEqual(len(student_data), 2)
self.assertDictContainsSubset({
@@ -592,7 +594,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.assertEqual(len(student_data), 1)
self.assertDictContainsSubset({
@@ -607,6 +609,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.assertEqual(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.assertEqual(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(
@@ -623,14 +685,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'):
@@ -650,9 +712,43 @@ class TestProblemResponsesReport(TestReportMixin, InstructorTaskModuleTestCase):
report_store = ReportStore.from_config(config_name='GRADES_DOWNLOAD')
links = report_store.links_for(self.course.id)
self.assertEqual(len(links), 1)
self.assertDictContainsSubset({'attempted': 3, 'succeeded': 3, 'failed': 0}, result)
self.assertIn("report_name", result)
assert len(links) == 1
assert set(({'attempted': 3, 'succeeded': 3, 'failed': 0}).items()).issubset(set(result.items()))
assert "report_name" in result
@ddt.data(
('blkid', None, 'edx_1.23x_test_course_student_state_from_blkid_2020-01-01-0000.csv'),
('blkid', 'poll,survey', 'edx_1.23x_test_course_student_state_from_blkid_for_poll,survey_2020-01-01-0000.csv'),
('blkid1,blkid2', None, 'edx_1.23x_test_course_student_state_from_multiple_blocks_2020-01-01-0000.csv'),
(
'blkid1,blkid2',
'poll,survey',
'edx_1.23x_test_course_student_state_from_multiple_blocks_for_poll,survey_2020-01-01-0000.csv',
),
)
@ddt.unpack
def test_file_names(self, problem_locations, problem_types_filter, file_name):
task_input = {
'problem_locations': problem_locations,
'problem_types_filter': problem_types_filter,
'user_id': self.instructor.id
}
with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'), \
freeze_time('2020-01-01'):
with patch('lms.djangoapps.instructor_task.tasks_helper.grades'
'.ProblemResponses._build_student_data') as mock_build_student_data:
mock_build_student_data.return_value = (
[
{'username': 'user0', 'state': u'state0'},
{'username': 'user1', 'state': u'state1'},
{'username': 'user2', 'state': u'state2'},
],
['username', 'state']
)
result = ProblemResponses.generate(
None, None, self.course.id, task_input, 'calculated'
)
assert result.get('report_name') == file_name
@ddt.ddt