Skip to content
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

FC-73 pr for edx platform repository (Creating waffle flag for integration towards created_submission in the edx-submission repo) #36258

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft
781 changes: 781 additions & 0 deletions cms/static/TESTS.xml

Large diffs are not rendered by default.

58 changes: 58 additions & 0 deletions docs/decisions/0022-create-waffle-switch.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
###############################################################
Integration of Waffle Switch for XQueue Submission
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this ADR should be in the https://github.com/openedx/edx-platform/tree/58834c6bdc20fae182eca4aa4ce7402cc14dfd47/xmodule/docs/decisions directory, and the file name should include "xqueue" in it (we have a ton of waffle switches, so the title "create-waffle-switch" is really not specific enough)

###############################################################

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)
5 changes: 3 additions & 2 deletions scripts/xsslint_thresholds.json
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -36,5 +37,5 @@
"python-wrap-html": 0,
"underscore-not-escaped": 2
},
"total": 64
}
"total": 65
}
2 changes: 2 additions & 0 deletions xmodule/capa/capa_problem.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ class LoncapaSystem(object):
See :class:`DescriptorSystem` for documentation of other attributes.

"""

def __init__(
self,
ajax_url,
Expand Down Expand Up @@ -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):
"""
Expand Down
1 change: 1 addition & 0 deletions xmodule/capa/registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ class TagRegistry(object):

(A dictionary with some extra error checking.)
"""

def __init__(self):
self._mapping = {}

Expand Down
1 change: 1 addition & 0 deletions xmodule/capa/tests/test_answer_pool.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
22 changes: 11 additions & 11 deletions xmodule/capa/tests/test_capa_problem.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -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"""
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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):
"""
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions xmodule/capa/tests/test_customrender.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion xmodule/capa/tests/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ def test_remove_markup(self):
Test for markup removal with nh3.
"""
assert remove_markup('The <mark>Truth</mark> is <em>Out There</em> & you need to <strong>find</strong> it') ==\
'The Truth is Out There &amp; you need to find it'
'The Truth is Out There &amp; you need to find it'

@ddt.data(
'When the root level failš the whole hierarchy won’t work anymore.',
Expand Down
60 changes: 59 additions & 1 deletion xmodule/capa/tests/test_xqueue_interface.py
Original file line number Diff line number Diff line change
@@ -1,20 +1,25 @@
"""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


@pytest.mark.django_db
@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")
Expand Down Expand Up @@ -52,3 +57,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={}
)
78 changes: 78 additions & 0 deletions xmodule/capa/tests/test_xqueue_submission.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
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 opaque_keys.edx.keys import UsageKey, CourseKey
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


@pytest.mark.django_db
@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
)
20 changes: 17 additions & 3 deletions xmodule/capa/xqueue_interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down Expand Up @@ -135,7 +137,14 @@ def _send_to_queue(self, header, body, files_to_upload): # lint-amnesty, pylint
for f in files_to_upload:
files.update({f.name: f})

return self._http_post(self.url + '/xqueue/submit/', payload, files=files)
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 None, ''

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:
Expand Down Expand Up @@ -184,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),
Expand Down
Loading