From 1252760de4858efb3e2d99d9bac6b9fe5efcbe30 Mon Sep 17 00:00:00 2001
From: Kshitij Sobti <kshitij@sobti.in>
Date: Sat, 24 Mar 2018 02:12:06 +0530
Subject: [PATCH 01/19] Allow list_problem_responses to limit the number of
 responses Allow problem_location to be a UsageKey or string in
 list_problem_responses

---
 lms/djangoapps/instructor_analytics/basic.py | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/lms/djangoapps/instructor_analytics/basic.py b/lms/djangoapps/instructor_analytics/basic.py
index 237e030ccd..0851f4a6ac 100644
--- a/lms/djangoapps/instructor_analytics/basic.py
+++ b/lms/djangoapps/instructor_analytics/basic.py
@@ -406,7 +406,7 @@ def coupon_codes_features(features, coupons_list, course_id):
     return [extract_coupon(coupon, features) for coupon in coupons_list]
 
 
-def list_problem_responses(course_key, problem_location):
+def list_problem_responses(course_key, problem_location, limit_responses=None):
     """
     Return responses to a given problem as a dict.
 
@@ -421,7 +421,10 @@ def list_problem_responses(course_key, problem_location):
     where `state` represents a student's response to the problem
     identified by `problem_location`.
     """
-    problem_key = UsageKey.from_string(problem_location)
+    if isinstance(problem_location, UsageKey):
+        problem_key = problem_location
+    else:
+        problem_key = UsageKey.from_string(problem_location)
     # Are we dealing with an "old-style" problem location?
     run = problem_key.run
     if not run:
@@ -434,6 +437,8 @@ def list_problem_responses(course_key, problem_location):
         module_state_key=problem_key
     )
     smdat = smdat.order_by('student')
+    if limit_responses is not None:
+        smdat = smdat[:limit_responses]
 
     return [
         {'username': response.student.username, 'state': response.state}

From e322a3a7594146ce13ffd9a48326a79daf47e9c2 Mon Sep 17 00:00:00 2001
From: Kshitij Sobti <kshitij@sobti.in>
Date: Sat, 24 Mar 2018 02:43:31 +0530
Subject: [PATCH 02/19] ProblemResponses can now iterate over all the problems
 in a chapter, sequential or vertical It is now possible to limit the number
 of rows in the problem response csv

---
 lms/djangoapps/instructor_task/api.py         |  2 +-
 .../instructor_task/tasks_helper/grades.py    | 22 ++++++++++++++++++-
 lms/envs/common.py                            |  3 +++
 3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/lms/djangoapps/instructor_task/api.py b/lms/djangoapps/instructor_task/api.py
index 99f2b7b4b9..da8c284b97 100644
--- a/lms/djangoapps/instructor_task/api.py
+++ b/lms/djangoapps/instructor_task/api.py
@@ -330,7 +330,7 @@ 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}
+    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)
diff --git a/lms/djangoapps/instructor_task/tasks_helper/grades.py b/lms/djangoapps/instructor_task/tasks_helper/grades.py
index 42e9e47e22..e3979c6cde 100644
--- a/lms/djangoapps/instructor_task/tasks_helper/grades.py
+++ b/lms/djangoapps/instructor_task/tasks_helper/grades.py
@@ -8,10 +8,14 @@ from datetime import datetime
 from itertools import chain, izip, izip_longest
 from time import time
 
+from django.contrib.auth import get_user_model
+from django.conf import settings
 from lazy import lazy
+from opaque_keys.edx.keys import UsageKey
 from pytz import UTC
 from six import text_type
 
+from course_blocks.api import get_course_blocks
 from courseware.courses import get_course_by_id
 from instructor_analytics.basic import list_problem_responses
 from instructor_analytics.csvs import format_dictlist
@@ -563,7 +567,23 @@ class ProblemResponses(object):
 
         # Compute result table and format it
         problem_location = task_input.get('problem_location')
-        student_data = list_problem_responses(course_id, problem_location)
+        problem_key = UsageKey.from_string(problem_location)
+
+        user_id = task_input.get('user_id')
+        user = get_user_model().objects.get(pk=user_id)
+        course_blocks = get_course_blocks(user, problem_key)
+
+        student_data = []
+        max_count = settings.FEATURES.get('MAX_PROBLEM_RESPONSES_COUNT')
+
+        for block in course_blocks:
+            if block.block_type == 'problem':
+                problem_responses = list_problem_responses(course_id, block, max_count)
+                student_data += problem_responses
+                max_count -= len(problem_responses)
+            if max_count <= 0:
+                break
+
         features = ['username', 'state']
         header, rows = format_dictlist(student_data, features)
 
diff --git a/lms/envs/common.py b/lms/envs/common.py
index 7dd42d762c..f456bb7ca8 100644
--- a/lms/envs/common.py
+++ b/lms/envs/common.py
@@ -206,6 +206,9 @@ FEATURES = {
     # Automatically approve student identity verification attempts
     'AUTOMATIC_VERIFY_STUDENT_IDENTITY_FOR_TESTING': False,
 
+    # Maximum number of rows to include in the csv file for downloading problem responses.
+    'MAX_PROBLEM_RESPONSES_COUNT': 5000,
+
     # whether to use password policy enforcement or not
     'ENFORCE_PASSWORD_POLICY': True,
 

From 9c1dc2c0a7e535d3caeecd182e86324cecbfa0a8 Mon Sep 17 00:00:00 2001
From: Kshitij Sobti <kshitij@sobti.in>
Date: Sat, 24 Mar 2018 05:08:31 +0530
Subject: [PATCH 03/19] Include title and location in problem response sheet

---
 .../instructor_task/tasks_helper/grades.py    | 28 ++++++++++++++-----
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/lms/djangoapps/instructor_task/tasks_helper/grades.py b/lms/djangoapps/instructor_task/tasks_helper/grades.py
index e3979c6cde..cd680b84a6 100644
--- a/lms/djangoapps/instructor_task/tasks_helper/grades.py
+++ b/lms/djangoapps/instructor_task/tasks_helper/grades.py
@@ -552,6 +552,19 @@ class ProblemGradeReport(object):
 
 
 class ProblemResponses(object):
+
+    @classmethod
+    def _build_problem_list(cls, course_blocks, root, path=None):
+        if path is None:
+            path = [course_blocks.get_xblock_field(root, 'display_name')]
+        for block in course_blocks.get_children(root):
+            display_name = course_blocks.get_xblock_field(block, 'display_name')
+            if block.block_type == 'problem':
+                yield display_name, path, block
+            else:
+                for result in ProblemResponses._build_problem_list(course_blocks, block, path + [display_name]):
+                    yield result
+
     @classmethod
     def generate(cls, _xmodule_instance_args, _entry_id, course_id, task_input, action_name):
         """
@@ -575,16 +588,17 @@ class ProblemResponses(object):
 
         student_data = []
         max_count = settings.FEATURES.get('MAX_PROBLEM_RESPONSES_COUNT')
-
-        for block in course_blocks:
-            if block.block_type == 'problem':
-                problem_responses = list_problem_responses(course_id, block, max_count)
-                student_data += problem_responses
-                max_count -= len(problem_responses)
+        for title, path, block in ProblemResponses._build_problem_list(course_blocks, problem_key):
+            problem_responses = list_problem_responses(course_id, block, max_count)
+            student_data += problem_responses
+            for response in problem_responses:
+                response['title'] = title
+                response['location'] = ' > '.join(path)
+            max_count -= len(problem_responses)
             if max_count <= 0:
                 break
 
-        features = ['username', 'state']
+        features = ['username', 'title', 'location', 'state']
         header, rows = format_dictlist(student_data, features)
 
         task_progress.attempted = task_progress.succeeded = len(rows)

From 00334a9ebb51a9c725b87a094d96b4ec10da41ac Mon Sep 17 00:00:00 2001
From: Kshitij Sobti <kshitij@sobti.in>
Date: Sat, 24 Mar 2018 05:38:00 +0530
Subject: [PATCH 04/19] Add block id to problem response report

---
 lms/djangoapps/instructor_task/tasks_helper/grades.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lms/djangoapps/instructor_task/tasks_helper/grades.py b/lms/djangoapps/instructor_task/tasks_helper/grades.py
index cd680b84a6..464c8a04d4 100644
--- a/lms/djangoapps/instructor_task/tasks_helper/grades.py
+++ b/lms/djangoapps/instructor_task/tasks_helper/grades.py
@@ -594,11 +594,12 @@ class ProblemResponses(object):
             for response in problem_responses:
                 response['title'] = title
                 response['location'] = ' > '.join(path)
+                response['block_id'] = block.block_id
             max_count -= len(problem_responses)
             if max_count <= 0:
                 break
 
-        features = ['username', 'title', 'location', 'state']
+        features = ['username', 'title', 'location', 'block_id', 'state']
         header, rows = format_dictlist(student_data, features)
 
         task_progress.attempted = task_progress.succeeded = len(rows)

From 1d507bf246ff896a0bee9f20f3dfe25046017b26 Mon Sep 17 00:00:00 2001
From: Kshitij Sobti <kshitij@sobti.in>
Date: Thu, 29 Mar 2018 05:08:42 +0530
Subject: [PATCH 05/19] Break out bits of generate method into separate method

---
 .../instructor_task/tasks_helper/grades.py    | 38 +++++++++++--------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/lms/djangoapps/instructor_task/tasks_helper/grades.py b/lms/djangoapps/instructor_task/tasks_helper/grades.py
index 464c8a04d4..736e5853dd 100644
--- a/lms/djangoapps/instructor_task/tasks_helper/grades.py
+++ b/lms/djangoapps/instructor_task/tasks_helper/grades.py
@@ -566,23 +566,8 @@ class ProblemResponses(object):
                     yield result
 
     @classmethod
-    def generate(cls, _xmodule_instance_args, _entry_id, course_id, task_input, action_name):
-        """
-        For a given `course_id`, generate a CSV file containing
-        all student answers to a given problem, and store using a `ReportStore`.
-        """
-        start_time = time()
-        start_date = datetime.now(UTC)
-        num_reports = 1
-        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)
-
-        # Compute result table and format it
-        problem_location = task_input.get('problem_location')
+    def _build_student_data(cls, user_id, course_id, problem_location):
         problem_key = UsageKey.from_string(problem_location)
-
-        user_id = task_input.get('user_id')
         user = get_user_model().objects.get(pk=user_id)
         course_blocks = get_course_blocks(user, problem_key)
 
@@ -599,6 +584,27 @@ class ProblemResponses(object):
             if max_count <= 0:
                 break
 
+        return student_data
+
+
+    @classmethod
+    def generate(cls, _xmodule_instance_args, _entry_id, course_id, task_input, action_name):
+        """
+        For a given `course_id`, generate a CSV file containing
+        all student answers to a given problem, and store using a `ReportStore`.
+        """
+        start_time = time()
+        start_date = datetime.now(UTC)
+        num_reports = 1
+        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)
+
+        # Compute result table and format it
+        student_data = cls._build_student_data(user_id=task_input.get('user_id'),
+                                               course_id=course_id,
+                                               problem_location=task_input.get('problem_location'))
+
         features = ['username', 'title', 'location', 'block_id', 'state']
         header, rows = format_dictlist(student_data, features)
 

From 888509279089a78b69e68c398c453b18c7799973 Mon Sep 17 00:00:00 2001
From: Kshitij Sobti <kshitij@sobti.in>
Date: Thu, 29 Mar 2018 05:11:59 +0530
Subject: [PATCH 06/19] Support setting MAX_PROBLEM_RESPONSES_COUNT to None for
 unrestricted row count

---
 lms/djangoapps/instructor_task/tasks_helper/grades.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lms/djangoapps/instructor_task/tasks_helper/grades.py b/lms/djangoapps/instructor_task/tasks_helper/grades.py
index 736e5853dd..add61f0b5f 100644
--- a/lms/djangoapps/instructor_task/tasks_helper/grades.py
+++ b/lms/djangoapps/instructor_task/tasks_helper/grades.py
@@ -580,9 +580,10 @@ class ProblemResponses(object):
                 response['title'] = title
                 response['location'] = ' > '.join(path)
                 response['block_id'] = block.block_id
-            max_count -= len(problem_responses)
-            if max_count <= 0:
-                break
+            if max_count is not None:
+                max_count -= len(problem_responses)
+                if max_count <= 0:
+                    break
 
         return student_data
 

From 0b75749c6e649c576f2365c5ed867ca982143af4 Mon Sep 17 00:00:00 2001
From: Kshitij Sobti <kshitij@sobti.in>
Date: Thu, 29 Mar 2018 05:12:51 +0530
Subject: [PATCH 07/19] Use cls instead of direct class name

---
 lms/djangoapps/instructor_task/tasks_helper/grades.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lms/djangoapps/instructor_task/tasks_helper/grades.py b/lms/djangoapps/instructor_task/tasks_helper/grades.py
index add61f0b5f..2d30fde7ed 100644
--- a/lms/djangoapps/instructor_task/tasks_helper/grades.py
+++ b/lms/djangoapps/instructor_task/tasks_helper/grades.py
@@ -562,7 +562,7 @@ class ProblemResponses(object):
             if block.block_type == 'problem':
                 yield display_name, path, block
             else:
-                for result in ProblemResponses._build_problem_list(course_blocks, block, path + [display_name]):
+                for result in cls._build_problem_list(course_blocks, block, path + [display_name]):
                     yield result
 
     @classmethod
@@ -573,7 +573,7 @@ class ProblemResponses(object):
 
         student_data = []
         max_count = settings.FEATURES.get('MAX_PROBLEM_RESPONSES_COUNT')
-        for title, path, block in ProblemResponses._build_problem_list(course_blocks, problem_key):
+        for title, path, block in cls._build_problem_list(course_blocks, problem_key):
             problem_responses = list_problem_responses(course_id, block, max_count)
             student_data += problem_responses
             for response in problem_responses:
@@ -587,7 +587,6 @@ class ProblemResponses(object):
 
         return student_data
 
-
     @classmethod
     def generate(cls, _xmodule_instance_args, _entry_id, course_id, task_input, action_name):
         """
@@ -600,11 +599,12 @@ 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')
 
         # Compute result table and format it
         student_data = cls._build_student_data(user_id=task_input.get('user_id'),
                                                course_id=course_id,
-                                               problem_location=task_input.get('problem_location'))
+                                               problem_location=problem_location)
 
         features = ['username', 'title', 'location', 'block_id', 'state']
         header, rows = format_dictlist(student_data, features)

From 7e31718f9110e963558ec9dafe700987a4c5d428 Mon Sep 17 00:00:00 2001
From: Kshitij Sobti <kshitij@sobti.in>
Date: Thu, 29 Mar 2018 19:53:54 +0530
Subject: [PATCH 08/19] Documentation and PEP8

---
 .../instructor_task/tasks_helper/grades.py    | 32 +++++++++++++------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/lms/djangoapps/instructor_task/tasks_helper/grades.py b/lms/djangoapps/instructor_task/tasks_helper/grades.py
index 2d30fde7ed..790986e50d 100644
--- a/lms/djangoapps/instructor_task/tasks_helper/grades.py
+++ b/lms/djangoapps/instructor_task/tasks_helper/grades.py
@@ -555,6 +555,10 @@ class ProblemResponses(object):
 
     @classmethod
     def _build_problem_list(cls, course_blocks, root, path=None):
+        """
+        Generate a tuple of display names, block location paths and block keys
+        for all problem blocks under the ``root`` block.
+        """
         if path is None:
             path = [course_blocks.get_xblock_field(root, 'display_name')]
         for block in course_blocks.get_children(root):
@@ -562,33 +566,40 @@ class ProblemResponses(object):
             if block.block_type == 'problem':
                 yield display_name, path, block
             else:
-                for result in cls._build_problem_list(course_blocks, block, path + [display_name]):
+                for result in cls._build_problem_list(course_blocks, block,
+                                                      path + [display_name]):
                     yield result
 
     @classmethod
     def _build_student_data(cls, user_id, course_id, problem_location):
+        """
+        Generate a list of problem responses for all problem under the
+        ``problem_location`` root.
+        """
         problem_key = UsageKey.from_string(problem_location)
         user = get_user_model().objects.get(pk=user_id)
         course_blocks = get_course_blocks(user, problem_key)
 
         student_data = []
         max_count = settings.FEATURES.get('MAX_PROBLEM_RESPONSES_COUNT')
-        for title, path, block in cls._build_problem_list(course_blocks, problem_key):
-            problem_responses = list_problem_responses(course_id, block, max_count)
-            student_data += problem_responses
-            for response in problem_responses:
+        for title, path, block in cls._build_problem_list(course_blocks,
+                                                          problem_key):
+            responses = list_problem_responses(course_id, block, max_count)
+            student_data += responses
+            for response in responses:
                 response['title'] = title
                 response['location'] = ' > '.join(path)
                 response['block_id'] = block.block_id
             if max_count is not None:
-                max_count -= len(problem_responses)
+                max_count -= len(responses)
                 if max_count <= 0:
                     break
 
         return student_data
 
     @classmethod
-    def generate(cls, _xmodule_instance_args, _entry_id, course_id, task_input, action_name):
+    def generate(cls, _xmodule_instance_args, _entry_id, course_id, task_input,
+                 action_name):
         """
         For a given `course_id`, generate a CSV file containing
         all student answers to a given problem, and store using a `ReportStore`.
@@ -602,9 +613,10 @@ class ProblemResponses(object):
         problem_location = task_input.get('problem_location')
 
         # Compute result table and format it
-        student_data = cls._build_student_data(user_id=task_input.get('user_id'),
-                                               course_id=course_id,
-                                               problem_location=problem_location)
+        student_data = cls._build_student_data(
+            user_id=task_input.get('user_id'),
+            course_id=course_id,
+            problem_location=problem_location)
 
         features = ['username', 'title', 'location', 'block_id', 'state']
         header, rows = format_dictlist(student_data, features)

From 94f1e0aa1c3ae8b5b102acfcf933a211c363396c Mon Sep 17 00:00:00 2001
From: Kshitij Sobti <kshitij@sobti.in>
Date: Thu, 29 Mar 2018 22:37:50 +0530
Subject: [PATCH 09/19] Fix error due to improper problem_key Add tests

---
 .../instructor_task/tasks_helper/grades.py    |  2 +-
 .../tests/test_tasks_helper.py                | 85 +++++++++++++------
 2 files changed, 59 insertions(+), 28 deletions(-)

diff --git a/lms/djangoapps/instructor_task/tasks_helper/grades.py b/lms/djangoapps/instructor_task/tasks_helper/grades.py
index 790986e50d..cde1f93c6c 100644
--- a/lms/djangoapps/instructor_task/tasks_helper/grades.py
+++ b/lms/djangoapps/instructor_task/tasks_helper/grades.py
@@ -576,7 +576,7 @@ class ProblemResponses(object):
         Generate a list of problem responses for all problem under the
         ``problem_location`` root.
         """
-        problem_key = UsageKey.from_string(problem_location)
+        problem_key = UsageKey.from_string(problem_location).map_into_course(course_id)
         user = get_user_model().objects.get(pk=user_id)
         course_blocks = get_course_blocks(user, problem_key)
 
diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py
index 992a115fab..91c50e8566 100644
--- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py
+++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py
@@ -16,23 +16,39 @@ from datetime import datetime, timedelta
 
 import ddt
 import unicodecsv
+from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory
+from course_modes.models import CourseMode
+from course_modes.tests.factories import CourseModeFactory
+from courseware.tests.factories import InstructorFactory
 from django.conf import settings
 from django.core.urlresolvers import reverse
 from django.test.utils import override_settings
 from freezegun import freeze_time
+from instructor_analytics.basic import UNAVAILABLE
 from mock import MagicMock, Mock, patch
 from nose.plugins.attrib import attr
 from pytz import UTC
+from shoppingcart.models import (
+    Coupon,
+    CourseRegistrationCode,
+    CourseRegistrationCodeInvoiceItem,
+    Invoice,
+    InvoiceTransaction,
+    Order,
+    PaidCourseRegistration
+)
 from six import text_type
+from student.models import ALLOWEDTOENROLL_TO_ENROLLED, CourseEnrollment, CourseEnrollmentAllowed, ManualEnrollmentAudit
+from student.tests.factories import CourseEnrollmentFactory, UserFactory
+from survey.models import SurveyAnswer, SurveyForm
+from xmodule.modulestore import ModuleStoreEnum
+from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
+from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls
+from xmodule.partitions.partitions import Group, UserPartition
 
 import openedx.core.djangoapps.user_api.course_tag.api as course_tag_api
-from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory
 from lms.djangoapps.certificates.models import CertificateStatuses, GeneratedCertificate
 from lms.djangoapps.certificates.tests.factories import CertificateWhitelistFactory, GeneratedCertificateFactory
-from course_modes.models import CourseMode
-from course_modes.tests.factories import CourseModeFactory
-from courseware.tests.factories import InstructorFactory
-from instructor_analytics.basic import UNAVAILABLE
 from lms.djangoapps.grades.models import PersistentCourseGrade
 from lms.djangoapps.grades.transformer import GradesTransformer
 from lms.djangoapps.instructor_task.tasks_helper.certs import generate_students_certificates
@@ -64,26 +80,9 @@ from lms.djangoapps.verify_student.tests.factories import SoftwareSecurePhotoVer
 from openedx.core.djangoapps.course_groups.models import CohortMembership, CourseUserGroupPartitionGroup
 from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory
 from openedx.core.djangoapps.credit.tests.factories import CreditCourseFactory
+from openedx.core.djangoapps.request_cache.middleware import RequestCache
 from openedx.core.djangoapps.user_api.partition_schemes import RandomUserPartitionScheme
 from openedx.core.djangoapps.util.testing import ContentGroupTestCase, TestConditionalContent
-from openedx.core.djangoapps.request_cache.middleware import RequestCache
-from shoppingcart.models import (
-    Coupon,
-    CourseRegistrationCode,
-    CourseRegistrationCodeInvoiceItem,
-    Invoice,
-    InvoiceTransaction,
-    Order,
-    PaidCourseRegistration
-)
-from student.models import ALLOWEDTOENROLL_TO_ENROLLED, CourseEnrollment, CourseEnrollmentAllowed, ManualEnrollmentAudit
-from student.tests.factories import CourseEnrollmentFactory, UserFactory
-from survey.models import SurveyAnswer, SurveyForm
-from xmodule.modulestore import ModuleStoreEnum
-from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
-from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls
-from xmodule.partitions.partitions import Group, UserPartition
-
 from ..models import ReportStore
 from ..tasks_helper.utils import UPDATE_STATUS_FAILED, UPDATE_STATUS_SUCCEEDED
 
@@ -462,19 +461,51 @@ class TestTeamGradeReport(InstructorGradeReportTestCase):
         self._verify_cell_data_for_user(self.student2.username, self.course.id, 'Team Name', team2.name)
 
 
-class TestProblemResponsesReport(TestReportMixin, InstructorTaskCourseTestCase):
+class TestProblemResponsesReport(TestReportMixin, InstructorTaskModuleTestCase):
     """
     Tests that generation of CSV files listing student answers to a
     given problem works.
     """
     def setUp(self):
         super(TestProblemResponsesReport, self).setUp()
-        self.course = CourseFactory.create()
+        self.initialize_course()
+        # self.course = CourseFactory.create()
+        self.instructor = self.create_instructor('instructor')
+        self.student = self.create_student('student')
+
+    @patch.dict("django.conf.settings.FEATURES", {"MAX_PROBLEM_RESPONSES_COUNT": 4})
+    def test_build_student_data_limit(self):
+        self.define_option_problem(u'Problem1')
+        for ctr in range(5):
+            student = self.create_student('student{}'.format(ctr))
+            self.submit_student_answer(student.username, u'Problem1', ['Option 1'])
+
+        student_data = ProblemResponses._build_student_data(user_id=self.instructor.id,
+                                                            course_id=self.course.id,
+                                                            problem_location=str(self.course.location))
+
+        self.assertEquals(len(student_data), 4)
+
+    def test_build_student_data(self):
+        self.define_option_problem(u'Problem1')
+        self.submit_student_answer(self.student.username, u'Problem1', ['Option 1'])
+
+        student_data = ProblemResponses._build_student_data(user_id=self.instructor.id,
+                                                            course_id=self.course.id,
+                                                            problem_location=str(self.course.location))
+        self.assertEquals(len(student_data), 1)
+        self.assertDictContainsSubset({
+            'username': 'student',
+            'location': 'test_course > Section > Subsection',
+            'block_id': 'Problem1',
+            'title': 'Problem1',
+        }, student_data[0])
 
     def test_success(self):
-        task_input = {'problem_location': ''}
+        task_input = {'problem_location': str(self.course.location), 'user_id': self.instructor.id}
         with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'):
-            with patch('lms.djangoapps.instructor_task.tasks_helper.grades.list_problem_responses') as patched_data_source:
+            with patch('lms.djangoapps.instructor_task.tasks_helper.grades'
+                       '.ProblemResponses._build_student_data') as patched_data_source:
                 patched_data_source.return_value = [
                     {'username': 'user0', 'state': u'state0'},
                     {'username': 'user1', 'state': u'state1'},

From 1a4fe2c527bfcf343ad8527e8e8bc812b72dc161 Mon Sep 17 00:00:00 2001
From: Kshitij Sobti <kshitij@sobti.in>
Date: Fri, 30 Mar 2018 00:38:38 +0530
Subject: [PATCH 10/19] Disable protected access pylint check for
 TestProblemResponsesReport

---
 lms/djangoapps/instructor_task/tests/test_tasks_helper.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py
index 91c50e8566..c60e129246 100644
--- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py
+++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py
@@ -461,6 +461,7 @@ class TestTeamGradeReport(InstructorGradeReportTestCase):
         self._verify_cell_data_for_user(self.student2.username, self.course.id, 'Team Name', team2.name)
 
 
+# pylint: disable=protected-access
 class TestProblemResponsesReport(TestReportMixin, InstructorTaskModuleTestCase):
     """
     Tests that generation of CSV files listing student answers to a

From 723a0639ca3820a70ff1d8e56f352cc95676acd3 Mon Sep 17 00:00:00 2001
From: Kshitij Sobti <kshitij@sobti.in>
Date: Sun, 1 Apr 2018 05:47:50 +0530
Subject: [PATCH 11/19] Fix: issue generating reports for single problem code
 style fixes

---
 .../instructor_task/tasks_helper/grades.py      | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/lms/djangoapps/instructor_task/tasks_helper/grades.py b/lms/djangoapps/instructor_task/tasks_helper/grades.py
index cde1f93c6c..48d80a63a1 100644
--- a/lms/djangoapps/instructor_task/tasks_helper/grades.py
+++ b/lms/djangoapps/instructor_task/tasks_helper/grades.py
@@ -559,15 +559,17 @@ class ProblemResponses(object):
         Generate a tuple of display names, block location paths and block keys
         for all problem blocks under the ``root`` block.
         """
+        display_name = course_blocks.get_xblock_field(root, 'display_name')
         if path is None:
-            path = [course_blocks.get_xblock_field(root, 'display_name')]
+            path = [display_name]
+        if root.block_type == 'problem':
+            yield display_name, path, root
         for block in course_blocks.get_children(root):
             display_name = course_blocks.get_xblock_field(block, 'display_name')
             if block.block_type == 'problem':
                 yield display_name, path, block
             else:
-                for result in cls._build_problem_list(course_blocks, block,
-                                                      path + [display_name]):
+                for result in cls._build_problem_list(course_blocks, block, path + [display_name]):
                     yield result
 
     @classmethod
@@ -582,8 +584,7 @@ class ProblemResponses(object):
 
         student_data = []
         max_count = settings.FEATURES.get('MAX_PROBLEM_RESPONSES_COUNT')
-        for title, path, block in cls._build_problem_list(course_blocks,
-                                                          problem_key):
+        for title, path, block in cls._build_problem_list(course_blocks, problem_key):
             responses = list_problem_responses(course_id, block, max_count)
             student_data += responses
             for response in responses:
@@ -598,8 +599,7 @@ class ProblemResponses(object):
         return student_data
 
     @classmethod
-    def generate(cls, _xmodule_instance_args, _entry_id, course_id, task_input,
-                 action_name):
+    def generate(cls, _xmodule_instance_args, _entry_id, course_id, task_input, action_name):
         """
         For a given `course_id`, generate a CSV file containing
         all student answers to a given problem, and store using a `ReportStore`.
@@ -616,7 +616,8 @@ class ProblemResponses(object):
         student_data = cls._build_student_data(
             user_id=task_input.get('user_id'),
             course_id=course_id,
-            problem_location=problem_location)
+            problem_location=problem_location
+        )
 
         features = ['username', 'title', 'location', 'block_id', 'state']
         header, rows = format_dictlist(student_data, features)

From d2b78a34f4c599f7615efc74f7f41f0e8e7c4573 Mon Sep 17 00:00:00 2001
From: Kshitij Sobti <kshitij@sobti.in>
Date: Sun, 1 Apr 2018 06:08:41 +0530
Subject: [PATCH 12/19] Unconditionally yield root block so it's always
 included in the report

---
 lms/djangoapps/instructor_task/tasks_helper/grades.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lms/djangoapps/instructor_task/tasks_helper/grades.py b/lms/djangoapps/instructor_task/tasks_helper/grades.py
index 48d80a63a1..e637881732 100644
--- a/lms/djangoapps/instructor_task/tasks_helper/grades.py
+++ b/lms/djangoapps/instructor_task/tasks_helper/grades.py
@@ -562,8 +562,7 @@ class ProblemResponses(object):
         display_name = course_blocks.get_xblock_field(root, 'display_name')
         if path is None:
             path = [display_name]
-        if root.block_type == 'problem':
-            yield display_name, path, root
+        yield display_name, path, root
         for block in course_blocks.get_children(root):
             display_name = course_blocks.get_xblock_field(block, 'display_name')
             if block.block_type == 'problem':

From c4bbe7f036268ca13e15bede03a0e3f1f2fce1ca Mon Sep 17 00:00:00 2001
From: Kshitij Sobti <kshitij@sobti.in>
Date: Mon, 2 Apr 2018 05:43:31 +0530
Subject: [PATCH 13/19] Only filter out verticals instead of all non-problem
 blocks Improve documentation

---
 .../instructor_task/tasks_helper/grades.py    | 35 ++++++++++++++++---
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/lms/djangoapps/instructor_task/tasks_helper/grades.py b/lms/djangoapps/instructor_task/tasks_helper/grades.py
index e637881732..2cef566ca3 100644
--- a/lms/djangoapps/instructor_task/tasks_helper/grades.py
+++ b/lms/djangoapps/instructor_task/tasks_helper/grades.py
@@ -558,14 +558,28 @@ class ProblemResponses(object):
         """
         Generate a tuple of display names, block location paths and block keys
         for all problem blocks under the ``root`` block.
+
+        Arguments:
+            course_blocks (BlockStructureBlockData): Block structure for a course.
+            root (UsageKey): This block and its children will be used to generate
+                the problem list
+            path (List[str]): The list of display names for the parent of root block
+
+        Yields:
+            Tuple[str, List[str], UsageKey]: tuple of a blocks display name, path, and
+                usage key
         """
         display_name = course_blocks.get_xblock_field(root, 'display_name')
         if path is None:
             path = [display_name]
+
         yield display_name, path, root
+
         for block in course_blocks.get_children(root):
             display_name = course_blocks.get_xblock_field(block, 'display_name')
-            if block.block_type == 'problem':
+            # Sequential blocks are filtered out since they include position state
+            # which isn't useful in this report.
+            if block.block_type != 'sequential':
                 yield display_name, path, block
             else:
                 for result in cls._build_problem_list(course_blocks, block, path + [display_name]):
@@ -576,6 +590,17 @@ class ProblemResponses(object):
         """
         Generate a list of problem responses for all problem under the
         ``problem_location`` root.
+
+        Arguments:
+            user_id (int): The user id for the user generating the report
+            course_id (UsageKey): The UsageKey for the course whose report is
+                being generated
+            problem_location (str): The generated report will include this
+                block and it child blocks.
+
+        Returns:
+              List[Dict]: Returns a list of dictionaries containing the student
+                data which will be included in the final csv.
         """
         problem_key = UsageKey.from_string(problem_location).map_into_course(course_id)
         user = get_user_model().objects.get(pk=user_id)
@@ -583,13 +608,13 @@ class ProblemResponses(object):
 
         student_data = []
         max_count = settings.FEATURES.get('MAX_PROBLEM_RESPONSES_COUNT')
-        for title, path, block in cls._build_problem_list(course_blocks, problem_key):
-            responses = list_problem_responses(course_id, block, max_count)
+        for title, path, block_key in cls._build_problem_list(course_blocks, problem_key):
+            responses = list_problem_responses(course_id, block_key, max_count)
             student_data += responses
             for response in responses:
                 response['title'] = title
                 response['location'] = ' > '.join(path)
-                response['block_id'] = block.block_id
+                response['block_key'] = str(block_key)
             if max_count is not None:
                 max_count -= len(responses)
                 if max_count <= 0:
@@ -618,7 +643,7 @@ class ProblemResponses(object):
             problem_location=problem_location
         )
 
-        features = ['username', 'title', 'location', 'block_id', 'state']
+        features = ['username', 'title', 'location', 'block_key', 'state']
         header, rows = format_dictlist(student_data, features)
 
         task_progress.attempted = task_progress.succeeded = len(rows)

From ba635383dba1161431b61b3d772627c99e91bf30 Mon Sep 17 00:00:00 2001
From: Kshitij Sobti <kshitij@sobti.in>
Date: Mon, 2 Apr 2018 10:03:49 +0530
Subject: [PATCH 14/19] Filter out sequentials in final report generation stage
 Fix tests to match code changes

---
 .../instructor_task/tasks_helper/grades.py    | 13 ++++----
 .../tests/test_tasks_helper.py                | 30 ++++++++++++-------
 2 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/lms/djangoapps/instructor_task/tasks_helper/grades.py b/lms/djangoapps/instructor_task/tasks_helper/grades.py
index 2cef566ca3..22dcb9f329 100644
--- a/lms/djangoapps/instructor_task/tasks_helper/grades.py
+++ b/lms/djangoapps/instructor_task/tasks_helper/grades.py
@@ -577,13 +577,8 @@ class ProblemResponses(object):
 
         for block in course_blocks.get_children(root):
             display_name = course_blocks.get_xblock_field(block, 'display_name')
-            # Sequential blocks are filtered out since they include position state
-            # which isn't useful in this report.
-            if block.block_type != 'sequential':
-                yield display_name, path, block
-            else:
-                for result in cls._build_problem_list(course_blocks, block, path + [display_name]):
-                    yield result
+            for result in cls._build_problem_list(course_blocks, block, path + [display_name]):
+                yield result
 
     @classmethod
     def _build_student_data(cls, user_id, course_id, problem_location):
@@ -609,6 +604,10 @@ class ProblemResponses(object):
         student_data = []
         max_count = settings.FEATURES.get('MAX_PROBLEM_RESPONSES_COUNT')
         for title, path, block_key in cls._build_problem_list(course_blocks, problem_key):
+            # Sequential blocks are filtered out since they include position state
+            # which isn't useful in this report.
+            if block_key.block_type == 'sequential':
+                continue
             responses = list_problem_responses(course_id, block_key, max_count)
             student_data += responses
             for response in responses:
diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py
index c60e129246..0bcfee9152 100644
--- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py
+++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py
@@ -470,7 +470,6 @@ class TestProblemResponsesReport(TestReportMixin, InstructorTaskModuleTestCase):
     def setUp(self):
         super(TestProblemResponsesReport, self).setUp()
         self.initialize_course()
-        # self.course = CourseFactory.create()
         self.instructor = self.create_instructor('instructor')
         self.student = self.create_student('student')
 
@@ -481,9 +480,11 @@ class TestProblemResponsesReport(TestReportMixin, InstructorTaskModuleTestCase):
             student = self.create_student('student{}'.format(ctr))
             self.submit_student_answer(student.username, u'Problem1', ['Option 1'])
 
-        student_data = ProblemResponses._build_student_data(user_id=self.instructor.id,
-                                                            course_id=self.course.id,
-                                                            problem_location=str(self.course.location))
+        student_data = ProblemResponses._build_student_data(
+            user_id=self.instructor.id,
+            course_id=self.course.id,
+            problem_location=str(self.course.location),
+        )
 
         self.assertEquals(len(student_data), 4)
 
@@ -491,19 +492,24 @@ class TestProblemResponsesReport(TestReportMixin, InstructorTaskModuleTestCase):
         self.define_option_problem(u'Problem1')
         self.submit_student_answer(self.student.username, u'Problem1', ['Option 1'])
 
-        student_data = ProblemResponses._build_student_data(user_id=self.instructor.id,
-                                                            course_id=self.course.id,
-                                                            problem_location=str(self.course.location))
+        student_data = ProblemResponses._build_student_data(
+            user_id=self.instructor.id,
+            course_id=self.course.id,
+            problem_location=str(self.course.location),
+        )
         self.assertEquals(len(student_data), 1)
         self.assertDictContainsSubset({
             'username': 'student',
-            'location': 'test_course > Section > Subsection',
-            'block_id': 'Problem1',
+            'location': 'test_course > Section > Subsection > Problem1',
+            'block_key': 'i4x://edx/1.23x/problem/Problem1',
             'title': 'Problem1',
         }, student_data[0])
 
     def test_success(self):
-        task_input = {'problem_location': str(self.course.location), 'user_id': self.instructor.id}
+        task_input = {
+            'problem_location': str(self.course.location),
+            'user_id': self.instructor.id
+        }
         with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'):
             with patch('lms.djangoapps.instructor_task.tasks_helper.grades'
                        '.ProblemResponses._build_student_data') as patched_data_source:
@@ -512,7 +518,9 @@ class TestProblemResponsesReport(TestReportMixin, InstructorTaskModuleTestCase):
                     {'username': 'user1', 'state': u'state1'},
                     {'username': 'user2', 'state': u'state2'},
                 ]
-                result = ProblemResponses.generate(None, None, self.course.id, task_input, 'calculated')
+                result = ProblemResponses.generate(
+                    None, None, self.course.id, task_input, 'calculated'
+                )
         report_store = ReportStore.from_config(config_name='GRADES_DOWNLOAD')
         links = report_store.links_for(self.course.id)
 

From 1aa55c6cfaef0537d53da529150349e5597a9e68 Mon Sep 17 00:00:00 2001
From: Kshitij Sobti <kshitij@sobti.in>
Date: Wed, 4 Apr 2018 12:37:23 +0530
Subject: [PATCH 15/19] Documentation grammar and accuracy fixes

---
 lms/djangoapps/instructor_task/tasks_helper/grades.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lms/djangoapps/instructor_task/tasks_helper/grades.py b/lms/djangoapps/instructor_task/tasks_helper/grades.py
index 22dcb9f329..a8f093797e 100644
--- a/lms/djangoapps/instructor_task/tasks_helper/grades.py
+++ b/lms/djangoapps/instructor_task/tasks_helper/grades.py
@@ -566,7 +566,7 @@ class ProblemResponses(object):
             path (List[str]): The list of display names for the parent of root block
 
         Yields:
-            Tuple[str, List[str], UsageKey]: tuple of a blocks display name, path, and
+            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')
@@ -588,8 +588,8 @@ class ProblemResponses(object):
 
         Arguments:
             user_id (int): The user id for the user generating the report
-            course_id (UsageKey): The UsageKey for the course whose report is
-                being generated
+            course_id (CourseKey): The ``CourseKey`` for the course whose report
+                is being generated
             problem_location (str): The generated report will include this
                 block and it child blocks.
 

From ccabab07712f2efd65dbef6a135e4d77f00188ec Mon Sep 17 00:00:00 2001
From: Kshitij Sobti <kshitij@sobti.in>
Date: Thu, 5 Apr 2018 00:57:48 +0530
Subject: [PATCH 16/19] Added chapter blocks to be excluded from report

---
 lms/djangoapps/instructor_task/tasks_helper/grades.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lms/djangoapps/instructor_task/tasks_helper/grades.py b/lms/djangoapps/instructor_task/tasks_helper/grades.py
index a8f093797e..2d524ae9ac 100644
--- a/lms/djangoapps/instructor_task/tasks_helper/grades.py
+++ b/lms/djangoapps/instructor_task/tasks_helper/grades.py
@@ -604,9 +604,9 @@ class ProblemResponses(object):
         student_data = []
         max_count = settings.FEATURES.get('MAX_PROBLEM_RESPONSES_COUNT')
         for title, path, block_key in cls._build_problem_list(course_blocks, problem_key):
-            # Sequential blocks are filtered out since they include position state
-            # which isn't useful in this report.
-            if block_key.block_type == 'sequential':
+            # 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
             responses = list_problem_responses(course_id, block_key, max_count)
             student_data += responses

From 7e01a065b9d4ee4e92d354933975b4e1d2e04bb7 Mon Sep 17 00:00:00 2001
From: Kshitij Sobti <kshitij@sobti.in>
Date: Sat, 21 Apr 2018 02:21:25 +0530
Subject: [PATCH 17/19] Use ``generate_report_data`` for getting report data if
 the block provides it

---
 .../instructor_task/tasks_helper/grades.py    | 66 +++++++++++------
 .../tests/test_tasks_helper.py                | 72 ++++++++++++++++---
 2 files changed, 106 insertions(+), 32 deletions(-)

diff --git a/lms/djangoapps/instructor_task/tasks_helper/grades.py b/lms/djangoapps/instructor_task/tasks_helper/grades.py
index 2d524ae9ac..f0c883a41e 100644
--- a/lms/djangoapps/instructor_task/tasks_helper/grades.py
+++ b/lms/djangoapps/instructor_task/tasks_helper/grades.py
@@ -17,6 +17,7 @@ from six import text_type
 
 from course_blocks.api import get_course_blocks
 from courseware.courses import get_course_by_id
+from courseware.user_state_client import DjangoXBlockUserStateClient
 from instructor_analytics.basic import list_problem_responses
 from instructor_analytics.csvs import format_dictlist
 from lms.djangoapps.certificates.models import CertificateWhitelist, GeneratedCertificate, certificate_info_for_user
@@ -581,43 +582,64 @@ class ProblemResponses(object):
                 yield result
 
     @classmethod
-    def _build_student_data(cls, user_id, course_id, problem_location):
+    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.
 
         Arguments:
             user_id (int): The user id for the user generating the report
-            course_id (CourseKey): The ``CourseKey`` for the course whose report
+            course_key (CourseKey): The ``CourseKey`` for the course whose report
                 is being generated
-            problem_location (str): The generated report will include this
+            usage_key_str (str): The generated report will include this
                 block and it child blocks.
 
         Returns:
               List[Dict]: Returns a list of dictionaries containing the student
                 data which will be included in the final csv.
         """
-        problem_key = UsageKey.from_string(problem_location).map_into_course(course_id)
+        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, problem_key)
+        course_blocks = get_course_blocks(user, usage_key)
 
         student_data = []
         max_count = settings.FEATURES.get('MAX_PROBLEM_RESPONSES_COUNT')
-        for title, path, block_key in cls._build_problem_list(course_blocks, problem_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
-            responses = list_problem_responses(course_id, block_key, max_count)
-            student_data += responses
-            for response in responses:
-                response['title'] = title
-                response['location'] = ' > '.join(path)
-                response['block_key'] = str(block_key)
-            if max_count is not None:
-                max_count -= len(responses)
-                if max_count <= 0:
-                    break
+
+        store = modulestore()
+        user_state_client = DjangoXBlockUserStateClient()
+
+        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
+
+                block = store.get_item(block_key)
+
+                # Blocks can implement the generate_report_data method to provide their own
+                # human-readable formatting for user state.
+                if hasattr(block, 'generate_report_data'):
+                    user_state_iterator = user_state_client.iter_all_for_block(block_key)
+                    responses = [
+                        {'username': username, 'state': state}
+                        for username, state in
+                        block.generate_report_data(user_state_iterator, max_count)
+                    ]
+                else:
+                    responses = list_problem_responses(course_key, block_key, max_count)
+
+                student_data += responses
+                for response in responses:
+                    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)
+                if max_count is not None:
+                    max_count -= len(responses)
+                    if max_count <= 0:
+                        break
 
         return student_data
 
@@ -638,8 +660,8 @@ class ProblemResponses(object):
         # Compute result table and format it
         student_data = cls._build_student_data(
             user_id=task_input.get('user_id'),
-            course_id=course_id,
-            problem_location=problem_location
+            course_key=course_id,
+            usage_key_str=problem_location
         )
 
         features = ['username', 'title', 'location', 'block_key', 'state']
diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py
index 0bcfee9152..f775ceb180 100644
--- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py
+++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py
@@ -24,8 +24,8 @@ from django.conf import settings
 from django.core.urlresolvers import reverse
 from django.test.utils import override_settings
 from freezegun import freeze_time
-from instructor_analytics.basic import UNAVAILABLE
-from mock import MagicMock, Mock, patch
+from instructor_analytics.basic import UNAVAILABLE, list_problem_responses
+from mock import MagicMock, Mock, patch, ANY
 from nose.plugins.attrib import attr
 from pytz import UTC
 from shoppingcart.models import (
@@ -473,8 +473,25 @@ class TestProblemResponsesReport(TestReportMixin, InstructorTaskModuleTestCase):
         self.instructor = self.create_instructor('instructor')
         self.student = self.create_student('student')
 
-    @patch.dict("django.conf.settings.FEATURES", {"MAX_PROBLEM_RESPONSES_COUNT": 4})
+    # Can be used once CapaDescriptor gets ``generate_report_data`` method.
+    # @contextmanager
+    # def _remove_report_generator(self):
+    #     """
+    #     Temporarily removes the generate_report_data method so we can test
+    #     report generation when it's absent.
+    #     """
+    #     from xmodule.capa_module import CapaDescriptor
+    #     generate_report_data = CapaDescriptor.generate_report_data
+    #     del CapaDescriptor.generate_report_data
+    #     yield
+    #     CapaDescriptor.generate_report_data = generate_report_data
+
+    @patch.dict('django.conf.settings.FEATURES', {'MAX_PROBLEM_RESPONSES_COUNT': 4})
     def test_build_student_data_limit(self):
+        """
+        Ensure that the _build_student_data method respects the global setting for
+        maximum responses to return in a report.
+        """
         self.define_option_problem(u'Problem1')
         for ctr in range(5):
             student = self.create_student('student{}'.format(ctr))
@@ -482,20 +499,28 @@ class TestProblemResponsesReport(TestReportMixin, InstructorTaskModuleTestCase):
 
         student_data = ProblemResponses._build_student_data(
             user_id=self.instructor.id,
-            course_id=self.course.id,
-            problem_location=str(self.course.location),
+            course_key=self.course.id,
+            usage_key_str=str(self.course.location),
         )
 
         self.assertEquals(len(student_data), 4)
 
-    def test_build_student_data(self):
+    @patch(
+        'lms.djangoapps.instructor_task.tasks_helper.grades.list_problem_responses',
+        wraps=list_problem_responses
+    )
+    def test_build_student_data_for_block_without_generate_report_data(self, mock_list_problem_responses):
+        """
+        Ensure that building student data for a block the doesn't have the
+        ``generate_report_data`` method works as expected.
+        """
         self.define_option_problem(u'Problem1')
         self.submit_student_answer(self.student.username, u'Problem1', ['Option 1'])
 
         student_data = ProblemResponses._build_student_data(
             user_id=self.instructor.id,
-            course_id=self.course.id,
-            problem_location=str(self.course.location),
+            course_key=self.course.id,
+            usage_key_str=str(self.course.location),
         )
         self.assertEquals(len(student_data), 1)
         self.assertDictContainsSubset({
@@ -504,6 +529,33 @@ class TestProblemResponsesReport(TestReportMixin, InstructorTaskModuleTestCase):
             'block_key': 'i4x://edx/1.23x/problem/Problem1',
             'title': 'Problem1',
         }, student_data[0])
+        self.assertIn('state', student_data[0])
+        mock_list_problem_responses.assert_called_with(self.course.id, ANY, ANY)
+
+    @patch('xmodule.capa_module.CapaDescriptor.generate_report_data', create=True)
+    def test_build_student_data_for_block_with_generate_report_data(self, mock_generate_report_data):
+        """
+        Ensure that building student data for a block that supports the
+        ``generate_report_data`` method works as expected.
+        """
+        self.define_option_problem(u'Problem1')
+        state = {'some': 'state'}
+        mock_generate_report_data.return_value = iter([
+            ('student', state),
+        ])
+        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.assertDictContainsSubset({
+            'username': 'student',
+            'location': 'test_course > Section > Subsection > Problem1',
+            'block_key': 'i4x://edx/1.23x/problem/Problem1',
+            'title': 'Problem1',
+            'state': state,
+        }, student_data[0])
 
     def test_success(self):
         task_input = {
@@ -512,8 +564,8 @@ class TestProblemResponsesReport(TestReportMixin, InstructorTaskModuleTestCase):
         }
         with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'):
             with patch('lms.djangoapps.instructor_task.tasks_helper.grades'
-                       '.ProblemResponses._build_student_data') as patched_data_source:
-                patched_data_source.return_value = [
+                       '.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'},

From 011b295890b13088f593fee3b28a7ae86290f9c9 Mon Sep 17 00:00:00 2001
From: Kshitij Sobti <kshitij@sobti.in>
Date: Sat, 19 May 2018 00:52:29 +0530
Subject: [PATCH 18/19] Handle NotImplementedError raised by
 generate_report_data

---
 .../instructor_task/tasks_helper/grades.py    | 15 +++++++------
 .../instructor_task/tests/test_base.py        | 12 +++++------
 .../tests/test_tasks_helper.py                | 21 +++++++++++++++++++
 3 files changed, 36 insertions(+), 12 deletions(-)

diff --git a/lms/djangoapps/instructor_task/tasks_helper/grades.py b/lms/djangoapps/instructor_task/tasks_helper/grades.py
index f0c883a41e..0d25d71912 100644
--- a/lms/djangoapps/instructor_task/tasks_helper/grades.py
+++ b/lms/djangoapps/instructor_task/tasks_helper/grades.py
@@ -620,12 +620,15 @@ class ProblemResponses(object):
                 # Blocks can implement the generate_report_data method to provide their own
                 # human-readable formatting for user state.
                 if hasattr(block, 'generate_report_data'):
-                    user_state_iterator = user_state_client.iter_all_for_block(block_key)
-                    responses = [
-                        {'username': username, 'state': state}
-                        for username, state in
-                        block.generate_report_data(user_state_iterator, max_count)
-                    ]
+                    try:
+                        user_state_iterator = user_state_client.iter_all_for_block(block_key)
+                        responses = [
+                            {'username': username, 'state': state}
+                            for username, state in
+                            block.generate_report_data(user_state_iterator, max_count)
+                        ]
+                    except NotImplementedError:
+                        responses = list_problem_responses(course_key, block_key, max_count)
                 else:
                     responses = list_problem_responses(course_key, block_key, max_count)
 
diff --git a/lms/djangoapps/instructor_task/tests/test_base.py b/lms/djangoapps/instructor_task/tests/test_base.py
index 26236f39f0..1bd6277bd4 100644
--- a/lms/djangoapps/instructor_task/tests/test_base.py
+++ b/lms/djangoapps/instructor_task/tests/test_base.py
@@ -233,12 +233,12 @@ class InstructorTaskModuleTestCase(InstructorTaskCourseTestCase):
         factory = OptionResponseXMLFactory()
         factory_args = self._option_problem_factory_args()
         problem_xml = factory.build_xml(**factory_args)
-        ItemFactory.create(parent_location=parent.location,
-                           parent=parent,
-                           category="problem",
-                           display_name=problem_url_name,
-                           data=problem_xml,
-                           **kwargs)
+        return ItemFactory.create(parent_location=parent.location,
+                                  parent=parent,
+                                  category="problem",
+                                  display_name=problem_url_name,
+                                  data=problem_xml,
+                                  **kwargs)
 
     def redefine_option_problem(self, problem_url_name, correct_answer=OPTION_1, num_inputs=1, num_responses=2):
         """Change the problem definition so the answer is Option 2"""
diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py
index f775ceb180..e4b772357b 100644
--- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py
+++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py
@@ -557,6 +557,27 @@ class TestProblemResponsesReport(TestReportMixin, InstructorTaskModuleTestCase):
             'state': state,
         }, student_data[0])
 
+    @patch('lms.djangoapps.instructor_task.tasks_helper.grades.list_problem_responses')
+    @patch('xmodule.capa_module.CapaDescriptor.generate_report_data', create=True)
+    def test_build_student_data_for_block_with_generate_report_data_not_implemented(
+            self,
+            mock_generate_report_data,
+            mock_list_problem_responses,
+    ):
+        """
+        Ensure that if ``generate_report_data`` raises a NotImplementedError,
+        the report falls back to the alternative method.
+        """
+        problem = self.define_option_problem(u'Problem1')
+        mock_generate_report_data.side_effect = NotImplementedError
+        ProblemResponses._build_student_data(
+            user_id=self.instructor.id,
+            course_key=self.course.id,
+            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_location': str(self.course.location),

From 73179edd18bac35982066e77f8dc05d177f4c825 Mon Sep 17 00:00:00 2001
From: Kshitij Sobti <kshitij@sobti.in>
Date: Fri, 25 May 2018 19:19:53 +0530
Subject: [PATCH 19/19] Adapt tests to work with the presence of
 generate_report_data Add integrationish test

---
 .../tests/test_tasks_helper.py                | 67 +++++++++++++------
 1 file changed, 46 insertions(+), 21 deletions(-)

diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py
index e4b772357b..be2a0feb47 100644
--- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py
+++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py
@@ -12,6 +12,7 @@ import os
 import shutil
 import tempfile
 import urllib
+from contextlib import contextmanager
 from datetime import datetime, timedelta
 
 import ddt
@@ -473,18 +474,17 @@ class TestProblemResponsesReport(TestReportMixin, InstructorTaskModuleTestCase):
         self.instructor = self.create_instructor('instructor')
         self.student = self.create_student('student')
 
-    # Can be used once CapaDescriptor gets ``generate_report_data`` method.
-    # @contextmanager
-    # def _remove_report_generator(self):
-    #     """
-    #     Temporarily removes the generate_report_data method so we can test
-    #     report generation when it's absent.
-    #     """
-    #     from xmodule.capa_module import CapaDescriptor
-    #     generate_report_data = CapaDescriptor.generate_report_data
-    #     del CapaDescriptor.generate_report_data
-    #     yield
-    #     CapaDescriptor.generate_report_data = generate_report_data
+    @contextmanager
+    def _remove_capa_report_generator(self):
+        """
+        Temporarily removes the generate_report_data method so we can test
+        report generation when it's absent.
+        """
+        from xmodule.capa_module import CapaDescriptor
+        generate_report_data = CapaDescriptor.generate_report_data
+        del CapaDescriptor.generate_report_data
+        yield
+        CapaDescriptor.generate_report_data = generate_report_data
 
     @patch.dict('django.conf.settings.FEATURES', {'MAX_PROBLEM_RESPONSES_COUNT': 4})
     def test_build_student_data_limit(self):
@@ -514,18 +514,18 @@ class TestProblemResponsesReport(TestReportMixin, InstructorTaskModuleTestCase):
         Ensure that building student data for a block the doesn't have the
         ``generate_report_data`` method works as expected.
         """
-        self.define_option_problem(u'Problem1')
+        problem = self.define_option_problem(u'Problem1')
         self.submit_student_answer(self.student.username, u'Problem1', ['Option 1'])
-
-        student_data = ProblemResponses._build_student_data(
-            user_id=self.instructor.id,
-            course_key=self.course.id,
-            usage_key_str=str(self.course.location),
-        )
+        with self._remove_capa_report_generator():
+            student_data = ProblemResponses._build_student_data(
+                user_id=self.instructor.id,
+                course_key=self.course.id,
+                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])
@@ -533,7 +533,7 @@ class TestProblemResponsesReport(TestReportMixin, InstructorTaskModuleTestCase):
         mock_list_problem_responses.assert_called_with(self.course.id, ANY, ANY)
 
     @patch('xmodule.capa_module.CapaDescriptor.generate_report_data', create=True)
-    def test_build_student_data_for_block_with_generate_report_data(self, mock_generate_report_data):
+    def test_build_student_data_for_block_with_mock_generate_report_data(self, mock_generate_report_data):
         """
         Ensure that building student data for a block that supports the
         ``generate_report_data`` method works as expected.
@@ -557,6 +557,31 @@ class TestProblemResponsesReport(TestReportMixin, InstructorTaskModuleTestCase):
             'state': state,
         }, student_data[0])
 
+    def test_build_student_data_for_block_with_real_generate_report_data(self):
+        """
+        Ensure that building student data for a block that supports the
+        ``generate_report_data`` method works as expected.
+        """
+        self.define_option_problem(u'Problem1')
+        self.submit_student_answer(self.student.username, u'Problem1', ['Option 1'])
+        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.assertDictContainsSubset({
+            'username': 'student',
+            'location': 'test_course > Section > Subsection > Problem1',
+            'block_key': 'i4x://edx/1.23x/problem/Problem1',
+            'title': 'Problem1',
+            'state': {
+                'Answer ID': 'i4x-edx-1_23x-problem-Problem1_2_1',
+                'Answer': 'Option 1',
+                'Question': u'The correct answer is Option 1'
+            },
+        }, student_data[0])
+
     @patch('lms.djangoapps.instructor_task.tasks_helper.grades.list_problem_responses')
     @patch('xmodule.capa_module.CapaDescriptor.generate_report_data', create=True)
     def test_build_student_data_for_block_with_generate_report_data_not_implemented(