Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "[BB-873] Support for filters, and multiple roots in problem response reports"

Browse Source
This commit is contained in:
Syed Muhammad Dawoud Sheraz Ali
2019-08-09 17:17:36 +05:00
committed by GitHub
parent 20b7f96770
commit 22d3f802f2
6 changed files with 68 additions and 180 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
lms/djangoapps/instructor/views/api.py
View File

@@ -1063,23 +1063,22 @@ def get_problem_responses(request, course_id):
Responds with BadRequest if problem location is faulty.
"""
course_key = CourseKey.from_string(course_id)
# 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', '')
problem_location = request.POST.get('problem_location', '')
report_type = _('problem responses')
try:
for problem_location in problem_locations.split(','):
problem_key = UsageKey.from_string(problem_location)
# Are we dealing with an "old-style" problem location?
run = problem_key.run
if not run:
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_locations, problem_types_filter,
request, course_key, problem_location
)
success_status = SUCCESS_MESSAGE_TEMPLATE.format(report_type=report_type)

10
lms/djangoapps/instructor_task/api.py
View File

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

6
lms/djangoapps/instructor_task/tasks.py
View File

@@ -161,11 +161,7 @@ def send_bulk_course_email(entry_id, _xmodule_instance_args):
return run_main_task(entry_id, visit_fcn, action_name)
@task(
name='lms.djangoapps.instructor_task.tasks.calculate_problem_responses_csv.v2',
base=BaseInstructorTask,
routing_key=settings.GRADES_DOWNLOAD_ROUTING_KEY,
)
@task(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

137
lms/djangoapps/instructor_task/tasks_helper/grades.py
View File

@@ -604,23 +604,6 @@ 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):
"""
@@ -649,9 +632,7 @@ class ProblemResponses(object):
yield result
@classmethod
def _build_student_data(
cls, user_id, course_key, usage_key_str_list, filter_types=None,
):
def _build_student_data(cls, user_id, course_key, usage_key_str):
"""
Generate a list of problem responses for all problem under the
``problem_location`` root.
@@ -660,21 +641,17 @@ 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_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.
usage_key_str (str): The generated report will include this
block and it child blocks.
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_keys = [
UsageKey.from_string(usage_key_str).map_into_course(course_key)
for usage_key_str in usage_key_str_list
]
usage_key = UsageKey.from_string(usage_key_str).map_into_course(course_key)
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')
@@ -685,61 +662,53 @@ class ProblemResponses(object):
student_data_keys = set()
with store.bulk_operations(course_key):
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
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
if filter_types is not None and block_key.block_type not in filter_types:
continue
block = store.get_item(block_key)
generated_report_data = defaultdict(list)
block = store.get_item(block_key)
generated_report_data = defaultdict(list)
# 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
# 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
responses = []
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(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)
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)
student_data += responses
student_data += responses
if max_count is not None:
max_count -= len(responses)
if max_count <= 0:
break
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
@@ -764,19 +733,13 @@ 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_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(',')
problem_location = task_input.get('problem_location')
# 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_list=problem_locations.split(','),
filter_types=filter_types,
usage_key_str=problem_location
)
for data in student_data:
@@ -794,9 +757,7 @@ class ProblemResponses(object):
task_progress.update_task_state(extra_meta=current_step)
# Perform the upload
# 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]
problem_location = re.sub(r'[:/]', '_', problem_location)
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}

2
lms/djangoapps/instructor_task/tests/test_api.py
View File

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

76
lms/djangoapps/instructor_task/tests/test_tasks_helper.py
View File

@@ -32,7 +32,6 @@ 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 lms.djangoapps.instructor_analytics.basic import UNAVAILABLE, list_problem_responses
from lms.djangoapps.certificates.models import CertificateStatuses, GeneratedCertificate
@@ -479,7 +478,6 @@ class TestTeamGradeReport(InstructorGradeReportTestCase):
# pylint: disable=protected-access
@ddt.ddt
class TestProblemResponsesReport(TestReportMixin, InstructorTaskModuleTestCase):
"""
Tests that generation of CSV files listing student answers to a
@@ -520,7 +518,7 @@ class TestProblemResponsesReport(TestReportMixin, InstructorTaskModuleTestCase):
student_data, _ = ProblemResponses._build_student_data(
user_id=self.instructor.id,
course_key=self.course.id,
usage_key_str_list=[str(self.course.location)],
usage_key_str=str(self.course.location),
)
self.assertEquals(len(student_data), 4)
@@ -540,12 +538,12 @@ class TestProblemResponsesReport(TestReportMixin, InstructorTaskModuleTestCase):
student_data, _ = ProblemResponses._build_student_data(
user_id=self.instructor.id,
course_key=self.course.id,
usage_key_str_list=[str(problem.location)],
usage_key_str=str(problem.location),
)
self.assertEquals(len(student_data), 1)
self.assertDictContainsSubset({
'username': 'student',
'location': 'test_course > Section > Subsection > Problem1',
'location': 'Problem1',
'block_key': 'i4x://edx/1.23x/problem/Problem1',
'title': 'Problem1',
}, student_data[0])
@@ -569,7 +567,7 @@ class TestProblemResponsesReport(TestReportMixin, InstructorTaskModuleTestCase):
student_data, _ = ProblemResponses._build_student_data(
user_id=self.instructor.id,
course_key=self.course.id,
usage_key_str_list=[str(self.course.location)],
usage_key_str=str(self.course.location),
)
self.assertEquals(len(student_data), 2)
self.assertDictContainsSubset({
@@ -600,7 +598,7 @@ class TestProblemResponsesReport(TestReportMixin, InstructorTaskModuleTestCase):
student_data, _ = ProblemResponses._build_student_data(
user_id=self.instructor.id,
course_key=self.course.id,
usage_key_str_list=[str(self.course.location)],
usage_key_str=str(self.course.location),
)
self.assertEquals(len(student_data), 1)
self.assertDictContainsSubset({
@@ -615,66 +613,6 @@ 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(
@@ -691,14 +629,14 @@ class TestProblemResponsesReport(TestReportMixin, InstructorTaskModuleTestCase):
ProblemResponses._build_student_data(
user_id=self.instructor.id,
course_key=self.course.id,
usage_key_str_list=[str(problem.location)],
usage_key_str=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_locations': str(self.course.location),
'problem_location': str(self.course.location),
'user_id': self.instructor.id
}
with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'):