diff --git a/.github/workflows/add-remove-label-on-comment.yml b/.github/workflows/add-remove-label-on-comment.yml index a658064f09f0..0f369db7d293 100644 --- a/.github/workflows/add-remove-label-on-comment.yml +++ b/.github/workflows/add-remove-label-on-comment.yml @@ -17,3 +17,4 @@ on: jobs: add_remove_labels: uses: openedx/.github/.github/workflows/add-remove-label-on-comment.yml@master + diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 2a2669bc6862..407aa6fa1fb6 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -94,6 +94,7 @@ jobs: - name: install requirements run: | make test-requirements + pip install django-simple-history==3.10.1 if [[ "${{ matrix.django-version }}" != "pinned" ]]; then pip install "django~=${{ matrix.django-version }}.0" pip check # fail if this test-reqs/Django combination is broken diff --git a/cms/djangoapps/contentstore/core/course_optimizer_provider.py b/cms/djangoapps/contentstore/core/course_optimizer_provider.py index 7b44e05a87c2..16aec9075d6c 100644 --- a/cms/djangoapps/contentstore/core/course_optimizer_provider.py +++ b/cms/djangoapps/contentstore/core/course_optimizer_provider.py @@ -2,15 +2,19 @@ Logic for handling actions in Studio related to Course Optimizer. """ import json + +from opaque_keys.edx.keys import CourseKey from user_tasks.conf import settings as user_tasks_settings from user_tasks.models import UserTaskArtifact, UserTaskStatus -from cms.djangoapps.contentstore.tasks import CourseLinkCheckTask, LinkState +from cms.djangoapps.contentstore.tasks import CourseLinkCheckTask, LinkState, extract_content_URLs_from_course +from cms.djangoapps.contentstore.utils import create_course_info_usage_key from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import get_xblock from cms.djangoapps.contentstore.xblock_storage_handlers.xblock_helpers import usage_key_with_run +from openedx.core.lib.xblock_utils import get_course_update_items from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore - +from xmodule.tabs import StaticTab # Restricts status in the REST API to only those which the requesting user has permission to view. # These can be overwritten in django settings. @@ -23,6 +27,7 @@ def get_link_check_data(request, course_id): """ Retrives data and formats it for the link check get request. """ + course_key = CourseKey.from_string(course_id) task_status = _latest_task_status(request, course_id) status = None created_at = None @@ -43,7 +48,7 @@ def get_link_check_data(request, course_id): with artifact.file as file: content = file.read() json_content = json.loads(content) - broken_links_dto = generate_broken_links_descriptor(json_content, request.user) + broken_links_dto = generate_broken_links_descriptor(json_content, request.user, course_key) elif task_status.state in (UserTaskStatus.FAILED, UserTaskStatus.CANCELED): errors = UserTaskArtifact.objects.filter(status=task_status, name='Error') if errors: @@ -53,7 +58,6 @@ def get_link_check_data(request, course_id): except ValueError: # Wasn't JSON, just use the value as a string pass - data = { 'LinkCheckStatus': status, **({'LinkCheckCreatedAt': created_at} if created_at else {}), @@ -76,13 +80,16 @@ def _latest_task_status(request, course_key_string, view_func=None): return task_status.order_by('-created').first() -def generate_broken_links_descriptor(json_content, request_user): +def generate_broken_links_descriptor(json_content, request_user, course_key): """ Returns a Data Transfer Object for frontend given a list of broken links. + Includes all link types: broken, locked, external-forbidden, and previous run links, + as well as links found in course updates, handouts, and custom pages. ** Example json_content structure ** Note: link_state is locked if the link is a studio link and returns 403 link_state is external-forbidden if the link is not a studio link and returns 403 + link_state is previous-run if the link points to a previous course run [ ['block_id_1', 'link_1', link_state], ['block_id_1', 'link_2', link_state], @@ -111,6 +118,7 @@ def generate_broken_links_descriptor(json_content, request_user): 'url': 'url/to/block', 'brokenLinks: [], 'lockedLinks: [], + 'previousRunLinks: [] }, ..., ] @@ -122,30 +130,40 @@ def generate_broken_links_descriptor(json_content, request_user): ] }, ..., + ], + 'course_updates': [ + { + 'name': 'published_date', + 'url': 'url', + 'brokenLinks': [], + 'lockedLinks': [], + 'externalForbiddenLinks': [], + 'previousRunLinks': [] + }, + ... + { + 'name': 'handouts', + 'url': 'url', + 'brokenLinks': [], + 'lockedLinks': [], + 'externalForbiddenLinks': [], + 'previousRunLinks': [] + } + ], + 'custom_pages': [ + { + 'name': 'page_name', + 'url': 'url', + 'brokenLinks': [], + 'lockedLinks': [], + 'externalForbiddenLinks': [], + 'previousRunLinks': [] + }, + ... ] } """ - xblock_node_tree = {} # tree representation of xblock relationships - xblock_dictionary = {} # dictionary of xblock attributes - - for item in json_content: - block_id, link, *rest = item - if rest: - link_state = rest[0] - else: - link_state = '' - - usage_key = usage_key_with_run(block_id) - block = get_xblock(usage_key, request_user) - xblock_node_tree, xblock_dictionary = _update_node_tree_and_dictionary( - block=block, - link=link, - link_state=link_state, - node_tree=xblock_node_tree, - dictionary=xblock_dictionary - ) - - return _create_dto_recursive(xblock_node_tree, xblock_dictionary) + return _generate_enhanced_links_descriptor(json_content, request_user, course_key) def _update_node_tree_and_dictionary(block, link, link_state, node_tree, dictionary): @@ -221,6 +239,8 @@ def _update_node_tree_and_dictionary(block, link, link_state, node_tree, diction updated_dictionary[xblock_id].setdefault('locked_links', []).append(link) elif link_state == LinkState.EXTERNAL_FORBIDDEN: updated_dictionary[xblock_id].setdefault('external_forbidden_links', []).append(link) + elif link_state == LinkState.PREVIOUS_RUN: + updated_dictionary[xblock_id].setdefault('previous_run_links', []).append(link) else: updated_dictionary[xblock_id].setdefault('broken_links', []).append(link) @@ -277,7 +297,8 @@ def _create_dto_recursive(xblock_node, xblock_dictionary, parent_id=None): 'url': xblock_data.get('url', ''), 'brokenLinks': xblock_data.get('broken_links', []), 'lockedLinks': xblock_data.get('locked_links', []), - 'externalForbiddenLinks': xblock_data.get('external_forbidden_links', []) + 'externalForbiddenLinks': xblock_data.get('external_forbidden_links', []), + 'previousRunLinks': xblock_data.get('previous_run_links', []) }) else: # Non-leaf node category = xblock_data.get('category', None) @@ -317,3 +338,271 @@ def sort_course_sections(course_key, data): ] return data + + +def _generate_links_descriptor_for_content(json_content, request_user): + """ + Creates a content tree of all links in a course and their states + Returns a structure containing all broken links and locked links for a course. + """ + xblock_node_tree = {} + xblock_dictionary = {} + + for item in json_content: + block_id, link, *rest = item + if rest: + link_state = rest[0] + else: + link_state = "" + + usage_key = usage_key_with_run(block_id) + block = get_xblock(usage_key, request_user) + xblock_node_tree, xblock_dictionary = _update_node_tree_and_dictionary( + block=block, + link=link, + link_state=link_state, + node_tree=xblock_node_tree, + dictionary=xblock_dictionary, + ) + + result = _create_dto_recursive(xblock_node_tree, xblock_dictionary) + # Ensure we always return a valid structure with sections + if not isinstance(result, dict): + result = {"sections": []} + + return result + + +def _generate_enhanced_links_descriptor(json_content, request_user, course_key): + """ + Generate enhanced link descriptor that includes course updates, handouts, and custom pages. + """ + + content_links = [] + course_updates_links = [] + handouts_links = [] + custom_pages_links = [] + course = modulestore().get_course(course_key) + + for item in json_content: + block_id, link, *rest = item + if "course_info" in block_id and "updates" in block_id: + course_updates_links.append(item) + elif "course_info" in block_id and "handouts" in block_id: + handouts_links.append(item) + elif "static_tab" in block_id: + custom_pages_links.append(item) + else: + content_links.append(item) + + try: + main_content = _generate_links_descriptor_for_content(content_links, request_user) + except Exception: # pylint: disable=broad-exception-caught + main_content = {"sections": []} + + course_updates_data = ( + _generate_course_updates_structure(course, course_updates_links) + if course_updates_links and course else [] + ) + + handouts_data = ( + _generate_handouts_structure(course, handouts_links) + if handouts_links and course else [] + ) + + custom_pages_data = ( + _generate_custom_pages_structure(course, custom_pages_links) + if custom_pages_links and course else [] + ) + + result = main_content.copy() + result["course_updates"] = course_updates_data + handouts_data + result["custom_pages"] = custom_pages_data + return result + + +def _generate_enhanced_content_structure(course, content_links, content_type): + """ + Unified function to generate structure for enhanced content (updates, handouts, custom pages). + + Args: + course: Course object + content_links: List of link items for this content type + content_type: 'updates', 'handouts', or 'custom_pages' + + Returns: + List of content items with categorized links + """ + result = [] + try: + if content_type == "custom_pages": + result = _generate_custom_pages_content(course, content_links) + elif content_type == "updates": + result = _generate_course_updates_content(course, content_links) + elif content_type == "handouts": + result = _generate_handouts_content(course, content_links) + return result + except Exception as e: # pylint: disable=broad-exception-caught + return result + + +def _generate_course_updates_content(course, updates_links): + """Generate course updates content with categorized links.""" + store = modulestore() + usage_key = create_course_info_usage_key(course, "updates") + updates_block = store.get_item(usage_key) + course_updates = [] + + if not (updates_block and hasattr(updates_block, "data")): + return course_updates + + update_items = get_course_update_items(updates_block) + if not update_items: + return course_updates + + # Create link state mapping + link_state_map = { + item[1]: item[2] if len(item) >= 3 else LinkState.BROKEN + for item in updates_links if len(item) >= 2 + } + + for update in update_items: + if update.get("status") != "deleted": + update_content = update.get("content", "") + update_links = extract_content_URLs_from_course(update_content) if update_content else [] + + # Match links with their states + update_link_data = _create_empty_links_data() + for link in update_links: + link_state = link_state_map.get(link) + if link_state is not None: + _categorize_link_by_state(link, link_state, update_link_data) + + course_updates.append( + { + "id": str(update.get("id")), + "displayName": update.get("date", "Unknown Date"), + "url": f"/course/{str(course.id)}/course_info", + **update_link_data, + } + ) + + return course_updates + + +def _generate_handouts_content(course, handouts_links): + """Generate handouts content with categorized links.""" + store = modulestore() + usage_key = create_course_info_usage_key(course, "handouts") + handouts_block = store.get_item(usage_key) + course_handouts = [] + + if not ( + handouts_block + and hasattr(handouts_block, "data") + and handouts_block.data + ): + return course_handouts + + # Create link state mapping for handouts + link_state_map = { + item[1]: item[2] if len(item) >= 3 else LinkState.BROKEN + for item in handouts_links if len(item) >= 2 + } + + links_data = _create_empty_links_data() + for link, link_state in link_state_map.items(): + _categorize_link_by_state(link, link_state, links_data) + + course_handouts = [ + { + "id": str(usage_key), + "displayName": "handouts", + "url": f"/course/{str(course.id)}/course_info", + **links_data, + } + ] + return course_handouts + + +def _generate_custom_pages_content(course, custom_pages_links): + """Generate custom pages content with categorized links.""" + custom_pages = [] + + if not course or not hasattr(course, "tabs"): + return custom_pages + + # Group links by block_id and categorize them + links_by_page = {} + for item in custom_pages_links: + if len(item) >= 2: + block_id, link = item[0], item[1] + link_state = item[2] if len(item) >= 3 else LinkState.BROKEN + links_by_page.setdefault(block_id, _create_empty_links_data()) + _categorize_link_by_state(link, link_state, links_by_page[block_id]) + + # Process static tabs and add their pages + for tab in course.tabs: + if isinstance(tab, StaticTab): + block_id = str(course.id.make_usage_key("static_tab", tab.url_slug)) + custom_pages.append({ + "id": block_id, + "displayName": tab.name, + "url": f"/course/{str(course.id)}/custom-pages", + **links_by_page.get(block_id, _create_empty_links_data()), + }) + + return custom_pages + + +def _generate_course_updates_structure(course, updates_links): + """Generate structure for course updates.""" + return _generate_enhanced_content_structure(course, updates_links, "updates") + + +def _generate_handouts_structure(course, handouts_links): + """Generate structure for course handouts.""" + return _generate_enhanced_content_structure(course, handouts_links, "handouts") + + +def _generate_custom_pages_structure(course, custom_pages_links): + """Generate structure for custom pages (static tabs).""" + return _generate_enhanced_content_structure( + course, custom_pages_links, "custom_pages" + ) + + +def _categorize_link_by_state(link, link_state, links_data): + """ + Helper function to categorize a link into the appropriate list based on its state. + + Args: + link (str): The URL link to categorize + link_state (str): The state of the link (broken, locked, external-forbidden, previous-run) + links_data (dict): Dictionary containing the categorized link lists + """ + state_to_key = { + LinkState.BROKEN: "brokenLinks", + LinkState.LOCKED: "lockedLinks", + LinkState.EXTERNAL_FORBIDDEN: "externalForbiddenLinks", + LinkState.PREVIOUS_RUN: "previousRunLinks" + } + + key = state_to_key.get(link_state) + if key: + links_data[key].append(link) + + +def _create_empty_links_data(): + """ + Helper function to create an empty links data structure. + + Returns: + dict: Dictionary with empty lists for each link type + """ + return { + "brokenLinks": [], + "lockedLinks": [], + "externalForbiddenLinks": [], + "previousRunLinks": [], + } diff --git a/cms/djangoapps/contentstore/core/tests/test_course_optimizer_provider.py b/cms/djangoapps/contentstore/core/tests/test_course_optimizer_provider.py index ca0b73af71da..9ce568fc980a 100644 --- a/cms/djangoapps/contentstore/core/tests/test_course_optimizer_provider.py +++ b/cms/djangoapps/contentstore/core/tests/test_course_optimizer_provider.py @@ -1,16 +1,22 @@ """ Tests for course optimizer """ + from unittest import mock from unittest.mock import Mock -from cms.djangoapps.contentstore.tests.utils import CourseTestCase +from opaque_keys.edx.keys import CourseKey + from cms.djangoapps.contentstore.core.course_optimizer_provider import ( - _update_node_tree_and_dictionary, _create_dto_recursive, + _update_node_tree_and_dictionary, + generate_broken_links_descriptor, sort_course_sections ) -from cms.djangoapps.contentstore.tasks import LinkState +from cms.djangoapps.contentstore.tasks import LinkState, extract_content_URLs_from_course +from cms.djangoapps.contentstore.tests.utils import CourseTestCase +from cms.djangoapps.contentstore.utils import contains_previous_course_reference +from xmodule.tabs import StaticTab class TestLinkCheckProvider(CourseTestCase): @@ -123,6 +129,7 @@ def test_create_dto_recursive_returns_for_leaf_node(self): 'brokenLinks': ['broken_link_1', 'broken_link_2'], 'lockedLinks': ['locked_link'], 'externalForbiddenLinks': ['forbidden_link_1'], + 'previousRunLinks': [], } ] } @@ -181,6 +188,7 @@ def test_create_dto_recursive_returns_for_full_tree(self): 'brokenLinks': ['broken_link_1', 'broken_link_2'], 'lockedLinks': ['locked_link'], 'externalForbiddenLinks': ['forbidden_link_1'], + 'previousRunLinks': [], } ] } @@ -295,3 +303,145 @@ def test_sorts_sections_correctly(self, mock_modulestore): ] assert result["LinkCheckOutput"]["sections"] == expected_sections + + def test_prev_run_link_detection(self): + """Test the core logic of separating previous run links from regular links.""" + + previous_course_key = CourseKey.from_string( + "course-v1:edX+DemoX+Demo_Course_2023" + ) + + test_cases = [ + (f"/courses/{previous_course_key}/info", True), + (f"/courses/{previous_course_key}/courseware", True), + (f"/courses/{str(previous_course_key).upper()}/page", True), + # Should NOT match + ("/courses/course-v1:edX+DemoX+Demo_Course_2024/info", False), + ("/static/image.png", False), + ("/assets/courseware/file.pdf", False), + ("", False), + (" ", False), + ] + + for url, expected_match in test_cases: + with self.subTest(url=url, expected=expected_match): + result = contains_previous_course_reference(url, previous_course_key) + self.assertEqual( + result, + expected_match, + f"URL '{url}' should {'match' if expected_match else 'not match'} previous course", + ) + + def test_enhanced_url_detection_edge_cases(self): + """Test edge cases for enhanced URL detection.""" + + test_cases = [ + ("", []), # Empty content + ("No URLs here", []), # Content without URLs + ( + "Visit https://example.com today!", + ["https://example.com"], + ), # URL in text + ('href="#anchor"', []), # Should exclude fragments + ('src="data:image/png;base64,123"', []), # Should exclude data URLs + ( + "Multiple URLs: http://site1.com and https://site2.com", + ["http://site1.com", "https://site2.com"], + ), # Multiple URLs + ( + "URL with params: https://example.com/page?param=value&other=123", + ["https://example.com/page?param=value&other=123"], + ), # URL with parameters + ] + + for content, expected_urls in test_cases: + with self.subTest(content=content): + urls = extract_content_URLs_from_course(content) + for expected_url in expected_urls: + self.assertIn( + expected_url, + urls, + f"Should find '{expected_url}' in content: {content}", + ) + + def test_course_updates_and_custom_pages_structure(self): + """Test that course_updates and custom_pages are properly structured in the response.""" + + json_content = [ + # Regular course content + [ + "course-v1:Test+Course+2024+type@html+block@content1", + "http://content-link.com", + "broken", + ], + [ + "course-v1:Test+Course+2024+type@vertical+block@unit1", + "http://unit-link.com", + "locked", + ], + # Course updates + [ + "course-v1:Test+Course+2024+type@course_info+block@updates", + "http://update1.com", + "broken", + ], + [ + "course-v1:Test+Course+2024+type@course_info+block@updates", + "http://update2.com", + "locked", + ], + # Handouts (should be merged into course_updates) + [ + "course-v1:Test+Course+2024+type@course_info+block@handouts", + "http://handout.com", + "broken", + ], + # Custom pages (static tabs) + [ + "course-v1:Test+Course+2024+type@static_tab+block@page1", + "http://page1.com", + "broken", + ], + [ + "course-v1:Test+Course+2024+type@static_tab+block@page2", + "http://page2.com", + "external-forbidden", + ], + ] + + with mock.patch( + "cms.djangoapps.contentstore.core.course_optimizer_provider._generate_links_descriptor_for_content" + ) as mock_content, mock.patch( + "cms.djangoapps.contentstore.core.course_optimizer_provider.modulestore" + ) as mock_modulestore: + + mock_content.return_value = {"sections": []} + mock_course = self.mock_course + mock_tab1 = StaticTab(name="Page1", url_slug="page1") + mock_tab2 = StaticTab(name="Page2", url_slug="page2") + mock_course.tabs = [mock_tab1, mock_tab2] + mock_course.id = CourseKey.from_string("course-v1:Test+Course+2024") + mock_modulestore.return_value.get_course.return_value = mock_course + + course_key = CourseKey.from_string("course-v1:Test+Course+2024") + result = generate_broken_links_descriptor( + json_content, self.user, course_key + ) + + # Verify top-level structure + self.assertIn("sections", result) + self.assertIn("course_updates", result) + self.assertIn("custom_pages", result) + self.assertNotIn("handouts", result) + + # Course updates should include both updates and handouts + self.assertGreaterEqual( + len(result["course_updates"]), + 1, + "Should have course updates/handouts", + ) + + # Custom pages should have custom pages data + self.assertGreaterEqual( + len(result["custom_pages"]), 1, "Should have custom pages" + ) diff --git a/cms/djangoapps/contentstore/rest_api/v0/serializers/course_optimizer.py b/cms/djangoapps/contentstore/rest_api/v0/serializers/course_optimizer.py index 7411192d16f4..9faef425e4f4 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/serializers/course_optimizer.py +++ b/cms/djangoapps/contentstore/rest_api/v0/serializers/course_optimizer.py @@ -13,6 +13,7 @@ class LinkCheckBlockSerializer(serializers.Serializer): brokenLinks = serializers.ListField(required=False) lockedLinks = serializers.ListField(required=False) externalForbiddenLinks = serializers.ListField(required=False) + previousRunLinks = serializers.ListField(required=False) class LinkCheckUnitSerializer(serializers.Serializer): @@ -39,6 +40,8 @@ class LinkCheckSectionSerializer(serializers.Serializer): class LinkCheckOutputSerializer(serializers.Serializer): """ Serializer for broken links output model data """ sections = LinkCheckSectionSerializer(many=True) + course_updates = LinkCheckBlockSerializer(many=True, required=False) + custom_pages = LinkCheckBlockSerializer(many=True, required=False) class LinkCheckSerializer(serializers.Serializer): diff --git a/cms/djangoapps/contentstore/rest_api/v0/views/course_optimizer.py b/cms/djangoapps/contentstore/rest_api/v0/views/course_optimizer.py index 24c8dd0d18f8..b98255ebd35c 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/views/course_optimizer.py +++ b/cms/djangoapps/contentstore/rest_api/v0/views/course_optimizer.py @@ -71,53 +71,49 @@ class LinkCheckStatusView(DeveloperErrorViewMixin, APIView): ) def get(self, request: Request, course_id: str): """ - GET handler to return the status of the link_check task from UserTaskStatus. - If no task has been started for the course, return 'Uninitiated'. - If link_check task was successful, an output result is also returned. + **Use Case** - For reference, the following status are in UserTaskStatus: - 'Pending', 'In Progress' (sent to frontend as 'In-Progress'), - 'Succeeded', 'Failed', 'Canceled', 'Retrying' - This function adds a status for when status from UserTaskStatus is None: - 'Uninitiated' + GET handler to return the status of the link_check task from UserTaskStatus. + If no task has been started for the course, return 'Uninitiated'. + If link_check task was successful, an output result is also returned. + + For reference, the following status are in UserTaskStatus: + 'Pending', 'In Progress' (sent to frontend as 'In-Progress'), + 'Succeeded', 'Failed', 'Canceled', 'Retrying' + This function adds a status for when status from UserTaskStatus is None: + 'Uninitiated' **Example Request** + GET /api/contentstore/v0/link_check_status/{course_id} **Example Response** + ```json { "LinkCheckStatus": "Succeeded", "LinkCheckCreatedAt": "2025-02-05T14:32:01.294587Z", "LinkCheckOutput": { - sections: [ + "sections": [ { - id: , - displayName: , - subsections: [ + "id": , + "displayName": , + "subsections": [ { - id: , - displayName: , - units: [ + "id": , + "displayName": , + "units": [ { - id: , - displayName: , - blocks: [ + "id": , + "displayName": , + "blocks": [ { - id: , - url: , - brokenLinks: [ - , - , - , - ..., - ], - lockedLinks: [ - , - , - , - ..., - ], + "id": , + "url": , + "brokenLinks": [, ...], + "lockedLinks": [, ...], + "externalForbiddenLinks": [, ...], + "previousRunLinks": [, ...] }, { }, ], @@ -130,6 +126,42 @@ def get(self, request: Request, course_id: str): }, { }, ], + "course_updates": [ + { + "id": , + "displayName": , + "url": , + "brokenLinks": [, ...], + "lockedLinks": [, ...], + "externalForbiddenLinks": [, ...], + "previousRunLinks": [, ...] + }, + ..., + { }, + ..., + { + "id": , + "displayName": "handouts", + "url": , + "brokenLinks": [, ...], + "lockedLinks": [, ...], + "externalForbiddenLinks": [, ...], + "previousRunLinks": [, ...] + } + ], + "custom_pages": [ + { + "id": , + "displayName": , + "url": , + "brokenLinks": [, ...], + "lockedLinks": [, ...], + "externalForbiddenLinks": [, ...], + "previousRunLinks": [, ...] + }, + ..., + { }, + ] }, } """ diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/course_waffle_flags.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/course_waffle_flags.py index dca8e25cb435..3efb7b6226d4 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers/course_waffle_flags.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/course_waffle_flags.py @@ -30,6 +30,7 @@ class CourseWaffleFlagsSerializer(serializers.Serializer): enable_course_optimizer = serializers.SerializerMethodField() use_react_markdown_editor = serializers.SerializerMethodField() use_video_gallery_flow = serializers.SerializerMethodField() + enable_course_optimizer_check_prev_run_links = serializers.SerializerMethodField() def get_course_key(self): """ @@ -167,3 +168,10 @@ def get_use_video_gallery_flow(self, obj): Method to get the use_video_gallery_flow waffle flag """ return toggles.use_video_gallery_flow() + + def get_enable_course_optimizer_check_prev_run_links(self, obj): + """ + Method to get the enable_course_optimizer_check_prev_run_links waffle flag + """ + course_key = self.get_course_key() + return toggles.enable_course_optimizer_check_prev_run_links(course_key) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_waffle_flags.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_waffle_flags.py index ad5696834af2..f45cc48810d6 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_waffle_flags.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_waffle_flags.py @@ -1,6 +1,7 @@ """ Unit tests for the course waffle flags view """ + from django.urls import reverse from cms.djangoapps.contentstore import toggles @@ -13,28 +14,30 @@ class CourseWaffleFlagsViewTest(CourseTestCase): Basic test for the CourseWaffleFlagsView endpoint, which returns waffle flag states for a specific course or globally if no course ID is provided. """ + maxDiff = None # Show the whole dictionary in the diff defaults = { - 'enable_course_optimizer': False, - 'use_new_advanced_settings_page': True, - 'use_new_certificates_page': True, - 'use_new_course_outline_page': True, - 'use_new_course_team_page': True, - 'use_new_custom_pages': True, - 'use_new_export_page': True, - 'use_new_files_uploads_page': True, - 'use_new_grading_page': True, - 'use_new_group_configurations_page': True, - 'use_new_home_page': True, - 'use_new_import_page': True, - 'use_new_schedule_details_page': True, - 'use_new_textbooks_page': True, - 'use_new_unit_page': True, - 'use_new_updates_page': True, - 'use_new_video_uploads_page': False, - 'use_react_markdown_editor': False, - 'use_video_gallery_flow': False, + "enable_course_optimizer": False, + "use_new_advanced_settings_page": True, + "use_new_certificates_page": True, + "use_new_course_outline_page": True, + "use_new_course_team_page": True, + "use_new_custom_pages": True, + "use_new_export_page": True, + "use_new_files_uploads_page": True, + "use_new_grading_page": True, + "use_new_group_configurations_page": True, + "use_new_home_page": True, + "use_new_import_page": True, + "use_new_schedule_details_page": True, + "use_new_textbooks_page": True, + "use_new_unit_page": True, + "use_new_updates_page": True, + "use_new_video_uploads_page": False, + "use_react_markdown_editor": False, + "use_video_gallery_flow": False, + "enable_course_optimizer_check_prev_run_links": False, } def setUp(self): @@ -44,6 +47,11 @@ def setUp(self): course_id=self.course.id, enabled=True, ) + WaffleFlagCourseOverrideModel.objects.create( + waffle_flag=toggles.ENABLE_COURSE_OPTIMIZER_CHECK_PREV_RUN_LINKS.name, + course_id=self.course.id, + enabled=True, + ) def test_global_defaults(self): url = reverse("cms.djangoapps.contentstore:v1:course_waffle_flags") @@ -59,4 +67,5 @@ def test_course_override(self): assert response.data == { **self.defaults, "enable_course_optimizer": True, + "enable_course_optimizer_check_prev_run_links": True, } diff --git a/cms/djangoapps/contentstore/tasks.py b/cms/djangoapps/contentstore/tasks.py index 239fb89840b0..1b5c1061e78a 100644 --- a/cms/djangoapps/contentstore/tasks.py +++ b/cms/djangoapps/contentstore/tasks.py @@ -28,13 +28,13 @@ set_code_owner_attribute, set_code_owner_attribute_from_module, set_custom_attribute, - set_custom_attributes_for_course_key, + set_custom_attributes_for_course_key ) from olxcleaner.exceptions import ErrorLevel from olxcleaner.reporting import report_error_summary, report_errors from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey, UsageKey -from opaque_keys.edx.locator import LibraryLocator, LibraryContainerLocator +from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocator from organizations.api import add_organization_course, ensure_organization from organizations.exceptions import InvalidOrganizationException from organizations.models import Organization @@ -47,19 +47,23 @@ from cms.djangoapps.contentstore.courseware_index import ( CoursewareSearchIndexer, LibrarySearchIndexer, - SearchIndexingError, + SearchIndexingError ) from cms.djangoapps.contentstore.storage import course_import_export_storage +from cms.djangoapps.contentstore.toggles import enable_course_optimizer_check_prev_run_links from cms.djangoapps.contentstore.utils import ( IMPORTABLE_FILE_TYPES, + contains_previous_course_reference, + get_previous_run_course_key, create_or_update_xblock_upstream_link, delete_course, initialize_permissions, reverse_usage_url, - translation_language, + translation_language ) from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import get_block_info from cms.djangoapps.models.settings.course_metadata import CourseMetadata +from cms.djangoapps.contentstore.utils import create_course_info_usage_key from common.djangoapps.course_action_state.models import CourseRerunState from common.djangoapps.static_replace import replace_static_urls from common.djangoapps.student.auth import has_course_author_access @@ -75,6 +79,7 @@ from openedx.core.djangoapps.embargo.models import CountryAccessRule, RestrictedCourse from openedx.core.lib import ensure_cms from openedx.core.lib.extract_archive import safe_extractall +from openedx.core.lib.xblock_utils import get_course_update_items from xmodule.contentstore.django import contentstore from xmodule.course_block import CourseFields from xmodule.exceptions import SerializationError @@ -83,8 +88,9 @@ from xmodule.modulestore.exceptions import DuplicateCourseError, InvalidProctoringProvider, ItemNotFoundError from xmodule.modulestore.xml_exporter import export_course_to_xml, export_library_to_xml from xmodule.modulestore.xml_importer import CourseImportException, import_course_from_xml, import_library_from_xml +from xmodule.tabs import StaticTab -from .models import ContainerLink, LearningContextLinksStatus, LearningContextLinksStatusChoices, ComponentLink +from .models import ComponentLink, ContainerLink, LearningContextLinksStatus, LearningContextLinksStatusChoices from .outlines import update_outline_from_modulestore from .outlines_regenerate import CourseOutlineRegenerate from .toggles import bypass_olx_failure_enabled @@ -116,6 +122,7 @@ class LinkState: BROKEN = 'broken' LOCKED = 'locked' EXTERNAL_FORBIDDEN = 'external-forbidden' + PREVIOUS_RUN = 'previous-run' def clone_instance(instance, field_values): @@ -1137,7 +1144,8 @@ def check_broken_links(self, user_id, course_key_string, language): def _check_broken_links(task_instance, user_id, course_key_string, language): """ - Checks for broken links in a course and store the results in a file. + Checks for broken links in a course and stores the results in a file. + Also checks for previous run links if the feature is enabled. """ user = _validate_user(task_instance, user_id, language) @@ -1145,13 +1153,29 @@ def _check_broken_links(task_instance, user_id, course_key_string, language): course_key = CourseKey.from_string(course_key_string) url_list = _scan_course_for_links(course_key) - validated_url_list = asyncio.run(_validate_urls_access_in_batches(url_list, course_key, batch_size=100)) + previous_run_links = [] + urls_to_validate = url_list + + if enable_course_optimizer_check_prev_run_links(course_key): + previous_run_course_key = get_previous_run_course_key(course_key) + if previous_run_course_key: + + # Separate previous run links from regular links BEFORE validation + urls_to_validate = [] + for block_id, url in url_list: + if contains_previous_course_reference(url, previous_run_course_key): + previous_run_links.append([block_id, url, LinkState.PREVIOUS_RUN]) + else: + urls_to_validate.append([block_id, url]) + + validated_url_list = asyncio.run(_validate_urls_access_in_batches(urls_to_validate, course_key, batch_size=100)) broken_or_locked_urls, retry_list = _filter_by_status(validated_url_list) if retry_list: retry_results = _retry_validation(retry_list, course_key, retry_count=3) broken_or_locked_urls.extend(retry_results) + all_links = broken_or_locked_urls + previous_run_links try: task_instance.status.increment_completed_steps() @@ -1160,9 +1184,9 @@ def _check_broken_links(task_instance, user_id, course_key_string, language): LOGGER.debug(f'[Link Check] json file being generated at {broken_links_file.name}') with open(broken_links_file.name, 'w') as file: - json.dump(broken_or_locked_urls, file, indent=4) + json.dump(all_links, file, indent=4) - _write_broken_links_to_file(broken_or_locked_urls, broken_links_file) + _write_broken_links_to_file(all_links, broken_links_file) artifact = UserTaskArtifact(status=task_instance.status, name='BrokenLinks') _save_broken_links_file(artifact, broken_links_file) @@ -1186,7 +1210,8 @@ def _validate_user(task, user_id, language): def _scan_course_for_links(course_key): """ - Scans a course for links found in the data contents of blocks. + Scans a course for links found in the data contents of + blocks, course updates, handouts, and custom pages. Returns: list: block id and URL pairs @@ -1205,6 +1230,7 @@ def _scan_course_for_links(course_key): ) blocks = [] urls_to_validate = [] + course = modulestore().get_course(course_key) for vertical in verticals: blocks.extend(vertical.get_children()) @@ -1217,16 +1243,34 @@ def _scan_course_for_links(course_key): block_id = str(block.usage_key) block_info = get_block_info(block) block_data = block_info['data'] - url_list = _get_urls(block_data) + url_list = extract_content_URLs_from_course(block_data) urls_to_validate += [[block_id, url] for url in url_list] + course_updates_data = _scan_course_updates_for_links(course) + handouts_data = _scan_course_handouts_for_links(course) + custom_pages_data = _scan_custom_pages_for_links(course) + + for update in course_updates_data: + for url in update['urls']: + urls_to_validate.append([update['block_id'], url]) + + for handout in handouts_data: + for url in handout['urls']: + urls_to_validate.append([handout['block_id'], url]) + + for page in custom_pages_data: + for url in page['urls']: + urls_to_validate.append([page['block_id'], url]) + return urls_to_validate -def _get_urls(content): +def extract_content_URLs_from_course(content): """ Finds and returns a list of URLs in the given content. - Includes strings following 'href=' and 'src='. + Uses multiple regex patterns to find URLs in various contexts: + - URLs in href and src attributes + - Standalone URLs starting with http(s):// Excludes strings that are only '#' or start with 'data:'. Arguments: @@ -1235,11 +1279,129 @@ def _get_urls(content): Returns: list: urls """ - regex = r'\s+(?:href|src)=["\'](?!#|data:)([^"\']*)["\']' - url_list = re.findall(regex, content) + url_list = set() + + # Regex to match URLs in href and src attributes, or standalone URLs + regex = ( + r'(?:href|src)=["\'](?!#|data:)([^"\']+)["\']' + r'|(?:^|[\s\'"(<>])((?:https?://|http://|https://|www\.)[^\s\'")<>]+)(?=[\s\'")<>]|$)' + ) + + # Update list to include URLs found in the content + matches = re.findall(regex, content, re.IGNORECASE) + for match in matches: + url = match[0] or match[1] + if url: + url_list.add(url) + return url_list +def _scan_course_updates_for_links(course): + """ + Scans course updates for links. + + Returns: + list: course update data with links + """ + course_updates = [] + try: + store = modulestore() + usage_key = create_course_info_usage_key(course, "updates") + updates_block = store.get_item(usage_key) + + if updates_block and hasattr(updates_block, "data"): + update_items = get_course_update_items(updates_block) + + for update in update_items: + if update.get("status") != "deleted": + update_content = update.get("content", "") + url_list = extract_content_URLs_from_course(update_content) + + course_updates.append( + { + "displayName": update.get("date", "Unknown"), + "block_id": str(usage_key), + "urls": url_list, + } + ) + + return course_updates + + return course_updates + except Exception as e: # pylint: disable=broad-exception-caught + LOGGER.debug(f"Error scanning course updates: {e}") + return course_updates + + +def _scan_course_handouts_for_links(course): + """ + Scans course handouts for links. + + Returns: + list: handouts data with links + """ + + course_handouts = [] + try: + store = modulestore() + usage_key = create_course_info_usage_key(course, "handouts") + handouts_block = store.get_item(usage_key) + + if handouts_block and hasattr(handouts_block, "data") and handouts_block.data: + url_list = extract_content_URLs_from_course(handouts_block.data) + course_handouts.append( + {"name": "handouts", "block_id": str(usage_key), "urls": url_list} + ) + + return course_handouts + except Exception as e: # pylint: disable=broad-exception-caught + LOGGER.debug(f"Error scanning course handouts: {e}") + return course_handouts + + +def _scan_custom_pages_for_links(course): + """ + Scans custom pages (static tabs) for links. + + Returns: + list: custom pages data with links + """ + + custom_pages = [] + try: + store = modulestore() + course_key = course.id + + for tab in course.tabs: + if isinstance(tab, StaticTab): + try: + # Get the static tab content + static_tab_loc = course_key.make_usage_key( + "static_tab", tab.url_slug + ) + static_tab_block = store.get_item(static_tab_loc) + + if static_tab_block and hasattr(static_tab_block, "data"): + url_list = extract_content_URLs_from_course(static_tab_block.data) + + custom_pages.append( + { + "displayName": tab.name, + "block_id": str(static_tab_loc), + "urls": url_list, + } + ) + except Exception as e: # pylint: disable=broad-exception-caught + LOGGER.debug(f"Error scanning static tab {tab.name}: {e}") + continue + + return custom_pages + except Exception as e: # pylint: disable=broad-exception-caught + LOGGER.debug(f"Error scanning custom pages: {e}") + return custom_pages + + async def _validate_urls_access_in_batches(url_list, course_key, batch_size=100): """ Returns the statuses of a list of URL requests. diff --git a/cms/djangoapps/contentstore/tests/test_import.py b/cms/djangoapps/contentstore/tests/test_import.py index 1f6b393adca1..8106d552d0ef 100644 --- a/cms/djangoapps/contentstore/tests/test_import.py +++ b/cms/djangoapps/contentstore/tests/test_import.py @@ -37,6 +37,7 @@ class ContentStoreImportTest(ModuleStoreTestCase): Tests that rely on the toy and test_import_course courses. NOTE: refactor using CourseFactory so they do not. """ + def setUp(self): super().setUp() @@ -281,19 +282,27 @@ def test_video_components_present_while_import(self): @override_settings( COURSE_IMPORT_EXPORT_STORAGE="cms.djangoapps.contentstore.storage.ImportExportS3Storage", - DEFAULT_FILE_STORAGE="django.core.files.storage.FileSystemStorage" + STORAGES={ + 'default': { + 'BACKEND': "django.core.files.storage.FileSystemStorage" + } + } ) def test_resolve_default_storage(self): """ Ensure the default storage is invoked, even if course export storage is configured """ storage = resolve_storage_backend( storage_key="default", - legacy_setting_key="DEFAULT_FILE_STORAGE" + legacy_setting_key="STORAGES" ) self.assertEqual(storage.__class__.__name__, "FileSystemStorage") @override_settings( COURSE_IMPORT_EXPORT_STORAGE="cms.djangoapps.contentstore.storage.ImportExportS3Storage", - DEFAULT_FILE_STORAGE="django.core.files.storage.FileSystemStorage", + STORAGES={ + 'default': { + 'BACKEND': "django.core.files.storage.FileSystemStorage" + } + }, COURSE_IMPORT_EXPORT_BUCKET="bucket_name_test" ) def test_resolve_happy_path_storage(self): diff --git a/cms/djangoapps/contentstore/tests/test_tasks.py b/cms/djangoapps/contentstore/tests/test_tasks.py index 8634e7c6e5e0..5a76b7d67fe7 100644 --- a/cms/djangoapps/contentstore/tests/test_tasks.py +++ b/cms/djangoapps/contentstore/tests/test_tasks.py @@ -37,11 +37,11 @@ rerun_course, _validate_urls_access_in_batches, _filter_by_status, - _get_urls, _check_broken_links, _is_studio_url, _scan_course_for_links, - _convert_to_standard_url + _convert_to_standard_url, + extract_content_URLs_from_course ) logging = logging.getLogger(__name__) @@ -347,7 +347,7 @@ def test_hash_tags_stripped_from_url_lists(self): # Correct for the two carriage returns surrounding the ''' marks original_lines = len(url_list.splitlines()) - 2 - processed_url_list = _get_urls(url_list) + processed_url_list = extract_content_URLs_from_course(url_list) processed_lines = len(processed_url_list) assert processed_lines == original_lines - NUM_HASH_TAG_LINES, \ @@ -390,15 +390,15 @@ def test_course_scan_occurs_on_published_version(self, mock_modulestore, mock_mo revision=mock_module_store_enum.RevisionOption.published_only ) - @mock.patch('cms.djangoapps.contentstore.tasks._get_urls', autospec=True) - def test_number_of_scanned_blocks_equals_blocks_in_course(self, mock_get_urls): + @mock.patch('cms.djangoapps.contentstore.tasks.extract_content_URLs_from_course', autospec=True) + def test_number_of_scanned_blocks_equals_blocks_in_course(self, mockextract_content_URLs_from_course): """ - _scan_course_for_links should call _get_urls once per block in course. + _scan_course_for_links should call extract_content_URLs_from_course once per block in course. """ expected_blocks = self.store.get_items(self.test_course.id) _scan_course_for_links(self.test_course.id) - self.assertEqual(len(expected_blocks), mock_get_urls.call_count) + self.assertEqual(len(expected_blocks), mockextract_content_URLs_from_course.call_count) @mock.patch('cms.djangoapps.contentstore.tasks.get_block_info', autospec=True) @mock.patch('cms.djangoapps.contentstore.tasks.modulestore', autospec=True) @@ -644,8 +644,8 @@ def test_convert_to_standard_url(self): f"Failed for URL: {url}", ) - def test_get_urls(self): - """Test _get_urls function for correct URL extraction.""" + def test_extract_content_URLs_from_course(self): + """Test extract_content_URLs_from_course function for correct URL extraction.""" content = ''' Link @@ -667,4 +667,4 @@ def test_get_urls(self): "https://validsite.com", "https://another-valid.com" ] - self.assertEqual(_get_urls(content), expected) + self.assertEqual(extract_content_URLs_from_course(content), set(expected)) diff --git a/cms/djangoapps/contentstore/toggles.py b/cms/djangoapps/contentstore/toggles.py index 232bfc45d242..21d0b90c2313 100644 --- a/cms/djangoapps/contentstore/toggles.py +++ b/cms/djangoapps/contentstore/toggles.py @@ -659,3 +659,26 @@ def use_legacy_logged_out_home(): If not, then we should just go to the login page w/ redirect to studio course listing. """ return LEGACY_STUDIO_LOGGED_OUT_HOME.is_enabled() + + +# .. toggle_name: contentstore.enable_course_optimizer_check_prev_run_links +# .. toggle_implementation: CourseWaffleFlag +# .. toggle_default: False +# .. toggle_description: When enabled, allows the Course Optimizer to detect and update links pointing to previous course runs. +# This feature enables instructors to fix internal course links that still point to old course runs +# after creating a course rerun. +# .. toggle_use_cases: temporary +# .. toggle_creation_date: 2025-07-21 +# .. toggle_target_removal_date: None +ENABLE_COURSE_OPTIMIZER_CHECK_PREV_RUN_LINKS = CourseWaffleFlag( + f'{CONTENTSTORE_NAMESPACE}.enable_course_optimizer_check_prev_run_links', + __name__, + CONTENTSTORE_LOG_PREFIX, +) + + +def enable_course_optimizer_check_prev_run_links(course_key): + """ + Returns a boolean if previous run course optimizer feature is enabled for the given course. + """ + return ENABLE_COURSE_OPTIMIZER_CHECK_PREV_RUN_LINKS.is_enabled(course_key) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index c4049a818fe1..0562886acd3d 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -705,6 +705,13 @@ def get_sequence_usage_keys(course): for subsection in section.get_children()] +def create_course_info_usage_key(course, section_key): + """ + Returns the usage key for the specified section's course info block. + """ + return course.id.make_usage_key('course_info', section_key) + + def reverse_url(handler_name, key_name=None, key_value=None, kwargs=None): """ Creates the URL for the given handler. @@ -2435,3 +2442,33 @@ def create_or_update_xblock_upstream_link(xblock, course_key: CourseKey, created # It is possible that the upstream is a container and UsageKeyV2 parse failed # Create upstream container link and raise InvalidKeyError if xblock.upstream is a valid key. _create_or_update_container_link(course_key, created, xblock) + + +def get_previous_run_course_key(course_key): + """ + Retrieves the course key of the previous run for a given course. + """ + try: + rerun_state = CourseRerunState.objects.get(course_key=course_key) + except CourseRerunState.DoesNotExist: + log.warning(f'[Link Check] No rerun state found for course {course_key}. Cannot find previous run.') + return None + + return rerun_state.source_course_key + + +def contains_previous_course_reference(url, previous_course_key): + """ + Checks if a URL contains references to the previous course. + + Arguments: + url: The URL to check + previous_course_key: The previous course key to look for + + Returns: + bool: True if URL contains reference to previous course + """ + if not previous_course_key: + return False + + return str(previous_course_key).lower() in url.lower() diff --git a/cms/djangoapps/export_course_metadata/test_signals.py b/cms/djangoapps/export_course_metadata/test_signals.py index de3aaf6df232..fab0d1174f08 100644 --- a/cms/djangoapps/export_course_metadata/test_signals.py +++ b/cms/djangoapps/export_course_metadata/test_signals.py @@ -60,7 +60,11 @@ def test_happy_path(self, patched_content, patched_storage): @override_settings( COURSE_METADATA_EXPORT_STORAGE="cms.djangoapps.export_course_metadata.storage.CourseMetadataExportS3Storage", - DEFAULT_FILE_STORAGE="django.core.files.storage.FileSystemStorage" + STORAGES={ + 'default': { + 'BACKEND': "django.core.files.storage.FileSystemStorage" + } + } ) def test_resolve_default_storage(self): """ Ensure the default storage is invoked, even if course export storage is configured """ @@ -69,7 +73,11 @@ def test_resolve_default_storage(self): @override_settings( COURSE_METADATA_EXPORT_STORAGE="cms.djangoapps.export_course_metadata.storage.CourseMetadataExportS3Storage", - DEFAULT_FILE_STORAGE="django.core.files.storage.FileSystemStorage", + STORAGES={ + 'default': { + 'BACKEND': "django.core.files.storage.FileSystemStorage" + } + }, COURSE_METADATA_EXPORT_BUCKET="bucket_name_test" ) def test_resolve_happy_path_storage(self): diff --git a/cms/djangoapps/import_from_modulestore/data.py b/cms/djangoapps/import_from_modulestore/data.py index 998ea8dfc745..fad1b96b0ef1 100644 --- a/cms/djangoapps/import_from_modulestore/data.py +++ b/cms/djangoapps/import_from_modulestore/data.py @@ -16,7 +16,7 @@ class ImportStatus(TextChoices): NOT_STARTED = 'not_started', _('Waiting to stage content') STAGING = 'staging', _('Staging content for import') - STAGING_FAILED = _('Failed to stage content') + STAGING_FAILED = 'Failed to stage content', _('Failed to stage content') STAGED = 'staged', _('Content is staged and ready for import') IMPORTING = 'importing', _('Importing staged content') IMPORTING_FAILED = 'importing_failed', _('Failed to import staged content') diff --git a/cms/envs/common.py b/cms/envs/common.py index a6896160b630..932e300a5e03 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -1156,7 +1156,6 @@ 'YUI_BINARY': 'yui-compressor', } -STATICFILES_STORAGE = 'openedx.core.storage.ProductionStorage' STATICFILES_STORAGE_KWARGS = {} # List of finder classes that know how to find static files in various locations. @@ -2386,7 +2385,14 @@ BULK_EMAIL_LOG_SENT_EMAILS = False ############### Settings for django file storage ################## -DEFAULT_FILE_STORAGE = 'django.core.files.storage.FileSystemStorage' +STORAGES = { + 'default': { + 'BACKEND': 'django.core.files.storage.FileSystemStorage', + }, + 'staticfiles': { + 'BACKEND': 'openedx.core.storage.ProductionStorage', + }, +} ###################### Grade Downloads ###################### # These keys are used for all of our asynchronous downloadable files, including diff --git a/cms/envs/devstack.py b/cms/envs/devstack.py index 6ff0b0378960..bda53a366ca2 100644 --- a/cms/envs/devstack.py +++ b/cms/envs/devstack.py @@ -9,7 +9,7 @@ from .production import * # pylint: disable=wildcard-import, unused-wildcard-import # Don't use S3 in devstack, fall back to filesystem -del DEFAULT_FILE_STORAGE +STORAGES['default']['BACKEND'] = 'django.core.files.storage.FileSystemStorage' COURSE_IMPORT_EXPORT_STORAGE = 'django.core.files.storage.FileSystemStorage' USER_TASKS_ARTIFACT_STORAGE = COURSE_IMPORT_EXPORT_STORAGE @@ -56,7 +56,7 @@ # Skip packaging and optimization in development PIPELINE['PIPELINE_ENABLED'] = False -STATICFILES_STORAGE = 'openedx.core.storage.DevelopmentStorage' +STORAGES['staticfiles']['BACKEND'] = 'openedx.core.storage.DevelopmentStorage' # Revert to the default set of finders as we don't want the production pipeline STATICFILES_FINDERS = [ diff --git a/cms/envs/mock.yml b/cms/envs/mock.yml index 2338c68187cb..a390e28d6404 100644 --- a/cms/envs/mock.yml +++ b/cms/envs/mock.yml @@ -246,7 +246,7 @@ CROSS_DOMAIN_CSRF_COOKIE_DOMAIN: .localhost CROSS_DOMAIN_CSRF_COOKIE_NAME: csrftoken CSRF_COOKIE_SECURE: true CSRF_TRUSTED_ORIGINS: -- .localhost +- https://*.localhost CSRF_TRUSTED_ORIGINS_WITH_SCHEME: - https://*.localhost DATABASES: @@ -293,7 +293,9 @@ DATABASES: USER: user DATA_DIR: /edx/var/edxapp DEFAULT_FEEDBACK_EMAIL: feedback@example.com -DEFAULT_FILE_STORAGE: storages.backends.s3boto3.S3Boto3Storage +STORAGES: + default: + BACKEND: storages.backends.s3boto3.S3Boto3Storage DEFAULT_FROM_EMAIL: no-reply@registration.localhost DEFAULT_HASHING_ALGORITHM: sha256 DEFAULT_JWT_ISSUER: diff --git a/cms/envs/production.py b/cms/envs/production.py index 12c7daed66e6..f023656284b5 100644 --- a/cms/envs/production.py +++ b/cms/envs/production.py @@ -154,7 +154,7 @@ def get_env_setting(setting): # we need to run asset collection twice, once for local disk and once for S3. # Once we have migrated to service assets off S3, then we can convert this back to # managed by the yaml file contents -STATICFILES_STORAGE = os.environ.get('STATICFILES_STORAGE', STATICFILES_STORAGE) +STATICFILES_STORAGE = [] # just to run migrations CSRF_TRUSTED_ORIGINS = _YAML_TOKENS.get('CSRF_TRUSTED_ORIGINS_WITH_SCHEME', []) MKTG_URL_LINK_MAP.update(_YAML_TOKENS.get('MKTG_URL_LINK_MAP', {})) diff --git a/cms/envs/test.py b/cms/envs/test.py index deef2b8ff323..23131c699f91 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -29,7 +29,6 @@ from lms.envs.test import ( # pylint: disable=wrong-import-order, disable=unused-import ACCOUNT_MICROFRONTEND_URL, COMPREHENSIVE_THEME_DIRS, # unimport:skip - DEFAULT_FILE_STORAGE, ECOMMERCE_API_URL, ENABLE_COMPREHENSIVE_THEMING, JWT_AUTH, @@ -91,7 +90,7 @@ # If we don't add these settings, then Django templates that can't # find pipelined assets will raise a ValueError. # http://stackoverflow.com/questions/12816941/unit-testing-with-django-pipeline -STATICFILES_STORAGE = "pipeline.storage.NonPackagingPipelineStorage" +STORAGES['staticfiles']['BACKEND'] = "pipeline.storage.NonPackagingPipelineStorage" STATIC_URL = "/static/" # Update module store settings per defaults for tests diff --git a/common/djangoapps/static_replace/test/test_static_replace.py b/common/djangoapps/static_replace/test/test_static_replace.py index c71f8fa15c6a..e9c9d9346381 100644 --- a/common/djangoapps/static_replace/test/test_static_replace.py +++ b/common/djangoapps/static_replace/test/test_static_replace.py @@ -143,7 +143,6 @@ def test_mongo_filestore(mock_get_excluded_extensions, mock_get_base_url, mock_m mock_static_content.get_canonicalized_asset_path.assert_called_once_with(COURSE_KEY, 'file.png', '', ['foobar']) - @patch('common.djangoapps.static_replace.settings', autospec=True) @patch('xmodule.modulestore.django.modulestore', autospec=True) @patch('common.djangoapps.static_replace.staticfiles_storage', autospec=True) diff --git a/common/djangoapps/student/tests/test_api.py b/common/djangoapps/student/tests/test_api.py index 7cb20380cf9a..4757790e727f 100644 --- a/common/djangoapps/student/tests/test_api.py +++ b/common/djangoapps/student/tests/test_api.py @@ -88,7 +88,7 @@ def test_get_course_enrollments(self): result = get_course_enrollments(self.user) - self.assertQuerySetEqual(expected, result) + self.assertEqual(list(expected), list(result)) def test_get_filtered_course_enrollments(self): """Verify a filtered subset of enrollments can be retrieved""" @@ -99,4 +99,4 @@ def test_get_filtered_course_enrollments(self): result = get_course_enrollments(self.user, True, course_ids=[course_2.id]) - self.assertQuerySetEqual(expected, result) + self.assertEqual(list(expected), list(result)) diff --git a/common/djangoapps/third_party_auth/tests/utils.py b/common/djangoapps/third_party_auth/tests/utils.py index 29dc75e44fa5..8d1bafcdd887 100644 --- a/common/djangoapps/third_party_auth/tests/utils.py +++ b/common/djangoapps/third_party_auth/tests/utils.py @@ -57,7 +57,7 @@ def _create_client(self): client_type=Application.CLIENT_PUBLIC, ) - def _setup_provider_response(self, success=False, email=''): + def _setup_provider_response(self, success=False, email='', profile_data=None): """ Register a mock response for the third party user information endpoint; success indicates whether the response status code should be 200 or 400 @@ -67,6 +67,10 @@ def _setup_provider_response(self, success=False, email=''): response = {self.UID_FIELD: self.social_uid} if email: response.update({'email': email}) + + if profile_data: + response.update(profile_data) + body = json.dumps(response) else: status = 400 diff --git a/common/djangoapps/util/file.py b/common/djangoapps/util/file.py index b2892e6f42c9..f35dedf1e72e 100644 --- a/common/djangoapps/util/file.py +++ b/common/djangoapps/util/file.py @@ -78,7 +78,7 @@ def store_uploaded_file( file_storage = DefaultStorage() # If a file already exists with the supplied name, file_storage will make the filename unique. stored_file_name = file_storage.save(stored_file_name, uploaded_file) - if is_private and settings.DEFAULT_FILE_STORAGE == 'storages.backends.s3boto3.S3Boto3Storage': + if is_private and settings.STORAGES['default']['BACKEND'] == 'storages.backends.s3boto3.S3Boto3Storage': S3Boto3Storage().connection.meta.client.put_object_acl( ACL='private', Bucket=settings.AWS_STORAGE_BUCKET_NAME, @@ -155,6 +155,7 @@ class UniversalNewlineIterator: object which does not inherently support being read in universal-newline mode. It returns a line at a time. """ + def __init__(self, original_file, buffer_size=4096): self.original_file = original_file self.buffer_size = buffer_size diff --git a/common/djangoapps/util/storage.py b/common/djangoapps/util/storage.py index 37f908cd2799..5c3fbd2148e4 100644 --- a/common/djangoapps/util/storage.py +++ b/common/djangoapps/util/storage.py @@ -70,5 +70,5 @@ def resolve_storage_backend( break storage_path = storage_path.get(deep_setting_key) - StorageClass = import_string(storage_path or settings.DEFAULT_FILE_STORAGE) + StorageClass = import_string(storage_path or settings.STORAGES['default']['BACKEND']) return StorageClass(**options) diff --git a/lms/djangoapps/courseware/tests/helpers.py b/lms/djangoapps/courseware/tests/helpers.py index 2b5a33b2ac0f..1cd8cf89037c 100644 --- a/lms/djangoapps/courseware/tests/helpers.py +++ b/lms/djangoapps/courseware/tests/helpers.py @@ -138,11 +138,11 @@ def setUp(self): self.setup_course() self.initialize_module(metadata=self.METADATA, data=self.DATA) - def get_url(self, dispatch): + def get_url(self, dispatch, handler_name='xmodule_handler'): """Return item url with dispatch.""" return reverse( 'xblock_handler', - args=(str(self.course.id), quote_slashes(self.item_url), 'xmodule_handler', dispatch) + args=(str(self.course.id), quote_slashes(self.item_url), handler_name, dispatch) ) diff --git a/lms/djangoapps/courseware/tests/test_word_cloud.py b/lms/djangoapps/courseware/tests/test_word_cloud.py index 06217628cbca..8e225e1433fb 100644 --- a/lms/djangoapps/courseware/tests/test_word_cloud.py +++ b/lms/djangoapps/courseware/tests/test_word_cloud.py @@ -1,23 +1,40 @@ """Word cloud integration tests using mongo modulestore.""" - - -import pytest - +import importlib import json +import re from operator import itemgetter +from unittest.mock import patch +from uuid import UUID +import pytest +from django.conf import settings +from django.test import override_settings +from xblock import plugin + +from common.djangoapps.student.tests.factories import RequestFactoryNoCsrf +from xmodule import word_cloud_block # noinspection PyUnresolvedReferences -from xmodule.tests.helpers import override_descriptor_system # pylint: disable=unused-import +from xmodule.tests.helpers import override_descriptor_system, mock_render_template # pylint: disable=unused-import from xmodule.x_module import STUDENT_VIEW - from .helpers import BaseTestXmodule @pytest.mark.usefixtures("override_descriptor_system") -class TestWordCloud(BaseTestXmodule): +class _TestWordCloudBase(BaseTestXmodule): """Integration test for Word Cloud Block.""" + __test__ = False CATEGORY = "word_cloud" + @classmethod + def setUpClass(cls): + super().setUpClass() + plugin.PLUGIN_CACHE = {} + importlib.reload(word_cloud_block) + + def setUp(self): + super().setUp() + self.request_factory = RequestFactoryNoCsrf() + def _get_users_state(self): """Return current state for each user: @@ -27,7 +44,18 @@ def _get_users_state(self): users_state = {} for user in self.users: - response = self.clients[user.username].post(self.get_url('get_state')) + if settings.USE_EXTRACTED_WORD_CLOUD_BLOCK: + # The extracted Word Cloud XBlock uses @XBlock.json_handler, which expects a different + # request format and url pattern + handler_url = self.get_url('', handler_name='handle_get_state') + response = self.clients[user.username].post( + handler_url, + data=json.dumps({}), + content_type='application/json', + HTTP_X_REQUESTED_WITH='XMLHttpRequest', + ) + else: + response = self.clients[user.username].post(self.get_url('get_state')) users_state[user.username] = json.loads(response.content.decode('utf-8')) return users_state @@ -40,11 +68,22 @@ def _post_words(self, words): users_state = {} for user in self.users: - response = self.clients[user.username].post( - self.get_url('submit'), - {'student_words[]': words}, - HTTP_X_REQUESTED_WITH='XMLHttpRequest' - ) + if settings.USE_EXTRACTED_WORD_CLOUD_BLOCK: + # The extracted Word Cloud XBlock uses @XBlock.json_handler, which expects a different + # request format and url pattern + handler_url = self.get_url('', handler_name='handle_submit_state') + response = self.clients[user.username].post( + handler_url, + data=json.dumps({'student_words': words}), + content_type='application/json', + HTTP_X_REQUESTED_WITH='XMLHttpRequest', + ) + else: + response = self.clients[user.username].post( + self.get_url('submit'), + {'student_words[]': words}, + HTTP_X_REQUESTED_WITH='XMLHttpRequest' + ) users_state[user.username] = json.loads(response.content.decode('utf-8')) return users_state @@ -52,7 +91,6 @@ def _post_words(self, words): def _check_response(self, response_contents, correct_jsons): """Utility function that compares correct and real responses.""" for username, content in response_contents.items(): - # Used in debugger for comparing objects. # self.maxDiff = None @@ -120,7 +158,6 @@ def test_post_words(self): correct_state = {} for index, user in enumerate(self.users): - correct_state[user.username] = { 'status': 'success', 'submitted': True, @@ -202,6 +239,14 @@ def test_handle_ajax_incorrect_dispatch(self): for user in self.users } + if settings.USE_EXTRACTED_WORD_CLOUD_BLOCK: + # The extracted Word Cloud XBlock uses @XBlock.json_handler to handle AJAX requests, + # which automatically returns a 404 for unknown requests, so there's no need to test + # the incorrect dispatch case in this scenario. + for username, response in responses.items(): + self.assertEqual(response.status_code, 404) + return + status_codes = {response.status_code for response in responses.values()} assert status_codes.pop() == 200 @@ -214,19 +259,44 @@ def test_handle_ajax_incorrect_dispatch(self): } ) - def test_word_cloud_constructor(self): + @patch('xblock.utils.resources.ResourceLoader.render_django_template', side_effect=mock_render_template) + def test_word_cloud_constructor(self, mock_render_django_template): """ Make sure that all parameters extracted correctly from xml. """ fragment = self.runtime.render(self.block, STUDENT_VIEW) expected_context = { - 'ajax_url': self.block.ajax_url, 'display_name': self.block.display_name, 'instructions': self.block.instructions, - 'element_class': self.block.location.block_type, - 'element_id': self.block.location.html_id(), + 'element_class': self.block.scope_ids.block_type, 'num_inputs': 5, # default value 'submitted': False, # default value, } - assert fragment.content == self.runtime.render_template('word_cloud.html', expected_context) + if settings.USE_EXTRACTED_WORD_CLOUD_BLOCK: + # If `USE_EXTRACTED_WORD_CLOUD_BLOCK` is enabled, the `expected_context` will be different + # because in the extracted Word Cloud XBlock, the expected context: + # - contains `range_num_inputs` + # - uses `UUID` for `element_id` instead of `html_id()` + # - does not include `ajax_url` since it uses the `@XBlock.json_handler` decorator for AJAX requests + expected_context['range_num_inputs'] = range(5) + uuid_str = re.search(r"UUID\('([a-f0-9\-]+)'\)", fragment.content).group(1) + expected_context['element_id'] = UUID(uuid_str) + mock_render_django_template.assert_called_once() + # Remove i18n service + fragment_content_clean = re.sub(r"\{.*?}", "{}", fragment.content) + assert fragment_content_clean == self.runtime.render_template('templates/word_cloud.html', expected_context) + else: + expected_context['ajax_url'] = self.block.ajax_url + expected_context['element_id'] = self.block.location.html_id() + assert fragment.content == self.runtime.render_template('word_cloud.html', expected_context) + + +@override_settings(USE_EXTRACTED_WORD_CLOUD_BLOCK=True) +class TestWordCloudExtracted(_TestWordCloudBase): + __test__ = True + + +@override_settings(USE_EXTRACTED_WORD_CLOUD_BLOCK=False) +class TestWordCloudBuiltIn(_TestWordCloudBase): + __test__ = True diff --git a/lms/djangoapps/verify_student/management/commands/tests/test_manual_verify_student.py b/lms/djangoapps/verify_student/management/commands/tests/test_manual_verify_student.py index 5ce26eeec2b8..704a66dc7bfe 100644 --- a/lms/djangoapps/verify_student/management/commands/tests/test_manual_verify_student.py +++ b/lms/djangoapps/verify_student/management/commands/tests/test_manual_verify_student.py @@ -77,7 +77,7 @@ def test_manual_verifications_created_date(self): created_at__gte=earliest_allowed_verification_date() ) - self.assertQuerysetEqual(verification1, [repr(r) for r in verification2], transform=repr) + self.assertEqual(list(map(repr, verification1)), list(map(repr, verification2))) def test_user_doesnot_exist_log(self): """ diff --git a/lms/envs/common.py b/lms/envs/common.py index 316acdbdd200..66741aeacbdd 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2182,7 +2182,6 @@ 'UGLIFYJS_BINARY': 'node_modules/.bin/uglifyjs', } -STATICFILES_STORAGE = 'openedx.core.storage.ProductionStorage' STATICFILES_STORAGE_KWARGS = {} # List of finder classes that know how to find static files in various locations. @@ -4599,7 +4598,14 @@ } ############### Settings for django file storage ################## -DEFAULT_FILE_STORAGE = 'django.core.files.storage.FileSystemStorage' +STORAGES = { + 'default': { + 'BACKEND': 'django.core.files.storage.FileSystemStorage', + }, + 'staticfiles': { + 'BACKEND': 'openedx.core.storage.ProductionStorage', + }, +} ### Proctoring configuration (redirct URLs and keys shared between systems) #### PROCTORING_BACKENDS = { diff --git a/lms/envs/devstack.py b/lms/envs/devstack.py index 20bdba0d7de9..41eacec2d51e 100644 --- a/lms/envs/devstack.py +++ b/lms/envs/devstack.py @@ -17,7 +17,7 @@ from .production import * # pylint: disable=wildcard-import, unused-wildcard-import # Don't use S3 in devstack, fall back to filesystem -del DEFAULT_FILE_STORAGE +STORAGES['default']['BACKEND'] = 'django.core.files.storage.FileSystemStorage' ORA2_FILEUPLOAD_BACKEND = 'django' @@ -120,7 +120,7 @@ def should_show_debug_toolbar(request): # lint-amnesty, pylint: disable=missing ########################### PIPELINE ################################# PIPELINE['PIPELINE_ENABLED'] = False -STATICFILES_STORAGE = 'openedx.core.storage.DevelopmentStorage' +STORAGES['staticfiles']['BACKEND'] = 'openedx.core.storage.DevelopmentStorage' # Revert to the default set of finders as we don't want the production pipeline STATICFILES_FINDERS = [ diff --git a/lms/envs/mock.yml b/lms/envs/mock.yml index 0bcdf0e84b19..4dbbc37ea204 100644 --- a/lms/envs/mock.yml +++ b/lms/envs/mock.yml @@ -329,7 +329,7 @@ CROSS_DOMAIN_CSRF_COOKIE_DOMAIN: '' CROSS_DOMAIN_CSRF_COOKIE_NAME: '' CSRF_COOKIE_SECURE: true CSRF_TRUSTED_ORIGINS: -- .sandbox.localhost +- https://*.sandbox.localhost CSRF_TRUSTED_ORIGINS_WITH_SCHEME: - https://*.sandbox.localhost DASHBOARD_COURSE_LIMIT: 250 @@ -378,7 +378,9 @@ DATABASES: DATA_DIR: /edx/var/edxapp DEFAULT_COURSE_VISIBILITY_IN_CATALOG: both DEFAULT_FEEDBACK_EMAIL: feedback@example.com -DEFAULT_FILE_STORAGE: storages.backends.s3boto3.S3Boto3Storage +STORAGES: + default: + BACKEND: storages.backends.s3boto3.S3Boto3Storage DEFAULT_FROM_EMAIL: sandbox-notifications@example.com DEFAULT_HASHING_ALGORITHM: sha256 DEFAULT_JWT_ISSUER: diff --git a/lms/envs/production.py b/lms/envs/production.py index 835abc0dcfbd..9df0ab27630b 100644 --- a/lms/envs/production.py +++ b/lms/envs/production.py @@ -222,7 +222,7 @@ def get_env_setting(setting): # Change to S3Boto3 if we haven't specified another default storage AND we have specified AWS creds. if (not _YAML_TOKENS.get('DEFAULT_FILE_STORAGE')) and AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY: - DEFAULT_FILE_STORAGE = 'storages.backends.s3boto3.S3Boto3Storage' + STORAGES['default']['BACKEND'] = 'storages.backends.s3boto3.S3Boto3Storage' # The normal database user does not have enough permissions to run migrations. # Migrations are run with separate credentials, given as DB_MIGRATION_* diff --git a/lms/envs/test.py b/lms/envs/test.py index 2fb61b5dc5a9..c78604c0c1fb 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -151,7 +151,7 @@ # If we don't add these settings, then Django templates that can't # find pipelined assets will raise a ValueError. # http://stackoverflow.com/questions/12816941/unit-testing-with-django-pipeline -STATICFILES_STORAGE = 'pipeline.storage.NonPackagingPipelineStorage' +STORAGES['staticfiles']['BACKEND'] = 'pipeline.storage.NonPackagingPipelineStorage' # Don't use compression during tests PIPELINE['JS_COMPRESSOR'] = None @@ -298,7 +298,7 @@ ]) ############################ STATIC FILES ############################# -DEFAULT_FILE_STORAGE = 'django.core.files.storage.FileSystemStorage' +STORAGES['default']['BACKEND'] = 'django.core.files.storage.FileSystemStorage' MEDIA_ROOT = TEST_ROOT / "uploads" MEDIA_URL = "/uploads/" STATICFILES_DIRS.append(("uploads", MEDIA_ROOT)) diff --git a/openedx/core/djangoapps/enrollments/tests/test_services.py b/openedx/core/djangoapps/enrollments/tests/test_services.py index 46cdf057dc25..1d976f919f6c 100644 --- a/openedx/core/djangoapps/enrollments/tests/test_services.py +++ b/openedx/core/djangoapps/enrollments/tests/test_services.py @@ -97,7 +97,8 @@ def test_get_enrollments_can_take_proctored_exams_by_course(self): {'username': 'user4', 'mode': 'professional'}, {'username': 'user5', 'mode': 'verified'} ] - self.assertQuerysetEqual(enrollments, expected_values, self.enrollment_to_dict) + actual_values = [self.enrollment_to_dict(e) for e in enrollments] + self.assertEqual(actual_values, expected_values) def test_get_enrollments_can_take_proctored_exams_by_course_ignore_inactive(self): """ @@ -141,7 +142,8 @@ def test_get_enrollments_can_take_proctored_exams_allow_honor(self): {'username': 'user5', 'mode': 'verified'} ] - self.assertQuerysetEqual(enrollments, expected_values, self.enrollment_to_dict) + actual_values = [self.enrollment_to_dict(e) for e in enrollments] + self.assertEqual(actual_values, expected_values) def test_get_enrollments_can_take_proctored_exams_not_enable_proctored_exams(self): self.course.enable_proctored_exams = False @@ -180,7 +182,8 @@ def test_text_search_partial_return_some(self): expected_values = [ {'username': 'user3', 'mode': 'masters'} ] - self.assertQuerysetEqual(enrollments, expected_values, self.enrollment_to_dict) + actual_values = [self.enrollment_to_dict(e) for e in enrollments] + self.assertEqual(actual_values, expected_values) @ddt.data('user1', 'USER1', 'LEARNER1@example.com', 'lEarNer1@eXAMPLE.com') def test_text_search_exact_return_one(self, text_search): @@ -192,7 +195,8 @@ def test_text_search_exact_return_one(self, text_search): expected_values = [ {'username': 'user1', 'mode': 'executive-education'} ] - self.assertQuerysetEqual(enrollments, expected_values, self.enrollment_to_dict) + actual_values = [self.enrollment_to_dict(e) for e in enrollments] + self.assertEqual(actual_values, expected_values) def test_text_search_return_none(self): enrollments = self.service.get_enrollments_can_take_proctored_exams( diff --git a/openedx/core/djangoapps/oauth_dispatch/jwt.py b/openedx/core/djangoapps/oauth_dispatch/jwt.py index de493715a713..e6dcc8038f3d 100644 --- a/openedx/core/djangoapps/oauth_dispatch/jwt.py +++ b/openedx/core/djangoapps/oauth_dispatch/jwt.py @@ -80,7 +80,7 @@ def create_jwt_token_dict(token_dict, oauth_adapter, use_asymmetric_key=None): # .. custom_attribute_name: create_jwt_grant_type # .. custom_attribute_description: The grant type of the newly created JWT. set_custom_attribute('create_jwt_grant_type', grant_type) - scopes = _get_updated_scopes(token_dict['scope'].split(' '), grant_type) + scopes = _get_updated_scopes(token_dict['scope'].split(), grant_type) jwt_access_token = _create_jwt( access_token.user, diff --git a/openedx/core/djangoapps/oauth_dispatch/tests/test_views.py b/openedx/core/djangoapps/oauth_dispatch/tests/test_views.py index ef03dab0ac8a..06031c37a1da 100644 --- a/openedx/core/djangoapps/oauth_dispatch/tests/test_views.py +++ b/openedx/core/djangoapps/oauth_dispatch/tests/test_views.py @@ -422,7 +422,8 @@ def _test_jwt_access_token(self, client_attr, token_type=None, headers=None, gra """ client = getattr(self, client_attr) self.oauth_client = client - self._setup_provider_response(success=True) + profile_data = {'given_name': self.user.first_name, 'family_name': self.user.last_name} + self._setup_provider_response(success=True, profile_data=profile_data) response = self._post_request(self.user, client, token_type=token_type, headers=headers or {}, asymmetric_jwt=asymmetric_jwt) assert response.status_code == 200 @@ -451,7 +452,8 @@ def test_access_token_exchange_calls_dispatched_view(self, client_attr): def test_jwt_access_token_exchange_calls_dispatched_view(self, client_attr): client = getattr(self, client_attr) self.oauth_client = client - self._setup_provider_response(success=True) + profile_data = {'given_name': self.user.first_name, 'family_name': self.user.last_name} + self._setup_provider_response(success=True, profile_data=profile_data) response = self._post_request(self.user, client, token_type='jwt') assert response.status_code == 200 data = json.loads(response.content.decode('utf-8')) @@ -470,7 +472,8 @@ def test_jwt_access_token_exchange_calls_dispatched_view(self, client_attr): def test_asymmetric_jwt_access_token_exchange_calls_dispatched_view(self, client_attr): client = getattr(self, client_attr) self.oauth_client = client - self._setup_provider_response(success=True) + profile_data = {'given_name': self.user.first_name, 'family_name': self.user.last_name} + self._setup_provider_response(success=True, profile_data=profile_data) response = self._post_request(self.user, client, token_type='jwt', asymmetric_jwt=True) assert response.status_code == 200 data = json.loads(response.content.decode('utf-8')) diff --git a/openedx/core/djangoapps/theming/tests/test_views.py b/openedx/core/djangoapps/theming/tests/test_views.py index 5b5503702cd8..8e8c6393baf3 100644 --- a/openedx/core/djangoapps/theming/tests/test_views.py +++ b/openedx/core/djangoapps/theming/tests/test_views.py @@ -100,7 +100,13 @@ def test_asset_no_theme(self): assert response.status_code == 302 assert response.url == "/static/images/logo.png" - @override_settings(STATICFILES_STORAGE="openedx.core.storage.DevelopmentStorage") + @override_settings( + STORAGES={ + "staticfiles": { + "BACKEND": "openedx.core.storage.DevelopmentStorage" + } + } + ) def test_asset_with_theme(self): """ Fetch theme asset when a theme is set. diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py index 466e1e278abd..36a7312821ac 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -262,6 +262,7 @@ class TestCancelAccountRetirementStatusView(UserAPITestCase): """ Unit tests for CancelAccountRetirementStatusView """ + def setUp(self): super().setUp() permission = PermissionFactory( @@ -1147,7 +1148,7 @@ def test_patch_invalid_language_proficiencies(self, patch_value, expected_error_ client = self.login_client("client", "user") response = self.send_patch(client, {"language_proficiencies": patch_value}, expected_status=400) assert response.data['field_errors']['language_proficiencies']['developer_message'] == \ - f"Value '{patch_value}' is not valid for field 'language_proficiencies': {expected_error_message}" + f"Value '{patch_value}' is not valid for field 'language_proficiencies': {expected_error_message}" @mock.patch('openedx.core.djangoapps.user_api.accounts.serializers.AccountUserSerializer.save') def test_patch_serializer_save_fails(self, serializer_save): @@ -1201,8 +1202,8 @@ def test_convert_relative_profile_url(self): self.client.login(username=self.user.username, password=TEST_PASSWORD) response = self.send_get(self.client) assert response.data['profile_image'] == \ - {'has_image': False, - 'image_url_full': 'http://testserver/static/default_50.png', + {'has_image': False, + 'image_url_full': 'http://testserver/static/default_50.png', 'image_url_small': 'http://testserver/static/default_10.png'} @override_settings( @@ -1231,7 +1232,6 @@ def test_profile_backend_with_profile_image_settings(self): ) def test_profile_backend_with_default_hardcoded_backend(self): """ In case of empty storages scenario uses the hardcoded backend.""" - del settings.DEFAULT_FILE_STORAGE del settings.STORAGES storage = get_profile_image_storage() self.assertIsInstance(storage, FileSystemStorage) diff --git a/openedx/core/djangoapps/util/management/commands/dump_settings.py b/openedx/core/djangoapps/util/management/commands/dump_settings.py index 004f83a91cef..a7d7765ed9cf 100644 --- a/openedx/core/djangoapps/util/management/commands/dump_settings.py +++ b/openedx/core/djangoapps/util/management/commands/dump_settings.py @@ -7,7 +7,7 @@ from django.conf import settings from django.core.management.base import BaseCommand - +from django.utils.functional import Promise SETTING_NAME_REGEX = re.compile(r'^[A-Z][A-Z0-9_]*$') @@ -78,10 +78,11 @@ def _to_json_friendly_repr(value: object) -> object: if not isinstance(subkey, (str, int)): raise ValueError(f"Unexpected dict key {subkey} of type {type(subkey)}") return {subkey: _to_json_friendly_repr(subval) for subkey, subval in value.items()} - if proxy_args := getattr(value, "_proxy____args", None): - if len(proxy_args) == 1 and isinstance(proxy_args[0], str): - # Print gettext_lazy as simply the wrapped string - return proxy_args[0] + + # Directly convert Promise objects (gettext_lazy) to their string representation + if isinstance(value, Promise): + return str(value) + try: module = value.__module__ qualname = value.__qualname__ diff --git a/openedx/core/djangoapps/util/tests/test_dump_settings.py b/openedx/core/djangoapps/util/tests/test_dump_settings.py index 90171eb48c95..830d2c0be280 100644 --- a/openedx/core/djangoapps/util/tests/test_dump_settings.py +++ b/openedx/core/djangoapps/util/tests/test_dump_settings.py @@ -10,7 +10,7 @@ from django.core.management import call_command -from openedx.core.djangolib.testing.utils import skip_unless_lms, skip_unless_cms +from openedx.core.djangolib.testing.utils import skip_unless_cms, skip_unless_lms @skip_unless_lms diff --git a/openedx/core/storage.py b/openedx/core/storage.py index 9e7e52d94c17..c87a0ceaa969 100644 --- a/openedx/core/storage.py +++ b/openedx/core/storage.py @@ -20,6 +20,7 @@ class PipelineForgivingMixin: """ An extension of the django-pipeline storage backend which forgives missing files. """ + def hashed_name(self, name, content=None, **kwargs): # lint-amnesty, pylint: disable=missing-function-docstring try: out = super().hashed_name(name, content, **kwargs) # lint-amnesty, pylint: disable=super-with-arguments @@ -53,8 +54,9 @@ class ProductionMixin( can be applied over an existing Storage. We use this version on production. """ + def __init__(self, *args, **kwargs): - kwargs.update(settings.STATICFILES_STORAGE_KWARGS.get(settings.STATICFILES_STORAGE, {})) + kwargs.update(settings.STATICFILES_STORAGE_KWARGS.get(settings.STORAGES['staticfiles']['BACKEND'], {})) super().__init__(*args, **kwargs) # lint-amnesty, pylint: disable=super-with-arguments diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 97437fa8cde8..d907ba1e459f 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -51,7 +51,7 @@ django-stubs<6 # The team that owns this package will manually bump this package rather than having it pulled in automatically. # This is to allow them to better control its deployment and to do it in a process that works better # for them. -edx-enterprise==6.2.13 +edx-enterprise==6.2.17 # Date: 2023-07-26 # Our legacy Sass code is incompatible with anything except this ancient libsass version. @@ -83,6 +83,11 @@ openai<=0.28.1 # Issue for unpinning: https://github.com/openedx/edx-platform/issues/35267 path<16.12.0 +# Date: 2025-05-11 +# Broke lxml[html_clean] extra dependency declaration +# Issue for unpinning: https://github.com/openedx/edx-platform/issues/37168 +pip-tools<7.5.0 + # Date: 2022-08-03 # pycodestyle==2.9.0 generates false positive error E275. # Constraint can be removed once the issue https://github.com/PyCQA/pycodestyle/issues/1090 is fixed. diff --git a/requirements/edx-sandbox/base.txt b/requirements/edx-sandbox/base.txt index 7a3695ba37b2..8ad2b9e19158 100644 --- a/requirements/edx-sandbox/base.txt +++ b/requirements/edx-sandbox/base.txt @@ -22,7 +22,7 @@ fonttools==4.59.0 # via matplotlib joblib==1.5.1 # via nltk -kiwisolver==1.4.8 +kiwisolver==1.4.9 # via matplotlib lxml[html-clean]==5.3.2 # via diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 907e7320304f..1c08307bdf4d 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -118,7 +118,7 @@ cffi==1.17.1 # snowflake-connector-python chardet==5.2.0 # via pysrt -charset-normalizer==3.4.2 +charset-normalizer==3.4.3 # via # requests # snowflake-connector-python @@ -191,6 +191,7 @@ django==5.2.5 # django-push-notifications # django-sekizai # django-ses + # django-simple-history # django-statici18n # django-storages # django-user-tasks @@ -476,7 +477,7 @@ edx-drf-extensions==10.6.0 # edxval # enterprise-integrated-channels # openedx-learning -edx-enterprise==6.2.13 +edx-enterprise==6.2.17 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in @@ -511,7 +512,7 @@ edx-opaque-keys[django]==3.0.0 # openedx-filters # ora2 # xblocks-contrib -edx-organizations==7.1.0 +edx-organizations==7.2.1 # via -r requirements/edx/kernel.in edx-proctoring==5.2.0 # via @@ -780,7 +781,7 @@ mpmath==1.3.0 # via sympy msgpack==1.1.1 # via cachecontrol -multidict==6.6.3 +multidict==6.6.4 # via # aiohttp # yarl @@ -1285,7 +1286,7 @@ xblock-utils==4.0.0 # via # edx-sga # xblock-poll -xblocks-contrib==0.4.0 +xblocks-contrib==0.5.0 # via -r requirements/edx/bundled.in xmlsec==1.3.14 # via diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 316ba8c1cf71..f38603cfbc03 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -216,7 +216,7 @@ chardet==5.2.0 # diff-cover # pysrt # tox -charset-normalizer==3.4.2 +charset-normalizer==3.4.3 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -359,6 +359,7 @@ django==5.2.5 # django-push-notifications # django-sekizai # django-ses + # django-simple-history # django-statici18n # django-storages # django-stubs @@ -560,7 +561,7 @@ django-ses==4.4.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -django-simple-history==3.1.1 +django-simple-history==3.8.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -750,7 +751,7 @@ edx-drf-extensions==10.6.0 # edxval # enterprise-integrated-channels # openedx-learning -edx-enterprise==6.2.13 +edx-enterprise==6.2.17 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt @@ -798,7 +799,7 @@ edx-opaque-keys[django]==3.0.0 # openedx-filters # ora2 # xblocks-contrib -edx-organizations==7.1.0 +edx-organizations==7.2.1 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -1300,7 +1301,7 @@ msgpack==1.1.1 # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # cachecontrol -multidict==6.6.3 +multidict==6.6.4 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -1617,7 +1618,7 @@ pylatexenc==2.10 # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # olxcleaner -pylint==3.3.7 +pylint==3.3.8 # via # -r requirements/edx/testing.txt # edx-lint @@ -2115,11 +2116,11 @@ tqdm==4.67.1 # -r requirements/edx/testing.txt # nltk # openai -types-pyyaml==6.0.12.20250516 +types-pyyaml==6.0.12.20250809 # via # django-stubs # djangorestframework-stubs -types-requests==2.32.4.20250611 +types-requests==2.32.4.20250809 # via djangorestframework-stubs typing-extensions==4.14.1 # via @@ -2290,7 +2291,7 @@ xblock-utils==4.0.0 # -r requirements/edx/testing.txt # edx-sga # xblock-poll -xblocks-contrib==0.4.0 +xblocks-contrib==0.5.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index d75bac493963..4ff587ecb6bf 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -162,7 +162,7 @@ chardet==5.2.0 # via # -r requirements/edx/base.txt # pysrt -charset-normalizer==3.4.2 +charset-normalizer==3.4.3 # via # -r requirements/edx/base.txt # requests @@ -249,6 +249,7 @@ django==5.2.5 # django-push-notifications # django-sekizai # django-ses + # django-simple-history # django-statici18n # django-storages # django-user-tasks @@ -413,7 +414,7 @@ django-sekizai==4.1.0 # openedx-django-wiki django-ses==4.4.0 # via -r requirements/edx/base.txt -django-simple-history==3.1.1 +django-simple-history==3.8.0 # via # -r requirements/edx/base.txt # edx-enterprise @@ -560,7 +561,7 @@ edx-drf-extensions==10.6.0 # edxval # enterprise-integrated-channels # openedx-learning -edx-enterprise==6.2.13 +edx-enterprise==6.2.17 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt @@ -595,7 +596,7 @@ edx-opaque-keys[django]==3.0.0 # openedx-filters # ora2 # xblocks-contrib -edx-organizations==7.1.0 +edx-organizations==7.2.1 # via -r requirements/edx/base.txt edx-proctoring==5.2.0 # via @@ -950,7 +951,7 @@ msgpack==1.1.1 # via # -r requirements/edx/base.txt # cachecontrol -multidict==6.6.3 +multidict==6.6.4 # via # -r requirements/edx/base.txt # aiohttp @@ -1616,7 +1617,7 @@ xblock-utils==4.0.0 # -r requirements/edx/base.txt # edx-sga # xblock-poll -xblocks-contrib==0.4.0 +xblocks-contrib==0.5.0 # via -r requirements/edx/base.txt xmlsec==1.3.14 # via diff --git a/requirements/edx/semgrep.txt b/requirements/edx/semgrep.txt index 4ae928fa1e50..f56582219b35 100644 --- a/requirements/edx/semgrep.txt +++ b/requirements/edx/semgrep.txt @@ -19,7 +19,7 @@ bracex==2.6 # via wcmatch certifi==2025.8.3 # via requests -charset-normalizer==3.4.2 +charset-normalizer==3.4.3 # via requests click==8.1.8 # via @@ -51,7 +51,7 @@ jsonschema==4.25.0 # via semgrep jsonschema-specifications==2025.4.1 # via jsonschema -markdown-it-py==3.0.0 +markdown-it-py==4.0.0 # via rich mdurl==0.1.2 # via markdown-it-py diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 46434e3e78f1..6c74d48a3e7c 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -164,7 +164,7 @@ chardet==5.2.0 # diff-cover # pysrt # tox -charset-normalizer==3.4.2 +charset-normalizer==3.4.3 # via # -r requirements/edx/base.txt # requests @@ -275,6 +275,7 @@ django==5.2.5 # django-push-notifications # django-sekizai # django-ses + # django-simple-history # django-statici18n # django-storages # django-user-tasks @@ -439,7 +440,7 @@ django-sekizai==4.1.0 # openedx-django-wiki django-ses==4.4.0 # via -r requirements/edx/base.txt -django-simple-history==3.1.1 +django-simple-history==3.8.0 # via # -r requirements/edx/base.txt # edx-enterprise @@ -581,7 +582,7 @@ edx-drf-extensions==10.6.0 # edxval # enterprise-integrated-channels # openedx-learning -edx-enterprise==6.2.13 +edx-enterprise==6.2.17 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt @@ -618,7 +619,7 @@ edx-opaque-keys[django]==3.0.0 # openedx-filters # ora2 # xblocks-contrib -edx-organizations==7.1.0 +edx-organizations==7.2.1 # via -r requirements/edx/base.txt edx-proctoring==5.2.0 # via @@ -996,7 +997,7 @@ msgpack==1.1.1 # via # -r requirements/edx/base.txt # cachecontrol -multidict==6.6.3 +multidict==6.6.4 # via # -r requirements/edx/base.txt # aiohttp @@ -1230,7 +1231,7 @@ pylatexenc==2.10 # via # -r requirements/edx/base.txt # olxcleaner -pylint==3.3.7 +pylint==3.3.8 # via # edx-lint # pylint-celery @@ -1699,7 +1700,7 @@ xblock-utils==4.0.0 # -r requirements/edx/base.txt # edx-sga # xblock-poll -xblocks-contrib==0.4.0 +xblocks-contrib==0.5.0 # via -r requirements/edx/base.txt xmlsec==1.3.14 # via diff --git a/scripts/user_retirement/requirements/base.txt b/scripts/user_retirement/requirements/base.txt index 8f34fa527bae..d17060c455b4 100644 --- a/scripts/user_retirement/requirements/base.txt +++ b/scripts/user_retirement/requirements/base.txt @@ -24,7 +24,7 @@ cffi==1.17.1 # via # cryptography # pynacl -charset-normalizer==3.4.2 +charset-normalizer==3.4.3 # via requests click==8.2.1 # via diff --git a/scripts/user_retirement/requirements/testing.txt b/scripts/user_retirement/requirements/testing.txt index 29ec1cf8a42f..646fe4450e11 100644 --- a/scripts/user_retirement/requirements/testing.txt +++ b/scripts/user_retirement/requirements/testing.txt @@ -37,7 +37,7 @@ cffi==1.17.1 # -r scripts/user_retirement/requirements/base.txt # cryptography # pynacl -charset-normalizer==3.4.2 +charset-normalizer==3.4.3 # via # -r scripts/user_retirement/requirements/base.txt # requests @@ -225,7 +225,7 @@ requests-toolbelt==1.0.0 # via # -r scripts/user_retirement/requirements/base.txt # zeep -responses==0.25.7 +responses==0.25.8 # via # -r scripts/user_retirement/requirements/testing.in # moto diff --git a/scripts/xblock/requirements.txt b/scripts/xblock/requirements.txt index 3cb259160b78..b45b71916c4c 100644 --- a/scripts/xblock/requirements.txt +++ b/scripts/xblock/requirements.txt @@ -6,7 +6,7 @@ # certifi==2025.8.3 # via requests -charset-normalizer==3.4.2 +charset-normalizer==3.4.3 # via requests idna==3.10 # via requests diff --git a/xmodule/tests/test_word_cloud.py b/xmodule/tests/test_word_cloud.py index 9fbd02a612db..bc3f18a83c54 100644 --- a/xmodule/tests/test_word_cloud.py +++ b/xmodule/tests/test_word_cloud.py @@ -1,38 +1,48 @@ """Test for Word Cloud Block functional logic.""" - import json +import os from unittest.mock import Mock +from django.conf import settings from django.test import TestCase +from django.test import override_settings from fs.memoryfs import MemoryFS from lxml import etree -from webob import Request from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator +from webob import Request from webob.multidict import MultiDict from xblock.field_data import DictFieldData +from xblock.fields import ScopeIds -from xmodule.word_cloud_block import WordCloudBlock +from xmodule import word_cloud_block from . import get_test_descriptor_system, get_test_system -class WordCloudBlockTest(TestCase): +class _TestWordCloudBase(TestCase): """ Logic tests for Word Cloud Block. """ - - raw_field_data = { - 'all_words': {'cat': 10, 'dog': 5, 'mom': 1, 'dad': 2}, - 'top_words': {'cat': 10, 'dog': 5, 'dad': 2}, - 'submitted': False, - 'display_name': 'Word Cloud Block', - 'instructions': 'Enter some random words that comes to your mind' - } + __test__ = False + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.word_cloud_class = word_cloud_block.reset_class() + + def setUp(self): + super().setUp() + self.raw_field_data = { + 'all_words': {'cat': 10, 'dog': 5, 'mom': 1, 'dad': 2}, + 'top_words': {'cat': 10, 'dog': 5, 'dad': 2}, + 'submitted': False, + 'display_name': 'Word Cloud Block', + 'instructions': 'Enter some random words that comes to your mind' + } def test_xml_import_export_cycle(self): """ Test the import export cycle. """ - runtime = get_test_descriptor_system() runtime.export_fs = MemoryFS() @@ -43,7 +53,11 @@ def test_xml_import_export_cycle(self): olx_element = etree.fromstring(original_xml) runtime.id_generator = Mock() - block = WordCloudBlock.parse_xml(olx_element, runtime, None) + + def_id = runtime.id_generator.create_definition(olx_element.tag, olx_element.get('url_name')) + keys = ScopeIds(None, olx_element.tag, def_id, runtime.id_generator.create_usage(def_id)) + block = self.word_cloud_class.parse_xml(olx_element, runtime, keys) + block.location = BlockUsageLocator( CourseLocator('org', 'course', 'run', branch='revision'), 'word_cloud', 'block_id' ) @@ -54,38 +68,70 @@ def test_xml_import_export_cycle(self): assert block.num_inputs == 3 assert block.num_top_words == 100 - node = etree.Element("unknown_root") - # This will export the olx to a separate file. - block.add_xml_to_node(node) + if settings.USE_EXTRACTED_WORD_CLOUD_BLOCK: + # For extracted XBlocks, we need to manually export the XML definition to a file to properly test the + # import/export cycle. This is because extracted XBlocks use XBlock core's `add_xml_to_node` method, + # which does not export the XML to a file like `XmlMixin.add_xml_to_node` does. + filepath = 'word_cloud/block_id.xml' + runtime.export_fs.makedirs(os.path.dirname(filepath), recreate=True) + with runtime.export_fs.open(filepath, 'wb') as fileObj: + runtime.export_to_xml(block, fileObj) + else: + node = etree.Element("unknown_root") + # This will export the olx to a separate file. + block.add_xml_to_node(node) + with runtime.export_fs.open('word_cloud/block_id.xml') as f: exported_xml = f.read() + if settings.USE_EXTRACTED_WORD_CLOUD_BLOCK: + # For extracted XBlocks, we need to remove the `xblock-family` attribute from the exported XML to ensure + # consistency with the original XML. + # This is because extracted XBlocks use the core XBlock's `add_xml_to_node` method, which includes this + # attribute, whereas `XmlMixin.add_xml_to_node` does not. + exported_xml_tree = etree.fromstring(exported_xml.encode('utf-8')) + etree.cleanup_namespaces(exported_xml_tree) + if 'xblock-family' in exported_xml_tree.attrib: + del exported_xml_tree.attrib['xblock-family'] + exported_xml = etree.tostring(exported_xml_tree, encoding='unicode', pretty_print=True) + assert exported_xml == original_xml def test_bad_ajax_request(self): """ Make sure that answer for incorrect request is error json. """ - module_system = get_test_system() - block = WordCloudBlock(module_system, DictFieldData(self.raw_field_data), Mock()) - - response = json.loads(block.handle_ajax('bad_dispatch', {})) - self.assertDictEqual(response, { - 'status': 'fail', - 'error': 'Unknown Command!' - }) + block = self.word_cloud_class(module_system, DictFieldData(self.raw_field_data), Mock()) + + if settings.USE_EXTRACTED_WORD_CLOUD_BLOCK: + # The extracted Word Cloud XBlock uses @XBlock.json_handler for handling AJAX requests, + # which requires a different way of method invocation. + with self.assertRaises(AttributeError) as context: + json.loads(block.bad_dispatch('bad_dispatch', {})) + self.assertIn("'WordCloudBlock' object has no attribute 'bad_dispatch'", str(context.exception)) + else: + response = json.loads(block.handle_ajax('bad_dispatch', {})) + self.assertDictEqual(response, { + 'status': 'fail', + 'error': 'Unknown Command!' + }) def test_good_ajax_request(self): """ Make sure that ajax request works correctly. """ - module_system = get_test_system() - block = WordCloudBlock(module_system, DictFieldData(self.raw_field_data), Mock()) - - post_data = MultiDict(('student_words[]', word) for word in ['cat', 'cat', 'dog', 'sun']) - response = json.loads(block.handle_ajax('submit', post_data)) + block = self.word_cloud_class(module_system, DictFieldData(self.raw_field_data), Mock()) + + if settings.USE_EXTRACTED_WORD_CLOUD_BLOCK: + # The extracted Word Cloud XBlock uses @XBlock.json_handler for handling AJAX requests. + # It expects a standard Python dictionary as POST data and returns a JSON object in response. + post_data = {'student_words': ['cat', 'cat', 'dog', 'sun']} + response = block.submit_state(post_data) + else: + post_data = MultiDict(('student_words[]', word) for word in ['cat', 'cat', 'dog', 'sun']) + response = json.loads(block.handle_ajax('submit', post_data)) assert response['status'] == 'success' assert response['submitted'] is True assert response['total_count'] == 22 @@ -109,9 +155,8 @@ def test_indexibility(self): """ Test indexibility of Word Cloud """ - module_system = get_test_system() - block = WordCloudBlock(module_system, DictFieldData(self.raw_field_data), Mock()) + block = self.word_cloud_class(module_system, DictFieldData(self.raw_field_data), Mock()) assert block.index_dictionary() ==\ {'content_type': 'Word Cloud', 'content': {'display_name': 'Word Cloud Block', @@ -128,13 +173,23 @@ def test_studio_submit_handler(self): 'num_top_words': 10, 'display_student_percents': 'False', } + if settings.USE_EXTRACTED_WORD_CLOUD_BLOCK: + # In the extracted Word Cloud XBlock, we use StudioEditableXBlockMixin.submit_studio_edits, + # which expects a different handler name and request JSON format. + handler_name = 'submit_studio_edits' + TEST_REQUEST_JSON = { + 'values': TEST_SUBMIT_DATA, + } + else: + handler_name = 'studio_submit' + TEST_REQUEST_JSON = TEST_SUBMIT_DATA module_system = get_test_system() - block = WordCloudBlock(module_system, DictFieldData(self.raw_field_data), Mock()) - body = json.dumps(TEST_SUBMIT_DATA) + block = self.word_cloud_class(module_system, DictFieldData(self.raw_field_data), Mock()) + body = json.dumps(TEST_REQUEST_JSON) request = Request.blank('/') request.method = 'POST' request.body = body.encode('utf-8') - res = block.handle('studio_submit', request) + res = block.handle(handler_name, request) assert json.loads(res.body.decode('utf8')) == {'result': 'success'} assert block.display_name == TEST_SUBMIT_DATA['display_name'] @@ -142,3 +197,13 @@ def test_studio_submit_handler(self): assert block.num_inputs == TEST_SUBMIT_DATA['num_inputs'] assert block.num_top_words == TEST_SUBMIT_DATA['num_top_words'] assert block.display_student_percents == (TEST_SUBMIT_DATA['display_student_percents'] == "True") + + +@override_settings(USE_EXTRACTED_WORD_CLOUD_BLOCK=True) +class TestWordCloudExtracted(_TestWordCloudBase): + __test__ = True + + +@override_settings(USE_EXTRACTED_WORD_CLOUD_BLOCK=False) +class TestWordCloudBuiltIn(_TestWordCloudBase): + __test__ = True diff --git a/xmodule/word_cloud_block.py b/xmodule/word_cloud_block.py index 37e82400df78..b22ecf3b7ab7 100644 --- a/xmodule/word_cloud_block.py +++ b/xmodule/word_cloud_block.py @@ -316,8 +316,17 @@ def index_dictionary(self): return xblock_body -WordCloudBlock = ( - _ExtractedWordCloudBlock if settings.USE_EXTRACTED_WORD_CLOUD_BLOCK - else _BuiltInWordCloudBlock -) +WordCloudBlock = None + + +def reset_class(): + """Reset class as per django settings flag""" + global WordCloudBlock + WordCloudBlock = ( + _ExtractedWordCloudBlock if settings.USE_EXTRACTED_WORD_CLOUD_BLOCK + else _BuiltInWordCloudBlock + ) + return WordCloudBlock + +reset_class() WordCloudBlock.__name__ = "WordCloudBlock"