feat: Improve robust score rendering with event-based architecture
This commit implements a comprehensive solution for test score integration in the enhancement system along with improvements to the score rendering mechanism. Key changes include: - Add event handler for rendering blocks with edx-submissions scores - Implement event-based mechanism to render XBlocks with scoring data - Create signal handlers in handlers.py to process external grader scores - Develop specialized XBlock loader for rendering without HTTP requests - Add queue_key propagation across the submission pipeline - Register submission URLs in LMS routing configuration - Add complete docstrings to score render module for better code maintainability - Add ADR for XBlock rendering with external grader integration - Add openedx-events fork branch as a dependency in testing.in - Upgrade edx submission dependency These changes support the migration from traditional XQueue callback HTTP requests to a more robust event-based architecture, improving performance and reliability when processing submission scores. The included ADR documents the architectural decision and implementation approach for this significant improvement to the external grading workflow.
This commit is contained in:
committed by
David Ormsbee
parent
e1747f3844
commit
70ea641c99
@@ -1,13 +1,14 @@
|
||||
"""
|
||||
Grades related signals.
|
||||
"""
|
||||
|
||||
|
||||
import json
|
||||
from contextlib import contextmanager
|
||||
from logging import getLogger
|
||||
|
||||
from django.dispatch import receiver
|
||||
from opaque_keys.edx.keys import LearningContextKey
|
||||
from opaque_keys.edx.keys import CourseKey, LearningContextKey, UsageKey
|
||||
from opaque_keys import InvalidKeyError
|
||||
from openedx_events.learning.signals import EXTERNAL_GRADER_SCORE_SUBMITTED
|
||||
from openedx_events.learning.signals import EXAM_ATTEMPT_REJECTED, EXAM_ATTEMPT_VERIFIED
|
||||
from submissions.models import score_reset, score_set
|
||||
from xblock.scorable import ScorableXBlockMixin, Score
|
||||
@@ -23,23 +24,24 @@ from lms.djangoapps.grades.tasks import (
|
||||
recalculate_subsection_grade_v3
|
||||
)
|
||||
from openedx.core.djangoapps.course_groups.signals.signals import COHORT_MEMBERSHIP_UPDATED
|
||||
from openedx.core.djangoapps.signals.signals import ( # lint-amnesty, pylint: disable=wrong-import-order
|
||||
COURSE_GRADE_NOW_FAILED,
|
||||
COURSE_GRADE_NOW_PASSED
|
||||
)
|
||||
from openedx.core.lib.grade_utils import is_score_higher_or_equal
|
||||
from xmodule.modulestore.django import modulestore
|
||||
|
||||
from .. import events
|
||||
from ..constants import GradeOverrideFeatureEnum, ScoreDatabaseTableEnum
|
||||
from ..course_grade_factory import CourseGradeFactory
|
||||
from ..scores import weighted_score
|
||||
from .signals import (
|
||||
COURSE_GRADE_PASSED_FIRST_TIME,
|
||||
PROBLEM_RAW_SCORE_CHANGED,
|
||||
PROBLEM_WEIGHTED_SCORE_CHANGED,
|
||||
SCORE_PUBLISHED,
|
||||
SUBSECTION_OVERRIDE_CHANGED,
|
||||
SUBSECTION_SCORE_CHANGED,
|
||||
COURSE_GRADE_PASSED_FIRST_TIME
|
||||
)
|
||||
from openedx.core.djangoapps.signals.signals import ( # lint-amnesty, pylint: disable=wrong-import-order
|
||||
COURSE_GRADE_NOW_FAILED,
|
||||
COURSE_GRADE_NOW_PASSED
|
||||
SUBSECTION_SCORE_CHANGED
|
||||
)
|
||||
|
||||
log = getLogger(__name__)
|
||||
@@ -347,3 +349,98 @@ def exam_attempt_rejected_event_handler(sender, signal, **kwargs): # pylint: di
|
||||
overrider=None,
|
||||
comment=None,
|
||||
)
|
||||
|
||||
|
||||
@receiver(EXTERNAL_GRADER_SCORE_SUBMITTED)
|
||||
def handle_external_grader_score(signal, sender, score, **kwargs):
|
||||
"""
|
||||
Event handler for external grader score submissions.
|
||||
|
||||
This function is triggered when an external grader submits a score through the
|
||||
EXTERNAL_GRADER_SCORE_SUBMITTED signal. It processes the score and updates
|
||||
the corresponding XBlock instance with the grading results.
|
||||
|
||||
Args:
|
||||
signal: The signal that triggered this handler
|
||||
sender: The object that sent the signal
|
||||
score: An object containing the score data with attributes:
|
||||
- score_msg: The actual score message/response from the grader
|
||||
- course_id: String ID of the course
|
||||
- user_id: ID of the user who submitted the problem
|
||||
- module_id: ID of the module/problem
|
||||
- submission_id: ID of the submission
|
||||
- queue_key: Key identifying the submission in the queue
|
||||
- queue_name: Name of the queue used for grading
|
||||
**kwargs: Additional keyword arguments passed with the signal
|
||||
|
||||
The function logs details about the score event, formats the grader message
|
||||
appropriately, and then calls the module's score_update handler to record
|
||||
the grade in the learning management system.
|
||||
"""
|
||||
|
||||
log.info(f"Received external grader score event: {signal}, {sender}, {score}, {kwargs}")
|
||||
|
||||
grader_msg = score.score_msg
|
||||
log.info(
|
||||
"External grader event score payload received: user_id=%s, module_id=%s, submission_id=%s, course_id=%s",
|
||||
score.user_id,
|
||||
score.module_id,
|
||||
score.submission_id,
|
||||
score.course_id,
|
||||
)
|
||||
|
||||
# Since we already confirm this in edx-submissions, it is safe to parse this
|
||||
grader_msg = json.loads(grader_msg)
|
||||
log.info(f"External grader score: {grader_msg['score']}")
|
||||
|
||||
data = {
|
||||
'xqueue_header': json.dumps({
|
||||
'lms_key': str(score.submission_id),
|
||||
'queue_name': score.queue_name
|
||||
}),
|
||||
'xqueue_body': json.dumps(grader_msg),
|
||||
'queuekey': score.queue_key
|
||||
}
|
||||
|
||||
try:
|
||||
course_key = CourseKey.from_string(score.course_id)
|
||||
course = modulestore().get_course(course_key, depth=0)
|
||||
except InvalidKeyError:
|
||||
log.error("Invalid course_id received from external grader: %s", score.course_id)
|
||||
return
|
||||
|
||||
try:
|
||||
usage_key = UsageKey.from_string(score.module_id)
|
||||
except InvalidKeyError:
|
||||
log.error("Invalid usage key received from external grader: %s", score.module_id)
|
||||
return
|
||||
|
||||
# pylint: disable=broad-exception-caught
|
||||
try:
|
||||
# Use our new function instead of load_single_xblock
|
||||
# NOTE: Importing this at module level causes a circular import because
|
||||
# score_render → block_render → grades signals → back into this module.
|
||||
# Keeping it inside the handler avoids that by loading it only when needed.
|
||||
from xmodule.capa.score_render import load_xblock_for_external_grader
|
||||
instance = load_xblock_for_external_grader(score.user_id,
|
||||
course_key,
|
||||
usage_key,
|
||||
course=course)
|
||||
|
||||
# Call the handler method (mirroring the original xqueue_callback)
|
||||
instance.handle_ajax('score_update', data)
|
||||
|
||||
# Save any state changes
|
||||
instance.save()
|
||||
|
||||
log.info(f"Successfully processed external grade for module {score.module_id}, user {score.user_id}")
|
||||
|
||||
except Exception as e:
|
||||
log.exception(
|
||||
"Error processing external grade for user_id=%s, module_id=%s, submission_id=%s: %s",
|
||||
score.user_id,
|
||||
score.module_id,
|
||||
score.submission_id,
|
||||
e,
|
||||
)
|
||||
raise
|
||||
|
||||
@@ -3597,6 +3597,12 @@ EVENT_BUS_PRODUCER_CONFIG = {
|
||||
"enabled": Derived(should_send_learning_badge_events),
|
||||
},
|
||||
},
|
||||
"org.openedx.learning.external_grader.score.submitted.v1": {
|
||||
"learning-external-grader-score-lifecycle": {
|
||||
"event_key_field": "score.submission_id",
|
||||
"enabled": False
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
#### Survey Report ####
|
||||
|
||||
13
lms/urls.py
13
lms/urls.py
@@ -12,6 +12,7 @@ from django.utils.translation import gettext_lazy as _
|
||||
from django.views.generic.base import RedirectView
|
||||
from edx_api_doc_tools import make_docs_urls
|
||||
from edx_django_utils.plugins import get_plugin_url_patterns
|
||||
from submissions import urls as submissions_urls
|
||||
|
||||
from common.djangoapps.student import views as student_views
|
||||
from common.djangoapps.util import views as util_views
|
||||
@@ -355,6 +356,14 @@ urlpatterns += [
|
||||
name='xqueue_callback',
|
||||
),
|
||||
|
||||
re_path(
|
||||
r'^courses/{}/xqueue/(?P<userid>[^/]*)/(?P<mod_id>.*?)/(?P<dispatch>[^/]*)$'.format(
|
||||
settings.COURSE_ID_PATTERN,
|
||||
),
|
||||
xqueue_callback,
|
||||
name='callback_submission',
|
||||
),
|
||||
|
||||
# TODO: These views need to be updated before they work
|
||||
path('calculate', util_views.calculate),
|
||||
|
||||
@@ -1052,3 +1061,7 @@ urlpatterns += [
|
||||
urlpatterns += [
|
||||
path('api/notifications/', include('openedx.core.djangoapps.notifications.urls')),
|
||||
]
|
||||
|
||||
urlpatterns += [
|
||||
path('xqueue/', include((submissions_urls, 'submissions'), namespace='submissions')),
|
||||
]
|
||||
|
||||
@@ -526,7 +526,7 @@ edx-search==4.3.0
|
||||
# openedx-forum
|
||||
edx-sga==0.27.0
|
||||
# via -r requirements/edx/bundled.in
|
||||
edx-submissions==3.12.1
|
||||
edx-submissions==3.12.2
|
||||
# via
|
||||
# -r requirements/edx/kernel.in
|
||||
# ora2
|
||||
|
||||
@@ -828,7 +828,7 @@ edx-sga==0.27.0
|
||||
# via
|
||||
# -r requirements/edx/doc.txt
|
||||
# -r requirements/edx/testing.txt
|
||||
edx-submissions==3.12.1
|
||||
edx-submissions==3.12.2
|
||||
# via
|
||||
# -r requirements/edx/doc.txt
|
||||
# -r requirements/edx/testing.txt
|
||||
|
||||
@@ -615,7 +615,7 @@ edx-search==4.3.0
|
||||
# openedx-forum
|
||||
edx-sga==0.27.0
|
||||
# via -r requirements/edx/base.txt
|
||||
edx-submissions==3.12.1
|
||||
edx-submissions==3.12.2
|
||||
# via
|
||||
# -r requirements/edx/base.txt
|
||||
# ora2
|
||||
|
||||
@@ -80,7 +80,6 @@
|
||||
|
||||
# ... add dependencies here
|
||||
|
||||
|
||||
##############################################################################
|
||||
# Critical fixes for packages that are not yet available in a PyPI release.
|
||||
##############################################################################
|
||||
|
||||
@@ -639,7 +639,7 @@ edx-search==4.3.0
|
||||
# openedx-forum
|
||||
edx-sga==0.27.0
|
||||
# via -r requirements/edx/base.txt
|
||||
edx-submissions==3.12.1
|
||||
edx-submissions==3.12.2
|
||||
# via
|
||||
# -r requirements/edx/base.txt
|
||||
# ora2
|
||||
|
||||
90
xmodule/capa/score_render.py
Normal file
90
xmodule/capa/score_render.py
Normal file
@@ -0,0 +1,90 @@
|
||||
"""
|
||||
Score rendering when submission is evaluated for external grader and has been saved successfully
|
||||
"""
|
||||
import logging
|
||||
from functools import partial
|
||||
|
||||
from django.http import Http404
|
||||
from edx_when.field_data import DateLookupFieldData
|
||||
from opaque_keys.edx.keys import CourseKey, UsageKey
|
||||
from xblock.runtime import KvsFieldData
|
||||
|
||||
from common.djangoapps.student.models import AnonymousUserId
|
||||
from lms.djangoapps.courseware.block_render import prepare_runtime_for_user
|
||||
from lms.djangoapps.courseware.field_overrides import OverrideFieldData
|
||||
from lms.djangoapps.courseware.model_data import DjangoKeyValueStore, FieldDataCache
|
||||
from lms.djangoapps.lms_xblock.field_data import LmsFieldData
|
||||
from xmodule.modulestore.django import modulestore
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def load_xblock_for_external_grader(
|
||||
user_id: str,
|
||||
course_key: CourseKey,
|
||||
usage_key: UsageKey,
|
||||
course=None,
|
||||
):
|
||||
"""
|
||||
Load a single XBlock for external grading without user access checks.
|
||||
"""
|
||||
|
||||
user = AnonymousUserId.objects.get(anonymous_user_id=user_id).user
|
||||
|
||||
# pylint: disable=broad-exception-caught
|
||||
try:
|
||||
block = modulestore().get_item(usage_key)
|
||||
except Exception as e:
|
||||
log.exception(f"Could not find block {usage_key} in modulestore: {e}")
|
||||
raise Http404(f"Module {usage_key} was not found") from e
|
||||
|
||||
field_data_cache = FieldDataCache.cache_for_block_descendents(
|
||||
course_key, user, block, depth=0
|
||||
)
|
||||
|
||||
student_kvs = DjangoKeyValueStore(field_data_cache)
|
||||
student_data = KvsFieldData(student_kvs)
|
||||
|
||||
instance = get_block_for_descriptor_without_access_check(
|
||||
user=user,
|
||||
block=block,
|
||||
student_data=student_data,
|
||||
course_key=course_key,
|
||||
course=course
|
||||
)
|
||||
|
||||
if instance is None:
|
||||
msg = f"Could not bind XBlock instance for usage key: {usage_key}"
|
||||
log.error(msg)
|
||||
raise Http404(msg)
|
||||
|
||||
return instance
|
||||
|
||||
|
||||
def get_block_for_descriptor_without_access_check(user, block, student_data, course_key, course=None):
|
||||
"""
|
||||
Modified version of get_block_for_descriptor that skips access checks for system operations.
|
||||
"""
|
||||
|
||||
prepare_runtime_for_user(
|
||||
user=user,
|
||||
student_data=student_data,
|
||||
runtime=block.runtime,
|
||||
course_id=course_key,
|
||||
course=course,
|
||||
track_function=lambda event_type, event: None,
|
||||
request_token="external-grader-token",
|
||||
position=None,
|
||||
wrap_xblock_display=True,
|
||||
)
|
||||
|
||||
block.bind_for_student(
|
||||
user.id,
|
||||
[
|
||||
partial(DateLookupFieldData, course_id=course_key, user=user),
|
||||
partial(OverrideFieldData.wrap, user, course),
|
||||
partial(LmsFieldData, student_data=student_data),
|
||||
],
|
||||
)
|
||||
|
||||
return block
|
||||
353
xmodule/capa/tests/test_score_render.py
Normal file
353
xmodule/capa/tests/test_score_render.py
Normal file
@@ -0,0 +1,353 @@
|
||||
"""
|
||||
Test for xmodule.capa.score_render module
|
||||
"""
|
||||
import json
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from django.http import Http404
|
||||
|
||||
from common.djangoapps.student.models import AnonymousUserId
|
||||
from common.djangoapps.student.tests.factories import UserFactory
|
||||
from lms.djangoapps.grades.signals.handlers import handle_external_grader_score
|
||||
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
|
||||
from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory
|
||||
from xmodule.capa.score_render import (
|
||||
load_xblock_for_external_grader,
|
||||
get_block_for_descriptor_without_access_check
|
||||
)
|
||||
from opaque_keys.edx.keys import CourseKey, UsageKey
|
||||
|
||||
|
||||
class ScoreEvent:
|
||||
"""
|
||||
Mock class to represent an external grader score event.
|
||||
"""
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
score_msg=None,
|
||||
course_id=None,
|
||||
user_id=None,
|
||||
module_id=None,
|
||||
submission_id=None,
|
||||
queue_key=None,
|
||||
queue_name=None
|
||||
):
|
||||
self.score_msg = score_msg
|
||||
self.course_id = course_id
|
||||
self.user_id = user_id
|
||||
self.module_id = module_id
|
||||
self.submission_id = submission_id
|
||||
self.queue_key = queue_key
|
||||
self.queue_name = queue_name
|
||||
|
||||
|
||||
class TestScoreRender(ModuleStoreTestCase):
|
||||
"""
|
||||
Tests for the score_render module which handles external grader score submissions.
|
||||
"""
|
||||
|
||||
def setUp(self):
|
||||
"""
|
||||
Set up the test environment.
|
||||
"""
|
||||
super().setUp()
|
||||
self.course = CourseFactory.create()
|
||||
self.user = UserFactory.create()
|
||||
self.problem = BlockFactory.create(
|
||||
category='problem',
|
||||
parent=self.course,
|
||||
display_name='Test Problem'
|
||||
)
|
||||
self.anonymous_user_id = '12345'
|
||||
# Create AnonymousUserId instance
|
||||
AnonymousUserId.objects.create(
|
||||
user=self.user,
|
||||
anonymous_user_id=self.anonymous_user_id,
|
||||
course_id=self.course.id
|
||||
)
|
||||
|
||||
@patch('xmodule.capa.score_render.modulestore')
|
||||
@patch('xmodule.capa.score_render.FieldDataCache')
|
||||
def test_load_xblock_for_external_grader(self, mock_field_data_cache, mock_modulestore):
|
||||
"""
|
||||
Test loading an XBlock for external grading.
|
||||
"""
|
||||
# Setup mock returns
|
||||
mock_modulestore.return_value = MagicMock()
|
||||
mock_modulestore.return_value.get_item.return_value = MagicMock()
|
||||
mock_field_data_cache.cache_for_block_descendents.return_value = MagicMock()
|
||||
|
||||
with patch('xmodule.capa.score_render.get_block_for_descriptor_without_access_check') as mock_get_block:
|
||||
mock_get_block.return_value = MagicMock()
|
||||
|
||||
# Call the function
|
||||
result = load_xblock_for_external_grader(
|
||||
self.anonymous_user_id,
|
||||
str(self.course.id),
|
||||
str(self.problem.location),
|
||||
self.course
|
||||
)
|
||||
|
||||
# Assertions
|
||||
self.assertIsNotNone(result, "Should return a block instance")
|
||||
mock_modulestore.return_value.get_item.assert_called_once()
|
||||
mock_field_data_cache.cache_for_block_descendents.assert_called_once()
|
||||
mock_get_block.assert_called_once()
|
||||
|
||||
@patch('xmodule.capa.score_render.modulestore')
|
||||
@patch('xmodule.capa.score_render.AnonymousUserId.objects.get')
|
||||
def test_load_xblock_for_external_grader_missing_block(self, mock_anon_user, mock_modulestore):
|
||||
"""
|
||||
Test that Http404 is raised when the block is not found.
|
||||
"""
|
||||
# Setup mock returns
|
||||
mock_anon_user.return_value = MagicMock(user=self.user)
|
||||
mock_modulestore.return_value = MagicMock()
|
||||
mock_modulestore.return_value.get_item.side_effect = Exception("Block not found")
|
||||
|
||||
# Test that Http404 is raised
|
||||
with self.assertRaises(Http404):
|
||||
load_xblock_for_external_grader(
|
||||
self.anonymous_user_id,
|
||||
str(self.course.id),
|
||||
str(self.problem.location),
|
||||
self.course
|
||||
)
|
||||
|
||||
@patch('xmodule.capa.score_render.prepare_runtime_for_user')
|
||||
def test_get_block_for_descriptor_without_access_check(self, mock_prepare_runtime):
|
||||
"""
|
||||
Test initializing an XBlock instance without access checks.
|
||||
"""
|
||||
# Setup mocks
|
||||
block = MagicMock()
|
||||
block.runtime = MagicMock()
|
||||
student_data = MagicMock()
|
||||
|
||||
# Call the function
|
||||
result = get_block_for_descriptor_without_access_check(
|
||||
self.user,
|
||||
block,
|
||||
student_data,
|
||||
self.course.id,
|
||||
self.course
|
||||
)
|
||||
|
||||
# Assertions
|
||||
self.assertIsNotNone(result, "Should return a block instance")
|
||||
mock_prepare_runtime.assert_called_once()
|
||||
block.bind_for_student.assert_called_once()
|
||||
|
||||
@patch('xmodule.capa.score_render.modulestore')
|
||||
@patch('xmodule.capa.score_render.load_xblock_for_external_grader')
|
||||
def test_handle_external_grader_score_json_string(self, mock_load_xblock, mock_modulestore):
|
||||
"""
|
||||
Test handling an external grader score with a JSON string message.
|
||||
"""
|
||||
# Setup mocks
|
||||
mock_modulestore.return_value = MagicMock()
|
||||
mock_instance = MagicMock()
|
||||
mock_load_xblock.return_value = mock_instance
|
||||
|
||||
# Create score event
|
||||
score = ScoreEvent(
|
||||
score_msg='{"score": 10, "feedback": "Great job!"}',
|
||||
course_id=str(self.course.id),
|
||||
user_id=self.anonymous_user_id,
|
||||
module_id=str(self.problem.location),
|
||||
submission_id='sub_123',
|
||||
queue_key='key_456',
|
||||
queue_name='test_queue'
|
||||
)
|
||||
|
||||
# Call the handler
|
||||
handle_external_grader_score(None, None, score)
|
||||
|
||||
# Assertions
|
||||
mock_load_xblock.assert_called_once()
|
||||
call_args, call_kwargs = mock_load_xblock.call_args
|
||||
|
||||
self.assertEqual(call_args[0], score.user_id)
|
||||
self.assertIsInstance(call_args[1], CourseKey)
|
||||
self.assertEqual(str(call_args[1]), score.course_id)
|
||||
self.assertIsInstance(call_args[2], UsageKey)
|
||||
self.assertEqual(str(call_args[2]), score.module_id)
|
||||
|
||||
self.assertIn('course', call_kwargs)
|
||||
|
||||
mock_instance.handle_ajax.assert_called_once()
|
||||
ajax_args, _ = mock_instance.handle_ajax.call_args
|
||||
self.assertEqual(ajax_args[0], 'score_update')
|
||||
self.assertIn('xqueue_header', ajax_args[1])
|
||||
self.assertIn('xqueue_body', ajax_args[1])
|
||||
self.assertIn('queuekey', ajax_args[1])
|
||||
mock_instance.save.assert_called_once()
|
||||
|
||||
@patch('xmodule.capa.score_render.modulestore')
|
||||
@patch('xmodule.capa.score_render.load_xblock_for_external_grader')
|
||||
def test_handle_external_grader_score_plain_text(self, mock_load_xblock, mock_modulestore):
|
||||
"""
|
||||
Test handling an external grader score with a plain text message.
|
||||
"""
|
||||
# Setup mocks
|
||||
mock_modulestore.return_value = MagicMock()
|
||||
mock_instance = MagicMock()
|
||||
mock_load_xblock.return_value = mock_instance
|
||||
|
||||
# Create score event with plain text
|
||||
plain_text = "Plain text feedback that is not JSON"
|
||||
score = ScoreEvent(
|
||||
score_msg=plain_text,
|
||||
course_id=str(self.course.id),
|
||||
user_id=self.anonymous_user_id,
|
||||
module_id=str(self.problem.location),
|
||||
submission_id='sub_123',
|
||||
queue_key='key_456',
|
||||
queue_name='test_queue'
|
||||
)
|
||||
|
||||
# json.loads must fail BEFORE anything else runs
|
||||
with self.assertRaises(json.JSONDecodeError):
|
||||
handle_external_grader_score(None, None, score)
|
||||
|
||||
# Assertions
|
||||
mock_load_xblock.assert_not_called()
|
||||
|
||||
mock_instance.handle_ajax.assert_not_called()
|
||||
|
||||
mock_instance.save.assert_not_called()
|
||||
|
||||
@patch('xmodule.capa.score_render.modulestore')
|
||||
@patch('xmodule.capa.score_render.load_xblock_for_external_grader')
|
||||
def test_handle_external_grader_score_exception(self, mock_load_xblock, mock_modulestore):
|
||||
"""
|
||||
Test handling an exception during score processing.
|
||||
"""
|
||||
# Setup mocks
|
||||
mock_modulestore.return_value = MagicMock()
|
||||
mock_load_xblock.side_effect = Exception("Test exception")
|
||||
|
||||
# Create score event
|
||||
score = ScoreEvent(
|
||||
score_msg='{"score": 10}',
|
||||
course_id=str(self.course.id),
|
||||
user_id=self.anonymous_user_id,
|
||||
module_id=str(self.problem.location),
|
||||
submission_id='sub_123',
|
||||
queue_key='key_456',
|
||||
queue_name='test_queue'
|
||||
)
|
||||
|
||||
# Call the handler and expect exception to be raised
|
||||
with self.assertRaises(Exception):
|
||||
handle_external_grader_score(None, None, score)
|
||||
|
||||
@patch('xmodule.capa.score_render.AnonymousUserId.objects.get')
|
||||
@patch('xmodule.capa.score_render.modulestore')
|
||||
@patch('xmodule.capa.score_render.FieldDataCache')
|
||||
@patch('xmodule.capa.score_render.get_block_for_descriptor_without_access_check')
|
||||
def test_load_xblock_for_external_grader_none_instance(self, mock_get_block, mock_field_data_cache,
|
||||
mock_modulestore, mock_anon_user):
|
||||
"""
|
||||
Test that Http404 is raised when get_block_for_descriptor_without_access_check returns None.
|
||||
"""
|
||||
# Setup mock returns
|
||||
mock_anon_user.return_value = MagicMock(user=self.user)
|
||||
mock_modulestore.return_value = MagicMock()
|
||||
mock_block = MagicMock()
|
||||
mock_modulestore.return_value.get_item.return_value = mock_block
|
||||
mock_field_data_cache.cache_for_block_descendents.return_value = MagicMock()
|
||||
mock_get_block.return_value = None
|
||||
|
||||
# Test that Http404 is raised
|
||||
with self.assertRaises(Http404) as context:
|
||||
load_xblock_for_external_grader(
|
||||
self.anonymous_user_id,
|
||||
str(self.course.id),
|
||||
str(self.problem.location)
|
||||
)
|
||||
|
||||
expected_msg = f"Could not bind XBlock instance for usage key: {str(self.problem.location)}"
|
||||
self.assertEqual(str(context.exception), expected_msg)
|
||||
|
||||
# Verify that all mocks were called
|
||||
mock_anon_user.assert_called_once()
|
||||
mock_modulestore.return_value.get_item.assert_called_once()
|
||||
mock_field_data_cache.cache_for_block_descendents.assert_called_once()
|
||||
mock_get_block.assert_called_once()
|
||||
|
||||
|
||||
class TestScoreRenderIntegration(ModuleStoreTestCase):
|
||||
"""
|
||||
Integration tests for the score_render module.
|
||||
"""
|
||||
|
||||
def setUp(self):
|
||||
"""
|
||||
Set up the test environment.
|
||||
"""
|
||||
super().setUp()
|
||||
self.course = CourseFactory.create()
|
||||
self.user = UserFactory.create()
|
||||
self.problem = BlockFactory.create(
|
||||
category='problem',
|
||||
parent=self.course,
|
||||
display_name='Test Problem'
|
||||
)
|
||||
self.anonymous_user_id = '12345'
|
||||
# Create AnonymousUserId instance
|
||||
AnonymousUserId.objects.create(
|
||||
user=self.user,
|
||||
anonymous_user_id=self.anonymous_user_id,
|
||||
course_id=self.course.id
|
||||
)
|
||||
|
||||
@patch('xmodule.capa.score_render.modulestore')
|
||||
def test_end_to_end_grading_flow(self, mock_modulestore):
|
||||
"""
|
||||
Test the end-to-end flow from receiving a score event to updating the grade.
|
||||
"""
|
||||
# Mock the internal call to load_xblock_for_external_grader
|
||||
with patch('xmodule.capa.score_render.load_xblock_for_external_grader') as mock_load_xblock:
|
||||
# Setup the mock XBlock instance
|
||||
mock_instance = MagicMock()
|
||||
mock_load_xblock.return_value = mock_instance
|
||||
|
||||
# Create a score event
|
||||
score = ScoreEvent(
|
||||
score_msg='{"score": 1, "max_score": 1, "correct": true}',
|
||||
course_id=str(self.course.id),
|
||||
user_id=self.anonymous_user_id,
|
||||
module_id=str(self.problem.location),
|
||||
submission_id='sub_123',
|
||||
queue_key='key_456',
|
||||
queue_name='test_queue'
|
||||
)
|
||||
|
||||
# Call the handler
|
||||
handle_external_grader_score(None, None, score)
|
||||
|
||||
# Assertions
|
||||
mock_load_xblock.assert_called_once()
|
||||
mock_instance.handle_ajax.assert_called_once()
|
||||
mock_instance.save.assert_called_once()
|
||||
|
||||
# Verify the data structure passed to handle_ajax
|
||||
handle_ajax_args = mock_instance.handle_ajax.call_args[0]
|
||||
self.assertEqual(handle_ajax_args[0], 'score_update')
|
||||
|
||||
data = handle_ajax_args[1]
|
||||
self.assertIn('xqueue_header', data)
|
||||
self.assertIn('xqueue_body', data)
|
||||
self.assertIn('queuekey', data)
|
||||
|
||||
header = json.loads(data['xqueue_header'])
|
||||
self.assertEqual(header['lms_key'], 'sub_123')
|
||||
self.assertEqual(header['queue_name'], 'test_queue')
|
||||
|
||||
# Verify the body is the correct JSON
|
||||
body = json.loads(data['xqueue_body'])
|
||||
self.assertEqual(body['score'], 1)
|
||||
self.assertEqual(body['max_score'], 1)
|
||||
self.assertTrue(body['correct'])
|
||||
@@ -90,26 +90,23 @@ def test_send_to_queue_with_flag_enabled(mock_send_to_submission, mock_flag):
|
||||
block = Mock() # Mock block for the constructor
|
||||
xqueue_interface = XQueueInterface(url, django_auth, block=block)
|
||||
|
||||
header = json.dumps(
|
||||
{
|
||||
"lms_callback_url": (
|
||||
"http://example.com/courses/course-v1:test_org+test_course+test_run/"
|
||||
"xqueue/block@item_id/type@problem"
|
||||
),
|
||||
}
|
||||
)
|
||||
body = json.dumps(
|
||||
{
|
||||
"student_info": json.dumps({"anonymous_student_id": "student_id"}),
|
||||
"student_response": "student_answer",
|
||||
}
|
||||
)
|
||||
header = json.dumps({
|
||||
"lms_callback_url": (
|
||||
"http://example.com/courses/course-v1:test_org+test_course+test_run/"
|
||||
"xqueue/block@item_id/type@problem"
|
||||
),
|
||||
"lms_key": "default"
|
||||
})
|
||||
body = json.dumps({
|
||||
"student_info": json.dumps({"anonymous_student_id": "student_id"}),
|
||||
"student_response": "student_answer",
|
||||
})
|
||||
files_to_upload = None
|
||||
|
||||
mock_send_to_submission.return_value = {"submission": "mock_submission"}
|
||||
error, msg = xqueue_interface.send_to_queue(header, body, files_to_upload)
|
||||
|
||||
mock_send_to_submission.assert_called_once_with(header, body, {})
|
||||
mock_send_to_submission.assert_called_once_with(header, body, "default", {})
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
@@ -122,20 +119,17 @@ def test_send_to_queue_with_flag_disabled(mock_http_post, mock_flag):
|
||||
block = Mock() # Mock block for the constructor
|
||||
xqueue_interface = XQueueInterface(url, django_auth, block=block)
|
||||
|
||||
header = json.dumps(
|
||||
{
|
||||
"lms_callback_url": (
|
||||
"http://example.com/courses/course-v1:test_org+test_course+test_run/"
|
||||
"xqueue/block@item_id/type@problem"
|
||||
),
|
||||
}
|
||||
)
|
||||
body = json.dumps(
|
||||
{
|
||||
"student_info": json.dumps({"anonymous_student_id": "student_id"}),
|
||||
"student_response": "student_answer",
|
||||
}
|
||||
)
|
||||
header = json.dumps({
|
||||
"lms_callback_url": (
|
||||
"http://example.com/courses/course-v1:test_org+test_course+test_run/"
|
||||
"xqueue/block@item_id/type@problem"
|
||||
),
|
||||
"lms_key": "default"
|
||||
})
|
||||
body = json.dumps({
|
||||
"student_info": json.dumps({"anonymous_student_id": "student_id"}),
|
||||
"student_response": "student_answer",
|
||||
})
|
||||
files_to_upload = None
|
||||
|
||||
mock_http_post.return_value = (0, "Submission sent successfully")
|
||||
|
||||
@@ -77,7 +77,7 @@ def test_send_to_submission(mock_create_external_grader_detail, xqueue_service):
|
||||
mock_response = {"submission": "mock_submission"}
|
||||
mock_create_external_grader_detail.return_value = mock_response
|
||||
|
||||
result = xqueue_service.send_to_submission(header, body)
|
||||
result = xqueue_service.send_to_submission(header, body, queue_key="default")
|
||||
|
||||
assert result == mock_response
|
||||
mock_create_external_grader_detail.assert_called_once_with(
|
||||
@@ -87,9 +87,10 @@ def test_send_to_submission(mock_create_external_grader_detail, xqueue_service):
|
||||
"course_id": "course-v1:test_org+test_course+test_run",
|
||||
"student_id": "student_id",
|
||||
},
|
||||
"student_answer",
|
||||
queue_name="default",
|
||||
grader_file_name="test.py",
|
||||
'student_answer',
|
||||
queue_name='default',
|
||||
queue_key='default',
|
||||
grader_file_name='test.py',
|
||||
points_possible=10,
|
||||
files=None,
|
||||
)
|
||||
@@ -115,7 +116,7 @@ def test_send_to_submission_with_missing_fields(mock_create_external_grader_deta
|
||||
}
|
||||
)
|
||||
|
||||
result = xqueue_service.send_to_submission(header, body)
|
||||
result = xqueue_service.send_to_submission(header, body, queue_key="default")
|
||||
|
||||
assert "error" in result
|
||||
assert "Validation error" in result["error"]
|
||||
|
||||
@@ -183,10 +183,12 @@ class XQueueInterface:
|
||||
return self._http_post(self.url + "/xqueue/submit/", payload, files=files)
|
||||
|
||||
course_key = self.block.scope_ids.usage_id.context_key
|
||||
header_info = json.loads(header)
|
||||
queue_key = header_info['lms_key']
|
||||
|
||||
if use_edx_submissions_for_xqueue(course_key):
|
||||
submission = self.submission.send_to_submission(header, body, files)
|
||||
return None, ""
|
||||
submission = self.submission.send_to_submission(header, body, queue_key, files)
|
||||
return None, ''
|
||||
|
||||
return self._http_post(self.url + "/xqueue/submit/", payload, files=files)
|
||||
|
||||
|
||||
@@ -81,7 +81,7 @@ class XQueueInterfaceSubmission:
|
||||
|
||||
return student_dict, student_answer, queue_name, grader_file_name, points_possible
|
||||
|
||||
def send_to_submission(self, header, body, files_to_upload=None):
|
||||
def send_to_submission(self, header, body, queue_key, files_to_upload=None):
|
||||
"""
|
||||
Submits the extracted student data to the edx-submissions system.
|
||||
"""
|
||||
@@ -95,6 +95,7 @@ class XQueueInterfaceSubmission:
|
||||
student_item,
|
||||
answer,
|
||||
queue_name=queue_name,
|
||||
queue_key=queue_key,
|
||||
grader_file_name=grader_file_name,
|
||||
points_possible=points_possible,
|
||||
files=files_to_upload,
|
||||
|
||||
@@ -0,0 +1,183 @@
|
||||
# 6. Event-based XBlock Rendering for External Grader Integration
|
||||
#################################################################
|
||||
|
||||
Status
|
||||
******
|
||||
|
||||
**Provisional** *2025-03-18*
|
||||
|
||||
Implemented by: https://github.com/openedx/edx-platform/pull/34888
|
||||
|
||||
Context
|
||||
*******
|
||||
|
||||
The Open edX platform currently renders XBlocks with scoring data through
|
||||
synchronous HTTP callback requests from XQueue. This approach introduces
|
||||
several challenges:
|
||||
|
||||
1. **Tight Coupling**: The XQueue service must know the specific callback URL
|
||||
for each XBlock, creating unnecessary coupling between services.
|
||||
|
||||
2. **HTTP Dependency**: Reliance on synchronous HTTP requests introduces
|
||||
potential points of failure, latency issues, and timeouts.
|
||||
|
||||
3. **Complex State Management**: Managing state across multiple services via
|
||||
HTTP callbacks makes tracking submission progress more difficult.
|
||||
|
||||
4. **Limited Scalability**: The callback model doesn't scale well in
|
||||
distributed environments, particularly with high loads.
|
||||
|
||||
5. **Consistency Issues**: HTTP failures can lead to discrepancies
|
||||
between the actual submission state and what's displayed to learners.
|
||||
|
||||
This ADR addresses the final component of the XQueue migration initiative,
|
||||
building upon previous decisions that established
|
||||
|
||||
Decision
|
||||
********
|
||||
|
||||
We will implement an event-driven approach to render XBlocks with scoring data,
|
||||
replacing the traditional HTTP callback mechanism. This involves:
|
||||
|
||||
1. **Event Handler Implementation**:
|
||||
|
||||
- Create a specialized event handler in the LMS to process the
|
||||
``EXTERNAL_GRADER_SCORE_SUBMITTED`` signal.
|
||||
- Implement a signal handler in ``handlers.py`` to react to score
|
||||
submission events.
|
||||
- Develop a dedicated XBlock loader in ``score_render.py`` that can render
|
||||
blocks without HTTP requests.
|
||||
|
||||
2. **Integration with Existing Event Structure**:
|
||||
|
||||
- Leverage the previously defined ``EXTERNAL_GRADER_SCORE_SUBMITTED``
|
||||
signal from edx-submissions.
|
||||
- Ensure propagation of the ``queue_key`` identifier across the submission
|
||||
pipeline.
|
||||
- Register appropriate URL handlers in the LMS for submission processing.
|
||||
|
||||
3. **Asynchronous Rendering Flow**:
|
||||
|
||||
- When a score is set via the edx-submissions service, emit the
|
||||
``EXTERNAL_GRADER_SCORE_SUBMITTED`` event.
|
||||
- The LMS event handler receives this event and initiates the XBlock
|
||||
rendering process.
|
||||
- The XBlock loader retrieves the necessary scoring data and updates
|
||||
the XBlock state.
|
||||
- The rendered XBlock is presented to the learner with updated scoring
|
||||
information.
|
||||
|
||||
Technical Components:
|
||||
|
||||
.. code-block:: python
|
||||
|
||||
# Signal handler registration
|
||||
@receiver(EXTERNAL_GRADER_SCORE_SUBMITTED)
|
||||
def handle_external_grader_score(sender, **kwargs):
|
||||
"""
|
||||
Handle the external grader score submitted event.
|
||||
Retrieves the scoring data and initiates XBlock rendering.
|
||||
"""
|
||||
score_data = kwargs.get('score_data')
|
||||
# Process score data and render XBlock
|
||||
render_xblock_with_score(score_data)
|
||||
|
||||
.. code-block:: python
|
||||
|
||||
def render_xblock_with_score(score_data):
|
||||
"""
|
||||
Render an XBlock with the provided scoring data.
|
||||
This replaces the traditional HTTP callback approach.
|
||||
"""
|
||||
# Retrieve the XBlock
|
||||
xblock = get_xblock_by_module_id(score_data.module_id)
|
||||
|
||||
# Update XBlock state with score information
|
||||
update_xblock_state(xblock, score_data)
|
||||
|
||||
# Trigger rendering process
|
||||
render_xblock(xblock)
|
||||
|
||||
Consequences
|
||||
************
|
||||
|
||||
Positive:
|
||||
---------
|
||||
|
||||
1. **Architectural Improvements**:
|
||||
|
||||
- Elimination of synchronous HTTP dependencies between services to
|
||||
render score.
|
||||
- More robust error handling.
|
||||
- Improved system observability through event tracking.
|
||||
|
||||
2. **Performance Benefits**:
|
||||
|
||||
- Reduced latency in score rendering and feedback presentation.
|
||||
- Better scalability in high-load environments.
|
||||
- More efficient resource utilization without blocking HTTP calls.
|
||||
|
||||
3. **User Experience**:
|
||||
|
||||
- More consistent experience for learners with faster score updates.
|
||||
- Reduced likelihood of rendering failures affecting feedback display.
|
||||
- Improved reliability in handling scoring events.
|
||||
|
||||
Negative:
|
||||
---------
|
||||
|
||||
1. **Implementation Complexity**:
|
||||
|
||||
- Requires additional signal handling infrastructure.
|
||||
- More complex testing scenarios to validate event-based flows.
|
||||
|
||||
2. **Operational Considerations**:
|
||||
|
||||
- Requires monitoring of event emission and consumption.
|
||||
- Debugging complexity increases with asynchronous flows.
|
||||
- Need for proper error recovery mechanisms if events are missed.
|
||||
|
||||
3. **Transition Challenges**:
|
||||
|
||||
- Temporary increased system complexity during migration period.
|
||||
- Careful coordination needed between edx-submissions and LMS changes.
|
||||
|
||||
Neutral:
|
||||
--------
|
||||
|
||||
1. **Documentation Needs**:
|
||||
|
||||
- Updated developer documentation for event-based architecture.
|
||||
- Event schema documentation for future integrations.
|
||||
|
||||
|
||||
References
|
||||
**********
|
||||
|
||||
Pull Requests:
|
||||
|
||||
* Initial Event Definition:
|
||||
https://github.com/openedx/edx-submissions/pull/283
|
||||
* ExternalGraderDetail Implementation:
|
||||
https://github.com/openedx/edx-submissions/pull/283
|
||||
* SubmissionFile Implementation:
|
||||
https://github.com/openedx/edx-submissions/pull/286
|
||||
* XQueueViewSet Implementation:
|
||||
https://github.com/openedx/edx-submissions/pull/287
|
||||
* Event Emission Implementation:
|
||||
https://github.com/openedx/edx-submissions/pull/292
|
||||
|
||||
Related Documentation:
|
||||
|
||||
* XQueue Migration Plan:
|
||||
https://github.com/openedx/edx-platform/pull/36258
|
||||
* Django Signals Documentation:
|
||||
https://docs.djangoproject.com/en/stable/topics/signals/
|
||||
* Open edX Events Framework: https://github.com/openedx/openedx-events
|
||||
|
||||
Architecture Guidelines:
|
||||
|
||||
* Open edX Architecture Guidelines:
|
||||
https://openedx.atlassian.net/wiki/spaces/AC/pages/124125264/Architecture+Guidelines
|
||||
* OEP-19: Developer Documentation:
|
||||
https://open-edx-proposals.readthedocs.io/en/latest/oep-0019-bp-developer-documentation.html
|
||||
Reference in New Issue
Block a user