From 5f86d4de98db4aa6110819252330d58563d9fe0a Mon Sep 17 00:00:00 2001 From: gabrielC1409 Date: Wed, 12 Feb 2025 21:42:59 -0400 Subject: [PATCH 1/4] fix: create function waffle switch for submit data to created_submission --- xmodule/capa/tests/test_xqueue_interface.py | 58 ++++++++- xmodule/capa/tests/test_xqueue_submission.py | 76 +++++++++++ xmodule/capa/xqueue_interface.py | 9 +- xmodule/capa/xqueue_submission.py | 129 +++++++++++++++++++ 4 files changed, 270 insertions(+), 2 deletions(-) create mode 100644 xmodule/capa/tests/test_xqueue_submission.py create mode 100644 xmodule/capa/xqueue_submission.py diff --git a/xmodule/capa/tests/test_xqueue_interface.py b/xmodule/capa/tests/test_xqueue_interface.py index 819fd73c798..fee1429fcd3 100644 --- a/xmodule/capa/tests/test_xqueue_interface.py +++ b/xmodule/capa/tests/test_xqueue_interface.py @@ -1,15 +1,18 @@ """Test the XQueue service and interface.""" from unittest import TestCase -from unittest.mock import Mock +from unittest.mock import Mock, patch from django.conf import settings from django.test.utils import override_settings from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator from xblock.fields import ScopeIds +from waffle.testutils import override_switch +import json from openedx.core.djangolib.testing.utils import skip_unless_lms from xmodule.capa.xqueue_interface import XQueueInterface, XQueueService +import pytest @skip_unless_lms @@ -52,3 +55,56 @@ def test_waittime(self): with override_settings(XQUEUE_WAITTIME_BETWEEN_REQUESTS=15): assert self.service.waittime == 15 + + +@pytest.mark.django_db +@override_switch('xqueue_submission.enabled', active=True) +@patch('xmodule.capa.xqueue_submission.XQueueInterfaceSubmission.send_to_submission') +def test_send_to_queue_with_waffle_enabled(mock_send_to_submission): + url = "http://example.com/xqueue" + django_auth = {"username": "user", "password": "pass"} + requests_auth = None + xqueue_interface = XQueueInterface(url, django_auth, requests_auth) + + 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' + }) + 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, {}) + + +@pytest.mark.django_db +@override_switch('xqueue_submission.enabled', active=False) +@patch('xmodule.capa.xqueue_interface.XQueueInterface._http_post') +def test_send_to_queue_with_waffle_disabled(mock_http_post): + + url = "http://example.com/xqueue" + django_auth = {"username": "user", "password": "pass"} + requests_auth = None + xqueue_interface = XQueueInterface(url, django_auth, requests_auth) + + 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' + }) + files_to_upload = None + + mock_http_post.return_value = (0, "Submission sent successfully") + error, msg = xqueue_interface.send_to_queue(header, body, files_to_upload) + + mock_http_post.assert_called_once_with( + 'http://example.com/xqueue/xqueue/submit/', + {'xqueue_header': header, 'xqueue_body': body}, + files={} + ) \ No newline at end of file diff --git a/xmodule/capa/tests/test_xqueue_submission.py b/xmodule/capa/tests/test_xqueue_submission.py new file mode 100644 index 00000000000..6fc74521c21 --- /dev/null +++ b/xmodule/capa/tests/test_xqueue_submission.py @@ -0,0 +1,76 @@ +import json +import pytest +from unittest.mock import Mock, patch +from django.conf import settings +from xmodule.capa.xqueue_submission import XQueueInterfaceSubmission +from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator +from xblock.fields import ScopeIds + + +@pytest.fixture +def xqueue_service(): + location = BlockUsageLocator(CourseLocator("test_org", "test_course", "test_run"), "problem", "ExampleProblem") + block = Mock(scope_ids=ScopeIds('user1', 'mock_problem', location, location)) + return XQueueInterfaceSubmission() + + +def test_extract_item_data(): + header = json.dumps({ + 'lms_callback_url': 'http://example.com/courses/course-v1:org+course+run/xqueue/5/block-v1:org+course+run+type@problem+block@item_id/score_update', + }) + payload = json.dumps({ + 'student_info': json.dumps({'anonymous_student_id': 'student_id'}), + 'student_response': 'student_answer', + 'grader_payload': json.dumps({'grader': 'test.py'}) + }) + with patch('lms.djangoapps.courseware.models.StudentModule.objects.filter') as mock_filter: + mock_filter.return_value.first.return_value = Mock(grade=0.85) + + student_item, student_answer, queue_name,grader, score = XQueueInterfaceSubmission().extract_item_data(header, payload) + + assert student_item == { + 'item_id': 'block-v1:org+course+run+type@problem+block@item_id', + 'item_type': 'problem', + 'course_id': 'course-v1:org+course+run', + 'student_id': 'student_id' + } + assert student_answer == 'student_answer' + assert queue_name == 'default' + assert grader == 'test.py' + assert score == 0.85 + + +@patch('submissions.api.create_submission') +def test_send_to_submission(mock_create_submission, xqueue_service): + header = json.dumps({ + 'lms_callback_url': 'http://example.com/courses/course-v1:test_org+test_course+test_run/xqueue/5/block-v1:test_org+test_course+test_run+type@problem+block@item_id/score_update', + }) + body = json.dumps({ + 'student_info': json.dumps({'anonymous_student_id': 'student_id'}), + 'student_response': 'student_answer', + 'grader_payload': json.dumps({'grader': 'test.py'}) + }) + + with patch('lms.djangoapps.courseware.models.StudentModule.objects.filter') as mock_filter: + mock_filter.return_value.first.return_value = Mock(grade=0.85) + + mock_create_submission.return_value = {'submission': 'mock_submission'} + + # Llamada a send_to_submission + result = xqueue_service.send_to_submission(header, body) + + # Afirmaciones + assert 'submission' in result + assert result['submission'] == 'mock_submission' + mock_create_submission.assert_called_once_with( + { + 'item_id': 'block-v1:test_org+test_course+test_run+type@problem+block@item_id', + 'item_type': 'problem', + 'course_id': 'course-v1:test_org+test_course+test_run', + 'student_id': 'student_id' + }, + 'student_answer', + queue_name='default', + grader='test.py', + score=0.85 + ) \ No newline at end of file diff --git a/xmodule/capa/xqueue_interface.py b/xmodule/capa/xqueue_interface.py index aee7232a413..e6a1273efd3 100644 --- a/xmodule/capa/xqueue_interface.py +++ b/xmodule/capa/xqueue_interface.py @@ -11,12 +11,14 @@ from django.conf import settings from django.urls import reverse from requests.auth import HTTPBasicAuth +from waffle import switch_is_active +from xmodule.capa.xqueue_submission import XQueueInterfaceSubmission if TYPE_CHECKING: from xmodule.capa_block import ProblemBlock log = logging.getLogger(__name__) -dateformat = '%Y%m%d%H%M%S' +dateformat = '%Y-%m-%dT%H:%M:%S' XQUEUE_METRIC_NAME = 'edxapp.xqueue' @@ -134,6 +136,11 @@ def _send_to_queue(self, header, body, files_to_upload): # lint-amnesty, pylint if files_to_upload is not None: for f in files_to_upload: files.update({f.name: f}) + + if switch_is_active('xqueue_submission.enabled'): + # Use the new edx-submissions workflow + submission = XQueueInterfaceSubmission().send_to_submission(header, body, files) + log.error(submission) 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 new file mode 100644 index 00000000000..94e63468822 --- /dev/null +++ b/xmodule/capa/xqueue_submission.py @@ -0,0 +1,129 @@ + +import hashlib +import json +import logging + +import requests +from django.conf import settings +from django.urls import reverse +from requests.auth import HTTPBasicAuth +import re +from typing import Dict, Optional, TYPE_CHECKING +from opaque_keys.edx.keys import CourseKey, UsageKey +from django.core.exceptions import ObjectDoesNotExist +if TYPE_CHECKING: + from xmodule.capa_block import ProblemBlock + +log = logging.getLogger(__name__) +dateformat = '%Y-%m-%dT%H:%M:%S' + +XQUEUE_METRIC_NAME = 'edxapp.xqueue' + +# Wait time for response from Xqueue. +XQUEUE_TIMEOUT = 35 # seconds +CONNECT_TIMEOUT = 3.05 # seconds +READ_TIMEOUT = 10 # seconds + +class XQueueInterfaceSubmission: + """ + Interface to the external grading system + """ + + def extract_item_data(self, header, payload): + from lms.djangoapps.courseware.models import StudentModule + from opaque_keys.edx.locator import BlockUsageLocator + if isinstance(header, str): + try: + header = json.loads(header) + except json.JSONDecodeError as e: + raise ValueError(f"Error to header: {e}") + + if isinstance(payload, str): + try: + payload = json.loads(payload) + except json.JSONDecodeError as e: + raise ValueError(f"Error to payload: {e}") + + callback_url = header.get('lms_callback_url') + queue_name = header.get('queue_name', 'default') + + if not callback_url: + raise ValueError("El header is not content 'lms_callback_url'.") + + match_item_id = re.search(r'(block-v1:[^/]+)', callback_url) + match_course_id = re.search(r'(course-v1:[^\/]+)', callback_url) + match_item_type = re.search(r'type@([^+]+)', callback_url) + + if not (match_item_id and match_item_type and match_course_id): + raise ValueError(f"The callback_url is not valid: {callback_url}") + + item_id = match_item_id.group(1) + item_type = match_item_type.group(1) + course_id = match_course_id.group(1) + + try: + student_info = json.loads(payload["student_info"]) + except json.JSONDecodeError as e: + raise ValueError(f"Error to student_info: {e}") + + usage_key = BlockUsageLocator.from_string(item_id) + course_key = CourseKey.from_string(course_id) + + try: + grader_payload = payload["grader_payload"] + if isinstance(grader_payload, str): + grader_payload = json.loads(grader_payload) + grader = grader_payload.get("grader", '') + except json.JSONDecodeError as e: + raise ValueError(f"Error grader_payload: {e}") + except KeyError as e: + raise ValueError(f"Error payload: {e}") + + + + student_id = student_info.get("anonymous_student_id") + if not student_id: + raise ValueError("The field 'anonymous_student_id' is not student_info.") + + student_dict = { + 'item_id': item_id, + 'item_type': item_type, + 'course_id': course_id, + 'student_id': student_id + } + + student_answer = payload.get("student_response") + if student_answer is None: + raise ValueError("The field 'student_response' do not exist.") + + student_module = StudentModule.objects.filter( + module_state_key=usage_key, + course_id=course_key + ).first() + + log.error(f"student_module: {student_module}") + + if student_module and student_module.grade is not None: + score = student_module.grade + else: + score = None + + log.error(f"Score: {student_id}: {score}") + + return student_dict, student_answer, queue_name, grader, score + + def send_to_submission(self, header, body, files_to_upload=None): + from submissions.api import create_submission + try: + student_item, answer, queue_name, grader, score = self.extract_item_data(header, body) + + log.error(f"student_item: {student_item}") + log.error(f"header: {header}") + log.error(f"body: {body}") + log.error(f"grader: {grader}") + + submission = create_submission(student_item, answer, queue_name=queue_name, grader=grader, score=score) + + return submission + except Exception as e: + return {"error": str(e)} \ No newline at end of file From b47cdc2c03d2cb459e8379d781d27048c696da94 Mon Sep 17 00:00:00 2001 From: gabrielC1409 Date: Wed, 12 Feb 2025 22:26:23 -0400 Subject: [PATCH 2/4] fix: docs ADR complete --- docs/decisions/0022-create-waffle-switch.rst | 58 +++++++++++++ docs/lms-openapi.yaml | 91 ++++++++++++++++++++ 2 files changed, 149 insertions(+) create mode 100644 docs/decisions/0022-create-waffle-switch.rst diff --git a/docs/decisions/0022-create-waffle-switch.rst b/docs/decisions/0022-create-waffle-switch.rst new file mode 100644 index 00000000000..055c40bf155 --- /dev/null +++ b/docs/decisions/0022-create-waffle-switch.rst @@ -0,0 +1,58 @@ +############################################################### +Integration of Waffle Switch for XQueue Submission +############################################################### + +Status +****** + +**Pending** *2025-02-11* + +Implementation in progress. + +Context +******* + +In the `edx-platform` repository, there was a need to implement a mechanism that allows conditional execution of a new functionality: sending a student's response within an exercise to the `created_submission` function. This mechanism should be easily toggleable without requiring code changes or deployments. + +Decision +******** + +A `waffle switch` named `xqueue_submission.enabled` was introduced within the Django admin interface. When this switch is activated, it enables the functionality to send data to the `send_to_submission` function, which parses and forwards the data to the `created_submission` function. + +The `created_submission` function resides in the `edx-submissions` repository and is responsible for storing the data in the submissions database. + +Implementation Details +---------------------- + +This functionality was implemented within the `edx-platform` repository by modifying the following files: + +1. **`xmodule/capa/xqueue_interfaces.py`** + - The `waffle switch` **`xqueue_submission.enabled`** was added here. + - This switch is checked before invoking `send_to_submission`, ensuring that the submission logic is only executed when enabled. + +2. **`xmodule/capa/xqueue_submission.py`** + - This file contains the newly implemented logic that parses the student’s response. + - It processes and formats the data before calling `created_submission`, ensuring that it is correctly stored in the **edx-submissions** repository. + +Consequences +************ + +Positive: +--------- + +- **Flexibility:** The use of a `waffle switch` allows administrators to enable or disable the new submission functionality without modifying the codebase or redeploying the application. +- **Control:** Administrators can manage the feature directly from the Django admin interface, providing a straightforward method to toggle the feature as needed. +- **Modular Design:** The logic was added in a way that allows future modifications without affecting existing submission workflows. + +Negative: +--------- + +- **Potential Misconfiguration:** If the `waffle switch` is not properly managed, there could be inconsistencies in submission processing. +- **Admin Overhead:** Requires monitoring to ensure the toggle is enabled when needed. + +References +********** + +- Commit implementing the change: [f50afcc301bdc3eeb42a6dc2c051ffb2d799f868#diff-9b4290d2b574f54e4eca7831368727f7ddbac8292aa75ba4b28651d4bf2bbe6b](https://github.com/aulasneo/edx-platform/commit/f50afcc301bdc3eeb42a6dc2c051ffb2d799f868#diff-9b4290d2b574f54e4eca7831368727f7ddbac8292aa75ba4b28651d4bf2bbe6b) +- Open edX Feature Toggles Documentation: [Feature Toggles — edx-platform documentation](https://docs.openedx.org/projects/edx-platform/en/latest/references/featuretoggles.html) +- `edx-submissions` Repository: [openedx/edx-submissions](https://github.com/openedx/edx-submissions) \ No newline at end of file diff --git a/docs/lms-openapi.yaml b/docs/lms-openapi.yaml index ad3a4fc40db..32f67203b45 100644 --- a/docs/lms-openapi.yaml +++ b/docs/lms-openapi.yaml @@ -7134,6 +7134,18 @@ paths: tags: - notifications parameters: [] + /notifications/configurations/: + get: + operationId: notifications_configurations_list + description: API view for getting the aggregate notification preferences for + the current user. + parameters: [] + responses: + '200': + description: '' + tags: + - notifications + parameters: [] /notifications/configurations/{course_key_string}: get: operationId: notifications_configurations_read @@ -7309,6 +7321,17 @@ paths: in: path required: true type: string + /notifications/preferences/update-all/: + post: + operationId: notifications_preferences_update-all_create + description: Update all notification preferences for the current user. + parameters: [] + responses: + '201': + description: '' + tags: + - notifications + parameters: [] /notifications/preferences/update/{username}/{patch}/: get: operationId: notifications_preferences_update_read @@ -9817,6 +9840,26 @@ paths: tags: - val parameters: [] + /val/v0/videos/video-transcripts/: + post: + operationId: val_v0_videos_video-transcripts_create + description: Creates a video transcript instance with the given information. + parameters: [] + responses: + '201': + description: '' + tags: + - val + delete: + operationId: val_v0_videos_video-transcripts_delete + description: Delete a video transcript instance with the given information. + parameters: [] + responses: + '204': + description: '' + tags: + - val + parameters: [] /val/v0/videos/video-transcripts/create/: post: operationId: val_v0_videos_video-transcripts_create_create @@ -9827,6 +9870,15 @@ paths: description: '' tags: - val + delete: + operationId: val_v0_videos_video-transcripts_create_delete + description: Delete a video transcript instance with the given information. + parameters: [] + responses: + '204': + description: '' + tags: + - val parameters: [] /val/v0/videos/{edx_video_id}: get: @@ -9955,6 +10007,21 @@ paths: in: path required: true type: string + /xblock/v2/xblocks/{usage_key}/olx/: + get: + operationId: xblock_v2_xblocks_olx_list + description: Get the OLX (XML serialization) of the specified XBlock + parameters: [] + responses: + '200': + description: '' + tags: + - xblock + parameters: + - name: usage_key + in: path + required: true + type: string /xblock/v2/xblocks/{usage_key}/view/{view_name}/: get: operationId: xblock_v2_xblocks_view_read @@ -10054,6 +10121,25 @@ paths: in: path required: true type: string + /xblock/v2/xblocks/{usage_key}@{version}/olx/: + get: + operationId: xblock_v2_xblocks_olx_list + description: Get the OLX (XML serialization) of the specified XBlock + parameters: [] + responses: + '200': + description: '' + tags: + - xblock + parameters: + - name: usage_key + in: path + required: true + type: string + - name: version + in: path + required: true + type: string /xblock/v2/xblocks/{usage_key}@{version}/view/{view_name}/: get: operationId: xblock_v2_xblocks_view_read @@ -12072,6 +12158,11 @@ definitions: format: uri maxLength: 200 x-nullable: true + course_id: + title: Course id + type: string + maxLength: 255 + x-nullable: true last_read: title: Last read type: string From 2fd7b96c47e1717337bfad023f196fc42677ae8d Mon Sep 17 00:00:00 2001 From: gabrielC1409 Date: Fri, 14 Feb 2025 14:13:13 -0400 Subject: [PATCH 3/4] test: run test make and pytest --- cms/static/TESTS.xml | 781 +++++++++++++++++++ scripts/xsslint_thresholds.json | 5 +- xmodule/capa/capa_problem.py | 2 + xmodule/capa/registry.py | 1 + xmodule/capa/tests/test_answer_pool.py | 1 + xmodule/capa/tests/test_capa_problem.py | 22 +- xmodule/capa/tests/test_customrender.py | 1 + xmodule/capa/tests/test_util.py | 2 +- xmodule/capa/tests/test_xqueue_interface.py | 9 +- xmodule/capa/tests/test_xqueue_submission.py | 16 +- xmodule/capa/xqueue_interface.py | 2 +- xmodule/capa/xqueue_submission.py | 25 +- 12 files changed, 827 insertions(+), 40 deletions(-) create mode 100644 cms/static/TESTS.xml diff --git a/cms/static/TESTS.xml b/cms/static/TESTS.xml new file mode 100644 index 00000000000..94bcb2bded1 --- /dev/null +++ b/cms/static/TESTS.xml @@ -0,0 +1,781 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/scripts/xsslint_thresholds.json b/scripts/xsslint_thresholds.json index 26c267c074c..f1c2293577d 100644 --- a/scripts/xsslint_thresholds.json +++ b/scripts/xsslint_thresholds.json @@ -9,6 +9,7 @@ "django-trans-escape-variable-mismatch": 0, "django-trans-invalid-escape-filter": 0, "django-trans-missing-escape": 0, + "django-trans-escape-filter-parse-error": 0, "javascript-concat-html": 2, "javascript-escape": 1, "javascript-jquery-append": 2, @@ -36,5 +37,5 @@ "python-wrap-html": 0, "underscore-not-escaped": 2 }, - "total": 64 -} + "total": 65 +} \ No newline at end of file diff --git a/xmodule/capa/capa_problem.py b/xmodule/capa/capa_problem.py index 6d78e156f29..3bf5b78cd4a 100644 --- a/xmodule/capa/capa_problem.py +++ b/xmodule/capa/capa_problem.py @@ -96,6 +96,7 @@ class LoncapaSystem(object): See :class:`DescriptorSystem` for documentation of other attributes. """ + def __init__( self, ajax_url, @@ -130,6 +131,7 @@ class LoncapaProblem(object): """ Main class for capa Problems. """ + def __init__(self, problem_text, id, capa_system, capa_block, # pylint: disable=redefined-builtin state=None, seed=None, minimal_init=False, extract_tree=True): """ diff --git a/xmodule/capa/registry.py b/xmodule/capa/registry.py index 1f0674771fd..fc8b853ee61 100644 --- a/xmodule/capa/registry.py +++ b/xmodule/capa/registry.py @@ -7,6 +7,7 @@ class TagRegistry(object): (A dictionary with some extra error checking.) """ + def __init__(self): self._mapping = {} diff --git a/xmodule/capa/tests/test_answer_pool.py b/xmodule/capa/tests/test_answer_pool.py index f4f65d3ee9d..d11df3a203f 100644 --- a/xmodule/capa/tests/test_answer_pool.py +++ b/xmodule/capa/tests/test_answer_pool.py @@ -13,6 +13,7 @@ class CapaAnswerPoolTest(unittest.TestCase): """Capa Answer Pool Test""" + def setUp(self): super(CapaAnswerPoolTest, self).setUp() # lint-amnesty, pylint: disable=super-with-arguments self.system = test_capa_system() diff --git a/xmodule/capa/tests/test_capa_problem.py b/xmodule/capa/tests/test_capa_problem.py index 74cf4d096fd..4e43efb5053 100644 --- a/xmodule/capa/tests/test_capa_problem.py +++ b/xmodule/capa/tests/test_capa_problem.py @@ -54,7 +54,7 @@ def test_label_and_description_inside_responsetype(self, question): """.format(question=question) problem = new_loncapa_problem(xml) assert problem.problem_data ==\ - {'1_2_1': {'label': question, 'descriptions': {'description_1_1_1': 'Only the paranoid survive.'}}} + {'1_2_1': {'label': question, 'descriptions': {'description_1_1_1': 'Only the paranoid survive.'}}} assert len(problem.tree.xpath('//label')) == 0 @ddt.unpack @@ -123,7 +123,7 @@ def test_neither_label_tag_nor_attribute(self, question1, question2): """.format(question1, question2) problem = new_loncapa_problem(xml) assert problem.problem_data ==\ - {'1_2_1': {'label': question1, 'descriptions': {}}, '1_3_1': {'label': question2, 'descriptions': {}}} + {'1_2_1': {'label': question1, 'descriptions': {}}, '1_3_1': {'label': question2, 'descriptions': {}}} for question in (question1, question2): assert len(problem.tree.xpath('//label[text()="{}"]'.format(question))) == 0 @@ -146,8 +146,8 @@ def test_multiple_descriptions(self): """.format(desc1, desc2) problem = new_loncapa_problem(xml) assert problem.problem_data ==\ - {'1_2_1': {'label': '___ requires sacrifices.', - 'descriptions': {'description_1_1_1': desc1, 'description_1_1_2': desc2}}} + {'1_2_1': {'label': '___ requires sacrifices.', + 'descriptions': {'description_1_1_1': desc1, 'description_1_1_2': desc2}}} def test_additional_answer_is_skipped_from_resulting_html(self): """Tests that additional_answer element is not present in transformed HTML""" @@ -236,10 +236,10 @@ def test_multiple_questions_problem(self): """ problem = new_loncapa_problem(xml) assert problem.problem_data ==\ - {'1_2_1': {'label': 'Select the correct synonym of paranoid?', - 'descriptions': {'description_1_1_1': 'Only the paranoid survive.'}}, - '1_3_1': {'label': 'What Apple device competed with the portable CD player?', - 'descriptions': {'description_1_2_1': 'Device looks like an egg plant.'}}} + {'1_2_1': {'label': 'Select the correct synonym of paranoid?', + 'descriptions': {'description_1_1_1': 'Only the paranoid survive.'}}, + '1_3_1': {'label': 'What Apple device competed with the portable CD player?', + 'descriptions': {'description_1_2_1': 'Device looks like an egg plant.'}}} assert len(problem.tree.xpath('//label')) == 0 def test_question_title_not_removed_got_children(self): @@ -291,8 +291,8 @@ def test_multiple_inputtypes(self, group_label): problem = new_loncapa_problem(xml) assert problem.problem_data ==\ - {'1_2_1': {'group_label': group_label, 'label': input1_label, 'descriptions': {}}, - '1_2_2': {'group_label': group_label, 'label': input2_label, 'descriptions': {}}} + {'1_2_1': {'group_label': group_label, 'label': input1_label, 'descriptions': {}}, + '1_2_2': {'group_label': group_label, 'label': input2_label, 'descriptions': {}}} def test_single_inputtypes(self): """ @@ -355,7 +355,7 @@ def assert_question_tag(self, question1, question2, tag, label_attr=False): ) problem = new_loncapa_problem(xml) assert problem.problem_data ==\ - {'1_2_1': {'label': question1, 'descriptions': {}}, '1_3_1': {'label': question2, 'descriptions': {}}} + {'1_2_1': {'label': question1, 'descriptions': {}}, '1_3_1': {'label': question2, 'descriptions': {}}} assert len(problem.tree.xpath('//{}'.format(tag))) == 0 @ddt.unpack diff --git a/xmodule/capa/tests/test_customrender.py b/xmodule/capa/tests/test_customrender.py index 93309608449..95dc7a68a0f 100644 --- a/xmodule/capa/tests/test_customrender.py +++ b/xmodule/capa/tests/test_customrender.py @@ -28,6 +28,7 @@ class HelperTest(unittest.TestCase): ''' Make sure that our helper function works! ''' + def check(self, d): xml = etree.XML(test_capa_system().render_template('blah', d)) assert d == extract_context(xml) diff --git a/xmodule/capa/tests/test_util.py b/xmodule/capa/tests/test_util.py index 51ffc623a98..dc356d39db4 100644 --- a/xmodule/capa/tests/test_util.py +++ b/xmodule/capa/tests/test_util.py @@ -145,7 +145,7 @@ def test_remove_markup(self): Test for markup removal with nh3. """ assert remove_markup('The Truth is Out There & you need to find it') ==\ - 'The Truth is Out There & you need to find it' + 'The Truth is Out There & you need to find it' @ddt.data( 'When the root level failš the whole hierarchy won’t work anymore.', diff --git a/xmodule/capa/tests/test_xqueue_interface.py b/xmodule/capa/tests/test_xqueue_interface.py index fee1429fcd3..ef73739aab2 100644 --- a/xmodule/capa/tests/test_xqueue_interface.py +++ b/xmodule/capa/tests/test_xqueue_interface.py @@ -18,6 +18,7 @@ @skip_unless_lms class XQueueServiceTest(TestCase): """Test the XQueue service methods.""" + def setUp(self): super().setUp() location = BlockUsageLocator(CourseLocator("test_org", "test_course", "test_run"), "problem", "ExampleProblem") @@ -77,7 +78,7 @@ def test_send_to_queue_with_waffle_enabled(mock_send_to_submission): 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, {}) @@ -85,7 +86,7 @@ def test_send_to_queue_with_waffle_enabled(mock_send_to_submission): @override_switch('xqueue_submission.enabled', active=False) @patch('xmodule.capa.xqueue_interface.XQueueInterface._http_post') def test_send_to_queue_with_waffle_disabled(mock_http_post): - + url = "http://example.com/xqueue" django_auth = {"username": "user", "password": "pass"} requests_auth = None @@ -102,9 +103,9 @@ def test_send_to_queue_with_waffle_disabled(mock_http_post): mock_http_post.return_value = (0, "Submission sent successfully") error, msg = xqueue_interface.send_to_queue(header, body, files_to_upload) - + mock_http_post.assert_called_once_with( 'http://example.com/xqueue/xqueue/submit/', {'xqueue_header': header, 'xqueue_body': body}, files={} - ) \ No newline at end of file + ) diff --git a/xmodule/capa/tests/test_xqueue_submission.py b/xmodule/capa/tests/test_xqueue_submission.py index 6fc74521c21..8eb7e596003 100644 --- a/xmodule/capa/tests/test_xqueue_submission.py +++ b/xmodule/capa/tests/test_xqueue_submission.py @@ -25,9 +25,9 @@ def test_extract_item_data(): }) with patch('lms.djangoapps.courseware.models.StudentModule.objects.filter') as mock_filter: mock_filter.return_value.first.return_value = Mock(grade=0.85) - - student_item, student_answer, queue_name,grader, score = XQueueInterfaceSubmission().extract_item_data(header, payload) - + + student_item, student_answer, queue_name, grader, score = XQueueInterfaceSubmission().extract_item_data(header, payload) + assert student_item == { 'item_id': 'block-v1:org+course+run+type@problem+block@item_id', 'item_type': 'problem', @@ -50,15 +50,15 @@ def test_send_to_submission(mock_create_submission, xqueue_service): 'student_response': 'student_answer', 'grader_payload': json.dumps({'grader': 'test.py'}) }) - + with patch('lms.djangoapps.courseware.models.StudentModule.objects.filter') as mock_filter: mock_filter.return_value.first.return_value = Mock(grade=0.85) - + mock_create_submission.return_value = {'submission': 'mock_submission'} - + # Llamada a send_to_submission result = xqueue_service.send_to_submission(header, body) - + # Afirmaciones assert 'submission' in result assert result['submission'] == 'mock_submission' @@ -73,4 +73,4 @@ def test_send_to_submission(mock_create_submission, xqueue_service): queue_name='default', grader='test.py', score=0.85 - ) \ No newline at end of file + ) diff --git a/xmodule/capa/xqueue_interface.py b/xmodule/capa/xqueue_interface.py index e6a1273efd3..f99b91c4649 100644 --- a/xmodule/capa/xqueue_interface.py +++ b/xmodule/capa/xqueue_interface.py @@ -136,7 +136,7 @@ def _send_to_queue(self, header, body, files_to_upload): # lint-amnesty, pylint if files_to_upload is not None: for f in files_to_upload: files.update({f.name: f}) - + if switch_is_active('xqueue_submission.enabled'): # Use the new edx-submissions workflow submission = XQueueInterfaceSubmission().send_to_submission(header, body, files) diff --git a/xmodule/capa/xqueue_submission.py b/xmodule/capa/xqueue_submission.py index 94e63468822..84bae7c3779 100644 --- a/xmodule/capa/xqueue_submission.py +++ b/xmodule/capa/xqueue_submission.py @@ -24,11 +24,12 @@ CONNECT_TIMEOUT = 3.05 # seconds READ_TIMEOUT = 10 # seconds + class XQueueInterfaceSubmission: """ Interface to the external grading system """ - + def extract_item_data(self, header, payload): from lms.djangoapps.courseware.models import StudentModule from opaque_keys.edx.locator import BlockUsageLocator @@ -65,10 +66,10 @@ def extract_item_data(self, header, payload): student_info = json.loads(payload["student_info"]) except json.JSONDecodeError as e: raise ValueError(f"Error to student_info: {e}") - + usage_key = BlockUsageLocator.from_string(item_id) course_key = CourseKey.from_string(course_id) - + try: grader_payload = payload["grader_payload"] if isinstance(grader_payload, str): @@ -78,8 +79,6 @@ def extract_item_data(self, header, payload): raise ValueError(f"Error grader_payload: {e}") except KeyError as e: raise ValueError(f"Error payload: {e}") - - student_id = student_info.get("anonymous_student_id") if not student_id: @@ -95,14 +94,14 @@ def extract_item_data(self, header, payload): student_answer = payload.get("student_response") if student_answer is None: raise ValueError("The field 'student_response' do not exist.") - + student_module = StudentModule.objects.filter( - module_state_key=usage_key, - course_id=course_key + module_state_key=usage_key, + course_id=course_key ).first() log.error(f"student_module: {student_module}") - + if student_module and student_module.grade is not None: score = student_module.grade else: @@ -116,14 +115,14 @@ def send_to_submission(self, header, body, files_to_upload=None): from submissions.api import create_submission try: student_item, answer, queue_name, grader, score = self.extract_item_data(header, body) - + log.error(f"student_item: {student_item}") log.error(f"header: {header}") log.error(f"body: {body}") log.error(f"grader: {grader}") - + submission = create_submission(student_item, answer, queue_name=queue_name, grader=grader, score=score) - + return submission except Exception as e: - return {"error": str(e)} \ No newline at end of file + return {"error": str(e)} From 422d08e8c8ead720622aa7ab32a108241a0159a5 Mon Sep 17 00:00:00 2001 From: gabrielC1409 Date: Tue, 18 Feb 2025 14:50:48 -0400 Subject: [PATCH 4/4] fix: update waffle switch in edx-platform --- xmodule/capa/tests/test_xqueue_interface.py | 1 + xmodule/capa/tests/test_xqueue_submission.py | 2 ++ xmodule/capa/xqueue_interface.py | 11 +++++++++-- xmodule/capa/xqueue_submission.py | 16 +++++----------- 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/xmodule/capa/tests/test_xqueue_interface.py b/xmodule/capa/tests/test_xqueue_interface.py index ef73739aab2..46c18869ba9 100644 --- a/xmodule/capa/tests/test_xqueue_interface.py +++ b/xmodule/capa/tests/test_xqueue_interface.py @@ -15,6 +15,7 @@ import pytest +@pytest.mark.django_db @skip_unless_lms class XQueueServiceTest(TestCase): """Test the XQueue service methods.""" diff --git a/xmodule/capa/tests/test_xqueue_submission.py b/xmodule/capa/tests/test_xqueue_submission.py index 8eb7e596003..5c6cf8ba0f3 100644 --- a/xmodule/capa/tests/test_xqueue_submission.py +++ b/xmodule/capa/tests/test_xqueue_submission.py @@ -4,6 +4,7 @@ from django.conf import settings from xmodule.capa.xqueue_submission import XQueueInterfaceSubmission from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator +from opaque_keys.edx.keys import UsageKey, CourseKey from xblock.fields import ScopeIds @@ -40,6 +41,7 @@ def test_extract_item_data(): assert score == 0.85 +@pytest.mark.django_db @patch('submissions.api.create_submission') def test_send_to_submission(mock_create_submission, xqueue_service): header = json.dumps({ diff --git a/xmodule/capa/xqueue_interface.py b/xmodule/capa/xqueue_interface.py index f99b91c4649..0c6b495bfbb 100644 --- a/xmodule/capa/xqueue_interface.py +++ b/xmodule/capa/xqueue_interface.py @@ -141,8 +141,10 @@ def _send_to_queue(self, header, body, files_to_upload): # lint-amnesty, pylint # Use the new edx-submissions workflow submission = XQueueInterfaceSubmission().send_to_submission(header, body, files) log.error(submission) + return None, '' - return self._http_post(self.url + '/xqueue/submit/', payload, files=files) + else: + return self._http_post(self.url + '/xqueue/submit/', payload, files=files) def _http_post(self, url, data, files=None): # lint-amnesty, pylint: disable=missing-function-docstring try: @@ -191,8 +193,13 @@ def construct_callback(self, dispatch: str = 'score_update') -> str: """ Return a fully qualified callback URL for external queueing system. """ + if switch_is_active('callback_submission.enabled'): + dispatch_callback = "callback_submission" + else: + dispatch_callback = 'xqueue_callback' + relative_xqueue_callback_url = reverse( - 'xqueue_callback', + dispatch_callback, kwargs=dict( course_id=str(self._block.scope_ids.usage_id.context_key), userid=str(self._block.scope_ids.user_id), diff --git a/xmodule/capa/xqueue_submission.py b/xmodule/capa/xqueue_submission.py index 84bae7c3779..6282883ac70 100644 --- a/xmodule/capa/xqueue_submission.py +++ b/xmodule/capa/xqueue_submission.py @@ -51,16 +51,9 @@ def extract_item_data(self, header, payload): if not callback_url: raise ValueError("El header is not content 'lms_callback_url'.") - match_item_id = re.search(r'(block-v1:[^/]+)', callback_url) - match_course_id = re.search(r'(course-v1:[^\/]+)', callback_url) - match_item_type = re.search(r'type@([^+]+)', callback_url) - - if not (match_item_id and match_item_type and match_course_id): - raise ValueError(f"The callback_url is not valid: {callback_url}") - - item_id = match_item_id.group(1) - item_type = match_item_type.group(1) - course_id = match_course_id.group(1) + item_id = re.search(r'block@([^\/]+)', callback_url).group(1) + item_type = re.search(r'type@([^+]+)', callback_url).group(1) + course_id = re.search(r'(course-v1:[^\/]+)', callback_url).group(1) try: student_info = json.loads(payload["student_info"]) @@ -70,6 +63,7 @@ def extract_item_data(self, header, payload): usage_key = BlockUsageLocator.from_string(item_id) course_key = CourseKey.from_string(course_id) + full_block_id = f"block-v1:{course_id.replace('course-v1:', '')}+type@{item_type}+block@{item_id}" try: grader_payload = payload["grader_payload"] if isinstance(grader_payload, str): @@ -85,7 +79,7 @@ def extract_item_data(self, header, payload): raise ValueError("The field 'anonymous_student_id' is not student_info.") student_dict = { - 'item_id': item_id, + 'item_id': full_block_id, 'item_type': item_type, 'course_id': course_id, 'student_id': student_id