fix: Inconsistent update state in Course Optimizer API (#37369)
* fix: Inconsistent update state in Course Optimizer API * fix: update re-run link issue
This commit is contained in:
@@ -491,17 +491,24 @@ def _generate_course_updates_content(course, updates_links, course_key=None):
|
||||
if not update_items:
|
||||
return course_updates
|
||||
|
||||
# Group links by update ID and categorize them
|
||||
links_by_update = {}
|
||||
for item in updates_links:
|
||||
if len(item) >= 2:
|
||||
update_id, link = item[0], item[1]
|
||||
link_state = item[2] if len(item) >= 3 else LinkState.BROKEN
|
||||
links_by_update.setdefault(update_id, _create_empty_links_data())
|
||||
_categorize_link_by_state(link, link_state, links_by_update[update_id], course_key)
|
||||
|
||||
for update in update_items:
|
||||
if update.get("status") != "deleted":
|
||||
update_content = update.get("content", "")
|
||||
update_link_data = _process_content_links(update_content, updates_links, course_key)
|
||||
|
||||
update_id = update.get("id")
|
||||
course_updates.append(
|
||||
{
|
||||
"id": str(update.get("id")),
|
||||
"id": str(update_id),
|
||||
"displayName": update.get("date", "Unknown Date"),
|
||||
"url": f"/course/{str(course.id)}/course_info",
|
||||
**update_link_data,
|
||||
**links_by_update.get(update_id, _create_empty_links_data()),
|
||||
}
|
||||
)
|
||||
|
||||
@@ -522,14 +529,21 @@ def _generate_handouts_content(course, handouts_links, course_key=None):
|
||||
):
|
||||
return course_handouts
|
||||
|
||||
links_data = _process_content_links(handouts_block.data, handouts_links, course_key)
|
||||
# Group links by block_id and categorize them
|
||||
links_by_handout = {}
|
||||
for item in handouts_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_handout.setdefault(block_id, _create_empty_links_data())
|
||||
_categorize_link_by_state(link, link_state, links_by_handout[block_id], course_key)
|
||||
|
||||
course_handouts = [
|
||||
{
|
||||
"id": str(usage_key),
|
||||
"displayName": "handouts",
|
||||
"url": f"/course/{str(course.id)}/course_info",
|
||||
**links_data,
|
||||
**links_by_handout.get(str(usage_key), _create_empty_links_data()),
|
||||
}
|
||||
]
|
||||
return course_handouts
|
||||
|
||||
@@ -15,7 +15,7 @@ from cms.djangoapps.contentstore.core.course_optimizer_provider import (
|
||||
)
|
||||
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_course_reference
|
||||
from xmodule.tabs import StaticTab
|
||||
|
||||
|
||||
@@ -329,7 +329,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_course_reference(url, previous_course_key)
|
||||
self.assertEqual(
|
||||
result,
|
||||
expected_match,
|
||||
|
||||
@@ -56,7 +56,7 @@ 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,
|
||||
contains_course_reference,
|
||||
create_course_info_usage_key,
|
||||
create_or_update_xblock_upstream_link,
|
||||
delete_course,
|
||||
@@ -1190,7 +1190,7 @@ def _check_broken_links(task_instance, user_id, course_key_string, language):
|
||||
# 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_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])
|
||||
@@ -1917,28 +1917,45 @@ def _course_link_update_required(url, course_key, prev_run_course_key):
|
||||
Args:
|
||||
url: The URL to check
|
||||
course_key: The current course key
|
||||
prev_run_course_key: The previous course run key
|
||||
|
||||
Returns:
|
||||
bool: True if the link needs updating
|
||||
"""
|
||||
|
||||
if not url or not course_key:
|
||||
if not all((url, course_key, prev_run_course_key)):
|
||||
return False
|
||||
|
||||
course_id_match = contains_previous_course_reference(url, prev_run_course_key)
|
||||
if not course_id_match:
|
||||
return False
|
||||
|
||||
# Check if it's the same org and course but different run
|
||||
if (
|
||||
prev_run_course_key.org == course_key.org
|
||||
and prev_run_course_key.course == course_key.course
|
||||
and prev_run_course_key.run != course_key.run
|
||||
):
|
||||
course_id_match = contains_course_reference(url, prev_run_course_key)
|
||||
if course_id_match:
|
||||
return True
|
||||
|
||||
return False
|
||||
|
||||
|
||||
def _replace_exact_course_reference(url, old_course_key, new_course_key):
|
||||
"""
|
||||
Replaces exact course key references in a URL, avoiding partial matches.
|
||||
|
||||
Args:
|
||||
url: The URL to update
|
||||
old_course_key: The course key to replace
|
||||
new_course_key: The course key to replace with
|
||||
|
||||
Returns:
|
||||
str: Updated URL with exact course key replacements
|
||||
"""
|
||||
if not old_course_key or not new_course_key or not url:
|
||||
return url
|
||||
|
||||
old_course_pattern = re.escape(str(old_course_key))
|
||||
|
||||
# Ensure the course key is followed by '/' or end of string
|
||||
pattern = old_course_pattern + r'(?=/|$)'
|
||||
|
||||
return re.sub(pattern, str(new_course_key), url, flags=re.IGNORECASE)
|
||||
|
||||
|
||||
def _determine_link_type(block_id):
|
||||
"""
|
||||
Determines the type of link based on block_id and URL.
|
||||
@@ -1987,22 +2004,13 @@ def _update_link_to_latest_rerun(link_data, course_key, prev_run_course_key, use
|
||||
if not original_url:
|
||||
return original_url
|
||||
|
||||
prev_run_course_org = prev_run_course_key.org if prev_run_course_key else None
|
||||
prev_run_course_course = (
|
||||
prev_run_course_key.course if prev_run_course_key else None
|
||||
)
|
||||
|
||||
if prev_run_course_key == course_key:
|
||||
return original_url
|
||||
|
||||
# Validate url based on previous-run org
|
||||
if (
|
||||
prev_run_course_org != course_key.org
|
||||
or prev_run_course_course != course_key.course
|
||||
):
|
||||
return original_url
|
||||
new_url = _replace_exact_course_reference(original_url, prev_run_course_key, course_key)
|
||||
|
||||
new_url = original_url.replace(str(prev_run_course_key), str(course_key))
|
||||
if new_url == original_url:
|
||||
return original_url
|
||||
|
||||
# condition because we're showing handouts as updates
|
||||
if link_type == "course_updates" and "handouts" in str(block_id):
|
||||
|
||||
@@ -2477,18 +2477,24 @@ 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_course_reference(url, course_key):
|
||||
"""
|
||||
Checks if a URL contains references to the previous course.
|
||||
Checks if a URL contains an exact reference to the specified course key.
|
||||
Uses specific delimiter matching to ensure exact matching and avoid partial matches.
|
||||
|
||||
Arguments:
|
||||
Args:
|
||||
url: The URL to check
|
||||
previous_course_key: The previous course key to look for
|
||||
course_key: The course key to look for
|
||||
|
||||
Returns:
|
||||
bool: True if URL contains reference to previous course
|
||||
bool: True if URL contains exact reference to the course
|
||||
"""
|
||||
if not previous_course_key:
|
||||
if not course_key or not url:
|
||||
return False
|
||||
|
||||
return str(previous_course_key).lower() in url.lower()
|
||||
course_key_pattern = re.escape(str(course_key))
|
||||
|
||||
# Ensure the course key is followed by '/' or end of string
|
||||
pattern = course_key_pattern + r'(?=/|$)'
|
||||
|
||||
return bool(re.search(pattern, url, re.IGNORECASE))
|
||||
|
||||
Reference in New Issue
Block a user