fix: Ensure problem response reports include all descendant problems regardless of nesting or randomization (#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.
This commit is contained in:
@@ -24,7 +24,8 @@ from common.djangoapps.student.models import CourseEnrollment
|
||||
from common.djangoapps.student.roles import BulkRoleCache
|
||||
from lms.djangoapps.certificates import api as certs_api
|
||||
from lms.djangoapps.certificates.api import get_certificates_for_course_and_users
|
||||
from lms.djangoapps.course_blocks.api import get_course_blocks
|
||||
from lms.djangoapps.course_blocks.api import get_course_block_access_transformers, get_course_blocks
|
||||
from lms.djangoapps.course_blocks.transformers import library_content
|
||||
from lms.djangoapps.courseware.user_state_client import DjangoXBlockUserStateClient
|
||||
from lms.djangoapps.grades.api import CourseGradeFactory
|
||||
from lms.djangoapps.grades.api import context as grades_context
|
||||
@@ -39,11 +40,13 @@ from lms.djangoapps.instructor_task.config.waffle import (
|
||||
from lms.djangoapps.teams.models import CourseTeamMembership
|
||||
from lms.djangoapps.verify_student.services import IDVerificationService
|
||||
from openedx.core.djangoapps.content.block_structure.api import get_course_in_cache
|
||||
from openedx.core.djangoapps.content.block_structure.transformers import BlockStructureTransformers
|
||||
from openedx.core.djangoapps.course_groups.cohorts import bulk_cache_cohorts, get_cohort, is_course_cohorted
|
||||
from openedx.core.djangoapps.user_api.course_tag.api import BulkCourseTags
|
||||
from openedx.core.lib.cache_utils import get_cache
|
||||
from openedx.core.lib.courses import get_course_by_id
|
||||
from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order
|
||||
from xmodule.modulestore.exceptions import ItemNotFoundError
|
||||
from xmodule.partitions.partitions_service import PartitionService # lint-amnesty, pylint: disable=wrong-import-order
|
||||
from xmodule.split_test_block import get_split_user_partitions # lint-amnesty, pylint: disable=wrong-import-order
|
||||
|
||||
@@ -834,13 +837,28 @@ class ProblemResponses:
|
||||
Arguments:
|
||||
course_blocks (BlockStructureBlockData): Block structure for a course.
|
||||
root (UsageKey): This block and its children will be used to generate
|
||||
the problem list
|
||||
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 block's display name, path, and
|
||||
usage key
|
||||
usage key.
|
||||
"""
|
||||
name = course_blocks.get_xblock_field(root, 'display_name') or root.block_type
|
||||
name = course_blocks.get_xblock_field(root, 'display_name')
|
||||
if not name or name == 'problem':
|
||||
# Fallback: CourseBlocks may not have display_name cached for all blocks,
|
||||
# especially for dynamically generated content or library_content blocks.
|
||||
# Loading the full block is necessary to get meaningful names for CSV reports
|
||||
TASK_LOG.debug(
|
||||
"ProblemResponses: display_name missing in course_blocks for %s, falling back to modulestore. "
|
||||
"Occasional occurrences of this message are expected (e.g., library_content children); "
|
||||
"frequent occurrences may indicate a cache or transformer issue.",
|
||||
root,
|
||||
)
|
||||
try:
|
||||
block = modulestore().get_item(root)
|
||||
name = getattr(block, 'display_name', None) or root.block_type
|
||||
except ItemNotFoundError:
|
||||
name = root.block_type
|
||||
if path is None:
|
||||
path = [name]
|
||||
|
||||
@@ -874,8 +892,24 @@ class ProblemResponses:
|
||||
UsageKey.from_string(usage_key_str).map_into_course(course_key)
|
||||
for usage_key_str in usage_key_str_list
|
||||
]
|
||||
|
||||
user = get_user_model().objects.get(pk=user_id)
|
||||
|
||||
# For reporting, we want the full set of descendant blocks including all children
|
||||
# of library_content blocks (randomized content). The default transformer list includes
|
||||
# ContentLibraryTransformer which filters children based on per-user selections.
|
||||
# For staff-generated reports, we bypass those library transformers to see all problems.
|
||||
report_transformers = BlockStructureTransformers([
|
||||
transformer for transformer in get_course_block_access_transformers(user)
|
||||
if not isinstance(
|
||||
transformer,
|
||||
(
|
||||
library_content.ContentLibraryTransformer,
|
||||
library_content.ContentLibraryOrderTransformer,
|
||||
)
|
||||
)
|
||||
])
|
||||
|
||||
student_data = []
|
||||
max_count = settings.FEATURES.get('MAX_PROBLEM_RESPONSES_COUNT')
|
||||
|
||||
@@ -890,12 +924,15 @@ class ProblemResponses:
|
||||
for usage_key in usage_keys: # lint-amnesty, pylint: disable=too-many-nested-blocks
|
||||
if max_count is not None and max_count <= 0:
|
||||
break
|
||||
course_blocks = get_course_blocks(user, usage_key)
|
||||
course_blocks = get_course_blocks(user, usage_key, transformers=report_transformers)
|
||||
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'):
|
||||
# Chapter, sequential, library_content, and itembank blocks are filtered out
|
||||
# since they include state which isn't useful for this report.
|
||||
# library_content (V1) and itembank (V2) state contains internal selection
|
||||
# metadata (which problems were randomly assigned to each user), not actual
|
||||
# student responses.
|
||||
if block_key.block_type in ('sequential', 'chapter', 'library_content', 'itembank'):
|
||||
continue
|
||||
|
||||
if filter_types is not None and block_key.block_type not in filter_types:
|
||||
|
||||
@@ -27,6 +27,7 @@ from pytz import UTC
|
||||
import openedx.core.djangoapps.user_api.course_tag.api as course_tag_api
|
||||
import openedx.core.djangoapps.content.block_structure.api as bs_api
|
||||
from xmodule.capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory # lint-amnesty, pylint: disable=wrong-import-order
|
||||
from lms.djangoapps.course_blocks.transformers import library_content
|
||||
from common.djangoapps.course_modes.models import CourseMode
|
||||
from common.djangoapps.student.models import CourseEnrollment, CourseEnrollmentAllowed
|
||||
from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory
|
||||
@@ -524,6 +525,48 @@ class TestProblemResponsesReport(TestReportMixin, InstructorTaskModuleTestCase):
|
||||
|
||||
assert len(student_data) == 4
|
||||
|
||||
@patch('lms.djangoapps.instructor_task.tasks_helper.grades.list_problem_responses', return_value=[])
|
||||
def test_problem_responses_excludes_library_content_transformers(self, _mock_list_problem_responses):
|
||||
"""Ensure ProblemResponses bypasses per-user library_content transformers.
|
||||
|
||||
The default course block access transformers include library_content transformers
|
||||
that filter children based on the requesting user's selections. Reports must exclude
|
||||
those transformers so output is not dependent on the instructor running the report.
|
||||
"""
|
||||
problem = self.define_option_problem('Problem1')
|
||||
|
||||
captured = {}
|
||||
|
||||
class _FakeCourseBlocks:
|
||||
"""Minimal fake CourseBlocks object for testing."""
|
||||
def get_xblock_field(self, _usage_key, field_name):
|
||||
if field_name == 'display_name':
|
||||
return 'Problem1'
|
||||
return None
|
||||
|
||||
def get_children(self, _usage_key):
|
||||
return []
|
||||
|
||||
def _fake_get_course_blocks(_user, _usage_key, transformers=None, **_kwargs):
|
||||
captured['transformers'] = transformers
|
||||
return _FakeCourseBlocks()
|
||||
|
||||
with patch(
|
||||
'lms.djangoapps.instructor_task.tasks_helper.grades.get_course_blocks',
|
||||
side_effect=_fake_get_course_blocks,
|
||||
):
|
||||
ProblemResponses._build_student_data(
|
||||
user_id=self.instructor.id,
|
||||
course_key=self.course.id,
|
||||
usage_key_str_list=[str(problem.location)],
|
||||
)
|
||||
|
||||
transformers = captured.get('transformers')
|
||||
assert transformers is not None
|
||||
all_transformers = transformers._transformers['supports_filter'] + transformers._transformers['no_filter']
|
||||
assert not any(isinstance(t, library_content.ContentLibraryTransformer) for t in all_transformers)
|
||||
assert not any(isinstance(t, library_content.ContentLibraryOrderTransformer) for t in all_transformers)
|
||||
|
||||
@patch(
|
||||
'lms.djangoapps.instructor_task.tasks_helper.grades.list_problem_responses',
|
||||
wraps=list_problem_responses
|
||||
|
||||
Reference in New Issue
Block a user