diff --git a/lms/djangoapps/courseware/tests/test_submitting_problems.py b/lms/djangoapps/courseware/tests/test_submitting_problems.py index 4d04204078de..16439fb0f4b3 100644 --- a/lms/djangoapps/courseware/tests/test_submitting_problems.py +++ b/lms/djangoapps/courseware/tests/test_submitting_problems.py @@ -29,7 +29,6 @@ SchematicResponseXMLFactory ) from xmodule.capa.tests.test_util import use_unsafe_codejail -from xmodule.capa.xqueue_interface import XQueueInterface from common.djangoapps.course_modes.models import CourseMode from lms.djangoapps.courseware.models import BaseStudentModuleHistory, StudentModule from lms.djangoapps.courseware.tests.helpers import LoginEnrollmentTestCase @@ -44,6 +43,12 @@ from xmodule.partitions.partitions import Group, UserPartition # lint-amnesty, pylint: disable=wrong-import-order +if settings.USE_EXTRACTED_PROBLEM_BLOCK: + from xblocks_contrib.problem.capa.xqueue_interface import XQueueInterface +else: + from xmodule.capa.xqueue_interface import XQueueInterface + + class ProblemSubmissionTestMixin(TestCase): """ TestCase mixin that provides functions to submit answers to problems. diff --git a/lms/djangoapps/instructor_task/tests/test_integration.py b/lms/djangoapps/instructor_task/tests/test_integration.py index 267f8021cd9d..4098b324e41c 100644 --- a/lms/djangoapps/instructor_task/tests/test_integration.py +++ b/lms/djangoapps/instructor_task/tests/test_integration.py @@ -19,6 +19,7 @@ from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user from django.test.utils import override_settings from django.urls import reverse +from django.conf import settings from xmodule.capa.responsetypes import StudentInputError from xmodule.capa.tests.response_xml_factory import CodeResponseXMLFactory, CustomResponseXMLFactory @@ -45,6 +46,8 @@ from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.factories import BlockFactory # lint-amnesty, pylint: disable=wrong-import-order +PREFIX_CAPA = "xblocks_contrib.problem" if settings.USE_EXTRACTED_PROBLEM_BLOCK else "xmodule" + log = logging.getLogger(__name__) @@ -275,7 +278,7 @@ def test_rescoring_failure(self): self.submit_student_answer('u1', problem_url_name, [OPTION_1, OPTION_1]) expected_message = "bad things happened" - with patch('xmodule.capa.capa_problem.LoncapaProblem.get_grade_from_current_answers') as mock_rescore: + with patch(f'{PREFIX_CAPA}.capa.capa_problem.LoncapaProblem.get_grade_from_current_answers') as mock_rescore: mock_rescore.side_effect = ZeroDivisionError(expected_message) instructor_task = self.submit_rescore_all_student_answers('instructor', problem_url_name) self._assert_task_failure( @@ -295,7 +298,7 @@ def test_rescoring_bad_unicode_input(self): # return an input error as if it were a numerical response, with an embedded unicode character: expected_message = "Could not interpret '2/3\u03a9' as a number" - with patch('xmodule.capa.capa_problem.LoncapaProblem.get_grade_from_current_answers') as mock_rescore: + with patch(f'{PREFIX_CAPA}.capa.capa_problem.LoncapaProblem.get_grade_from_current_answers') as mock_rescore: mock_rescore.side_effect = StudentInputError(expected_message) instructor_task = self.submit_rescore_all_student_answers('instructor', problem_url_name) diff --git a/lms/envs/common.py b/lms/envs/common.py index 2075f3e70fac..6f05c2d2fbf3 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1584,7 +1584,7 @@ # Import after sys.path fixup from xmodule.modulestore.edit_info import EditInfoMixin # lint-amnesty, pylint: disable=wrong-import-order, wrong-import-position from xmodule.modulestore.inheritance import InheritanceMixin # lint-amnesty, pylint: disable=wrong-import-order, wrong-import-position -from xmodule.x_module import XModuleMixin # lint-amnesty, pylint: disable=wrong-import-order, wrong-import-position +from xmodule.x_module import XModuleMixin, ResourceTemplates # lint-amnesty, pylint: disable=wrong-import-order, wrong-import-position # These are the Mixins that will be added to every Blocklike upon instantiation. # DO NOT EXPAND THIS LIST!! We want it eventually to be EMPTY. Why? Because dynamically adding functions/behaviors to @@ -1597,6 +1597,7 @@ # (b) refactor their functionality out of the Blocklike objects and into the edx-platform block runtimes. LmsBlockMixin, InheritanceMixin, + ResourceTemplates, XModuleMixin, EditInfoMixin, ) @@ -3164,6 +3165,9 @@ "openedx_learning.apps.authoring.units", "openedx_learning.apps.authoring.subsections", "openedx_learning.apps.authoring.sections", + + # Extracted problem xblock + "xblocks_contrib.problem.capa" ] ######################### CSRF ######################################### diff --git a/openedx/envs/common.py b/openedx/envs/common.py index c888ec8fd73f..c11b6f8274c1 100644 --- a/openedx/envs/common.py +++ b/openedx/envs/common.py @@ -763,7 +763,7 @@ def _make_locale_paths(settings): # .. toggle_warning: Not production-ready until relevant subtask https://github.com/openedx/edx-platform/issues/34827 is done. # .. toggle_creation_date: 2024-11-10 # .. toggle_target_removal_date: 2025-06-01 -USE_EXTRACTED_PROBLEM_BLOCK = False +USE_EXTRACTED_PROBLEM_BLOCK = True # .. toggle_name: USE_EXTRACTED_VIDEO_BLOCK # .. toggle_default: False diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index d01ab82fb83c..c375412eab0a 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -1280,7 +1280,7 @@ xblock-utils==4.0.0 # via # edx-sga # xblock-poll -xblocks-contrib==0.6.0 +git+https://github.com/openedx/xblocks-contrib.git@problemblock # via -r requirements/edx/bundled.in xmlsec==1.3.14 # via diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 6c75a51c6e81..f06aa5b44c9c 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -2285,7 +2285,7 @@ xblock-utils==4.0.0 # -r requirements/edx/testing.txt # edx-sga # xblock-poll -xblocks-contrib==0.6.0 +git+https://github.com/openedx/xblocks-contrib.git@problemblock # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 0e87d53dd6e4..70d8ecb1219f 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -1609,7 +1609,7 @@ xblock-utils==4.0.0 # -r requirements/edx/base.txt # edx-sga # xblock-poll -xblocks-contrib==0.6.0 +git+https://github.com/openedx/xblocks-contrib.git@problemblock # via -r requirements/edx/base.txt xmlsec==1.3.14 # via diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 577ee62d146d..6df871a615ce 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -1691,7 +1691,7 @@ xblock-utils==4.0.0 # -r requirements/edx/base.txt # edx-sga # xblock-poll -xblocks-contrib==0.6.0 +git+https://github.com/openedx/xblocks-contrib.git@problemblock # via -r requirements/edx/base.txt xmlsec==1.3.14 # via diff --git a/xmodule/modulestore/tests/factories.py b/xmodule/modulestore/tests/factories.py index 4467215fac4a..dc3d6c26ed93 100644 --- a/xmodule/modulestore/tests/factories.py +++ b/xmodule/modulestore/tests/factories.py @@ -397,7 +397,8 @@ def _create(cls, target_class, **kwargs): # lint-amnesty, pylint: disable=argum if 'boilerplate' in kwargs: template_id = kwargs.pop('boilerplate') clz = XBlock.load_class(category) - template = clz.get_template(template_id) + dynamic_clz = parent.runtime.mixologist.mix(clz) + template = dynamic_clz.get_template(template_id) assert template is not None metadata.update(template.get('metadata', {})) if not isinstance(data, str): diff --git a/xmodule/tests/__init__.py b/xmodule/tests/__init__.py index 786836c050b5..7c1c4b462d17 100644 --- a/xmodule/tests/__init__.py +++ b/xmodule/tests/__init__.py @@ -164,6 +164,7 @@ def get_block(block): descriptor_system.get_block_for_descriptor = get_block # lint-amnesty, pylint: disable=attribute-defined-outside-init descriptor_system._services.update(services) # lint-amnesty, pylint: disable=protected-access + descriptor_system.render_template = Mock(side_effect=render_template) return descriptor_system diff --git a/xmodule/tests/test_capa_block.py b/xmodule/tests/test_capa_block.py index 21582e55eac3..dbdfa529cd18 100644 --- a/xmodule/tests/test_capa_block.py +++ b/xmodule/tests/test_capa_block.py @@ -12,15 +12,14 @@ import unittest from unittest.mock import DEFAULT, Mock, PropertyMock, patch -import pytest import ddt +import pytest import requests import webob from codejail.safe_exec import SafeExecException from django.conf import settings from django.test import override_settings from django.utils.encoding import smart_str -from lms.djangoapps.courseware.user_state_client import XBlockUserState from lxml import etree from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator from pytz import UTC @@ -29,19 +28,33 @@ from xblock.fields import ScopeIds from xblock.scorable import Score -import xmodule +from lms.djangoapps.courseware.user_state_client import XBlockUserState from openedx.core.djangolib.testing.utils import skip_unless_lms from xmodule.capa import responsetypes -from xmodule.capa.correctmap import CorrectMap -from xmodule.capa.responsetypes import LoncapaProblemError, ResponseError, StudentInputError -from xmodule.capa.xqueue_interface import XQueueInterface +from xmodule.capa.tests.test_util import use_unsafe_codejail from xmodule.capa_block import ComplexEncoder, ProblemBlock from xmodule.tests import DATA_DIR -from xmodule.capa.tests.test_util import use_unsafe_codejail from ..capa_block import RANDOMIZATION, SHOWANSWER from . import get_test_system +if settings.USE_EXTRACTED_PROBLEM_BLOCK: + PREFIX_CAPA = "xblocks_contrib.problem" + PREFIX_BLOCK = "xblocks_contrib.problem.problem" + + from xblocks_contrib.problem.capa.correctmap import CorrectMap + from xblocks_contrib.problem.capa.responsetypes import LoncapaProblemError, ResponseError, StudentInputError + from xblocks_contrib.problem.capa.xqueue_interface import XQueueInterface + from xblocks_contrib.problem.problem import NotFoundError + +else: + PREFIX_CAPA = "xmodule" + PREFIX_BLOCK = "xmodule.capa_block" + + from xmodule.capa.correctmap import CorrectMap + from xmodule.capa.responsetypes import LoncapaProblemError, ResponseError, StudentInputError + from xmodule.capa.xqueue_interface import XQueueInterface + from xmodule.capa_block import NotFoundError FEATURES_WITH_GRADING_METHOD_IN_PROBLEMS = settings.FEATURES.copy() FEATURES_WITH_GRADING_METHOD_IN_PROBLEMS['ENABLE_GRADING_METHOD_IN_PROBLEMS'] = True @@ -1035,7 +1048,7 @@ def test_submit_problem_correct_highest_score(self, mock_html: Mock, mock_is_cor assert block.score == Score(raw_earned=1, raw_possible=1) @override_settings(FEATURES=FEATURES_WITH_GRADING_METHOD_IN_PROBLEMS) - @patch('xmodule.capa.correctmap.CorrectMap.is_correct') + @patch(f'{PREFIX_CAPA}.capa.correctmap.CorrectMap.is_correct') @patch('xmodule.capa_block.ProblemBlock.get_problem_html') def test_submit_problem_correct_first_score(self, mock_html: Mock, mock_is_correct: Mock): """ @@ -1068,7 +1081,7 @@ def test_submit_problem_correct_first_score(self, mock_html: Mock, mock_is_corre assert block.score == Score(raw_earned=0, raw_possible=1) @override_settings(FEATURES=FEATURES_WITH_GRADING_METHOD_IN_PROBLEMS) - @patch('xmodule.capa.correctmap.CorrectMap.is_correct') + @patch(f'{PREFIX_CAPA}.capa.correctmap.CorrectMap.is_correct') @patch('xmodule.capa_block.ProblemBlock.get_problem_html') def test_submit_problem_correct_average_score(self, mock_html: Mock, mock_is_correct: Mock): """ @@ -1147,7 +1160,7 @@ def test_submit_problem_closed(self): # Simulate that ProblemBlock.closed() always returns True with patch('xmodule.capa_block.ProblemBlock.closed') as mock_closed: mock_closed.return_value = True - with pytest.raises(xmodule.exceptions.NotFoundError): + with pytest.raises(NotFoundError): get_request_dict = {CapaFactory.input_key(): '3.14'} block.submit_problem(get_request_dict) @@ -1166,7 +1179,7 @@ def test_submit_problem_resubmitted_with_randomize(self, rerandomize): block.done = True # Expect that we cannot submit - with pytest.raises(xmodule.exceptions.NotFoundError): + with pytest.raises(NotFoundError): get_request_dict = {CapaFactory.input_key(): '3.14'} block.submit_problem(get_request_dict) @@ -1197,7 +1210,7 @@ def test_submit_problem_queued(self): # Simulate that the problem is queued multipatch = patch.multiple( - 'xmodule.capa.capa_problem.LoncapaProblem', + f'{PREFIX_CAPA}.capa.capa_problem.LoncapaProblem', is_queued=DEFAULT, get_recentmost_queuetime=DEFAULT ) @@ -1309,7 +1322,7 @@ def test_submit_problem_error(self): block = CapaFactory.create(attempts=1, user_is_staff=False) # Simulate answering a problem that raises the exception - with patch('xmodule.capa.capa_problem.LoncapaProblem.grade_answers') as mock_grade: + with patch(f'{PREFIX_CAPA}.capa.capa_problem.LoncapaProblem.grade_answers') as mock_grade: mock_grade.side_effect = exception_class('test error') get_request_dict = {CapaFactory.input_key(): '3.14'} @@ -1337,7 +1350,7 @@ def test_submit_problem_error_with_codejail_exception(self): block = CapaFactory.create(attempts=1, user_is_staff=False) # Simulate a codejail exception "Exception: Couldn't execute jailed code" - with patch('xmodule.capa.capa_problem.LoncapaProblem.grade_answers') as mock_grade: + with patch(f'{PREFIX_CAPA}.capa.capa_problem.LoncapaProblem.grade_answers') as mock_grade: try: raise ResponseError( 'Couldn\'t execute jailed code: stdout: \'\', ' @@ -1371,7 +1384,7 @@ def test_submit_problem_other_errors(self): block = CapaFactory.create(attempts=1, user_is_staff=False) # Simulate answering a problem that raises the exception - with patch('xmodule.capa.capa_problem.LoncapaProblem.grade_answers') as mock_grade: + with patch(f'{PREFIX_CAPA}.capa.capa_problem.LoncapaProblem.grade_answers') as mock_grade: error_msg = "Superterrible error happened: ☠" mock_grade.side_effect = Exception(error_msg) @@ -1406,7 +1419,7 @@ def test_submit_problem_error_nonascii(self): block = CapaFactory.create(attempts=1, user_is_staff=False) # Simulate answering a problem that raises the exception - with patch('xmodule.capa.capa_problem.LoncapaProblem.grade_answers') as mock_grade: + with patch(f'{PREFIX_CAPA}.capa.capa_problem.LoncapaProblem.grade_answers') as mock_grade: mock_grade.side_effect = exception_class("ȧƈƈḗƞŧḗḓ ŧḗẋŧ ƒǿř ŧḗşŧīƞɠ") get_request_dict = {CapaFactory.input_key(): '3.14'} @@ -1432,7 +1445,7 @@ def test_submit_problem_error_with_staff_user(self): block = CapaFactory.create(attempts=1, user_is_staff=True) # Simulate answering a problem that raises an exception - with patch('xmodule.capa.capa_problem.LoncapaProblem.grade_answers') as mock_grade: + with patch(f'{PREFIX_CAPA}.capa.capa_problem.LoncapaProblem.grade_answers') as mock_grade: mock_grade.side_effect = exception_class('test error') get_request_dict = {CapaFactory.input_key(): '3.14'} @@ -1464,7 +1477,7 @@ def test_handle_ajax_show_correctness(self, show_correctness, is_correct, expect correct=is_correct) # Simulate marking the input correct/incorrect - with patch('xmodule.capa.correctmap.CorrectMap.is_correct') as mock_is_correct: + with patch(f'{PREFIX_CAPA}.capa.correctmap.CorrectMap.is_correct') as mock_is_correct: mock_is_correct.return_value = is_correct # Check the problem @@ -1541,7 +1554,7 @@ def test_rescore_problem_correct(self): correctness='correct', npoints=1, ) - with patch('xmodule.capa.correctmap.CorrectMap.is_correct') as mock_is_correct: + with patch(f'{PREFIX_CAPA}.capa.correctmap.CorrectMap.is_correct') as mock_is_correct: mock_is_correct.return_value = True # Check the problem @@ -1581,10 +1594,10 @@ def test_rescore_problem_additional_correct(self): # In case of rescore with only_if_higher=True it should update score of block # if previous score was lower - with patch('xmodule.capa.correctmap.CorrectMap.is_correct') as mock_is_correct: + with patch(f'{PREFIX_CAPA}.capa.correctmap.CorrectMap.is_correct') as mock_is_correct: mock_is_correct.return_value = True block.set_score(block.score_from_lcp(block.lcp)) - with patch('xmodule.capa.responsetypes.NumericalResponse.get_staff_ans') as get_staff_ans: + with patch(f'{PREFIX_CAPA}.capa.responsetypes.NumericalResponse.get_staff_ans') as get_staff_ans: get_staff_ans.return_value = 1 + 0j block.rescore(only_if_higher=True) @@ -1689,7 +1702,7 @@ def test_rescore_problem_grading_method_disable_to_enable(self, mock_publish_gra return_value=True ): with patch( - 'xmodule.capa.capa_problem.LoncapaProblem.is_grading_method_enabled', + f'{PREFIX_CAPA}.capa.capa_problem.LoncapaProblem.is_grading_method_enabled', new_callable=PropertyMock, return_value=True ): @@ -1743,7 +1756,7 @@ def test_rescore_problem_grading_method_enable_to_disable(self, mock_publish_gra return_value=True ): with patch( - 'xmodule.capa.capa_problem.LoncapaProblem.is_grading_method_enabled', + f'{PREFIX_CAPA}.capa.capa_problem.LoncapaProblem.is_grading_method_enabled', new_callable=PropertyMock, return_value=True ): @@ -1838,14 +1851,14 @@ def test_rescore_problem_not_done(self): block = CapaFactory.create(done=False) # Try to rescore the problem, and get exception - with pytest.raises(xmodule.exceptions.NotFoundError): + with pytest.raises(NotFoundError): block.rescore(only_if_higher=False) def test_rescore_problem_not_supported(self): block = CapaFactory.create(done=True) # Try to rescore the problem, and get exception - with patch('xmodule.capa.capa_problem.LoncapaProblem.supports_rescoring') as mock_supports_rescoring: + with patch(f'{PREFIX_CAPA}.capa.capa_problem.LoncapaProblem.supports_rescoring') as mock_supports_rescoring: mock_supports_rescoring.return_value = False with pytest.raises(NotImplementedError): block.rescore(only_if_higher=False) @@ -1978,7 +1991,7 @@ def test_get_score_with_grading_method_calls_grading_method_handler(self): block = CapaFactory.create(attempts=1) current_score = Score(raw_earned=0, raw_possible=1) - with patch('xmodule.capa_block.GradingMethodHandler') as mock_handler: + with patch(f'{PREFIX_BLOCK}.GradingMethodHandler') as mock_handler: mock_handler.return_value.get_score.return_value = current_score block.get_score_with_grading_method(current_score) mock_handler.assert_called_once_with( @@ -2024,7 +2037,7 @@ def _rescore_problem_error_helper(self, exception_class): block.submit_problem(get_request_dict) # Simulate answering a problem that raises the exception - with patch('xmodule.capa.capa_problem.LoncapaProblem.get_grade_from_current_answers') as mock_rescore: + with patch(f'{PREFIX_CAPA}.capa.capa_problem.LoncapaProblem.get_grade_from_current_answers') as mock_rescore: mock_rescore.side_effect = exception_class('test error \u03a9') with pytest.raises(exception_class): block.rescore(only_if_higher=False) @@ -2296,7 +2309,7 @@ def test_get_problem_html(self): block.should_show_save_button = Mock(return_value=show_save_button) # Patch the capa problem's HTML rendering - with patch('xmodule.capa.capa_problem.LoncapaProblem.get_html') as mock_html: + with patch(f'{PREFIX_CAPA}.capa.capa_problem.LoncapaProblem.get_html') as mock_html: mock_html.return_value = "
Test Problem HTML
" # Render the problem HTML @@ -2690,8 +2703,8 @@ def test_random_seed_bins(self, rerandomize): assert 0 <= block.seed < 1000 i -= 1 - @patch('xmodule.capa_block.log') - @patch('xmodule.capa_block.Progress') + @patch(f'{PREFIX_BLOCK}.log') + @patch(f'{PREFIX_BLOCK}.Progress') def test_get_progress_error(self, mock_progress, mock_log): """ Check that an exception given in `Progress` produces a `log.exception` call. @@ -2716,7 +2729,7 @@ def test_get_progress_no_error_if_weight_zero(self, mock_progress): assert progress is None assert not mock_progress.called - @patch('xmodule.capa_block.Progress') + @patch(f'{PREFIX_BLOCK}.Progress') def test_get_progress_calculate_progress_fraction(self, mock_progress): """ Check that score and total are calculated correctly for the progress fraction. @@ -3934,7 +3947,7 @@ class ProblemBlockReportGenerationTest(unittest.TestCase): def setUp(self): # lint-amnesty, pylint: disable=super-method-not-called self.find_question_label_patcher = patch( - 'xmodule.capa.capa_problem.LoncapaProblem.find_question_label', + f'{PREFIX_CAPA}.capa.capa_problem.LoncapaProblem.find_question_label', lambda self, answer_id: answer_id ) self.find_answer_text_patcher = patch( @@ -4005,7 +4018,7 @@ def test_generate_report_data_skip_dynamath(self): def test_generate_report_data_report_loncapa_error(self): #Test to make sure reports continue despite loncappa errors, and write them into the report. block = self._get_block() - with patch('xmodule.capa_block.LoncapaProblem') as mock_LoncapaProblem: + with patch(f'{PREFIX_BLOCK}.LoncapaProblem') as mock_LoncapaProblem: mock_LoncapaProblem.side_effect = LoncapaProblemError report_data = list(block.generate_report_data( self._mock_user_state_generator( diff --git a/xmodule/tests/test_delay_between_attempts.py b/xmodule/tests/test_delay_between_attempts.py index 9b1c58d8aaf3..983b0a48dcde 100644 --- a/xmodule/tests/test_delay_between_attempts.py +++ b/xmodule/tests/test_delay_between_attempts.py @@ -14,17 +14,22 @@ from unittest.mock import Mock import pytest +from django.conf import settings from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator from pytz import UTC from xblock.field_data import DictFieldData from xblock.fields import ScopeIds from xblock.scorable import Score -import xmodule from xmodule.capa_block import ProblemBlock from . import get_test_system +if settings.USE_EXTRACTED_PROBLEM_BLOCK: + from xblocks_contrib.problem.problem import NotFoundError +else: + from xmodule.capa_block import NotFoundError + class CapaFactoryWithDelay: """ @@ -244,7 +249,7 @@ def test_still_cannot_submit_after_max_attempts(self): # Already attempted once (just now) and thus has a submitted time num_attempts = 99 # Regular create_and_check should fail - with pytest.raises(xmodule.exceptions.NotFoundError): + with pytest.raises(NotFoundError): (block, unused_result) = self.create_and_check( num_attempts=num_attempts, last_submission_time=datetime.datetime(2013, 12, 6, 0, 17, 36, tzinfo=UTC),