diff --git a/common/lib/xmodule/xmodule/tabs.py b/common/lib/xmodule/xmodule/tabs.py index 1b94f3e..64dfe0e 100644 --- a/common/lib/xmodule/xmodule/tabs.py +++ b/common/lib/xmodule/xmodule/tabs.py @@ -57,7 +57,7 @@ class CourseTab(object): # pylint: disable=incomplete-protocol self.link_func = link_func - def can_display(self, course, settings, is_user_authenticated, is_user_staff): # pylint: disable=unused-argument + def can_display(self, course, settings, is_user_authenticated, is_user_staff, is_user_enrolled): # pylint: disable=unused-argument """ Determines whether the tab should be displayed in the UI for the given course and a particular user. This method is to be overridden by subclasses when applicable. The base class implementation @@ -78,6 +78,8 @@ class CourseTab(object): # pylint: disable=incomplete-protocol is_user_staff: Indicates whether the user has staff access to the course. If the tab is of type StaffTab and this value is False, then can_display will return False. + is_user_enrolled: Indicates whether the user is enrolled in the course + Returns: A boolean value to indicate whether this instance of the tab should be displayed to a given user for the given course. @@ -212,7 +214,7 @@ class AuthenticatedCourseTab(CourseTab): """ Abstract class for tabs that can be accessed by only authenticated users. """ - def can_display(self, course, settings, is_user_authenticated, is_user_staff): + def can_display(self, course, settings, is_user_authenticated, is_user_staff, is_user_enrolled): return is_user_authenticated @@ -220,9 +222,17 @@ class StaffTab(AuthenticatedCourseTab): """ Abstract class for tabs that can be accessed by only users with staff access. """ - def can_display(self, course, settings, is_user_authenticated, is_user_staff): # pylint: disable=unused-argument + def can_display(self, course, settings, is_user_authenticated, is_user_staff, is_user_enrolled): # pylint: disable=unused-argument return is_user_staff +class EnrolledOrStaffTab(CourseTab): + """ + Abstract class for tabs that can be accessed by only users with staff access + or users enrolled in the course. + """ + def can_display(self, course, settings, is_user_authenticated, is_user_staff, is_user_enrolled): # pylint: disable=unused-argument + return is_user_authenticated and (is_user_staff or is_user_enrolled) + class HideableTab(CourseTab): """ @@ -262,7 +272,7 @@ class HideableTab(CourseTab): return self.is_hidden == other.get('is_hidden', False) -class CoursewareTab(CourseTab): +class CoursewareTab(EnrolledOrStaffTab): """ A tab containing the course content. """ @@ -300,7 +310,7 @@ class CourseInfoTab(CourseTab): return super(CourseInfoTab, cls).validate(tab_dict, raise_error) and need_name(tab_dict, raise_error) -class ProgressTab(AuthenticatedCourseTab): +class ProgressTab(EnrolledOrStaffTab): """ A tab containing information about the authenticated user's progress. """ @@ -315,8 +325,11 @@ class ProgressTab(AuthenticatedCourseTab): link_func=link_reverse_func(self.type), ) - def can_display(self, course, settings, is_user_authenticated, is_user_staff): - return not course.hide_progress_tab + def can_display(self, course, settings, is_user_authenticated, is_user_staff, is_user_enrolled): + super_can_display = super(ProgressTab, self).can_display( + course, settings, is_user_authenticated, is_user_staff, is_user_enrolled + ) + return super_can_display and not course.hide_progress_tab @classmethod def validate(cls, tab_dict, raise_error=True): @@ -339,15 +352,17 @@ class WikiTab(HideableTab): tab_dict=tab_dict, ) - def can_display(self, course, settings, is_user_authenticated, is_user_staff): - return settings.WIKI_ENABLED + def can_display(self, course, settings, is_user_authenticated, is_user_staff, is_user_enrolled): + return settings.WIKI_ENABLED and ( + course.allow_public_wiki_access or is_user_enrolled or is_user_staff + ) @classmethod def validate(cls, tab_dict, raise_error=True): return super(WikiTab, cls).validate(tab_dict, raise_error) and need_name(tab_dict, raise_error) -class DiscussionTab(CourseTab): +class DiscussionTab(EnrolledOrStaffTab): """ A tab only for the new Berkeley discussion forums. """ @@ -362,8 +377,11 @@ class DiscussionTab(CourseTab): link_func=link_reverse_func('django_comment_client.forum.views.forum_form_discussion'), ) - def can_display(self, course, settings, is_user_authenticated, is_user_staff): - return settings.FEATURES.get('ENABLE_DISCUSSION_SERVICE') + def can_display(self, course, settings, is_user_authenticated, is_user_staff, is_user_enrolled): + super_can_display = super(DiscussionTab, self).can_display( + course, settings, is_user_authenticated, is_user_staff, is_user_enrolled + ) + return settings.FEATURES.get('ENABLE_DISCUSSION_SERVICE') and super_can_display @classmethod def validate(cls, tab_dict, raise_error=True): @@ -529,7 +547,7 @@ class TextbookTabs(TextbookTabsBase): tab_id=self.type, ) - def can_display(self, course, settings, is_user_authenticated, is_user_staff): + def can_display(self, course, settings, is_user_authenticated, is_user_staff, is_user_enrolled): return settings.FEATURES.get('ENABLE_TEXTBOOK') def items(self, course): @@ -642,7 +660,7 @@ class SyllabusTab(CourseTab): """ type = 'syllabus' - def can_display(self, course, settings, is_user_authenticated, is_user_staff): + def can_display(self, course, settings, is_user_authenticated, is_user_staff, is_user_enrolled): return hasattr(course, 'syllabus_present') and course.syllabus_present def __init__(self, tab_dict=None): # pylint: disable=unused-argument @@ -660,7 +678,7 @@ class NotesTab(AuthenticatedCourseTab): """ type = 'notes' - def can_display(self, course, settings, is_user_authenticated, is_user_staff): + def can_display(self, course, settings, is_user_authenticated, is_user_staff, is_user_enrolled): return settings.FEATURES.get('ENABLE_STUDENT_NOTES') def __init__(self, tab_dict=None): @@ -773,6 +791,7 @@ class CourseTabList(List): settings, is_user_authenticated=True, is_user_staff=True, + is_user_enrolled=False ): """ Generator method for iterating through all tabs that can be displayed for the given course and @@ -780,7 +799,7 @@ class CourseTabList(List): """ for tab in course.tabs: if tab.can_display( - course, settings, is_user_authenticated, is_user_staff + course, settings, is_user_authenticated, is_user_staff, is_user_enrolled ) and (not tab.is_hideable or not tab.is_hidden): if tab.is_collection: for item in tab.items(course): @@ -788,7 +807,7 @@ class CourseTabList(List): else: yield tab instructor_tab = InstructorTab() - if instructor_tab.can_display(course, settings, is_user_authenticated, is_user_staff): + if instructor_tab.can_display(course, settings, is_user_authenticated, is_user_staff, is_user_enrolled): yield instructor_tab @staticmethod @@ -801,7 +820,7 @@ class CourseTabList(List): with the provided settings. """ for tab in course.tabs: - if tab.can_display(course, settings, is_user_authenticated=True, is_user_staff=True): + if tab.can_display(course, settings, is_user_authenticated=True, is_user_staff=True, is_user_enrolled=True): if tab.is_collection and not len(list(tab.items(course))): # do not yield collections that have no items continue diff --git a/common/lib/xmodule/xmodule/tests/test_tabs.py b/common/lib/xmodule/xmodule/tests/test_tabs.py index 6d40d04..537f1fb 100644 --- a/common/lib/xmodule/xmodule/tests/test_tabs.py +++ b/common/lib/xmodule/xmodule/tests/test_tabs.py @@ -90,22 +90,42 @@ class TabTestCase(unittest.TestCase): deserialized_tab = tab.from_json(serialized_tab) self.assertEquals(serialized_tab, deserialized_tab) - def check_can_display_results(self, tab, expected_value=True, for_authenticated_users_only=False, for_staff_only=False): + def check_can_display_results( + self, + tab, + expected_value=True, + for_authenticated_users_only=False, + for_staff_only=False, + for_enrolled_users_only=False + ): """Checks can display results for various users""" if for_staff_only: self.assertEquals( expected_value, - tab.can_display(self.course, self.settings, is_user_authenticated=False, is_user_staff=True) + tab.can_display( + self.course, self.settings, is_user_authenticated=True, is_user_staff=True, is_user_enrolled=True + ) ) if for_authenticated_users_only: self.assertEquals( expected_value, - tab.can_display(self.course, self.settings, is_user_authenticated=True, is_user_staff=False) + tab.can_display( + self.course, self.settings, is_user_authenticated=True, is_user_staff=False, is_user_enrolled=False + ) + ) + if not for_staff_only and not for_authenticated_users_only and not for_enrolled_users_only: + self.assertEquals( + expected_value, + tab.can_display( + self.course, self.settings, is_user_authenticated=False, is_user_staff=False, is_user_enrolled=False + ) ) - if not for_staff_only and not for_authenticated_users_only: + if for_enrolled_users_only: self.assertEquals( expected_value, - tab.can_display(self.course, self.settings, is_user_authenticated=False, is_user_staff=False) + tab.can_display( + self.course, self.settings, is_user_authenticated=True, is_user_staff=False, is_user_enrolled=True + ) ) def check_get_and_set_methods(self, tab): @@ -147,11 +167,15 @@ class ProgressTestCase(TabTestCase): self.course.hide_progress_tab = False tab = self.check_progress_tab() - self.check_can_display_results(tab, for_authenticated_users_only=True) + self.check_can_display_results( + tab, for_staff_only=True, for_enrolled_users_only=True + ) self.course.hide_progress_tab = True self.check_progress_tab() - self.check_can_display_results(tab, for_authenticated_users_only=True, expected_value=False) + self.check_can_display_results( + tab, for_staff_only=True, for_enrolled_users_only=True, expected_value=False + ) class WikiTestCase(TabTestCase): @@ -167,13 +191,25 @@ class WikiTestCase(TabTestCase): invalid_dict_tab=self.fake_dict_tab, ) - def test_wiki_enabled(self): - """Test wiki tab when Enabled setting is True""" - + def test_wiki_enabled_and_public(self): + """ + Test wiki tab when Enabled setting is True and the wiki is open to + the public. + """ self.settings.WIKI_ENABLED = True + self.course.allow_public_wiki_access = True tab = self.check_wiki_tab() self.check_can_display_results(tab) + def test_wiki_enabled_and_not_public(self): + """ + Test wiki when it is enabled but not open to the public + """ + self.settings.WIKI_ENABLED = True + self.course.allow_public_wiki_access = False + tab = self.check_wiki_tab() + self.check_can_display_results(tab, for_enrolled_users_only=True, for_staff_only=True) + def test_wiki_enabled_false(self): """Test wiki tab when Enabled setting is False""" @@ -611,7 +647,14 @@ class DiscussionLinkTestCase(TabTestCase): return "default_discussion_link" return reverse_discussion_link - def check_discussion(self, tab_list, expected_discussion_link, expected_can_display_value, discussion_link_in_course=""): + def check_discussion( + self, tab_list, + expected_discussion_link, + expected_can_display_value, + discussion_link_in_course="", + is_staff=True, + is_enrolled=True, + ): """Helper function to verify whether the discussion tab exists and can be displayed""" self.course.tabs = tab_list self.course.discussion_link = discussion_link_in_course @@ -619,7 +662,7 @@ class DiscussionLinkTestCase(TabTestCase): self.assertEquals( ( discussion is not None and - discussion.can_display(self.course, self.settings, True, True) and + discussion.can_display(self.course, self.settings, True, is_staff, is_enrolled) and (discussion.link_func(self.course, self._reverse(self.course)) == expected_discussion_link) ), expected_can_display_value @@ -662,3 +705,25 @@ class DiscussionLinkTestCase(TabTestCase): expected_discussion_link=not None, expected_can_display_value=False, ) + + def test_tabs_enrolled_or_staff(self): + self.settings.FEATURES['ENABLE_DISCUSSION_SERVICE'] = True + for is_enrolled, is_staff in [(True, False), (False, True)]: + self.check_discussion( + tab_list=self.tabs_with_discussion, + expected_discussion_link="default_discussion_link", + expected_can_display_value=True, + is_enrolled=is_enrolled, + is_staff=is_staff + ) + + def test_tabs_not_enrolled_or_staff(self): + self.settings.FEATURES['ENABLE_DISCUSSION_SERVICE'] = True + is_enrolled = is_staff = False + self.check_discussion( + tab_list=self.tabs_with_discussion, + expected_discussion_link="default_discussion_link", + expected_can_display_value=False, + is_enrolled=is_enrolled, + is_staff=is_staff + ) diff --git a/lms/djangoapps/courseware/tests/test_tabs.py b/lms/djangoapps/courseware/tests/test_tabs.py index 3b36599..8dad034 100644 --- a/lms/djangoapps/courseware/tests/test_tabs.py +++ b/lms/djangoapps/courseware/tests/test_tabs.py @@ -95,4 +95,3 @@ class StaticTabDateTestCaseXML(LoginEnrollmentTestCase, ModuleStoreTestCase): resp = self.client.get(url) self.assertEqual(resp.status_code, 200) self.assertIn(self.xml_data, resp.content) - diff --git a/lms/templates/courseware/course_navigation.html b/lms/templates/courseware/course_navigation.html index bff88e1..267bc68 100644 --- a/lms/templates/courseware/course_navigation.html +++ b/lms/templates/courseware/course_navigation.html @@ -18,12 +18,16 @@ def url_class(is_active): <%! from django.core.urlresolvers import reverse %> <%! from django.utils.translation import ugettext as _ %> <%! from courseware.views import notification_image_for_tab %> +<%! from student.models import CourseEnrollment%> +<% + user_is_enrolled = user.is_authenticated() and CourseEnrollment.is_enrolled(user, course.id) +%> % if disable_tabs is UNDEFINED or not disable_tabs: <nav class="${active_page} course-material"> <div class="inner-wrapper"> <ol class="course-tabs"> - % for tab in CourseTabList.iterate_displayable(course, settings, user.is_authenticated(), has_access(user, 'staff', course, course.id)): + % for tab in CourseTabList.iterate_displayable(course, settings, user.is_authenticated(), has_access(user, 'staff', course, course.id), user_is_enrolled): <% tab_is_active = (tab.tab_id == active_page) or (tab.tab_id == default_tab) tab_image = notification_image_for_tab(tab, user, course) diff --git a/lms/templates/help_modal.html b/lms/templates/help_modal.html index d39ca20..57f8b87 100644 --- a/lms/templates/help_modal.html +++ b/lms/templates/help_modal.html @@ -37,7 +37,7 @@ <% discussion_tab = CourseTabList.get_discussion(course) if course else None - discussion_link = discussion_tab.link_func(course, reverse) if (discussion_tab and discussion_tab.can_display(course, settings, True, True)) else None + discussion_link = discussion_tab.link_func(course, reverse) if (discussion_tab and discussion_tab.can_display(course, settings, True, True, True)) else None %> % if discussion_link: