Skip to content

Commit fd644d4

Browse files
authored
fix: Ensure problem response reports include all descendant problems regardless of nesting or randomization (openedx#36677)
This PR ensures that the instructor "Problem Responses" report in Open edX includes all student responses to all problems under any selected block, including those that are nested or randomized (such as those from legacy library_content blocks). Previously, the report could miss responses to problems that were not directly visible to the instructor or admin user generating the report, especially in courses using randomized content blocks or deep nesting. In courses that use randomized content (e.g., legacy library_content blocks) or have deeply nested structures, the instructor dashboard’s problem response report was incomplete. It only included responses to problems visible in the block tree for the user generating the report (typically the admin or instructor). As a result, responses to problems served randomly to students, or problems nested in containers, were omitted from the CSV export. This led to inaccurate reporting and made it difficult for instructors to audit all student answers.
1 parent a0359a5 commit fd644d4

File tree

2 files changed

+89
-9
lines changed

2 files changed

+89
-9
lines changed

lms/djangoapps/instructor_task/tasks_helper/grades.py

Lines changed: 46 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@
2424
from common.djangoapps.student.roles import BulkRoleCache
2525
from lms.djangoapps.certificates import api as certs_api
2626
from lms.djangoapps.certificates.api import get_certificates_for_course_and_users
27-
from lms.djangoapps.course_blocks.api import get_course_blocks
27+
from lms.djangoapps.course_blocks.api import get_course_block_access_transformers, get_course_blocks
28+
from lms.djangoapps.course_blocks.transformers import library_content
2829
from lms.djangoapps.courseware.user_state_client import DjangoXBlockUserStateClient
2930
from lms.djangoapps.grades.api import CourseGradeFactory
3031
from lms.djangoapps.grades.api import context as grades_context
@@ -39,11 +40,13 @@
3940
from lms.djangoapps.teams.models import CourseTeamMembership
4041
from lms.djangoapps.verify_student.services import IDVerificationService
4142
from openedx.core.djangoapps.content.block_structure.api import get_course_in_cache
43+
from openedx.core.djangoapps.content.block_structure.transformers import BlockStructureTransformers
4244
from openedx.core.djangoapps.course_groups.cohorts import bulk_cache_cohorts, get_cohort, is_course_cohorted
4345
from openedx.core.djangoapps.user_api.course_tag.api import BulkCourseTags
4446
from openedx.core.lib.cache_utils import get_cache
4547
from openedx.core.lib.courses import get_course_by_id
4648
from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order
49+
from xmodule.modulestore.exceptions import ItemNotFoundError
4750
from xmodule.partitions.partitions_service import PartitionService # lint-amnesty, pylint: disable=wrong-import-order
4851
from xmodule.split_test_block import get_split_user_partitions # lint-amnesty, pylint: disable=wrong-import-order
4952

@@ -834,13 +837,28 @@ def _build_problem_list(cls, course_blocks, root, path=None):
834837
Arguments:
835838
course_blocks (BlockStructureBlockData): Block structure for a course.
836839
root (UsageKey): This block and its children will be used to generate
837-
the problem list
840+
the problem list.
838841
path (List[str]): The list of display names for the parent of root block
839842
Yields:
840843
Tuple[str, List[str], UsageKey]: tuple of a block's display name, path, and
841-
usage key
842-
"""
843-
name = course_blocks.get_xblock_field(root, 'display_name') or root.block_type
844+
usage key.
845+
"""
846+
name = course_blocks.get_xblock_field(root, 'display_name')
847+
if not name or name == 'problem':
848+
# Fallback: CourseBlocks may not have display_name cached for all blocks,
849+
# especially for dynamically generated content or library_content blocks.
850+
# Loading the full block is necessary to get meaningful names for CSV reports
851+
TASK_LOG.debug(
852+
"ProblemResponses: display_name missing in course_blocks for %s, falling back to modulestore. "
853+
"Occasional occurrences of this message are expected (e.g., library_content children); "
854+
"frequent occurrences may indicate a cache or transformer issue.",
855+
root,
856+
)
857+
try:
858+
block = modulestore().get_item(root)
859+
name = getattr(block, 'display_name', None) or root.block_type
860+
except ItemNotFoundError:
861+
name = root.block_type
844862
if path is None:
845863
path = [name]
846864

@@ -874,8 +892,24 @@ def _build_student_data(
874892
UsageKey.from_string(usage_key_str).map_into_course(course_key)
875893
for usage_key_str in usage_key_str_list
876894
]
895+
877896
user = get_user_model().objects.get(pk=user_id)
878897

898+
# For reporting, we want the full set of descendant blocks including all children
899+
# of library_content blocks (randomized content). The default transformer list includes
900+
# ContentLibraryTransformer which filters children based on per-user selections.
901+
# For staff-generated reports, we bypass those library transformers to see all problems.
902+
report_transformers = BlockStructureTransformers([
903+
transformer for transformer in get_course_block_access_transformers(user)
904+
if not isinstance(
905+
transformer,
906+
(
907+
library_content.ContentLibraryTransformer,
908+
library_content.ContentLibraryOrderTransformer,
909+
)
910+
)
911+
])
912+
879913
student_data = []
880914
max_count = settings.FEATURES.get('MAX_PROBLEM_RESPONSES_COUNT')
881915

@@ -890,12 +924,15 @@ def _build_student_data(
890924
for usage_key in usage_keys: # lint-amnesty, pylint: disable=too-many-nested-blocks
891925
if max_count is not None and max_count <= 0:
892926
break
893-
course_blocks = get_course_blocks(user, usage_key)
927+
course_blocks = get_course_blocks(user, usage_key, transformers=report_transformers)
894928
base_path = cls._build_block_base_path(store.get_item(usage_key))
895929
for title, path, block_key in cls._build_problem_list(course_blocks, usage_key):
896-
# Chapter and sequential blocks are filtered out since they include state
897-
# which isn't useful for this report.
898-
if block_key.block_type in ('sequential', 'chapter'):
930+
# Chapter, sequential, library_content, and itembank blocks are filtered out
931+
# since they include state which isn't useful for this report.
932+
# library_content (V1) and itembank (V2) state contains internal selection
933+
# metadata (which problems were randomly assigned to each user), not actual
934+
# student responses.
935+
if block_key.block_type in ('sequential', 'chapter', 'library_content', 'itembank'):
899936
continue
900937

901938
if filter_types is not None and block_key.block_type not in filter_types:

lms/djangoapps/instructor_task/tests/test_tasks_helper.py

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import openedx.core.djangoapps.user_api.course_tag.api as course_tag_api
2828
import openedx.core.djangoapps.content.block_structure.api as bs_api
2929
from xmodule.capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory # lint-amnesty, pylint: disable=wrong-import-order
30+
from lms.djangoapps.course_blocks.transformers import library_content
3031
from common.djangoapps.course_modes.models import CourseMode
3132
from common.djangoapps.student.models import CourseEnrollment, CourseEnrollmentAllowed
3233
from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory
@@ -524,6 +525,48 @@ def test_build_student_data_limit(self):
524525

525526
assert len(student_data) == 4
526527

528+
@patch('lms.djangoapps.instructor_task.tasks_helper.grades.list_problem_responses', return_value=[])
529+
def test_problem_responses_excludes_library_content_transformers(self, _mock_list_problem_responses):
530+
"""Ensure ProblemResponses bypasses per-user library_content transformers.
531+
532+
The default course block access transformers include library_content transformers
533+
that filter children based on the requesting user's selections. Reports must exclude
534+
those transformers so output is not dependent on the instructor running the report.
535+
"""
536+
problem = self.define_option_problem('Problem1')
537+
538+
captured = {}
539+
540+
class _FakeCourseBlocks:
541+
"""Minimal fake CourseBlocks object for testing."""
542+
def get_xblock_field(self, _usage_key, field_name):
543+
if field_name == 'display_name':
544+
return 'Problem1'
545+
return None
546+
547+
def get_children(self, _usage_key):
548+
return []
549+
550+
def _fake_get_course_blocks(_user, _usage_key, transformers=None, **_kwargs):
551+
captured['transformers'] = transformers
552+
return _FakeCourseBlocks()
553+
554+
with patch(
555+
'lms.djangoapps.instructor_task.tasks_helper.grades.get_course_blocks',
556+
side_effect=_fake_get_course_blocks,
557+
):
558+
ProblemResponses._build_student_data(
559+
user_id=self.instructor.id,
560+
course_key=self.course.id,
561+
usage_key_str_list=[str(problem.location)],
562+
)
563+
564+
transformers = captured.get('transformers')
565+
assert transformers is not None
566+
all_transformers = transformers._transformers['supports_filter'] + transformers._transformers['no_filter']
567+
assert not any(isinstance(t, library_content.ContentLibraryTransformer) for t in all_transformers)
568+
assert not any(isinstance(t, library_content.ContentLibraryOrderTransformer) for t in all_transformers)
569+
527570
@patch(
528571
'lms.djangoapps.instructor_task.tasks_helper.grades.list_problem_responses',
529572
wraps=list_problem_responses

0 commit comments

Comments
 (0)