From e2ce7debf2c1b84f8151bbf61ba9ed9d20fe1c70 Mon Sep 17 00:00:00 2001 From: Devasia Joseph Date: Thu, 7 Aug 2025 15:29:14 +0530 Subject: [PATCH] fix: refactor code based on pep-8 guideline --- .../core/course_optimizer_provider.py | 13 ++++++----- .../tests/test_course_optimizer_provider.py | 8 +++---- .../v0/serializers/course_optimizer.py | 14 ++---------- .../rest_api/v0/views/course_optimizer.py | 9 +++++--- cms/djangoapps/contentstore/tasks.py | 22 +++++++++---------- .../contentstore/tests/test_tasks.py | 20 ++++++++--------- cms/djangoapps/contentstore/utils.py | 4 ++-- cms/lib/spectacular.py | 3 +-- 8 files changed, 44 insertions(+), 49 deletions(-) diff --git a/cms/djangoapps/contentstore/core/course_optimizer_provider.py b/cms/djangoapps/contentstore/core/course_optimizer_provider.py index b93994d4c3..3e0cd86bc4 100644 --- a/cms/djangoapps/contentstore/core/course_optimizer_provider.py +++ b/cms/djangoapps/contentstore/core/course_optimizer_provider.py @@ -7,7 +7,7 @@ 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, _get_urls +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 @@ -469,7 +469,7 @@ def _generate_course_updates_content(course, updates_links): for update in update_items: if update.get("status") != "deleted": update_content = update.get("content", "") - update_links = _get_urls(update_content) if update_content else [] + 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() @@ -480,7 +480,8 @@ def _generate_course_updates_content(course, updates_links): course_updates.append( { - "name": update.get("date", "Unknown Date"), + "id": str(update.get("id")), + "displayName": update.get("date", "Unknown Date"), "url": f"/course/{str(course.id)}/course_info", **update_link_data, } @@ -515,7 +516,8 @@ def _generate_handouts_content(course, handouts_links): course_handouts = [ { - "name": "handouts", + "id": str(usage_key), + "displayName": "handouts", "url": f"/course/{str(course.id)}/course_info", **links_data, } @@ -544,7 +546,8 @@ def _generate_custom_pages_content(course, custom_pages_links): if isinstance(tab, StaticTab): block_id = str(course.id.make_usage_key("static_tab", tab.url_slug)) custom_pages.append({ - "name": tab.name, + "id": block_id, + "displayName": tab.name, "url": f"/course/{str(course.id)}/custom-pages", **links_by_page.get(block_id, _create_empty_links_data()), }) 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 8cb7c38bc1..9ce568fc98 100644 --- a/cms/djangoapps/contentstore/core/tests/test_course_optimizer_provider.py +++ b/cms/djangoapps/contentstore/core/tests/test_course_optimizer_provider.py @@ -13,9 +13,9 @@ from cms.djangoapps.contentstore.core.course_optimizer_provider import ( generate_broken_links_descriptor, sort_course_sections ) -from cms.djangoapps.contentstore.tasks import LinkState, _get_urls +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 cms.djangoapps.contentstore.utils import contains_previous_course_reference from xmodule.tabs import StaticTab @@ -325,7 +325,7 @@ class TestLinkCheckProvider(CourseTestCase): for url, expected_match in test_cases: with self.subTest(url=url, expected=expected_match): - result = _contains_previous_course_reference(url, previous_course_key) + result = contains_previous_course_reference(url, previous_course_key) self.assertEqual( result, expected_match, @@ -356,7 +356,7 @@ class TestLinkCheckProvider(CourseTestCase): for content, expected_urls in test_cases: with self.subTest(content=content): - urls = _get_urls(content) + urls = extract_content_URLs_from_course(content) for expected_url in expected_urls: self.assertIn( expected_url, 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 e6879203f7..9faef425e4 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/serializers/course_optimizer.py +++ b/cms/djangoapps/contentstore/rest_api/v0/serializers/course_optimizer.py @@ -37,21 +37,11 @@ class LinkCheckSectionSerializer(serializers.Serializer): subsections = LinkCheckSubsectionSerializer(many=True) -class LinkCheckContentItemSerializer(serializers.Serializer): - """ Serializer for course content items like updates, handouts, and custom pages """ - name = serializers.CharField(required=True, allow_null=False, allow_blank=False) - url = serializers.CharField(required=True, allow_null=False, allow_blank=False) - brokenLinks = serializers.ListField(required=False) - lockedLinks = serializers.ListField(required=False) - externalForbiddenLinks = serializers.ListField(required=False) - previousRunLinks = serializers.ListField(required=False) - - class LinkCheckOutputSerializer(serializers.Serializer): """ Serializer for broken links output model data """ sections = LinkCheckSectionSerializer(many=True) - course_updates = LinkCheckContentItemSerializer(many=True, required=False) - custom_pages = LinkCheckContentItemSerializer(many=True, required=False) + 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 67bcd8309a..b98255ebd3 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/views/course_optimizer.py +++ b/cms/djangoapps/contentstore/rest_api/v0/views/course_optimizer.py @@ -128,7 +128,8 @@ class LinkCheckStatusView(DeveloperErrorViewMixin, APIView): ], "course_updates": [ { - "name": , + "id": , + "displayName": , "url": , "brokenLinks": [, ...], "lockedLinks": [, ...], @@ -139,7 +140,8 @@ class LinkCheckStatusView(DeveloperErrorViewMixin, APIView): { }, ..., { - "name": "handouts", + "id": , + "displayName": "handouts", "url": , "brokenLinks": [, ...], "lockedLinks": [, ...], @@ -149,7 +151,8 @@ class LinkCheckStatusView(DeveloperErrorViewMixin, APIView): ], "custom_pages": [ { - "name": , + "id": , + "displayName": , "url": , "brokenLinks": [, ...], "lockedLinks": [, ...], diff --git a/cms/djangoapps/contentstore/tasks.py b/cms/djangoapps/contentstore/tasks.py index 5e86bbae3b..1b5c1061e7 100644 --- a/cms/djangoapps/contentstore/tasks.py +++ b/cms/djangoapps/contentstore/tasks.py @@ -53,8 +53,8 @@ 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, + contains_previous_course_reference, + get_previous_run_course_key, create_or_update_xblock_upstream_link, delete_course, initialize_permissions, @@ -1157,13 +1157,13 @@ def _check_broken_links(task_instance, user_id, course_key_string, language): 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) + 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): + 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]) @@ -1243,7 +1243,7 @@ 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) @@ -1265,7 +1265,7 @@ def _scan_course_for_links(course_key): 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. Uses multiple regex patterns to find URLs in various contexts: @@ -1316,11 +1316,11 @@ def _scan_course_updates_for_links(course): for update in update_items: if update.get("status") != "deleted": update_content = update.get("content", "") - url_list = _get_urls(update_content) + url_list = extract_content_URLs_from_course(update_content) course_updates.append( { - "name": update.get("date", "Unknown"), + "displayName": update.get("date", "Unknown"), "block_id": str(usage_key), "urls": url_list, } @@ -1349,7 +1349,7 @@ def _scan_course_handouts_for_links(course): handouts_block = store.get_item(usage_key) if handouts_block and hasattr(handouts_block, "data") and handouts_block.data: - url_list = _get_urls(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} ) @@ -1383,11 +1383,11 @@ def _scan_custom_pages_for_links(course): static_tab_block = store.get_item(static_tab_loc) if static_tab_block and hasattr(static_tab_block, "data"): - url_list = _get_urls(static_tab_block.data) + url_list = extract_content_URLs_from_course(static_tab_block.data) custom_pages.append( { - "name": tab.name, + "displayName": tab.name, "block_id": str(static_tab_loc), "urls": url_list, } diff --git a/cms/djangoapps/contentstore/tests/test_tasks.py b/cms/djangoapps/contentstore/tests/test_tasks.py index fe7c33ac0d..5a76b7d67f 100644 --- a/cms/djangoapps/contentstore/tests/test_tasks.py +++ b/cms/djangoapps/contentstore/tests/test_tasks.py @@ -37,11 +37,11 @@ from ..tasks import ( 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 @@ class CheckBrokenLinksTaskTest(ModuleStoreTestCase): # 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 @@ class CheckBrokenLinksTaskTest(ModuleStoreTestCase): 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 @@ class CheckBrokenLinksTaskTest(ModuleStoreTestCase): 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 @@ class CheckBrokenLinksTaskTest(ModuleStoreTestCase): "https://validsite.com", "https://another-valid.com" ] - self.assertEqual(_get_urls(content), set(expected)) + self.assertEqual(extract_content_URLs_from_course(content), set(expected)) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 03422a1714..0562886acd 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -2444,7 +2444,7 @@ def create_or_update_xblock_upstream_link(xblock, course_key: CourseKey, created _create_or_update_container_link(course_key, created, xblock) -def _get_previous_run_course_key(course_key): +def get_previous_run_course_key(course_key): """ Retrieves the course key of the previous run for a given course. """ @@ -2457,7 +2457,7 @@ def _get_previous_run_course_key(course_key): return rerun_state.source_course_key -def _contains_previous_course_reference(url, previous_course_key): +def contains_previous_course_reference(url, previous_course_key): """ Checks if a URL contains references to the previous course. diff --git a/cms/lib/spectacular.py b/cms/lib/spectacular.py index 2e05290fc0..0a4ac831d3 100644 --- a/cms/lib/spectacular.py +++ b/cms/lib/spectacular.py @@ -10,8 +10,7 @@ def cms_api_filter(endpoints): """ filtered = [] CMS_PATH_PATTERN = re.compile( - r"^/api/contentstore/v0/(xblock|videos|video_transcripts|file_assets|" - r"youtube_transcripts|link_check|link_check_status)" + r"^/api/contentstore/v0/(xblock|videos|video_transcripts|file_assets|youtube_transcripts)" ) for path, path_regex, method, callback in endpoints: