Merge pull request #19507 from open-craft/kshitij/fix-problem-response-report

[BB-753] Fix issue with multiple responses for a single user returned by generate_report_data

lms/djangoapps/instructor_task/tasks_helper/grades.py

@@ -3,7 +3,7 @@ Functionality for generating grade reports.
 """
 import logging
 import re
-from collections import OrderedDict
+from collections import defaultdict, OrderedDict
 from datetime import datetime
 from itertools import chain, izip, izip_longest
 from time import time
@@ -615,15 +615,15 @@ class ProblemResponses(object):
             Tuple[str, List[str], UsageKey]: tuple of a block's display name, path, and
                 usage key
         """
-        display_name = course_blocks.get_xblock_field(root, 'display_name')
+        name = course_blocks.get_xblock_field(root, 'display_name') or root.category
         if path is None:
-            path = [display_name]
+            path = [name]
 
-        yield display_name, path, root
+        yield name, path, root
 
         for block in course_blocks.get_children(root):
-            display_name = course_blocks.get_xblock_field(block, 'display_name')
-            for result in cls._build_problem_list(course_blocks, block, path + [display_name]):
+            name = course_blocks.get_xblock_field(block, 'display_name') or block.category
+            for result in cls._build_problem_list(course_blocks, block, path + [name]):
                 yield result
 
     @classmethod
@@ -664,33 +664,42 @@ class ProblemResponses(object):
                     continue
 
                 block = store.get_item(block_key)
-                generated_report_data = {}
+                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)
-                        generated_report_data = {
-                            username: state
-                            for username, state in
-                            block.generate_report_data(user_state_iterator, max_count)
-                        }
+                        for username, state in block.generate_report_data(user_state_iterator, max_count):
+                            generated_report_data[username].append(state)
                     except NotImplementedError:
                         pass
 
-                responses = list_problem_responses(course_key, block_key, max_count)
+                responses = []
 
-                student_data += responses
-                for response in 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)
-                    user_data = generated_report_data.get(response['username'], {})
-                    response.update(user_data)
-                    student_data_keys = student_data_keys.union(user_data.keys())
+                    # 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(user_state.keys())
+                            responses.append(user_response)
+                    else:
+                        responses.append(response)
+
+                student_data += responses
+
                 if max_count is not None:
                     max_count -= len(responses)
                     if max_count <= 0:

lms/djangoapps/instructor_task/tests/test_tasks_helper.py

@@ -558,24 +558,35 @@ class TestProblemResponsesReport(TestReportMixin, InstructorTaskModuleTestCase):
         """
         self.define_option_problem(u'Problem1')
         self.submit_student_answer(self.student.username, u'Problem1', ['Option 1'])
-        state = {'some': 'state', 'more': 'state!'}
+        state1 = {'some': 'state1', 'more': 'state1!'}
+        state2 = {'some': 'state2', 'more': 'state2!'}
         mock_generate_report_data.return_value = iter([
-            ('student', state),
+            ('student', state1),
+            ('student', state2),
         ])
         student_data, _ = ProblemResponses._build_student_data(
             user_id=self.instructor.id,
             course_key=self.course.id,
             usage_key_str=str(self.course.location),
         )
-        self.assertEquals(len(student_data), 1)
+        self.assertEquals(len(student_data), 2)
         self.assertDictContainsSubset({
             'username': 'student',
             'location': 'test_course > Section > Subsection > Problem1',
             'block_key': 'i4x://edx/1.23x/problem/Problem1',
             'title': 'Problem1',
-            'some': 'state',
-            'more': 'state!',
+            'some': 'state1',
+            'more': 'state1!',
         }, student_data[0])
+        self.assertDictContainsSubset({
+            'username': 'student',
+            'location': 'test_course > Section > Subsection > Problem1',
+            'block_key': 'i4x://edx/1.23x/problem/Problem1',
+            'title': 'Problem1',
+            'some': 'state2',
+            'more': 'state2!',
+        }, student_data[1])
+        self.assertEquals(student_data[0]['state'], student_data[1]['state'])
 
     def test_build_student_data_for_block_with_real_generate_report_data(self):
         """