only load tabs if user is enrolled in course (TNL-286)
add tests for static tabs
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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:
|
||||
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)
|
||||
tab.can_display(
|
||||
self.course, self.settings, is_user_authenticated=False, is_user_staff=False, is_user_enrolled=False
|
||||
)
|
||||
)
|
||||
if for_enrolled_users_only:
|
||||
self.assertEquals(
|
||||
expected_value,
|
||||
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
|
||||
)
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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:
|
||||
|
||||
Reference in New Issue
Block a user