-
Notifications
You must be signed in to change notification settings - Fork 4.2k
FC-73 Implement receiver EXTERNAL_GRADER_SCORE_SUBMITTED signal to render score to student (Feature: Improve robust score rendering) #36408
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,13 @@ | ||
| """ | ||
| 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 | ||
| 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 +23,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 +348,84 @@ 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(f"---------------------> course_id: {score.course_id}") | ||
| log.info(f"---------------------> user_id: {score.user_id}") | ||
| log.info(f"---------------------> module_id: {score.module_id}") | ||
| log.info(f"---------------------> submission_id: {score.submission_id}") | ||
| log.info(f"---------------------> queue_key: {score.queue_key}") | ||
| log.info(f"---------------------> queue_name: {score.queue_name}") | ||
| log.info(f"---------------------> score reply: {grader_msg}") | ||
|
Comment on lines
+380
to
+389
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is pretty verbose logging that I would expect for debug, but not "info" level. Try to limit info-level debugging to just one or two lines per request. |
||
|
|
||
| if isinstance(grader_msg, str): | ||
| try: | ||
| # Try to parse it as JSON if it's a string | ||
| grader_msg = json.loads(grader_msg) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the value we actually expect for |
||
| except json.JSONDecodeError: | ||
| # If it's not valid JSON, keep it as is | ||
| pass | ||
|
|
||
| data = { | ||
| 'xqueue_header': json.dumps({ | ||
| 'lms_key': str(score.submission_id), | ||
| 'queue_name': score.queue_name | ||
| }), | ||
| 'xqueue_body': json.dumps(grader_msg) if isinstance(grader_msg, dict) else grader_msg, | ||
| 'queuekey': str(score.queue_key) | ||
| } | ||
|
|
||
| course_key = CourseKey.from_string(score.course_id) | ||
| # with modulestore().bulk_operations(course_key): TODO: Remove this when PR will be convert to open PR | ||
| course = modulestore().get_course(course_key, depth=0) | ||
|
|
||
| # pylint: disable=broad-exception-caught | ||
| try: | ||
| # Use our new function instead of load_single_xblock | ||
| from xmodule.capa.score_render import load_xblock_for_external_grader | ||
| instance = load_xblock_for_external_grader(score.user_id, | ||
| score.course_id, | ||
| score.module_id, | ||
| course=course) | ||
|
|
||
| # Call the handler method (mirroring the original xqueue_callback) | ||
| instance.handle_ajax('score_update', data) | ||
|
|
||
| # Save any state changes | ||
| instance.save() | ||
|
Comment on lines
+415
to
+425
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does this handler code live here, instead of in |
||
|
|
||
| log.info(f"Successfully processed external grade for module {score.module_id}, user {score.user_id}") | ||
|
|
||
| except Exception as e: | ||
| log.exception(f"Error processing external grade: {e}") | ||
| raise | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5521,6 +5521,12 @@ def _should_send_learning_badge_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": Derived(_should_send_learning_badge_events) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this bit about |
||
| }, | ||
| }, | ||
| } | ||
|
|
||
| BEAMER_PRODUCT_ID = "" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -348,6 +348,14 @@ | |
| 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), | ||
|
|
||
|
|
@@ -1047,3 +1055,9 @@ | |
| urlpatterns += [ | ||
| path('api/notifications/', include('openedx.core.djangoapps.notifications.urls')), | ||
| ] | ||
|
|
||
| from submissions import urls as submissions_urls | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please group this import with the others. |
||
|
|
||
| urlpatterns += [ | ||
| path('xqueue/', include((submissions_urls, 'submissions'), namespace='submissions')), | ||
| ] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,3 @@ | ||
| -r kernel.in # Core dependencies required for the platform to run. | ||
| -r bundled.in # Additional packages usually bundled with the platform | ||
| edx-submissions @ git+https://github.com/aulasneo/edx-submissions.git@XQU-44-add-signal-to-call-xqueue-callback |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| """ | ||
| 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, course_id, usage_key_string, course=None): | ||
| """ | ||
| Load a single XBlock for external grading without user access checks. | ||
| """ | ||
|
|
||
| usage_key = UsageKey.from_string(usage_key_string) | ||
| course_key = CourseKey.from_string(course_id) | ||
| usage_key = usage_key.map_into_course(course_key) | ||
|
|
||
| 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"No se pudo encontrar el bloque {usage_key} en modulestore: {e}") | ||
| raise Http404(f"No se encontró el módulo {usage_key_string}") 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"It {usage_key_string}" | ||
| 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason we need to import it out of order here?