diff --git a/cms/djangoapps/contentstore/rest_api/v0/tests/test_tabs.py b/cms/djangoapps/contentstore/rest_api/v0/tests/test_tabs.py index 5162b50b06..5daf2f55a0 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/tests/test_tabs.py +++ b/cms/djangoapps/contentstore/rest_api/v0/tests/test_tabs.py @@ -43,15 +43,11 @@ class TabsAPITests(CourseTestCase): ) # add a static tab to the course, for code coverage - # add 4 static tabs to the course, for code coverage - self.test_tabs = [] - for i in range(1, 5): - tab = ItemFactory.create( - parent_location=self.course.location, - category="static_tab", - display_name=f"Static_{i}" - ) - self.test_tabs.append(tab) + self.test_tab = ItemFactory.create( + parent_location=self.course.location, + category="static_tab", + display_name="Static_1", + ) self.reload_course() def check_invalid_response(self, resp): @@ -129,27 +125,6 @@ class TabsAPITests(CourseTestCase): new_tab_ids = [tab.tab_id for tab in self.course.tabs] assert new_tab_ids == reordered_tab_ids - def test_reorder_tabs_invalid_list(self): - """ - Test re-ordering of tabs with invalid tab list. - - Not all tabs can be rearranged. Here we are trying to swap the first - two tabs, which is disallowed since the first tab is the "Course" tab - which is immovable. - """ - - orig_tab_ids = [tab.tab_id for tab in self.course.tabs] - tab_ids = list(orig_tab_ids) - - # reorder the first two tabs - tab_ids[0], tab_ids[1] = tab_ids[1], tab_ids[0] - - # post the request - resp = self.make_reorder_tabs_request([{"tab_id": tab_id} for tab_id in tab_ids]) - assert resp.status_code == 400 - error = self.check_invalid_response(resp) - assert "error" in error - def test_reorder_tabs_invalid_tab_ids(self): """ Test re-ordering of tabs with invalid tab. diff --git a/cms/djangoapps/contentstore/rest_api/v0/views/tabs.py b/cms/djangoapps/contentstore/rest_api/v0/views/tabs.py index ce5e72de43..518381a85a 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/views/tabs.py +++ b/cms/djangoapps/contentstore/rest_api/v0/views/tabs.py @@ -13,7 +13,7 @@ from xmodule.modulestore.exceptions import ItemNotFoundError from common.djangoapps.student.auth import has_studio_read_access, has_studio_write_access from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, verify_course_exists, view_auth_classes from ..serializers import CourseTabSerializer, CourseTabUpdateSerializer, TabIDLocatorSerializer -from ....views.tabs import edit_tab_handler, get_course_static_tabs, reorder_tabs_handler +from ....views.tabs import edit_tab_handler, get_course_tabs, reorder_tabs_handler @view_auth_classes(is_authenticated=True) @@ -34,7 +34,7 @@ class CourseTabListView(DeveloperErrorViewMixin, APIView): @verify_course_exists() def get(self, request: Request, course_id: str) -> Response: """ - Get a list of all the static tabs in a course including hidden tabs. + Get a list of all the tabs in a course including hidden tabs. **Example Request** @@ -82,7 +82,7 @@ class CourseTabListView(DeveloperErrorViewMixin, APIView): self.permission_denied(request) course_module = modulestore().get_course(course_key) - tabs_to_render = get_course_static_tabs(course_module, request.user) + tabs_to_render = get_course_tabs(course_module, request.user) return Response(CourseTabSerializer(tabs_to_render, many=True).data) diff --git a/cms/djangoapps/contentstore/views/tabs.py b/cms/djangoapps/contentstore/views/tabs.py index 85f6b89850..72c7c90c72 100644 --- a/cms/djangoapps/contentstore/views/tabs.py +++ b/cms/djangoapps/contentstore/views/tabs.py @@ -19,7 +19,7 @@ from xmodule.tabs import CourseTab, CourseTabList, InvalidTabsException, StaticT from common.djangoapps.edxmako.shortcuts import render_to_response from common.djangoapps.student.auth import has_course_author_access from common.djangoapps.util.json_request import JsonResponse, JsonResponseBadRequest, expect_json -from ..utils import get_lms_link_for_item +from ..utils import get_lms_link_for_item, get_pages_and_resources_url __all__ = ["tabs_handler", "update_tabs_handler"] @@ -61,10 +61,10 @@ def tabs_handler(request, course_key_string): return JsonResponse() elif request.method == "GET": # assume html - # get all tabs from the tabs list and select only static tabs (a.k.a. user-created tabs) + # get all tabs from the tabs list: static tabs (a.k.a. user-created tabs) and built-in tabs # present in the same order they are displayed in LMS - tabs_to_render = list(get_course_static_tabs(course_item, request.user)) + tabs_to_render = list(get_course_tabs(course_item, request.user)) return render_to_response( "edit-tabs.html", @@ -78,9 +78,9 @@ def tabs_handler(request, course_key_string): return HttpResponseNotFound() -def get_course_static_tabs(course_item: CourseBlock, user: User) -> Iterable[CourseTab]: +def get_course_tabs(course_item: CourseBlock, user: User) -> Iterable[CourseTab]: """ - Yields all the static tabs in a course including hidden tabs. + Yields all the course tabs in a course including hidden tabs. Args: course_item (CourseBlock): The course object from which to get the tabs @@ -90,12 +90,14 @@ def get_course_static_tabs(course_item: CourseBlock, user: User) -> Iterable[Cou Iterable[CourseTab]: An iterable containing course tab objects from the course """ - + pages_and_resources_mfe_enabled = bool(get_pages_and_resources_url(course_item.id)) for tab in CourseTabList.iterate_displayable(course_item, user=user, inline_collections=False, include_hidden=True): if isinstance(tab, StaticTab): # static tab needs its locator information to render itself as an xmodule static_tab_loc = course_item.id.make_usage_key("static_tab", tab.url_slug) tab.locator = static_tab_loc + # If the course apps MFE is set up and pages and resources is enabled, then only show static tabs + if isinstance(tab, StaticTab) or not pages_and_resources_mfe_enabled: yield tab @@ -119,7 +121,7 @@ def update_tabs_handler(course_item: CourseBlock, tabs_data: Dict, user: User) - def reorder_tabs_handler(course_item, tabs_data, user): """ - Helper function for handling reorder of static tabs request + Helper function for handling reorder of tabs request """ # Static tabs are identified by locators (a UsageKey) instead of a tab id like @@ -152,9 +154,8 @@ def create_new_list(tab_locators, old_tab_list): tab = get_tab_by_tab_id_locator(old_tab_list, tab_locator) if tab is None: raise ValidationError({"error": f"Tab with id_locator '{tab_locator}' does not exist."}) - if not isinstance(tab, StaticTab): - raise ValidationError({"error": f"Cannot reorder tabs of type '{tab.type}'"}) - new_tab_list.append(tab) + if isinstance(tab, StaticTab): + new_tab_list.append(tab) # the old_tab_list may contain additional tabs that were not rendered in the UI because of # global or course settings. so add those to the end of the list. diff --git a/cms/djangoapps/contentstore/views/tests/test_tabs.py b/cms/djangoapps/contentstore/views/tests/test_tabs.py index 58a6393585..8df8c7b3c9 100644 --- a/cms/djangoapps/contentstore/views/tests/test_tabs.py +++ b/cms/djangoapps/contentstore/views/tests/test_tabs.py @@ -1,5 +1,6 @@ """ Tests for tab functions (just primitive). """ + import json import random @@ -26,15 +27,12 @@ class TabsPageTests(CourseTestCase): # Set the URL for tests self.url = reverse_course_url('tabs_handler', self.course.id) - # add 4 static tabs to the course, for code coverage - self.test_tabs = [] - for i in range(1, 5): - tab = ItemFactory.create( - parent_location=self.course.location, - category="static_tab", - display_name=f"Static_{i}" - ) - self.test_tabs.append(tab) + # add a static tab to the course, for code coverage + self.test_tab = ItemFactory.create( + parent_location=self.course.location, + category="static_tab", + display_name="Static_1" + ) self.reload_course() def check_invalid_tab_id_response(self, resp): @@ -95,7 +93,7 @@ class TabsPageTests(CourseTestCase): # Remove one tab randomly. This shouldn't delete the tab. tabs_data.pop() - # post the request with the reordered static tabs only + # post the request resp = self.client.ajax_post( self.url, data={ @@ -167,7 +165,7 @@ class TabsPageTests(CourseTestCase): """ Verify that the static tab renders itself with the correct HTML """ - preview_url = f'/xblock/{self.test_tabs[0].location}/{STUDENT_VIEW}' + preview_url = f'/xblock/{self.test_tab.location}/{STUDENT_VIEW}' resp = self.client.get(preview_url, HTTP_ACCEPT='application/json') assert resp.status_code == 200 diff --git a/cms/static/images/preview-lms-staticpage.png b/cms/static/images/preview-lms-staticpage.png deleted file mode 100644 index 7940a106bf..0000000000 Binary files a/cms/static/images/preview-lms-staticpage.png and /dev/null differ diff --git a/cms/templates/edit-tabs.html b/cms/templates/edit-tabs.html index 03e5bc6a4d..7952da3386 100644 --- a/cms/templates/edit-tabs.html +++ b/cms/templates/edit-tabs.html @@ -59,7 +59,7 @@