Skip to content

Commit 3834f20

Browse files
authored
fix: sort sections in course-optimizer before returning result (#36441)
1 parent 1ca57ec commit 3834f20

3 files changed

Lines changed: 102 additions & 3 deletions

File tree

cms/djangoapps/contentstore/core/course_optimizer_provider.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
from cms.djangoapps.contentstore.tasks import CourseLinkCheckTask, LinkState
99
from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import get_xblock
1010
from cms.djangoapps.contentstore.xblock_storage_handlers.xblock_helpers import usage_key_with_run
11+
from xmodule.modulestore import ModuleStoreEnum
12+
from xmodule.modulestore.django import modulestore
1113

1214

1315
# Restricts status in the REST API to only those which the requesting user has permission to view.
@@ -285,3 +287,26 @@ def _create_dto_recursive(xblock_node, xblock_dictionary):
285287
xblock_children.append(xblock_entry)
286288

287289
return {level: xblock_children} if level else None
290+
291+
292+
def sort_course_sections(course_key, data):
293+
"""Retrieve and sort course sections based on the published course structure."""
294+
course_blocks = modulestore().get_items(
295+
course_key,
296+
qualifiers={'category': 'course'},
297+
revision=ModuleStoreEnum.RevisionOption.published_only
298+
)
299+
300+
if not course_blocks or 'LinkCheckOutput' not in data or 'sections' not in data['LinkCheckOutput']:
301+
return data # Return unchanged data if course_blocks or required keys are missing
302+
303+
sorted_section_ids = [section.location.block_id for section in course_blocks[0].get_children()]
304+
305+
sections_map = {section['id']: section for section in data['LinkCheckOutput']['sections']}
306+
data['LinkCheckOutput']['sections'] = [
307+
sections_map[section_id]
308+
for section_id in sorted_section_ids
309+
if section_id in sections_map
310+
]
311+
312+
return data

cms/djangoapps/contentstore/core/tests/test_course_optimizer_provider.py

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
"""
22
Tests for course optimizer
33
"""
4+
from unittest import mock
45
from unittest.mock import Mock
56

67
from cms.djangoapps.contentstore.tests.utils import CourseTestCase
78
from cms.djangoapps.contentstore.core.course_optimizer_provider import (
89
_update_node_tree_and_dictionary,
9-
_create_dto_recursive
10+
_create_dto_recursive,
11+
sort_course_sections
1012
)
1113
from cms.djangoapps.contentstore.tasks import LinkState
1214

@@ -222,3 +224,74 @@ def test_create_dto_recursive_returns_for_full_tree(self):
222224
expected = _create_dto_recursive(mock_node_tree, mock_dictionary)
223225

224226
self.assertEqual(expected_result, expected)
227+
228+
@mock.patch('cms.djangoapps.contentstore.core.course_optimizer_provider.modulestore', autospec=True)
229+
def test_returns_unchanged_data_if_no_course_blocks(self, mock_modulestore):
230+
"""Test that the function returns unchanged data if no course blocks exist."""
231+
mock_modulestore_instance = Mock()
232+
mock_modulestore.return_value = mock_modulestore_instance
233+
mock_modulestore_instance.get_items.return_value = []
234+
235+
data = {}
236+
result = sort_course_sections("course-v1:Test+Course", data)
237+
assert result == data # Should return the original data
238+
239+
@mock.patch('cms.djangoapps.contentstore.core.course_optimizer_provider.modulestore', autospec=True)
240+
def test_returns_unchanged_data_if_linkcheckoutput_missing(self, mock_modulestore):
241+
"""Test that the function returns unchanged data if 'LinkCheckOutput' is missing."""
242+
243+
mock_modulestore_instance = Mock()
244+
mock_modulestore.return_value = mock_modulestore_instance
245+
246+
data = {'LinkCheckStatus': 'Uninitiated'} # No 'LinkCheckOutput'
247+
mock_modulestore_instance.get_items.return_value = data
248+
249+
result = sort_course_sections("course-v1:Test+Course", data)
250+
assert result == data
251+
252+
@mock.patch('cms.djangoapps.contentstore.core.course_optimizer_provider.modulestore', autospec=True)
253+
def test_returns_unchanged_data_if_sections_missing(self, mock_modulestore):
254+
"""Test that the function returns unchanged data if 'sections' is missing."""
255+
256+
mock_modulestore_instance = Mock()
257+
mock_modulestore.return_value = mock_modulestore_instance
258+
259+
data = {'LinkCheckStatus': 'Success', 'LinkCheckOutput': {}} # No 'LinkCheckOutput'
260+
mock_modulestore_instance.get_items.return_value = data
261+
262+
result = sort_course_sections("course-v1:Test+Course", data)
263+
assert result == data
264+
265+
@mock.patch('cms.djangoapps.contentstore.core.course_optimizer_provider.modulestore', autospec=True)
266+
def test_sorts_sections_correctly(self, mock_modulestore):
267+
"""Test that the function correctly sorts sections based on published course structure."""
268+
269+
mock_course_block = Mock()
270+
mock_course_block.get_children.return_value = [
271+
Mock(location=Mock(block_id="section2")),
272+
Mock(location=Mock(block_id="section3")),
273+
Mock(location=Mock(block_id="section1")),
274+
]
275+
276+
mock_modulestore_instance = Mock()
277+
mock_modulestore.return_value = mock_modulestore_instance
278+
mock_modulestore_instance.get_items.return_value = [mock_course_block]
279+
280+
data = {
281+
"LinkCheckOutput": {
282+
"sections": [
283+
{"id": "section1", "name": "Intro"},
284+
{"id": "section2", "name": "Advanced"},
285+
{"id": "section3", "name": "Bonus"}, # Not in course structure
286+
]
287+
}
288+
}
289+
290+
result = sort_course_sections("course-v1:Test+Course", data)
291+
expected_sections = [
292+
{"id": "section2", "name": "Advanced"},
293+
{"id": "section3", "name": "Bonus"},
294+
{"id": "section1", "name": "Intro"},
295+
]
296+
297+
assert result["LinkCheckOutput"]["sections"] == expected_sections

cms/djangoapps/contentstore/rest_api/v0/views/course_optimizer.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from rest_framework.response import Response
77
from user_tasks.models import UserTaskStatus
88

9-
from cms.djangoapps.contentstore.core.course_optimizer_provider import get_link_check_data
9+
from cms.djangoapps.contentstore.core.course_optimizer_provider import get_link_check_data, sort_course_sections
1010
from cms.djangoapps.contentstore.rest_api.v0.serializers.course_optimizer import LinkCheckSerializer
1111
from cms.djangoapps.contentstore.tasks import check_broken_links
1212
from common.djangoapps.student.auth import has_course_author_access, has_studio_read_access
@@ -139,6 +139,7 @@ def get(self, request: Request, course_id: str):
139139
self.permission_denied(request)
140140

141141
data = get_link_check_data(request, course_id)
142-
serializer = LinkCheckSerializer(data)
142+
data = sort_course_sections(course_key, data)
143143

144+
serializer = LinkCheckSerializer(data)
144145
return Response(serializer.data)

0 commit comments

Comments
 (0)