From 7f4da14185328ea5c7ec6d74417fb7bdabc2ce22 Mon Sep 17 00:00:00 2001 From: Adam Palay Date: Tue, 16 Sep 2014 10:58:30 -0400 Subject: [PATCH 1/5] Add middleware optionally to catch unenrolled students who fail has_access (TNL-286) --- lms/djangoapps/courseware/courses.py | 14 ++++- lms/djangoapps/courseware/middleware.py | 23 ++++++++ .../courseware/tests/test_middleware.py | 53 +++++++++++++++++++ .../django_comment_client/forum/tests.py | 24 +++++++++ .../django_comment_client/forum/views.py | 2 +- lms/envs/common.py | 3 ++ 6 files changed, 117 insertions(+), 2 deletions(-) create mode 100644 lms/djangoapps/courseware/middleware.py create mode 100644 lms/djangoapps/courseware/tests/test_middleware.py diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index dee09c0d59..7d3c9d53a3 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -20,6 +20,7 @@ from xmodule.x_module import STUDENT_VIEW from courseware.access import has_access from courseware.model_data import FieldDataCache from courseware.module_render import get_module +from student.models import CourseEnrollment import branding log = logging.getLogger(__name__) @@ -72,7 +73,13 @@ def get_course_by_id(course_key, depth=0): raise Http404("Course not found.") -def get_course_with_access(user, action, course_key, depth=0): +class UserNotEnrolled(Http404): + def __init__(self, course_key): + super(UserNotEnrolled, self).__init__() + self.course_key = course_key + + +def get_course_with_access(user, action, course_key, depth=0, check_if_enrolled=False): """ Given a course_key, look up the corresponding course descriptor, check that the user has the access to perform the specified action @@ -86,6 +93,11 @@ def get_course_with_access(user, action, course_key, depth=0): course = get_course_by_id(course_key, depth=depth) if not has_access(user, action, course, course_key): + if check_if_enrolled and not CourseEnrollment.is_enrolled(user, course_key): + # If user is not enrolled, raise UserNotEnrolled exception that will + # be caught by middleware + raise UserNotEnrolled(course_key) + # Deliberately return a non-specific error message to avoid # leaking info about access control settings raise Http404("Course not found.") diff --git a/lms/djangoapps/courseware/middleware.py b/lms/djangoapps/courseware/middleware.py new file mode 100644 index 0000000000..7ef0844db5 --- /dev/null +++ b/lms/djangoapps/courseware/middleware.py @@ -0,0 +1,23 @@ +""" +Middleware for the courseware app +""" + +from django.shortcuts import redirect +from django.core.urlresolvers import reverse + +from courseware.courses import UserNotEnrolled + +class RedirectUnenrolledMiddleware(object): + """ + Catch UserNotEnrolled errors thrown by `get_course_with_access` and redirect + users to the course about page + """ + def process_exception(self, request, exception): + if isinstance(exception, UserNotEnrolled): + course_key = exception.course_key + return redirect( + reverse( + 'courseware.views.course_about', + args=[course_key.to_deprecated_string()] + ) + ) diff --git a/lms/djangoapps/courseware/tests/test_middleware.py b/lms/djangoapps/courseware/tests/test_middleware.py new file mode 100644 index 0000000000..ff92b33e92 --- /dev/null +++ b/lms/djangoapps/courseware/tests/test_middleware.py @@ -0,0 +1,53 @@ +""" +Tests for courseware middleware +""" + +from django.core.urlresolvers import reverse +from django.test.utils import override_settings +from django.test.client import RequestFactory +from django.http import Http404 +from mock import patch + +from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE +import courseware.courses as courses +from courseware.middleware import RedirectUnenrolledMiddleware +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + + +@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) +class CoursewareMiddlewareTestCase(ModuleStoreTestCase): + """Tests that courseware middleware is correctly redirected""" + + def setUp(self): + self.course = CourseFactory.create() + + def check_user_not_enrolled_redirect(self): + """A UserNotEnrolled exception should trigger a redirect""" + request = RequestFactory().get("dummy_url") + response = RedirectUnenrolledMiddleware().process_exception( + request, courses.UserNotEnrolled(self.course.id) + ) + self.assertEqual(response.status_code, 302) + # make sure we redirect to the course about page + expected_url = reverse( + "about_course", args=[self.course.id.to_deprecated_string()] + ) + + target_url = response._headers['location'][1] + self.assertTrue(target_url.endswith(expected_url)) + + def test_user_not_enrolled_redirect(self): + self.check_user_not_enrolled_redirect() + + @patch.dict("django.conf.settings.FEATURES", {"ENABLE_MKTG_SITE": True}) + def test_user_not_enrolled_redirect_mktg(self): + self.check_user_not_enrolled_redirect() + + def test_process_404(self): + """A 404 should not trigger anything""" + request = RequestFactory().get("dummy_url") + response = RedirectUnenrolledMiddleware().process_exception( + request, Http404() + ) + self.assertIsNone(response) diff --git a/lms/djangoapps/django_comment_client/forum/tests.py b/lms/djangoapps/django_comment_client/forum/tests.py index 6951318a1f..5cc8150770 100644 --- a/lms/djangoapps/django_comment_client/forum/tests.py +++ b/lms/djangoapps/django_comment_client/forum/tests.py @@ -20,6 +20,7 @@ from django_comment_client.tests.utils import CohortedContentTestCase from django_comment_client.forum import views from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE +from courseware.courses import UserNotEnrolled from nose.tools import assert_true # pylint: disable=E0611 from mock import patch, Mock, ANY, call @@ -913,3 +914,26 @@ class FollowedThreadsUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin): response_data = json.loads(response.content) self.assertEqual(response_data["discussion_data"][0]["title"], text) self.assertEqual(response_data["discussion_data"][0]["body"], text) + + +@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) +class EnrollmentTestCase(ModuleStoreTestCase): + """ + Tests for the behavior of views depending on if the student is enrolled + in the course + """ + + @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) + def setUp(self): + super(EnrollmentTestCase, self).setUp() + self.course = CourseFactory.create() + self.student = UserFactory.create() + + @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) + @patch('lms.lib.comment_client.utils.requests.request') + def test_unenrolled(self, mock_request): + mock_request.side_effect = make_mock_request_impl('dummy') + request = RequestFactory().get('dummy_url') + request.user = self.student + with self.assertRaises(UserNotEnrolled): + views.forum_form_discussion(request, course_id=self.course.id.to_deprecated_string()) diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index 0f450e66ec..73582db039 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/lms/djangoapps/django_comment_client/forum/views.py @@ -163,7 +163,7 @@ def forum_form_discussion(request, course_id): course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) nr_transaction = newrelic.agent.current_transaction() - course = get_course_with_access(request.user, 'load_forum', course_key) + course = get_course_with_access(request.user, 'load_forum', course_key, check_if_enrolled=True) course_settings = make_course_settings(course, include_category_map=True) user = cc.User.from_django_user(request.user) diff --git a/lms/envs/common.py b/lms/envs/common.py index 5c045bd0d5..b331e39340 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -941,6 +941,9 @@ MIDDLEWARE_CLASSES = ( # use Django built in clickjacking protection 'django.middleware.clickjacking.XFrameOptionsMiddleware', + # to redirected unenrolled students to the course info page + 'courseware.middleware.RedirectUnenrolledMiddleware', + 'course_wiki.middleware.WikiAccessMiddleware', ) From e569e090c2be42c45267d3e16a14869d96457ebd Mon Sep 17 00:00:00 2001 From: Adam Palay Date: Tue, 16 Sep 2014 11:00:47 -0400 Subject: [PATCH 2/5] redirect users from about page to info page when there is a marketing site (TNL-286) --- lms/djangoapps/courseware/tests/test_about.py | 13 +++++++++++++ lms/djangoapps/courseware/views.py | 8 ++++---- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_about.py b/lms/djangoapps/courseware/tests/test_about.py index 44bb1e2b87..f4870a6b06 100644 --- a/lms/djangoapps/courseware/tests/test_about.py +++ b/lms/djangoapps/courseware/tests/test_about.py @@ -46,6 +46,19 @@ class AboutTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): # Check that registration button is present self.assertIn(REG_STR, resp.content) + @patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True}) + def test_logged_in_marketing(self): + self.setup_user() + url = reverse('about_course', args=[self.course.id.to_deprecated_string()]) + resp = self.client.get(url) + # should be redirected + self.assertEqual(resp.status_code, 302) + # follow this time, and check we're redirected to the course info page + resp = self.client.get(url, follow=True) + target_url = resp.redirect_chain[-1][0] + info_url = reverse('info', args=[self.course.id.to_deprecated_string()]) + self.assertTrue(target_url.endswith(info_url)) + @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) class AboutTestCaseXML(LoginEnrollmentTestCase, ModuleStoreTestCase): diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 5df386c957..35adef43d6 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -658,15 +658,15 @@ def course_about(request, course_id): Assumes the course_id is in a valid format. """ + course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) + course = get_course_with_access(request.user, 'see_exists', course_key) + if microsite.get_value( 'ENABLE_MKTG_SITE', settings.FEATURES.get('ENABLE_MKTG_SITE', False) ): - raise Http404 + return redirect(reverse('info', args=[course.id.to_deprecated_string()])) - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) - - course = get_course_with_access(request.user, 'see_exists', course_key) registered = registered_for_course(course, request.user) staff_access = has_access(request.user, 'staff', course) studio_url = get_studio_url(course, 'settings/details') From 1b06edb6ffdb06e1990176c293da782820e518b5 Mon Sep 17 00:00:00 2001 From: Adam Palay Date: Thu, 11 Sep 2014 14:54:16 -0400 Subject: [PATCH 3/5] display banner on course info page if user is not enrolled (TNL-286) write failing tests for info page banner --- .../courseware/tests/test_course_info.py | 9 ++++++++- lms/djangoapps/courseware/views.py | 14 +++++++++++-- lms/templates/courseware/info.html | 20 ++++++++++++++++++- 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_course_info.py b/lms/djangoapps/courseware/tests/test_course_info.py index ba69b94166..d1f01b5288 100644 --- a/lms/djangoapps/courseware/tests/test_course_info.py +++ b/lms/djangoapps/courseware/tests/test_course_info.py @@ -20,12 +20,19 @@ class CourseInfoTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): data="OOGIE BLOOGIE", display_name="updates" ) - def test_logged_in(self): + def test_logged_in_unenrolled(self): self.setup_user() url = reverse('info', args=[self.course.id.to_deprecated_string()]) resp = self.client.get(url) self.assertEqual(resp.status_code, 200) self.assertIn("OOGIE BLOOGIE", resp.content) + self.assertIn("You are not currently enrolled in this course", resp.content) + + def test_logged_in_enrolled(self): + self.enroll(self.course) + url = reverse('info', args=[self.course.id.to_deprecated_string()]) + resp = self.client.get(url) + self.assertNotIn("You are not currently enrolled in this course", resp.content) def test_anonymous_user(self): url = reverse('info', args=[self.course.id.to_deprecated_string()]) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 35adef43d6..20164db01f 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -19,7 +19,7 @@ from django.contrib.auth.decorators import login_required from django.views.decorators.http import require_GET from django.http import Http404, HttpResponse from django.shortcuts import redirect -from edxmako.shortcuts import render_to_response, render_to_string +from edxmako.shortcuts import render_to_response, render_to_string, marketing_link from django_future.csrf import ensure_csrf_cookie from django.views.decorators.cache import cache_control from django.db import transaction @@ -569,6 +569,14 @@ def course_info(request, course_id): reverifications = fetch_reverify_banner_info(request, course_key) studio_url = get_studio_url(course, 'course_info') + # link to where the student should go to enroll in the course: + # about page if there is not marketing site, SITE_NAME if there is + url_to_enroll = reverse(course_about, args=[course_id]) + if settings.FEATURES.get('ENABLE_MKTG_SITE'): + url_to_enroll = marketing_link('COURSES') + + show_enroll_banner = request.user.is_authenticated() and not CourseEnrollment.is_enrolled(request.user, course.id) + context = { 'request': request, 'course_id': course_key.to_deprecated_string(), @@ -578,6 +586,8 @@ def course_info(request, course_id): 'masquerade': masq, 'studio_url': studio_url, 'reverifications': reverifications, + 'show_enroll_banner': show_enroll_banner, + 'url_to_enroll': url_to_enroll, } return render_to_response('courseware/info.html', context) @@ -806,7 +816,7 @@ def _progress(request, course_key, student_id): Course staff are allowed to see the progress of students in their class. """ - course = get_course_with_access(request.user, 'load', course_key, depth=None) + course = get_course_with_access(request.user, 'load', course_key, depth=None, check_if_enrolled=True) staff_access = has_access(request.user, 'staff', course) if student_id is None or student_id == request.user.id: diff --git a/lms/templates/courseware/info.html b/lms/templates/courseware/info.html index 30470382c1..9b8b5b5e28 100644 --- a/lms/templates/courseware/info.html +++ b/lms/templates/courseware/info.html @@ -13,6 +13,24 @@ <%include file="/dashboard/_dashboard_prompt_midcourse_reverify.html" /> +% if show_enroll_banner: +
+
+
+

${_("You are not enrolled yet")}

+
+

+ ${_(u"You are not currently enrolled in this course. Sign up for it {link_start}here{link_end}!").format( + link_start=u"".format(url_to_enroll), + link_end=u"" + )} +

+
+
+
+
+% endif + <%include file="/courseware/course_navigation.html" args="active_page='info'" /> <%block name="js_extra"> @@ -36,7 +54,7 @@ $(document).ready(function(){ % endif % endif - +

${_("Course Updates & News")}

${get_course_info_section(request, course, 'updates')} From ac862b7a9779bf421cbb65e13f4f0985fc0ce39e Mon Sep 17 00:00:00 2001 From: Adam Palay Date: Thu, 11 Sep 2014 10:55:18 -0400 Subject: [PATCH 4/5] only load tabs if user is enrolled in course (TNL-286) add tests for static tabs --- common/lib/xmodule/xmodule/tabs.py | 55 ++++++++---- common/lib/xmodule/xmodule/tests/test_tabs.py | 89 ++++++++++++++++--- lms/djangoapps/courseware/tests/test_tabs.py | 1 - .../courseware/course_navigation.html | 6 +- lms/templates/help_modal.html | 2 +- 5 files changed, 120 insertions(+), 33 deletions(-) diff --git a/common/lib/xmodule/xmodule/tabs.py b/common/lib/xmodule/xmodule/tabs.py index 1b94f3ee02..64dfe0ee30 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 6d40d04317..537f1fb2e0 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: + 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 + ) diff --git a/lms/djangoapps/courseware/tests/test_tabs.py b/lms/djangoapps/courseware/tests/test_tabs.py index 3b36599d3f..8dad0342c5 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 bff88e1464..267bc6896a 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: