Commit 81d8c40e by John Eskew

Merge pull request #5281 from edx/adam/hide-tabs-if-not-enrolled-rebase-2

Deployed as hotfix
parents 6bf52074 0a385544
...@@ -260,12 +260,12 @@ class EmbargoMiddlewareTests(ModuleStoreTestCase): ...@@ -260,12 +260,12 @@ class EmbargoMiddlewareTests(ModuleStoreTestCase):
profile.save() profile.save()
# Warm the cache # Warm the cache
with self.assertNumQueries(14): with self.assertNumQueries(16):
self.client.get(self.embargoed_page) self.client.get(self.embargoed_page)
# Access the page multiple times, but expect that we hit # Access the page multiple times, but expect that we hit
# the database to check the user's profile only once # the database to check the user's profile only once
with self.assertNumQueries(8): with self.assertNumQueries(10):
self.client.get(self.embargoed_page) self.client.get(self.embargoed_page)
def test_embargo_profile_country_db_null(self): def test_embargo_profile_country_db_null(self):
......
...@@ -57,7 +57,7 @@ class CourseTab(object): # pylint: disable=incomplete-protocol ...@@ -57,7 +57,7 @@ class CourseTab(object): # pylint: disable=incomplete-protocol
self.link_func = link_func 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. 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 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 ...@@ -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 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. 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: Returns:
A boolean value to indicate whether this instance of the tab should be displayed to a A boolean value to indicate whether this instance of the tab should be displayed to a
given user for the given course. given user for the given course.
...@@ -212,7 +214,7 @@ class AuthenticatedCourseTab(CourseTab): ...@@ -212,7 +214,7 @@ class AuthenticatedCourseTab(CourseTab):
""" """
Abstract class for tabs that can be accessed by only authenticated users. 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 return is_user_authenticated
...@@ -220,9 +222,17 @@ class StaffTab(AuthenticatedCourseTab): ...@@ -220,9 +222,17 @@ class StaffTab(AuthenticatedCourseTab):
""" """
Abstract class for tabs that can be accessed by only users with staff access. 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 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): class HideableTab(CourseTab):
""" """
...@@ -262,7 +272,7 @@ class HideableTab(CourseTab): ...@@ -262,7 +272,7 @@ class HideableTab(CourseTab):
return self.is_hidden == other.get('is_hidden', False) return self.is_hidden == other.get('is_hidden', False)
class CoursewareTab(CourseTab): class CoursewareTab(EnrolledOrStaffTab):
""" """
A tab containing the course content. A tab containing the course content.
""" """
...@@ -300,7 +310,7 @@ class CourseInfoTab(CourseTab): ...@@ -300,7 +310,7 @@ class CourseInfoTab(CourseTab):
return super(CourseInfoTab, cls).validate(tab_dict, raise_error) and need_name(tab_dict, raise_error) 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. A tab containing information about the authenticated user's progress.
""" """
...@@ -315,8 +325,11 @@ class ProgressTab(AuthenticatedCourseTab): ...@@ -315,8 +325,11 @@ class ProgressTab(AuthenticatedCourseTab):
link_func=link_reverse_func(self.type), link_func=link_reverse_func(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 not course.hide_progress_tab 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 @classmethod
def validate(cls, tab_dict, raise_error=True): def validate(cls, tab_dict, raise_error=True):
...@@ -339,15 +352,17 @@ class WikiTab(HideableTab): ...@@ -339,15 +352,17 @@ class WikiTab(HideableTab):
tab_dict=tab_dict, tab_dict=tab_dict,
) )
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.WIKI_ENABLED return settings.WIKI_ENABLED and (
course.allow_public_wiki_access or is_user_enrolled or is_user_staff
)
@classmethod @classmethod
def validate(cls, tab_dict, raise_error=True): def validate(cls, tab_dict, raise_error=True):
return super(WikiTab, cls).validate(tab_dict, raise_error) and need_name(tab_dict, raise_error) 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. A tab only for the new Berkeley discussion forums.
""" """
...@@ -362,8 +377,11 @@ class DiscussionTab(CourseTab): ...@@ -362,8 +377,11 @@ class DiscussionTab(CourseTab):
link_func=link_reverse_func('django_comment_client.forum.views.forum_form_discussion'), 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): def can_display(self, course, settings, is_user_authenticated, is_user_staff, is_user_enrolled):
return settings.FEATURES.get('ENABLE_DISCUSSION_SERVICE') 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 @classmethod
def validate(cls, tab_dict, raise_error=True): def validate(cls, tab_dict, raise_error=True):
...@@ -529,7 +547,7 @@ class TextbookTabs(TextbookTabsBase): ...@@ -529,7 +547,7 @@ class TextbookTabs(TextbookTabsBase):
tab_id=self.type, 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') return settings.FEATURES.get('ENABLE_TEXTBOOK')
def items(self, course): def items(self, course):
...@@ -642,7 +660,7 @@ class SyllabusTab(CourseTab): ...@@ -642,7 +660,7 @@ class SyllabusTab(CourseTab):
""" """
type = 'syllabus' 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 return hasattr(course, 'syllabus_present') and course.syllabus_present
def __init__(self, tab_dict=None): # pylint: disable=unused-argument def __init__(self, tab_dict=None): # pylint: disable=unused-argument
...@@ -660,7 +678,7 @@ class NotesTab(AuthenticatedCourseTab): ...@@ -660,7 +678,7 @@ class NotesTab(AuthenticatedCourseTab):
""" """
type = 'notes' 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') return settings.FEATURES.get('ENABLE_STUDENT_NOTES')
def __init__(self, tab_dict=None): def __init__(self, tab_dict=None):
...@@ -773,6 +791,7 @@ class CourseTabList(List): ...@@ -773,6 +791,7 @@ class CourseTabList(List):
settings, settings,
is_user_authenticated=True, is_user_authenticated=True,
is_user_staff=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 Generator method for iterating through all tabs that can be displayed for the given course and
...@@ -780,7 +799,7 @@ class CourseTabList(List): ...@@ -780,7 +799,7 @@ class CourseTabList(List):
""" """
for tab in course.tabs: for tab in course.tabs:
if tab.can_display( 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): ) and (not tab.is_hideable or not tab.is_hidden):
if tab.is_collection: if tab.is_collection:
for item in tab.items(course): for item in tab.items(course):
...@@ -788,7 +807,7 @@ class CourseTabList(List): ...@@ -788,7 +807,7 @@ class CourseTabList(List):
else: else:
yield tab yield tab
instructor_tab = InstructorTab() 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 yield instructor_tab
@staticmethod @staticmethod
...@@ -801,7 +820,7 @@ class CourseTabList(List): ...@@ -801,7 +820,7 @@ class CourseTabList(List):
with the provided settings. with the provided settings.
""" """
for tab in course.tabs: 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))): if tab.is_collection and not len(list(tab.items(course))):
# do not yield collections that have no items # do not yield collections that have no items
continue continue
......
...@@ -90,22 +90,42 @@ class TabTestCase(unittest.TestCase): ...@@ -90,22 +90,42 @@ class TabTestCase(unittest.TestCase):
deserialized_tab = tab.from_json(serialized_tab) deserialized_tab = tab.from_json(serialized_tab)
self.assertEquals(serialized_tab, deserialized_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""" """Checks can display results for various users"""
if for_staff_only: if for_staff_only:
self.assertEquals( self.assertEquals(
expected_value, 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: if for_authenticated_users_only:
self.assertEquals( self.assertEquals(
expected_value, 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( self.assertEquals(
expected_value, 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): def check_get_and_set_methods(self, tab):
...@@ -147,11 +167,15 @@ class ProgressTestCase(TabTestCase): ...@@ -147,11 +167,15 @@ class ProgressTestCase(TabTestCase):
self.course.hide_progress_tab = False self.course.hide_progress_tab = False
tab = self.check_progress_tab() 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.course.hide_progress_tab = True
self.check_progress_tab() 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): class WikiTestCase(TabTestCase):
...@@ -167,13 +191,25 @@ class WikiTestCase(TabTestCase): ...@@ -167,13 +191,25 @@ class WikiTestCase(TabTestCase):
invalid_dict_tab=self.fake_dict_tab, invalid_dict_tab=self.fake_dict_tab,
) )
def test_wiki_enabled(self): def test_wiki_enabled_and_public(self):
"""Test wiki tab when Enabled setting is True""" """
Test wiki tab when Enabled setting is True and the wiki is open to
the public.
"""
self.settings.WIKI_ENABLED = True self.settings.WIKI_ENABLED = True
self.course.allow_public_wiki_access = True
tab = self.check_wiki_tab() tab = self.check_wiki_tab()
self.check_can_display_results(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): def test_wiki_enabled_false(self):
"""Test wiki tab when Enabled setting is False""" """Test wiki tab when Enabled setting is False"""
...@@ -611,7 +647,14 @@ class DiscussionLinkTestCase(TabTestCase): ...@@ -611,7 +647,14 @@ class DiscussionLinkTestCase(TabTestCase):
return "default_discussion_link" return "default_discussion_link"
return reverse_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""" """Helper function to verify whether the discussion tab exists and can be displayed"""
self.course.tabs = tab_list self.course.tabs = tab_list
self.course.discussion_link = discussion_link_in_course self.course.discussion_link = discussion_link_in_course
...@@ -619,7 +662,7 @@ class DiscussionLinkTestCase(TabTestCase): ...@@ -619,7 +662,7 @@ class DiscussionLinkTestCase(TabTestCase):
self.assertEquals( self.assertEquals(
( (
discussion is not None and 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) (discussion.link_func(self.course, self._reverse(self.course)) == expected_discussion_link)
), ),
expected_can_display_value expected_can_display_value
...@@ -662,3 +705,25 @@ class DiscussionLinkTestCase(TabTestCase): ...@@ -662,3 +705,25 @@ class DiscussionLinkTestCase(TabTestCase):
expected_discussion_link=not None, expected_discussion_link=not None,
expected_can_display_value=False, 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
)
...@@ -20,6 +20,7 @@ from xmodule.x_module import STUDENT_VIEW ...@@ -20,6 +20,7 @@ from xmodule.x_module import STUDENT_VIEW
from courseware.access import has_access from courseware.access import has_access
from courseware.model_data import FieldDataCache from courseware.model_data import FieldDataCache
from courseware.module_render import get_module from courseware.module_render import get_module
from student.models import CourseEnrollment
import branding import branding
log = logging.getLogger(__name__) log = logging.getLogger(__name__)
...@@ -72,7 +73,13 @@ def get_course_by_id(course_key, depth=0): ...@@ -72,7 +73,13 @@ def get_course_by_id(course_key, depth=0):
raise Http404("Course not found.") 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, Given a course_key, look up the corresponding course descriptor,
check that the user has the access to perform the specified action 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): ...@@ -86,6 +93,11 @@ def get_course_with_access(user, action, course_key, depth=0):
course = get_course_by_id(course_key, depth=depth) course = get_course_by_id(course_key, depth=depth)
if not has_access(user, action, course, course_key): 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 # Deliberately return a non-specific error message to avoid
# leaking info about access control settings # leaking info about access control settings
raise Http404("Course not found.") raise Http404("Course not found.")
......
"""
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()]
)
)
...@@ -46,6 +46,19 @@ class AboutTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): ...@@ -46,6 +46,19 @@ class AboutTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase):
# Check that registration button is present # Check that registration button is present
self.assertIn(REG_STR, resp.content) 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) @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
class AboutTestCaseXML(LoginEnrollmentTestCase, ModuleStoreTestCase): class AboutTestCaseXML(LoginEnrollmentTestCase, ModuleStoreTestCase):
......
...@@ -20,12 +20,19 @@ class CourseInfoTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): ...@@ -20,12 +20,19 @@ class CourseInfoTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase):
data="OOGIE BLOOGIE", display_name="updates" data="OOGIE BLOOGIE", display_name="updates"
) )
def test_logged_in(self): def test_logged_in_unenrolled(self):
self.setup_user() self.setup_user()
url = reverse('info', args=[self.course.id.to_deprecated_string()]) url = reverse('info', args=[self.course.id.to_deprecated_string()])
resp = self.client.get(url) resp = self.client.get(url)
self.assertEqual(resp.status_code, 200) self.assertEqual(resp.status_code, 200)
self.assertIn("OOGIE BLOOGIE", resp.content) 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): def test_anonymous_user(self):
url = reverse('info', args=[self.course.id.to_deprecated_string()]) url = reverse('info', args=[self.course.id.to_deprecated_string()])
......
"""
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)
...@@ -95,4 +95,3 @@ class StaticTabDateTestCaseXML(LoginEnrollmentTestCase, ModuleStoreTestCase): ...@@ -95,4 +95,3 @@ class StaticTabDateTestCaseXML(LoginEnrollmentTestCase, ModuleStoreTestCase):
resp = self.client.get(url) resp = self.client.get(url)
self.assertEqual(resp.status_code, 200) self.assertEqual(resp.status_code, 200)
self.assertIn(self.xml_data, resp.content) self.assertIn(self.xml_data, resp.content)
...@@ -19,7 +19,7 @@ from django.contrib.auth.decorators import login_required ...@@ -19,7 +19,7 @@ from django.contrib.auth.decorators import login_required
from django.views.decorators.http import require_GET from django.views.decorators.http import require_GET
from django.http import Http404, HttpResponse from django.http import Http404, HttpResponse
from django.shortcuts import redirect 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_future.csrf import ensure_csrf_cookie
from django.views.decorators.cache import cache_control from django.views.decorators.cache import cache_control
from django.db import transaction from django.db import transaction
...@@ -569,6 +569,14 @@ def course_info(request, course_id): ...@@ -569,6 +569,14 @@ def course_info(request, course_id):
reverifications = fetch_reverify_banner_info(request, course_key) reverifications = fetch_reverify_banner_info(request, course_key)
studio_url = get_studio_url(course, 'course_info') 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 = { context = {
'request': request, 'request': request,
'course_id': course_key.to_deprecated_string(), 'course_id': course_key.to_deprecated_string(),
...@@ -578,6 +586,8 @@ def course_info(request, course_id): ...@@ -578,6 +586,8 @@ def course_info(request, course_id):
'masquerade': masq, 'masquerade': masq,
'studio_url': studio_url, 'studio_url': studio_url,
'reverifications': reverifications, 'reverifications': reverifications,
'show_enroll_banner': show_enroll_banner,
'url_to_enroll': url_to_enroll,
} }
return render_to_response('courseware/info.html', context) return render_to_response('courseware/info.html', context)
...@@ -658,15 +668,15 @@ def course_about(request, course_id): ...@@ -658,15 +668,15 @@ def course_about(request, course_id):
Assumes the course_id is in a valid format. 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( if microsite.get_value(
'ENABLE_MKTG_SITE', 'ENABLE_MKTG_SITE',
settings.FEATURES.get('ENABLE_MKTG_SITE', False) 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) registered = registered_for_course(course, request.user)
staff_access = has_access(request.user, 'staff', course) staff_access = has_access(request.user, 'staff', course)
studio_url = get_studio_url(course, 'settings/details') studio_url = get_studio_url(course, 'settings/details')
...@@ -806,7 +816,7 @@ def _progress(request, course_key, student_id): ...@@ -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 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) staff_access = has_access(request.user, 'staff', course)
if student_id is None or student_id == request.user.id: if student_id is None or student_id == request.user.id:
......
...@@ -20,6 +20,7 @@ from django_comment_client.tests.utils import CohortedContentTestCase ...@@ -20,6 +20,7 @@ from django_comment_client.tests.utils import CohortedContentTestCase
from django_comment_client.forum import views from django_comment_client.forum import views
from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE 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 nose.tools import assert_true # pylint: disable=E0611
from mock import patch, Mock, ANY, call from mock import patch, Mock, ANY, call
...@@ -913,3 +914,26 @@ class FollowedThreadsUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin): ...@@ -913,3 +914,26 @@ class FollowedThreadsUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin):
response_data = json.loads(response.content) response_data = json.loads(response.content)
self.assertEqual(response_data["discussion_data"][0]["title"], text) self.assertEqual(response_data["discussion_data"][0]["title"], text)
self.assertEqual(response_data["discussion_data"][0]["body"], 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())
...@@ -163,7 +163,7 @@ def forum_form_discussion(request, course_id): ...@@ -163,7 +163,7 @@ def forum_form_discussion(request, course_id):
course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
nr_transaction = newrelic.agent.current_transaction() 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) course_settings = make_course_settings(course, include_category_map=True)
user = cc.User.from_django_user(request.user) user = cc.User.from_django_user(request.user)
......
...@@ -941,6 +941,9 @@ MIDDLEWARE_CLASSES = ( ...@@ -941,6 +941,9 @@ MIDDLEWARE_CLASSES = (
# use Django built in clickjacking protection # use Django built in clickjacking protection
'django.middleware.clickjacking.XFrameOptionsMiddleware', 'django.middleware.clickjacking.XFrameOptionsMiddleware',
# to redirected unenrolled students to the course info page
'courseware.middleware.RedirectUnenrolledMiddleware',
'course_wiki.middleware.WikiAccessMiddleware', 'course_wiki.middleware.WikiAccessMiddleware',
) )
......
...@@ -18,12 +18,16 @@ def url_class(is_active): ...@@ -18,12 +18,16 @@ def url_class(is_active):
<%! from django.core.urlresolvers import reverse %> <%! from django.core.urlresolvers import reverse %>
<%! from django.utils.translation import ugettext as _ %> <%! from django.utils.translation import ugettext as _ %>
<%! from courseware.views import notification_image_for_tab %> <%! 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: % if disable_tabs is UNDEFINED or not disable_tabs:
<nav class="${active_page} course-material"> <nav class="${active_page} course-material">
<div class="inner-wrapper"> <div class="inner-wrapper">
<ol class="course-tabs"> <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_is_active = (tab.tab_id == active_page) or (tab.tab_id == default_tab)
tab_image = notification_image_for_tab(tab, user, course) tab_image = notification_image_for_tab(tab, user, course)
......
...@@ -13,6 +13,24 @@ ...@@ -13,6 +13,24 @@
<%include file="/dashboard/_dashboard_prompt_midcourse_reverify.html" /> <%include file="/dashboard/_dashboard_prompt_midcourse_reverify.html" />
% if show_enroll_banner:
<div class="wrapper-msg urgency-low" id="failed-verification-banner">
<div class="msg msg-reverify is-dismissable">
<div class="msg-content">
<h2 class="title">${_("You are not enrolled yet")}</h2>
<div class="copy">
<p class='enroll-message'>
${_(u"You are not currently enrolled in this course. Sign up for it {link_start}here{link_end}!").format(
link_start=u"<a href={}>".format(url_to_enroll),
link_end=u"</a>"
)}
</p>
</div>
</div>
</div>
</div>
% endif
<%include file="/courseware/course_navigation.html" args="active_page='info'" /> <%include file="/courseware/course_navigation.html" args="active_page='info'" />
<%block name="js_extra"> <%block name="js_extra">
...@@ -36,7 +54,7 @@ $(document).ready(function(){ ...@@ -36,7 +54,7 @@ $(document).ready(function(){
</div> </div>
% endif % endif
% endif % endif
<h1>${_("Course Updates &amp; News")}</h1> <h1>${_("Course Updates &amp; News")}</h1>
${get_course_info_section(request, course, 'updates')} ${get_course_info_section(request, course, 'updates')}
</section> </section>
......
...@@ -37,7 +37,7 @@ ...@@ -37,7 +37,7 @@
<% <%
discussion_tab = CourseTabList.get_discussion(course) if course else None 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: % if discussion_link:
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment