From e8e8f4acbe6741387234d52f0cf607011cc4544b Mon Sep 17 00:00:00 2001 From: Kshitij Sobti Date: Mon, 22 Nov 2021 06:17:30 +0000 Subject: [PATCH] feat!: Change the way tabs are ordered [BD-38] [TNL-9174] [BB-5076] (#29262) * feat!: Change the way tabs are ordered The change imposes a new ordering for tabs based on their new priority. When reordering tabs, this ordering will be maintained. * fix: Apply suggestions from code review Co-authored-by: Farhaan Bukhsh * fix: review feedback Co-authored-by: Farhaan Bukhsh --- .../rest_api/v0/tests/test_tabs.py | 37 +++++---- .../rest_api/v0/views/advanced_settings.py | 2 +- .../contentstore/rest_api/v0/views/tabs.py | 18 +--- cms/djangoapps/contentstore/views/tabs.py | 58 +++++-------- .../contentstore/views/tests/test_tabs.py | 83 ++++++++----------- cms/templates/edit-tabs.html | 6 +- common/lib/xmodule/xmodule/tabs.py | 13 ++- lms/djangoapps/ccx/plugins.py | 1 + lms/djangoapps/course_wiki/tab.py | 1 + lms/djangoapps/courseware/tabs.py | 22 ++--- lms/djangoapps/courseware/tests/test_tabs.py | 7 +- lms/djangoapps/discussion/plugins.py | 2 +- lms/djangoapps/edxnotes/plugins.py | 1 + .../instructor/views/instructor_dashboard.py | 1 + lms/djangoapps/teams/plugins.py | 1 + openedx/features/lti_course_tab/tab.py | 2 + 16 files changed, 115 insertions(+), 140 deletions(-) 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 3a159cf042..b5e53fadb9 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/tests/test_tabs.py +++ b/cms/djangoapps/contentstore/rest_api/v0/tests/test_tabs.py @@ -6,6 +6,7 @@ import json from urllib.parse import urlencode import ddt +import random from django.urls import reverse from xmodule.modulestore.tests.factories import ItemFactory from xmodule.tabs import CourseTabList @@ -95,36 +96,38 @@ class TabsAPITests(CourseTestCase): content_type="application/json", ) - def test_reorder_tabs(self): + def test_reorder_static_tabs(self): """ - Test re-ordering of tabs + Test re-ordering of static tabs in a course. """ - # get the original tab ids - orig_tab_ids = [tab.tab_id for tab in self.course.tabs] - tab_ids = list(orig_tab_ids) - num_orig_tabs = len(orig_tab_ids) + # get the original tabs + course_tabs = list(self.course.tabs) + num_orig_tabs = len(self.course.tabs) # make sure we have enough tabs to play around with assert num_orig_tabs >= 5 - # 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] + # Randomize the order of static tabs, leaving the rest intact + course_tabs.sort(key=lambda tab: (100 + random.random()) if tab.type == 'static_tab' else tab.priority) - # 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 - 3) - self.assertEqual(len(tab_ids), num_orig_tabs - 1) + tabs_data = [ + {'tab_locator': str(self.course.id.make_usage_key("static_tab", tab.url_slug))} + for tab in course_tabs + if tab.type == 'static_tab' + ] + # Remove one tab randomly. This shouldn't delete the tab. + tabs_data.pop() - # post the request - resp = self.make_reorder_tabs_request([{"tab_id": tab_id} for tab_id in tab_ids if 'static' in tab_id]) + # post the request with the reordered static tabs only + resp = self.make_reorder_tabs_request(tabs_data) assert resp.status_code == 204 - # reload the course and verify the new tab order + # Reload the course and verify the new tab order self.reload_course() + reordered_tab_ids = [tab.tab_id for tab in course_tabs] new_tab_ids = [tab.tab_id for tab in self.course.tabs] - assert new_tab_ids == tab_ids + [removed_tab] - assert new_tab_ids != orig_tab_ids + assert new_tab_ids == reordered_tab_ids def test_reorder_tabs_invalid_list(self): """ diff --git a/cms/djangoapps/contentstore/rest_api/v0/views/advanced_settings.py b/cms/djangoapps/contentstore/rest_api/v0/views/advanced_settings.py index 320ee9054d..899513dfd1 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/views/advanced_settings.py +++ b/cms/djangoapps/contentstore/rest_api/v0/views/advanced_settings.py @@ -24,7 +24,7 @@ class AdvancedCourseSettingsView(DeveloperErrorViewMixin, APIView): class FilterQuery(forms.Form): """ - Form for validating query marameters passed to advanced course settings view + Form for validating query parameters passed to advanced course settings view to filter the data it returns. """ filter_fields = forms.CharField(strip=True, empty_value=None, required=False) diff --git a/cms/djangoapps/contentstore/rest_api/v0/views/tabs.py b/cms/djangoapps/contentstore/rest_api/v0/views/tabs.py index e7f52d2143..ce5e72de43 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/views/tabs.py +++ b/cms/djangoapps/contentstore/rest_api/v0/views/tabs.py @@ -177,7 +177,7 @@ class CourseTabReorderView(DeveloperErrorViewMixin, APIView): return super().handle_exception(exc) @apidocs.schema( - body=[TabIDLocatorSerializer], + body=TabIDLocatorSerializer(many=True), parameters=[ apidocs.string_parameter("course_id", apidocs.ParameterLocation.PATH, description="Course ID"), ], @@ -198,24 +198,12 @@ class CourseTabReorderView(DeveloperErrorViewMixin, APIView): Move course tabs: POST /api/contentstore/v0/tabs/{course_id}/reorder [ - { - "tab_id": "info" - }, - { - "tab_id": "courseware" - }, { "tab_locator": "block-v1:TstX+DemoX+Demo+type@static_tab+block@d26fcb0e93824fbfa5c9e5f100e2511a" }, { - "tab_id": "wiki" + "tab_locator": "block-v1:TstX+DemoX+Demo+type@static_tab+block@a011f1bd05af4578ae397ed8cabccf62" }, - { - "tab_id": "discussion" - }, - { - "tab_id": "progress" - } ] @@ -233,7 +221,7 @@ class CourseTabReorderView(DeveloperErrorViewMixin, APIView): tab_id_locators.is_valid(raise_exception=True) reorder_tabs_handler( course_module, - {"tabs": tab_id_locators.validated_data}, + tab_id_locators.validated_data, request.user, ) return Response(status=status.HTTP_204_NO_CONTENT) diff --git a/cms/djangoapps/contentstore/views/tabs.py b/cms/djangoapps/contentstore/views/tabs.py index 4788ddf4e7..5482be63dc 100644 --- a/cms/djangoapps/contentstore/views/tabs.py +++ b/cms/djangoapps/contentstore/views/tabs.py @@ -1,7 +1,7 @@ """ Views related to course tabs """ -from typing import Dict, Iterable, List, Optional +from typing import Dict, Iterable, List, Optional, Union from django.contrib.auth import get_user_model from django.contrib.auth.decorators import login_required @@ -110,7 +110,7 @@ def update_tabs_handler(course_item: CourseBlock, tabs_data: Dict, user: User) - """ if "tabs" in tabs_data: - reorder_tabs_handler(course_item, tabs_data, user) + reorder_tabs_handler(course_item, tabs_data["tabs"], user) elif "tab_id_locator" in tabs_data: edit_tab_handler(course_item, tabs_data, user) else: @@ -122,58 +122,45 @@ def reorder_tabs_handler(course_item, tabs_data, user): Helper function for handling reorder of static tabs request """ - # Tabs are identified by tab_id or locators. - # The locators are used to identify static tabs since they are xmodules. + # Static tabs are identified by locators (a UsageKey) instead of a tab id like + # other tabs. These can be used to identify static tabs since they are xmodules. # Although all tabs have tab_ids, newly created static tabs do not know # their tab_ids since the xmodule editor uses only locators to identify new objects. - requested_tab_id_locators = tabs_data["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()) - - 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] + new_tab_list = create_new_list(tabs_data, course_item.tabs) # 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) + 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 = full_new_tab_list + course_item.tabs = new_tab_list + modulestore().update_item(course_item, user.id) -def create_new_list(requested_tab_id_locators, old_tab_list): +def create_new_list(tab_locators, old_tab_list): """ - Helper function for creating a new course tab list in the new order - of reordered tabs + Helper function for creating a new course tab list in the new order of + reordered tabs. + + It will take tab_locators for static tabs and resolve them to actual tab + instances. """ 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) + for tab_locator in tab_locators: + 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_id_locator}' does not exist."}) + 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) # 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. non_displayed_tabs = set(old_tab_list) - set(new_tab_list) new_tab_list.extend(non_displayed_tabs) - return new_tab_list + return sorted(new_tab_list, key=lambda item: item.priority or float('inf')) def edit_tab_handler(course_item: CourseBlock, tabs_data: Dict, user: User): @@ -212,11 +199,12 @@ def get_tab_by_tab_id_locator(tab_list: List[CourseTab], tab_id_locator: Dict[st return tab -def get_tab_by_locator(tab_list: List[CourseTab], usage_key_string: str) -> Optional[CourseTab]: +def get_tab_by_locator(tab_list: List[CourseTab], tab_location: Union[str, UsageKey]) -> Optional[CourseTab]: """ Look for a tab with the specified locator. Returns the first matching tab. """ - tab_location = UsageKey.from_string(usage_key_string) + if isinstance(tab_location, str): + tab_location = UsageKey.from_string(tab_location) item = modulestore().get_item(tab_location) static_tab = StaticTab( name=item.display_name, diff --git a/cms/djangoapps/contentstore/views/tests/test_tabs.py b/cms/djangoapps/contentstore/views/tests/test_tabs.py index 0ccc8878ca..5ba7a4a9f1 100644 --- a/cms/djangoapps/contentstore/views/tests/test_tabs.py +++ b/cms/djangoapps/contentstore/views/tests/test_tabs.py @@ -1,6 +1,8 @@ """ Tests for tab functions (just primitive). """ import json +import random + from cms.djangoapps.contentstore.tests.utils import CourseTestCase from cms.djangoapps.contentstore.utils import reverse_course_url @@ -38,9 +40,9 @@ class TabsPageTests(CourseTestCase): def check_invalid_tab_id_response(self, resp): """Verify response is an error listing the invalid_tab_id""" - self.assertEqual(resp.status_code, 400) + assert resp.status_code == 400 resp_content = json.loads(resp.content.decode('utf-8')) - self.assertIn("error", resp_content) + assert "error" in resp_content def test_not_implemented(self): """Verify not implemented errors""" @@ -75,52 +77,38 @@ class TabsPageTests(CourseTestCase): def test_reorder_tabs(self): """Test re-ordering of tabs""" - # get the original tab ids - orig_tab_ids = [tab.tab_id for tab in self.course.tabs] - tab_ids = list(orig_tab_ids) - num_orig_tabs = len(orig_tab_ids) + # get the original tabs + course_tabs = list(self.course.tabs) + num_orig_tabs = len(self.course.tabs) # make sure we have enough tabs to play around with - self.assertGreaterEqual(num_orig_tabs, 5) + assert num_orig_tabs >= 5 - # 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] + # Randomise the order of static tabs, leave the rest intact + course_tabs.sort(key=lambda tab: (100 + random.random()) if tab.type == 'static_tab' else tab.priority) - # 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 - 3) - self.assertEqual(len(tab_ids), num_orig_tabs - 1) + tabs_data = [ + {'tab_locator': str(self.course.id.make_usage_key("static_tab", tab.url_slug))} + for tab in course_tabs + if tab.type == 'static_tab' + ] + # Remove one tab randomly. This shouldn't delete the tab. + tabs_data.pop() # 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 if 'static' in tab_id]}, + data={ + 'tabs': tabs_data + }, ) - self.assertEqual(resp.status_code, 204) + assert resp.status_code == 204 # reload the course and verify the new tab order self.reload_course() + reordered_tab_ids = [tab.tab_id for tab in course_tabs] new_tab_ids = [tab.tab_id for tab in self.course.tabs] - self.assertEqual(new_tab_ids, tab_ids + [removed_tab]) - self.assertNotEqual(new_tab_ids, orig_tab_ids) - - def test_reorder_tabs_invalid_list(self): - """Test re-ordering of tabs with invalid tab list""" - - 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.client.ajax_post( - self.url, - data={'tabs': [{'tab_id': tab_id} for tab_id in tab_ids]}, - ) - self.assertEqual(resp.status_code, 400) - resp_content = json.loads(resp.content.decode('utf-8')) - self.assertIn("error", resp_content) + assert new_tab_ids == reordered_tab_ids def test_reorder_tabs_invalid_tab(self): """Test re-ordering of tabs with invalid tab""" @@ -141,7 +129,7 @@ class TabsPageTests(CourseTestCase): old_tab = CourseTabList.get_tab_by_type(self.course.tabs, tab_type) # visibility should be different from new setting - self.assertNotEqual(old_tab.is_hidden, new_is_hidden_setting) + assert old_tab.is_hidden != new_is_hidden_setting # post the request resp = self.client.ajax_post( @@ -151,12 +139,12 @@ class TabsPageTests(CourseTestCase): 'is_hidden': new_is_hidden_setting, }), ) - self.assertEqual(resp.status_code, 204) + assert resp.status_code == 204 # reload the course and verify the new visibility setting self.reload_course() new_tab = CourseTabList.get_tab_by_type(self.course.tabs, tab_type) - self.assertEqual(new_tab.is_hidden, new_is_hidden_setting) + assert new_tab.is_hidden == new_is_hidden_setting def test_toggle_tab_visibility(self): """Test toggling of tab visibility""" @@ -182,15 +170,15 @@ class TabsPageTests(CourseTestCase): 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) + assert resp.status_code == 200 resp_content = json.loads(resp.content.decode('utf-8')) html = resp_content['html'] # Verify that the HTML contains the expected elements - self.assertIn('Edit', html) - self.assertIn('Duplicate this component', html) - self.assertIn('Delete this component', html) - self.assertIn('', html) + assert 'Edit' in html + assert 'Duplicate this component' in html + assert 'Delete this component' in html + assert '' in html class PrimitiveTabEdit(ModuleStoreTestCase): @@ -205,16 +193,17 @@ class PrimitiveTabEdit(ModuleStoreTestCase): tabs.primitive_delete(course, 1) with self.assertRaises(IndexError): tabs.primitive_delete(course, 7) + assert course.tabs[2] != {'type': 'discussion', 'name': 'Discussion'} tabs.primitive_delete(course, 2) - self.assertNotIn({'type': 'textbooks'}, course.tabs) + assert {'type': 'progress'} not in course.tabs # Check that discussion has shifted up - self.assertEqual(course.tabs[2], {'type': 'discussion', 'name': 'Discussion'}) + assert course.tabs[2] == {'type': 'discussion', 'name': 'Discussion'} def test_insert(self): """Test primitive tab insertion.""" course = CourseFactory.create() tabs.primitive_insert(course, 2, 'pdf_textbooks', 'aname') - self.assertEqual(course.tabs[2], {'type': 'pdf_textbooks', 'name': 'aname'}) + assert course.tabs[2] == {'type': 'pdf_textbooks', 'name': 'aname'} with self.assertRaises(ValueError): tabs.primitive_insert(course, 0, 'pdf_textbooks', 'aname') with self.assertRaises(ValueError): @@ -225,4 +214,4 @@ class PrimitiveTabEdit(ModuleStoreTestCase): course = CourseFactory.create() tabs.primitive_insert(course, 3, 'pdf_textbooks', 'aname') course2 = modulestore().get_course(course.id) - self.assertEqual(course2.tabs[3], {'type': 'pdf_textbooks', 'name': 'aname'}) + assert course2.tabs[3] == {'type': 'pdf_textbooks', 'name': 'aname'} diff --git a/cms/templates/edit-tabs.html b/cms/templates/edit-tabs.html index 056bff979d..472f3fa908 100644 --- a/cms/templates/edit-tabs.html +++ b/cms/templates/edit-tabs.html @@ -9,7 +9,7 @@ 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 name="title">${_("Custom pages")} <%block name="bodyclass">is-signedin course view-static-pages <%block name="header_extras"> @@ -52,14 +52,14 @@

- > ${_("Custom Pages")} + > ${_("Custom pages")}

% else:

${_("Content")} ## Translators: Custom Pages refer to the tabs that appear in the top navigation of each course. - > ${_("Custom Pages")} + > ${_("Custom pages")}

% endif diff --git a/common/lib/xmodule/xmodule/tabs.py b/common/lib/xmodule/xmodule/tabs.py index 5ebe9b0095..099aa84a45 100644 --- a/common/lib/xmodule/xmodule/tabs.py +++ b/common/lib/xmodule/xmodule/tabs.py @@ -302,6 +302,7 @@ class StaticTab(CourseTab): type = 'static_tab' is_default = False # A static tab is never added to a course by default allow_multiple = True + priority = 100 def __init__(self, tab_dict=None, name=None, url_slug=None): def link_func(course, reverse_func): @@ -381,14 +382,14 @@ class CourseTabList(List): within the course. """ - course.tabs.extend([ + course_tabs = [ CourseTab.load('course_info'), CourseTab.load('courseware') - ]) + ] # Presence of syllabus tab is indicated by a course attribute if hasattr(course, 'syllabus_present') and course.syllabus_present: - course.tabs.append(CourseTab.load('syllabus')) + course_tabs.append(CourseTab.load('syllabus')) # If the course has a discussion link specified, use that even if we feature # flag discussions off. Disabling that is mostly a server safety feature @@ -400,12 +401,16 @@ class CourseTabList(List): else: discussion_tab = CourseTab.load('discussion') - course.tabs.extend([ + course_tabs.extend([ CourseTab.load('textbooks'), discussion_tab, CourseTab.load('wiki'), CourseTab.load('progress'), ]) + # While you should be able to do `tab.priority`, a lot of tests mock tabs to be a dict + # which causes them to throw an error on this line + course_tabs.sort(key=lambda tab: getattr(tab, 'priority', None) or float('inf')) + course.tabs.extend(course_tabs) @staticmethod def get_discussion(course): diff --git a/lms/djangoapps/ccx/plugins.py b/lms/djangoapps/ccx/plugins.py index 39e321bfee..4ddca4e211 100644 --- a/lms/djangoapps/ccx/plugins.py +++ b/lms/djangoapps/ccx/plugins.py @@ -18,6 +18,7 @@ class CcxCourseTab(CourseTab): """ type = "ccx_coach" + priority = 310 title = gettext_noop("CCX Coach") view_name = "ccx_coach_dashboard" is_dynamic = True # The CCX view is dynamically added to the set of tabs when it is enabled diff --git a/lms/djangoapps/course_wiki/tab.py b/lms/djangoapps/course_wiki/tab.py index 1bb352020b..d4b5ad23d7 100644 --- a/lms/djangoapps/course_wiki/tab.py +++ b/lms/djangoapps/course_wiki/tab.py @@ -20,6 +20,7 @@ class WikiTab(EnrolledTab): view_name = "course_wiki" is_hideable = True is_default = False + priority = 70 def __init__(self, tab_dict): # Default to hidden diff --git a/lms/djangoapps/courseware/tabs.py b/lms/djangoapps/courseware/tabs.py index ad351c93d0..3e7ebed546 100644 --- a/lms/djangoapps/courseware/tabs.py +++ b/lms/djangoapps/courseware/tabs.py @@ -34,7 +34,7 @@ class CoursewareTab(EnrolledTab): """ type = 'courseware' title = gettext_noop('Course') - priority = 10 + priority = 11 view_name = 'courseware' is_movable = False is_default = False @@ -69,7 +69,7 @@ class CourseInfoTab(CourseTab): """ type = 'course_info' title = gettext_noop('Home') - priority = 20 + priority = 10 view_name = 'info' tab_id = 'info' is_movable = False @@ -86,7 +86,7 @@ class SyllabusTab(EnrolledTab): """ type = 'syllabus' title = gettext_noop('Syllabus') - priority = 30 + priority = 80 view_name = 'syllabus' allow_multiple = True is_default = False @@ -104,7 +104,7 @@ class ProgressTab(EnrolledTab): """ type = 'progress' title = gettext_noop('Progress') - priority = 40 + priority = 20 view_name = 'progress' is_hideable = True is_default = False @@ -153,7 +153,7 @@ class TextbookTabs(TextbookTabsBase): A tab representing the collection of all textbook tabs. """ type = 'textbooks' - priority = None + priority = 200 view_name = 'book' @classmethod @@ -177,7 +177,7 @@ class PDFTextbookTabs(TextbookTabsBase): A tab representing the collection of all PDF textbook tabs. """ type = 'pdf_textbooks' - priority = None + priority = 201 view_name = 'pdf_book' @classmethod @@ -196,7 +196,7 @@ class HtmlTextbookTabs(TextbookTabsBase): A tab representing the collection of all Html textbook tabs. """ type = 'html_textbooks' - priority = None + priority = 202 view_name = 'html_book' @classmethod @@ -285,7 +285,7 @@ class ExternalLinkCourseTab(LinkTab): A course tab containing an external link. """ type = 'external_link' - priority = None + priority = 110 is_default = False # An external link tab is not added to a course by default allow_multiple = True @@ -326,9 +326,9 @@ class DatesTab(EnrolledTab): A tab representing the relevant dates for a course. """ type = "dates" - title = gettext_noop( - "Dates") # We don't have the user in this context, so we don't want to translate it at this level. - priority = 50 + # We don't have the user in this context, so we don't want to translate it at this level. + title = gettext_noop("Dates") + priority = 30 view_name = "dates" is_dynamic = True diff --git a/lms/djangoapps/courseware/tests/test_tabs.py b/lms/djangoapps/courseware/tests/test_tabs.py index 065834a240..5d80fa59aa 100644 --- a/lms/djangoapps/courseware/tests/test_tabs.py +++ b/lms/djangoapps/courseware/tests/test_tabs.py @@ -767,12 +767,7 @@ class CourseInfoTabTestCase(TabTestCase): def test_default_tab(self): # Verify that the course info tab is the first tab tabs = get_course_tab_list(self.user, self.course) - # So I know this means course_info is not the first tab, but it is going to be - # retired soon (https://openedx.atlassian.net/browse/TNL-7061) and also it has - # a lower priority than courseware so seems odd that it would ever be first. - # As such, I feel comfortable updating this test so it passes until it is removed - # as part of the linked ticket - assert tabs[1].type == 'course_info' + assert tabs[0].type == 'course_info' @override_waffle_flag(DISABLE_UNIFIED_COURSE_TAB_FLAG, active=False) def test_default_tab_for_new_course_experience(self): diff --git a/lms/djangoapps/discussion/plugins.py b/lms/djangoapps/discussion/plugins.py index d835761ea3..b9774d4cd8 100644 --- a/lms/djangoapps/discussion/plugins.py +++ b/lms/djangoapps/discussion/plugins.py @@ -19,7 +19,7 @@ class DiscussionTab(TabFragmentViewMixin, EnrolledTab): type = 'discussion' title = gettext_noop('Discussion') - priority = None + priority = 40 view_name = 'forum_form_discussion' fragment_view_name = 'lms.djangoapps.discussion.views.DiscussionBoardFragmentView' is_hideable = settings.FEATURES.get('ALLOW_HIDING_DISCUSSION_TAB', False) diff --git a/lms/djangoapps/edxnotes/plugins.py b/lms/djangoapps/edxnotes/plugins.py index 2e79706b82..0214f68a16 100644 --- a/lms/djangoapps/edxnotes/plugins.py +++ b/lms/djangoapps/edxnotes/plugins.py @@ -26,6 +26,7 @@ class EdxNotesTab(EnrolledTab): type = "edxnotes" title = _("Notes") view_name = "edxnotes" + priority = 50 @classmethod def is_enabled(cls, course, user=None): diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index 52983f656c..73f922f3be 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -81,6 +81,7 @@ class InstructorDashboardTab(CourseTab): title = gettext_noop('Instructor') view_name = "instructor_dashboard" is_dynamic = True # The "Instructor" tab is instead dynamically added when it is enabled + priority = 300 @classmethod def is_enabled(cls, course, user=None): diff --git a/lms/djangoapps/teams/plugins.py b/lms/djangoapps/teams/plugins.py index 33542ba7cd..3e62c23312 100644 --- a/lms/djangoapps/teams/plugins.py +++ b/lms/djangoapps/teams/plugins.py @@ -28,6 +28,7 @@ class TeamsTab(EnrolledTab): type = "teams" title = _("Teams") view_name = "teams_dashboard" + priority = 60 @classmethod def is_enabled(cls, course, user=None): diff --git a/openedx/features/lti_course_tab/tab.py b/openedx/features/lti_course_tab/tab.py index 0f880764f6..fa44ac24e4 100644 --- a/openedx/features/lti_course_tab/tab.py +++ b/openedx/features/lti_course_tab/tab.py @@ -195,6 +195,7 @@ class LtiCourseTab(LtiCourseLaunchMixin, EnrolledTab): A tab to add custom LTI components to a course in a tab. """ type = 'lti_tab' + priority = 120 is_default = False allow_multiple = True @@ -266,6 +267,7 @@ class DiscussionLtiCourseTab(LtiCourseLaunchMixin, TabFragmentViewMixin, Enrolle Course tab that loads the associated LTI-based discussion provider in a tab. """ type = 'lti_discussion' + priority = 41 allow_multiple = False is_dynamic = True title = gettext_lazy("Discussion")