diff --git a/lms/djangoapps/grades/signals/handlers.py b/lms/djangoapps/grades/signals/handlers.py index 58c5fb90aace..01e270ee23f0 100644 --- a/lms/djangoapps/grades/signals/handlers.py +++ b/lms/djangoapps/grades/signals/handlers.py @@ -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 @@ 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 diff --git a/lms/envs/common.py b/lms/envs/common.py index 4c3c1f79c835..2abd0824489a 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -3597,6 +3597,12 @@ def _should_send_certificate_events(settings): "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 #### diff --git a/lms/urls.py b/lms/urls.py index 9668a394d6d9..ca6744305a54 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -12,6 +12,7 @@ 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 @@ name='xqueue_callback', ), + re_path( + r'^courses/{}/xqueue/(?P[^/]*)/(?P.*?)/(?P[^/]*)$'.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 += [ path('api/notifications/', include('openedx.core.djangoapps.notifications.urls')), ] + +urlpatterns += [ + path('xqueue/', include((submissions_urls, 'submissions'), namespace='submissions')), +] diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 8d59db2051b6..9526ba655aa6 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -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 diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index b4d1b0c1274a..6c5f815f4eb0 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -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 diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index e25a04053f57..be4eacfb123a 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.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 diff --git a/requirements/edx/github.in b/requirements/edx/github.in index cfaa2d0f3d7f..3c01759768c9 100644 --- a/requirements/edx/github.in +++ b/requirements/edx/github.in @@ -80,7 +80,6 @@ # ... add dependencies here - ############################################################################## # Critical fixes for packages that are not yet available in a PyPI release. ############################################################################## diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 240aad2d630e..72ef61b538e5 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -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 diff --git a/xmodule/capa/score_render.py b/xmodule/capa/score_render.py new file mode 100644 index 000000000000..9d1b9e58ffba --- /dev/null +++ b/xmodule/capa/score_render.py @@ -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 diff --git a/xmodule/capa/tests/test_score_render.py b/xmodule/capa/tests/test_score_render.py new file mode 100644 index 000000000000..5a309e669665 --- /dev/null +++ b/xmodule/capa/tests/test_score_render.py @@ -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']) diff --git a/xmodule/capa/tests/test_xqueue_interface.py b/xmodule/capa/tests/test_xqueue_interface.py index 91f7e838578e..879cc261c54e 100644 --- a/xmodule/capa/tests/test_xqueue_interface.py +++ b/xmodule/capa/tests/test_xqueue_interface.py @@ -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") diff --git a/xmodule/capa/tests/test_xqueue_submission.py b/xmodule/capa/tests/test_xqueue_submission.py index 806a103f6ac2..a315f52ffa4c 100644 --- a/xmodule/capa/tests/test_xqueue_submission.py +++ b/xmodule/capa/tests/test_xqueue_submission.py @@ -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"] diff --git a/xmodule/capa/xqueue_interface.py b/xmodule/capa/xqueue_interface.py index 85d3b0e5dd8e..91b4e6f80d94 100644 --- a/xmodule/capa/xqueue_interface.py +++ b/xmodule/capa/xqueue_interface.py @@ -183,10 +183,12 @@ def _send_to_queue(self, header, body, files_to_upload): # lint-amnesty, pylint 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) diff --git a/xmodule/capa/xqueue_submission.py b/xmodule/capa/xqueue_submission.py index b45e1b8ce87e..ec4772280fa0 100644 --- a/xmodule/capa/xqueue_submission.py +++ b/xmodule/capa/xqueue_submission.py @@ -81,7 +81,7 @@ def get_submission_params(self, header, payload): 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 @@ def send_to_submission(self, header, body, files_to_upload=None): 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, diff --git a/xmodule/docs/decisions/0006-xblock-rendering-for-external-grader-integration.rst b/xmodule/docs/decisions/0006-xblock-rendering-for-external-grader-integration.rst new file mode 100644 index 000000000000..0b536e7059ea --- /dev/null +++ b/xmodule/docs/decisions/0006-xblock-rendering-for-external-grader-integration.rst @@ -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