diff --git a/common/lib/xmodule/xmodule/tabs.py b/common/lib/xmodule/xmodule/tabs.py index 8a8f123c86..71be051417 100644 --- a/common/lib/xmodule/xmodule/tabs.py +++ b/common/lib/xmodule/xmodule/tabs.py @@ -414,7 +414,6 @@ class CourseTabList(List): discussion_tab, CourseTab.load('wiki'), CourseTab.load('progress'), - CourseTab.load('dates'), ]) @staticmethod diff --git a/lms/djangoapps/courseware/tabs.py b/lms/djangoapps/courseware/tabs.py index 99b184afe9..3b952443e1 100644 --- a/lms/djangoapps/courseware/tabs.py +++ b/lms/djangoapps/courseware/tabs.py @@ -348,6 +348,12 @@ def get_course_tab_list(user, course): if tab.type == 'static_tab' and tab.course_staff_only and \ not bool(user and has_access(user, 'staff', course, course.id)): continue + # We had initially created a CourseTab.load() for dates that ended up + # persisting the dates tab tomodulestore on Course Run creation, but + # ignoring any static dates tab here we can fix forward without + # allowing the bug to continue to surface + if tab.type == 'dates': + continue course_tab_list.append(tab) # Add in any dynamic tabs, i.e. those that are not persisted diff --git a/lms/djangoapps/courseware/tests/test_tabs.py b/lms/djangoapps/courseware/tests/test_tabs.py index 0516ca00cc..b5d621beff 100644 --- a/lms/djangoapps/courseware/tests/test_tabs.py +++ b/lms/djangoapps/courseware/tests/test_tabs.py @@ -17,6 +17,7 @@ from lms.djangoapps.courseware.courses import get_course_by_id from lms.djangoapps.courseware.tabs import ( CourseInfoTab, CoursewareTab, + DatesTab, ExternalDiscussionCourseTab, ExternalLinkCourseTab, ProgressTab, @@ -543,6 +544,7 @@ class TabListTestCase(TabTestCase): [ {'type': CoursewareTab.type}, {'type': CourseInfoTab.type, 'name': 'fake_name'}, + {'type': DatesTab.type}, # Add this even though we filter it out, for testing purposes {'type': 'discussion', 'name': 'fake_name'}, {'type': ExternalLinkCourseTab.type, 'name': 'fake_name', 'link': 'fake_link'}, {'type': ExternalLinkCourseTab.type, 'name': 'fake_name', 'link': 'fake_link'}, @@ -656,13 +658,16 @@ class CourseTabListTestCase(TabListTestCase): # enumerate the tabs with a staff user user = UserFactory(is_staff=True) CourseEnrollment.enroll(user, self.course.id) - for i, tab in enumerate(xmodule_tabs.CourseTabList.iterate_displayable(self.course, user=user)): - if getattr(tab, 'is_collection_item', False): - # a collection item was found as a result of a collection tab - self.assertTrue(getattr(self.course.tabs[i], 'is_collection', False)) - else: - # all other tabs must match the expected type - self.assertEqual(tab.type, self.course.tabs[i].type) + # Need to mock this flag as we care that orders match, and a tab not enabled will result in a failure + with patch('lms.djangoapps.courseware.tabs.RELATIVE_DATES_FLAG') as mock_flag: + mock_flag.is_enabled().return_value = True + for i, tab in enumerate(xmodule_tabs.CourseTabList.iterate_displayable(self.course, user=user)): + if getattr(tab, 'is_collection_item', False): + # a collection item was found as a result of a collection tab + self.assertTrue(getattr(self.course.tabs[i], 'is_collection', False)) + else: + # all other tabs must match the expected type + self.assertEqual(tab.type, self.course.tabs[i].type) # test including non-empty collections self.assertIn( @@ -895,3 +900,29 @@ class DiscussionLinkTestCase(TabTestCase): is_enrolled=is_enrolled, is_staff=is_staff ) + + +class DatesTabTestCase(TabListTestCase): + """Test cases for making sure no persisted dates tab is surfaced""" + + def test_singular_dates_tab(self): + user = self.create_mock_user() + self.course.tabs = self.all_valid_tab_list + self.course.save() + + # Verify that there is a dates tab in the modulestore + has_dates_tab = False + for tab in self.course.tabs: + if tab.type == 'dates': + has_dates_tab = True + self.assertTrue(has_dates_tab) + + # Verify that there is only 1 'dates' tab in the returned result from get_course_tab_list() + with patch('lms.djangoapps.courseware.tabs.RELATIVE_DATES_FLAG') as mock_flag: + mock_flag.is_enabled().return_value = True + tabs = get_course_tab_list(user, self.course) + num_dates_tabs = 0 + for tab in tabs: + if tab.type == 'dates': + num_dates_tabs += 1 + self.assertEqual(num_dates_tabs, 1) diff --git a/openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py b/openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py index f5f8b95a0e..05bb64cb22 100644 --- a/openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py +++ b/openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py @@ -65,7 +65,7 @@ class CourseOverviewTestCase(CatalogIntegrationMixin, ModuleStoreTestCase, Cache None: None, } - COURSE_OVERVIEW_TABS = {'courseware', 'info', 'textbooks', 'discussion', 'wiki', 'progress', 'dates'} + COURSE_OVERVIEW_TABS = {'courseware', 'info', 'textbooks', 'discussion', 'wiki', 'progress'} ENABLED_SIGNALS = ['course_deleted', 'course_published']