fix: refactor code based on pep-8 guideline
This commit is contained in:
committed by
Muhammad Faraz Maqsood
parent
ca45009a31
commit
e2ce7debf2
@@ -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()),
|
||||
})
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -128,7 +128,8 @@ class LinkCheckStatusView(DeveloperErrorViewMixin, APIView):
|
||||
],
|
||||
"course_updates": [
|
||||
{
|
||||
"name": <string>,
|
||||
"id": <string>,
|
||||
"displayName": <string>,
|
||||
"url": <string>,
|
||||
"brokenLinks": [<string>, ...],
|
||||
"lockedLinks": [<string>, ...],
|
||||
@@ -139,7 +140,8 @@ class LinkCheckStatusView(DeveloperErrorViewMixin, APIView):
|
||||
{ <another course-updates> },
|
||||
...,
|
||||
{
|
||||
"name": "handouts",
|
||||
"id": <string>,
|
||||
"displayName": "handouts",
|
||||
"url": <string>,
|
||||
"brokenLinks": [<string>, ...],
|
||||
"lockedLinks": [<string>, ...],
|
||||
@@ -149,7 +151,8 @@ class LinkCheckStatusView(DeveloperErrorViewMixin, APIView):
|
||||
],
|
||||
"custom_pages": [
|
||||
{
|
||||
"name": <string>,
|
||||
"id": <string>,
|
||||
"displayName": <string>,
|
||||
"url": <string>,
|
||||
"brokenLinks": [<string>, ...],
|
||||
"lockedLinks": [<string>, ...],
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
|
||||
@@ -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 = '''
|
||||
<a href="https://example.com">Link</a>
|
||||
@@ -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))
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
@@ -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:
|
||||
|
||||
Reference in New Issue
Block a user