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 6722ed2d84..3a159cf042 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/tests/test_tabs.py +++ b/cms/djangoapps/contentstore/rest_api/v0/tests/test_tabs.py @@ -42,11 +42,15 @@ class TabsAPITests(CourseTestCase): ) # 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", - ) + # 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.reload_course() def check_invalid_response(self, resp): @@ -107,13 +111,13 @@ class TabsAPITests(CourseTestCase): # reorder the last two tabs tab_ids[num_orig_tabs - 1], tab_ids[num_orig_tabs - 2] = tab_ids[num_orig_tabs - 2], tab_ids[num_orig_tabs - 1] - # remove the middle tab + # remove the third to the last tab, the removed tab has to be a static tab # (the code needs to handle the case where tabs requested for re-ordering is a subset of the tabs in the course) - removed_tab = tab_ids.pop(num_orig_tabs // 2) - assert len(tab_ids) == num_orig_tabs - 1 + removed_tab = tab_ids.pop(num_orig_tabs - 3) + self.assertEqual(len(tab_ids), num_orig_tabs - 1) # post the request - resp = self.make_reorder_tabs_request([{"tab_id": tab_id} for tab_id in tab_ids]) + resp = self.make_reorder_tabs_request([{"tab_id": tab_id} for tab_id in tab_ids if 'static' in tab_id]) assert resp.status_code == 204 # reload the course and verify the new tab order diff --git a/cms/djangoapps/contentstore/rest_api/v0/views/tabs.py b/cms/djangoapps/contentstore/rest_api/v0/views/tabs.py index 0f6ddd778e..58e0e147f1 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_tabs, reorder_tabs_handler +from ....views.tabs import edit_tab_handler, get_course_static_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 tabs in a course including hidden tabs. + Get a list of all the static 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_tabs(course_module, request.user) + tabs_to_render = get_course_static_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 356087f51f..4788ddf4e7 100644 --- a/cms/djangoapps/contentstore/views/tabs.py +++ b/cms/djangoapps/contentstore/views/tabs.py @@ -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: static tabs (a.k.a. user-created tabs) and built-in tabs + # get all tabs from the tabs list and select only static tabs (a.k.a. user-created tabs) # present in the same order they are displayed in LMS - tabs_to_render = list(get_course_tabs(course_item, request.user)) + tabs_to_render = list(get_course_static_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_tabs(course_item: CourseBlock, user: User) -> Iterable[CourseTab]: +def get_course_static_tabs(course_item: CourseBlock, user: User) -> Iterable[CourseTab]: """ - Yields all the course tabs in a course including hidden tabs. + Yields all the static tabs in a course including hidden tabs. Args: course_item (CourseBlock): The course object from which to get the tabs @@ -96,7 +96,7 @@ def get_course_tabs(course_item: CourseBlock, user: User) -> Iterable[CourseTab] # 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 - yield tab + yield tab def update_tabs_handler(course_item: CourseBlock, tabs_data: Dict, user: User) -> None: @@ -119,7 +119,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 tabs request + Helper function for handling reorder of static tabs request """ # Tabs are identified by tab_id or locators. @@ -128,10 +128,40 @@ def reorder_tabs_handler(course_item, tabs_data, user): # their tab_ids since the xmodule editor uses only locators to identify new objects. requested_tab_id_locators = tabs_data["tabs"] - # original tab list in original order - old_tab_list = course_item.tabs + #get original tab list of only static tabs with their original index(position) in the full course tabs list + old_tab_dict = {} + for idx, tab in enumerate(course_item.tabs): + if isinstance(tab, StaticTab): + old_tab_dict[tab] = idx + old_tab_list = list(old_tab_dict.keys()) - # create a new list in the new order + new_tab_list = create_new_list(requested_tab_id_locators, old_tab_list) + + # Creates a full new course tab list of both default and static course tabs + # by looping through the new tab list of static only tabs and + # putting them in their new position in the list of course item tabs + # original_idx gives the list of positions of all static tabs in course tabs originally + full_new_tab_list = course_item.tabs + original_idx = list(old_tab_dict.values()) + for i in range(len(new_tab_list)): + full_new_tab_list[original_idx[i]] = new_tab_list[i] + + # validate the tabs to make sure everything is Ok (e.g., did the client try to reorder unmovable tabs?) + try: + CourseTabList.validate_tabs(full_new_tab_list) + except InvalidTabsException as exception: + raise ValidationError({"error": f"New list of tabs is not valid: {str(exception)}."}) from exception + + # persist the new order of the tabs + course_item.tabs = full_new_tab_list + modulestore().update_item(course_item, user.id) + + +def create_new_list(requested_tab_id_locators, old_tab_list): + """ + Helper function for creating a new course tab list in the new order + of reordered tabs + """ new_tab_list = [] for tab_id_locator in requested_tab_id_locators: tab = get_tab_by_tab_id_locator(old_tab_list, tab_id_locator) @@ -143,16 +173,7 @@ def reorder_tabs_handler(course_item, tabs_data, user): # global or course settings. so add those to the end of the list. non_displayed_tabs = set(old_tab_list) - set(new_tab_list) new_tab_list.extend(non_displayed_tabs) - - # validate the tabs to make sure everything is Ok (e.g., did the client try to reorder unmovable tabs?) - try: - CourseTabList.validate_tabs(new_tab_list) - except InvalidTabsException as exception: - raise ValidationError({"error": f"New list of tabs is not valid: {str(exception)}."}) from exception - - # persist the new order of the tabs - course_item.tabs = new_tab_list - modulestore().update_item(course_item, user.id) + return new_tab_list def edit_tab_handler(course_item: CourseBlock, tabs_data: Dict, user: User): diff --git a/cms/djangoapps/contentstore/views/tests/test_tabs.py b/cms/djangoapps/contentstore/views/tests/test_tabs.py index 9d3ebee96d..0ccc8878ca 100644 --- a/cms/djangoapps/contentstore/views/tests/test_tabs.py +++ b/cms/djangoapps/contentstore/views/tests/test_tabs.py @@ -1,6 +1,5 @@ """ Tests for tab functions (just primitive). """ - import json from cms.djangoapps.contentstore.tests.utils import CourseTestCase @@ -25,12 +24,15 @@ class TabsPageTests(CourseTestCase): # Set the URL for tests self.url = reverse_course_url('tabs_handler', self.course.id) - # 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" - ) + # 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.reload_course() def check_invalid_tab_id_response(self, resp): @@ -39,7 +41,6 @@ class TabsPageTests(CourseTestCase): self.assertEqual(resp.status_code, 400) resp_content = json.loads(resp.content.decode('utf-8')) self.assertIn("error", resp_content) - self.assertIn("invalid_tab_id", resp_content['error']) def test_not_implemented(self): """Verify not implemented errors""" @@ -85,15 +86,15 @@ class TabsPageTests(CourseTestCase): # reorder the last two tabs tab_ids[num_orig_tabs - 1], tab_ids[num_orig_tabs - 2] = tab_ids[num_orig_tabs - 2], tab_ids[num_orig_tabs - 1] - # remove the middle tab + # remove the third to the last tab, the removed tab has to be a static tab # (the code needs to handle the case where tabs requested for re-ordering is a subset of the tabs in the course) - removed_tab = tab_ids.pop(num_orig_tabs // 2) + removed_tab = tab_ids.pop(num_orig_tabs - 3) self.assertEqual(len(tab_ids), num_orig_tabs - 1) - # post the request + # post the request with the reordered static tabs only resp = self.client.ajax_post( self.url, - data={'tabs': [{'tab_id': tab_id} for tab_id in tab_ids]}, + data={'tabs': [{'tab_id': tab_id} for tab_id in tab_ids if 'static' in tab_id]}, ) self.assertEqual(resp.status_code, 204) @@ -178,7 +179,7 @@ class TabsPageTests(CourseTestCase): """ Verify that the static tab renders itself with the correct HTML """ - preview_url = f'/xblock/{self.test_tab.location}/{STUDENT_VIEW}' + preview_url = f'/xblock/{self.test_tabs[0].location}/{STUDENT_VIEW}' resp = self.client.get(preview_url, HTTP_ACCEPT='application/json') self.assertEqual(resp.status_code, 200) diff --git a/cms/static/images/preview-lms-staticpage.png b/cms/static/images/preview-lms-staticpage.png new file mode 100644 index 0000000000..7940a106bf Binary files /dev/null and b/cms/static/images/preview-lms-staticpage.png differ diff --git a/cms/templates/edit-tabs.html b/cms/templates/edit-tabs.html index c9e30c3223..056bff979d 100644 --- a/cms/templates/edit-tabs.html +++ b/cms/templates/edit-tabs.html @@ -7,6 +7,7 @@ from django.urls import reverse from xmodule.tabs import StaticTab from openedx.core.djangolib.js_utils import js_escaped_string + from cms.djangoapps.contentstore.utils import get_pages_and_resources_url %> <%block name="title">${_("Pages")}%block> <%block name="bodyclass">is-signedin course view-static-pages%block> @@ -27,12 +28,40 @@ <%block name="content">