Commit da1ac466 by Andy Armstrong

Implement common redirect logic for all course tabs

LEARNER-76
parent dc036af8
...@@ -6,11 +6,12 @@ from datetime import datetime ...@@ -6,11 +6,12 @@ from datetime import datetime
from collections import defaultdict from collections import defaultdict
from fs.errors import ResourceNotFoundError from fs.errors import ResourceNotFoundError
import logging import logging
from path import Path as path from path import Path as path
import pytz import pytz
from django.http import Http404 from django.http import Http404
from django.conf import settings from django.conf import settings
from django.core.urlresolvers import reverse
from edxmako.shortcuts import render_to_string from edxmako.shortcuts import render_to_string
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
...@@ -27,8 +28,9 @@ from courseware.date_summary import ( ...@@ -27,8 +28,9 @@ from courseware.date_summary import (
VerifiedUpgradeDeadlineDate, VerifiedUpgradeDeadlineDate,
) )
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, get_module_for_descriptor
from lms.djangoapps.courseware.courseware_access_exception import CoursewareAccessException from lms.djangoapps.courseware.courseware_access_exception import CoursewareAccessException
from lms.djangoapps.courseware.exceptions import CourseAccessRedirect
from student.models import CourseEnrollment from student.models import CourseEnrollment
import branding import branding
...@@ -36,7 +38,6 @@ from opaque_keys.edx.keys import UsageKey ...@@ -36,7 +38,6 @@ from opaque_keys.edx.keys import UsageKey
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
log = logging.getLogger(__name__) log = logging.getLogger(__name__)
...@@ -72,12 +73,6 @@ def get_course_by_id(course_key, depth=0): ...@@ -72,12 +73,6 @@ def get_course_by_id(course_key, depth=0):
raise Http404("Course not found: {}.".format(unicode(course_key))) raise Http404("Course not found: {}.".format(unicode(course_key)))
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): 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,
...@@ -132,10 +127,10 @@ def check_course_access(course, user, action, check_if_enrolled=False): ...@@ -132,10 +127,10 @@ def check_course_access(course, user, action, check_if_enrolled=False):
if check_if_enrolled: if check_if_enrolled:
# Verify that the user is either enrolled in the course or a staff # Verify that the user is either enrolled in the course or a staff
# member. If user is not enrolled, raise UserNotEnrolled exception # member. If the user is not enrolled, raise a Redirect exception
# that will be caught by middleware. # that will be handled by middleware.
if not ((user.id and CourseEnrollment.is_enrolled(user, course.id)) or has_access(user, 'staff', course)): if not ((user.id and CourseEnrollment.is_enrolled(user, course.id)) or has_access(user, 'staff', course)):
raise UserNotEnrolled(course.id) raise CourseAccessRedirect(reverse('about_course', args=[unicode(course.id)]))
def find_file(filesystem, dirs, filename): def find_file(filesystem, dirs, filename):
...@@ -470,3 +465,81 @@ def get_problems_in_section(section): ...@@ -470,3 +465,81 @@ def get_problems_in_section(section):
problem_descriptors[unicode(component.location)] = component problem_descriptors[unicode(component.location)] = component
return problem_descriptors return problem_descriptors
def get_current_child(xmodule, min_depth=None, requested_child=None):
"""
Get the xmodule.position's display item of an xmodule that has a position and
children. If xmodule has no position or is out of bounds, return the first
child with children of min_depth.
For example, if chapter_one has no position set, with two child sections,
section-A having no children and section-B having a discussion unit,
`get_current_child(chapter, min_depth=1)` will return section-B.
Returns None only if there are no children at all.
"""
# TODO: convert this method to use the Course Blocks API
def _get_child(children):
"""
Returns either the first or last child based on the value of
the requested_child parameter. If requested_child is None,
returns the first child.
"""
if requested_child == 'first':
return children[0]
elif requested_child == 'last':
return children[-1]
else:
return children[0]
def _get_default_child_module(child_modules):
"""Returns the first child of xmodule, subject to min_depth."""
if min_depth <= 0:
return _get_child(child_modules)
else:
content_children = [
child for child in child_modules
if child.has_children_at_depth(min_depth - 1) and child.get_display_items()
]
return _get_child(content_children) if content_children else None
child = None
if hasattr(xmodule, 'position'):
children = xmodule.get_display_items()
if len(children) > 0:
if xmodule.position is not None and not requested_child:
pos = xmodule.position - 1 # position is 1-indexed
if 0 <= pos < len(children):
child = children[pos]
if min_depth > 0 and not child.has_children_at_depth(min_depth - 1):
child = None
if child is None:
child = _get_default_child_module(children)
return child
def get_last_accessed_courseware(course, request, user):
"""
Returns a tuple containing the courseware module (URL, id) that the user last accessed,
or (None, None) if it cannot be found.
"""
# TODO: convert this method to use the Course Blocks API
field_data_cache = FieldDataCache.cache_for_descriptor_descendents(
course.id, request.user, course, depth=2
)
course_module = get_module_for_descriptor(
user, request, course, field_data_cache, course.id, course=course
)
chapter_module = get_current_child(course_module)
if chapter_module is not None:
section_module = get_current_child(chapter_module)
if section_module is not None:
url = reverse('courseware_section', kwargs={
'course_id': unicode(course.id),
'chapter': chapter_module.url_name,
'section': section_module.url_name
})
return (url, section_module.url_name)
return (None, None)
...@@ -3,12 +3,8 @@ This file contains all entrance exam related utils/logic. ...@@ -3,12 +3,8 @@ This file contains all entrance exam related utils/logic.
""" """
from courseware.access import has_access from courseware.access import has_access
from courseware.model_data import FieldDataCache, ScoresClient
from opaque_keys.edx.keys import UsageKey
from opaque_keys.edx.locator import BlockUsageLocator
from student.models import EntranceExamConfiguration from student.models import EntranceExamConfiguration
from util.milestones_helpers import get_required_content, is_entrance_exams_enabled from util.milestones_helpers import get_required_content, is_entrance_exams_enabled
from util.module_utils import yield_dynamic_descriptor_descendants
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
......
...@@ -10,3 +10,10 @@ class Redirect(Exception): ...@@ -10,3 +10,10 @@ class Redirect(Exception):
def __init__(self, url): def __init__(self, url):
super(Redirect, self).__init__() super(Redirect, self).__init__()
self.url = url self.url = url
class CourseAccessRedirect(Redirect):
"""
Redirect raised when user does not have access to a course.
"""
pass
...@@ -3,22 +3,17 @@ Middleware for the courseware app ...@@ -3,22 +3,17 @@ Middleware for the courseware app
""" """
from django.shortcuts import redirect from django.shortcuts import redirect
from django.core.urlresolvers import reverse
from courseware.courses import UserNotEnrolled from lms.djangoapps.courseware.exceptions import Redirect
class RedirectUnenrolledMiddleware(object): class RedirectMiddleware(object):
""" """
Catch UserNotEnrolled errors thrown by `get_course_with_access` and redirect Catch Redirect exceptions and redirect the user to the expected URL.
users to the course about page
""" """
def process_exception(self, _request, exception): def process_exception(self, _request, exception):
if isinstance(exception, UserNotEnrolled): """
course_key = exception.course_key Catch Redirect exceptions and redirect the user to the expected URL.
return redirect( """
reverse( if isinstance(exception, Redirect):
'courseware.views.views.course_about', return redirect(exception.url)
args=[course_key.to_deprecated_string()]
)
)
...@@ -120,7 +120,7 @@ class SurveyViewsTests(LoginEnrollmentTestCase, SharedModuleStoreTestCase, XssTe ...@@ -120,7 +120,7 @@ class SurveyViewsTests(LoginEnrollmentTestCase, SharedModuleStoreTestCase, XssTe
def test_anonymous_user_visiting_course_with_survey(self): def test_anonymous_user_visiting_course_with_survey(self):
""" """
Verifies that anonymous user going to the courseware info with an unanswered survey is not Verifies that anonymous user going to the courseware info with an unanswered survey is not
redirected to survery and info page renders without server error. redirected to survey and info page renders without server error.
""" """
self.logout() self.logout()
resp = self.client.get( resp = self.client.get(
......
...@@ -21,6 +21,7 @@ from courseware.courses import ( ...@@ -21,6 +21,7 @@ from courseware.courses import (
get_course_info_section, get_course_info_section,
get_course_overview_with_access, get_course_overview_with_access,
get_course_with_access, get_course_with_access,
get_current_child,
) )
from courseware.module_render import get_module_for_descriptor from courseware.module_render import get_module_for_descriptor
from courseware.model_data import FieldDataCache from courseware.model_data import FieldDataCache
...@@ -160,6 +161,23 @@ class CoursesTest(ModuleStoreTestCase): ...@@ -160,6 +161,23 @@ class CoursesTest(ModuleStoreTestCase):
"testing get_courses with filter_={}".format(filter_), "testing get_courses with filter_={}".format(filter_),
) )
def test_get_current_child(self):
mock_xmodule = mock.MagicMock()
self.assertIsNone(get_current_child(mock_xmodule))
mock_xmodule.position = -1
mock_xmodule.get_display_items.return_value = ['one', 'two', 'three']
self.assertEqual(get_current_child(mock_xmodule), 'one')
mock_xmodule.position = 2
self.assertEqual(get_current_child(mock_xmodule), 'two')
self.assertEqual(get_current_child(mock_xmodule, requested_child='first'), 'one')
self.assertEqual(get_current_child(mock_xmodule, requested_child='last'), 'three')
mock_xmodule.position = 3
mock_xmodule.get_display_items.return_value = []
self.assertIsNone(get_current_child(mock_xmodule))
@attr(shard=1) @attr(shard=1)
class ModuleStoreBranchSettingTest(ModuleStoreTestCase): class ModuleStoreBranchSettingTest(ModuleStoreTestCase):
......
...@@ -2,17 +2,16 @@ ...@@ -2,17 +2,16 @@
Tests for courseware middleware Tests for courseware middleware
""" """
from django.core.urlresolvers import reverse
from django.test.client import RequestFactory from django.test.client import RequestFactory
from django.http import Http404 from django.http import Http404
from mock import patch
from nose.plugins.attrib import attr from nose.plugins.attrib import attr
import courseware.courses as courses
from courseware.middleware import RedirectUnenrolledMiddleware
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.factories import CourseFactory
from lms.djangoapps.courseware.exceptions import Redirect
from lms.djangoapps.courseware.middleware import RedirectMiddleware
@attr(shard=1) @attr(shard=1)
class CoursewareMiddlewareTestCase(SharedModuleStoreTestCase): class CoursewareMiddlewareTestCase(SharedModuleStoreTestCase):
...@@ -26,32 +25,24 @@ class CoursewareMiddlewareTestCase(SharedModuleStoreTestCase): ...@@ -26,32 +25,24 @@ class CoursewareMiddlewareTestCase(SharedModuleStoreTestCase):
def setUp(self): def setUp(self):
super(CoursewareMiddlewareTestCase, self).setUp() super(CoursewareMiddlewareTestCase, self).setUp()
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): def test_process_404(self):
"""A 404 should not trigger anything""" """A 404 should not trigger anything"""
request = RequestFactory().get("dummy_url") request = RequestFactory().get("dummy_url")
response = RedirectUnenrolledMiddleware().process_exception( response = RedirectMiddleware().process_exception(
request, Http404() request, Http404()
) )
self.assertIsNone(response) self.assertIsNone(response)
def test_redirect_exceptions(self):
"""
Unit tests for handling of Redirect exceptions.
"""
request = RequestFactory().get("dummy_url")
test_url = '/test_url'
exception = Redirect(test_url)
response = RedirectMiddleware().process_exception(
request, exception
)
self.assertEqual(response.status_code, 302)
target_url = response._headers['location'][1]
self.assertTrue(target_url.endswith(test_url))
...@@ -432,46 +432,6 @@ class ViewsTestCase(ModuleStoreTestCase): ...@@ -432,46 +432,6 @@ class ViewsTestCase(ModuleStoreTestCase):
self.assertEqual(response.status_code, 302) self.assertEqual(response.status_code, 302)
self.assertRedirects(response, '/courses/{}/about'.format(unicode(self.course_key))) self.assertRedirects(response, '/courses/{}/about'.format(unicode(self.course_key)))
def test_courseware_redirection(self):
"""
Tests that a global staff member is redirected to the staff enrollment page.
Un-enrolled Staff user should be redirected to the staff enrollment page accessing courseware,
user chooses to enroll in the course. User is enrolled and redirected to the requested url.
Scenario:
1. Un-enrolled staff tries to access any course vertical (courseware url).
2. User is redirected to the staff enrollment page.
3. User chooses to enroll in the course.
4. User is enrolled in the course and redirected to the requested courseware url.
"""
self._create_global_staff_user()
courseware_url, enroll_url = self._create_url_for_enroll_staff()
# Accessing the courseware url in which not enrolled & redirected to staff enrollment page
response = self.client.get(courseware_url, follow=True)
self.assertEqual(response.status_code, 200)
self.assertIn(302, response.redirect_chain[0])
self.assertEqual(len(response.redirect_chain), 1)
self.assertRedirects(response, enroll_url)
# Accessing the enroll staff url and verify the correct url
response = self.client.get(enroll_url)
self.assertEqual(response.status_code, 200)
response_content = response.content
self.assertIn('Enroll', response_content)
self.assertIn("dont_enroll", response_content)
# Post the valid data to enroll the staff in the course
response = self.client.post(enroll_url, data={'enroll': "Enroll"}, follow=True)
self.assertEqual(response.status_code, 200)
self.assertIn(302, response.redirect_chain[0])
self.assertEqual(len(response.redirect_chain), 1)
self.assertRedirects(response, courseware_url)
# Verify staff has been enrolled to the given course
self.assertTrue(CourseEnrollment.is_enrolled(self.global_staff, self.course.id))
@unittest.skipUnless(settings.FEATURES.get('ENABLE_SHOPPING_CART'), "Shopping Cart not enabled in settings") @unittest.skipUnless(settings.FEATURES.get('ENABLE_SHOPPING_CART'), "Shopping Cart not enabled in settings")
@patch.dict(settings.FEATURES, {'ENABLE_PAID_COURSE_REGISTRATION': True}) @patch.dict(settings.FEATURES, {'ENABLE_PAID_COURSE_REGISTRATION': True})
def test_course_about_in_cart(self): def test_course_about_in_cart(self):
...@@ -557,23 +517,6 @@ class ViewsTestCase(ModuleStoreTestCase): ...@@ -557,23 +517,6 @@ class ViewsTestCase(ModuleStoreTestCase):
mock_user.is_authenticated.return_value = False mock_user.is_authenticated.return_value = False
self.assertEqual(views.user_groups(mock_user), []) self.assertEqual(views.user_groups(mock_user), [])
def test_get_current_child(self):
mock_xmodule = MagicMock()
self.assertIsNone(views.get_current_child(mock_xmodule))
mock_xmodule.position = -1
mock_xmodule.get_display_items.return_value = ['one', 'two', 'three']
self.assertEqual(views.get_current_child(mock_xmodule), 'one')
mock_xmodule.position = 2
self.assertEqual(views.get_current_child(mock_xmodule), 'two')
self.assertEqual(views.get_current_child(mock_xmodule, requested_child='first'), 'one')
self.assertEqual(views.get_current_child(mock_xmodule, requested_child='last'), 'three')
mock_xmodule.position = 3
mock_xmodule.get_display_items.return_value = []
self.assertIsNone(views.get_current_child(mock_xmodule))
def test_get_redirect_url(self): def test_get_redirect_url(self):
self.assertIn( self.assertIn(
'activate_block_id', 'activate_block_id',
......
...@@ -2,9 +2,11 @@ ...@@ -2,9 +2,11 @@
Module to define url helpers functions Module to define url helpers functions
""" """
from urllib import urlencode from urllib import urlencode
from django.core.urlresolvers import reverse
from xmodule.modulestore.search import path_to_location, navigation_index from xmodule.modulestore.search import path_to_location, navigation_index
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
from django.core.urlresolvers import reverse
def get_redirect_url(course_key, usage_key): def get_redirect_url(course_key, usage_key):
...@@ -48,17 +50,3 @@ def get_redirect_url(course_key, usage_key): ...@@ -48,17 +50,3 @@ def get_redirect_url(course_key, usage_key):
) )
redirect_url += "?{}".format(urlencode({'activate_block_id': unicode(final_target_id)})) redirect_url += "?{}".format(urlencode({'activate_block_id': unicode(final_target_id)}))
return redirect_url return redirect_url
def get_redirect_url_for_global_staff(course_key, _next):
"""
Returns the redirect url for staff enrollment
Args:
course_key(str): Course key string
_next(str): Redirect url of course component
"""
redirect_url = ("{url}?next={redirect}".format(
url=reverse('enroll_staff', args=[unicode(course_key)]),
redirect=_next))
return redirect_url
...@@ -16,7 +16,6 @@ from django.views.decorators.csrf import ensure_csrf_cookie ...@@ -16,7 +16,6 @@ from django.views.decorators.csrf import ensure_csrf_cookie
from django.views.generic import View from django.views.generic import View
from django.shortcuts import redirect from django.shortcuts import redirect
from courseware.url_helpers import get_redirect_url_for_global_staff
from edxmako.shortcuts import render_to_response, render_to_string from edxmako.shortcuts import render_to_response, render_to_string
import logging import logging
...@@ -25,6 +24,7 @@ log = logging.getLogger("edx.courseware.views.index") ...@@ -25,6 +24,7 @@ log = logging.getLogger("edx.courseware.views.index")
import urllib import urllib
import waffle import waffle
from lms.djangoapps.courseware.exceptions import CourseAccessRedirect
from lms.djangoapps.gating.api import get_entrance_exam_score_ratio, get_entrance_exam_usage_key from lms.djangoapps.gating.api import get_entrance_exam_score_ratio, get_entrance_exam_usage_key
from lms.djangoapps.grades.new.course_grade_factory import CourseGradeFactory from lms.djangoapps.grades.new.course_grade_factory import CourseGradeFactory
from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.keys import CourseKey
...@@ -35,29 +35,25 @@ from openedx.core.djangoapps.monitoring_utils import set_custom_metrics_for_cour ...@@ -35,29 +35,25 @@ from openedx.core.djangoapps.monitoring_utils import set_custom_metrics_for_cour
from openedx.features.enterprise_support.api import data_sharing_consent_required from openedx.features.enterprise_support.api import data_sharing_consent_required
from request_cache.middleware import RequestCache from request_cache.middleware import RequestCache
from shoppingcart.models import CourseRegistrationCode from shoppingcart.models import CourseRegistrationCode
from student.models import CourseEnrollment
from student.views import is_course_blocked from student.views import is_course_blocked
from student.roles import GlobalStaff
from util.views import ensure_valid_course_key from util.views import ensure_valid_course_key
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
from xmodule.x_module import STUDENT_VIEW from xmodule.x_module import STUDENT_VIEW
from survey.utils import must_answer_survey
from web_fragments.fragment import Fragment from web_fragments.fragment import Fragment
from ..access import has_access, _adjust_start_date_for_beta_testers from ..access import has_access, _adjust_start_date_for_beta_testers
from ..access_utils import in_preview_mode from ..access_utils import in_preview_mode
from ..courses import get_studio_url, get_course_with_access from ..courses import get_current_child, get_studio_url, get_course_with_access
from ..entrance_exams import ( from ..entrance_exams import (
course_has_entrance_exam, course_has_entrance_exam,
get_entrance_exam_content, get_entrance_exam_content,
user_has_passed_entrance_exam, user_has_passed_entrance_exam,
user_can_skip_entrance_exam, user_can_skip_entrance_exam,
) )
from ..exceptions import Redirect
from ..masquerade import setup_masquerade from ..masquerade import setup_masquerade
from ..model_data import FieldDataCache from ..model_data import FieldDataCache
from ..module_render import toc_for_course, get_module_for_descriptor from ..module_render import toc_for_course, get_module_for_descriptor
from .views import get_current_child, registered_for_course from .views import CourseTabView, check_access_to_course
TEMPLATE_IMPORTS = {'urllib': urllib} TEMPLATE_IMPORTS = {'urllib': urllib}
...@@ -100,25 +96,23 @@ class CoursewareIndex(View): ...@@ -100,25 +96,23 @@ class CoursewareIndex(View):
self.section_url_name = section self.section_url_name = section
self.position = position self.position = position
self.chapter, self.section = None, None self.chapter, self.section = None, None
self.course = None
self.url = request.path self.url = request.path
try: try:
set_custom_metrics_for_course_key(self.course_key) set_custom_metrics_for_course_key(self.course_key)
self._clean_position() self._clean_position()
with modulestore().bulk_operations(self.course_key): with modulestore().bulk_operations(self.course_key):
self.course = get_course_with_access(request.user, 'load', self.course_key, depth=CONTENT_DEPTH) self.course = get_course_with_access(
request.user, 'load', self.course_key,
depth=CONTENT_DEPTH,
check_if_enrolled=True,
)
self.is_staff = has_access(request.user, 'staff', self.course) self.is_staff = has_access(request.user, 'staff', self.course)
self._setup_masquerade_for_effective_user() self._setup_masquerade_for_effective_user()
return self._get(request) return self._get(request)
except Redirect as redirect_error: except Exception as exception: # pylint: disable=broad-except
return redirect(redirect_error.url) return CourseTabView.handle_exceptions(request, self.course, exception)
except UnicodeEncodeError:
raise Http404("URL contains Unicode characters")
except Http404:
# let it propagate
raise
except Exception: # pylint: disable=broad-except
return self._handle_unexpected_error()
def _setup_masquerade_for_effective_user(self): def _setup_masquerade_for_effective_user(self):
""" """
...@@ -139,7 +133,8 @@ class CoursewareIndex(View): ...@@ -139,7 +133,8 @@ class CoursewareIndex(View):
""" """
Render the index page. Render the index page.
""" """
self._redirect_if_needed_to_access_course() check_access_to_course(request, self.course)
self._redirect_if_needed_to_pay_for_course()
self._prefetch_and_bind_course(request) self._prefetch_and_bind_course(request)
if self.course.has_children_at_depth(CONTENT_DEPTH): if self.course.has_children_at_depth(CONTENT_DEPTH):
...@@ -165,7 +160,7 @@ class CoursewareIndex(View): ...@@ -165,7 +160,7 @@ class CoursewareIndex(View):
self.chapter.url_name != self.original_chapter_url_name or self.chapter.url_name != self.original_chapter_url_name or
(self.original_section_url_name and self.section.url_name != self.original_section_url_name) (self.original_section_url_name and self.section.url_name != self.original_section_url_name)
): ):
raise Redirect( raise CourseAccessRedirect(
reverse( reverse(
'courseware_section', 'courseware_section',
kwargs={ kwargs={
...@@ -186,15 +181,6 @@ class CoursewareIndex(View): ...@@ -186,15 +181,6 @@ class CoursewareIndex(View):
except ValueError: except ValueError:
raise Http404(u"Position {} is not an integer!".format(self.position)) raise Http404(u"Position {} is not an integer!".format(self.position))
def _redirect_if_needed_to_access_course(self):
"""
Verifies that the user can enter the course.
"""
self._redirect_if_needed_to_pay_for_course()
self._redirect_if_needed_to_register()
self._redirect_if_needed_for_prereqs()
self._redirect_if_needed_for_course_survey()
def _redirect_if_needed_to_pay_for_course(self): def _redirect_if_needed_to_pay_for_course(self):
""" """
Redirect to dashboard if the course is blocked due to non-payment. Redirect to dashboard if the course is blocked due to non-payment.
...@@ -213,47 +199,7 @@ class CoursewareIndex(View): ...@@ -213,47 +199,7 @@ class CoursewareIndex(View):
self.real_user, self.real_user,
unicode(self.course_key), unicode(self.course_key),
) )
raise Redirect(reverse('dashboard')) raise CourseAccessRedirect(reverse('dashboard'))
def _redirect_if_needed_to_register(self):
"""
Verify that the user is registered in the course.
"""
if not registered_for_course(self.course, self.effective_user):
log.debug(
u'User %s tried to view course %s but is not enrolled',
self.effective_user,
unicode(self.course.id)
)
user_is_global_staff = GlobalStaff().has_user(self.effective_user)
user_is_enrolled = CourseEnrollment.is_enrolled(self.effective_user, self.course_key)
if user_is_global_staff and not user_is_enrolled:
redirect_url = get_redirect_url_for_global_staff(self.course_key, _next=self.url)
raise Redirect(redirect_url)
raise Redirect(reverse('about_course', args=[unicode(self.course.id)]))
def _redirect_if_needed_for_prereqs(self):
"""
See if all pre-requisites (as per the milestones app feature) have been
fulfilled. Note that if the pre-requisite feature flag has been turned off
(default) then this check will always pass.
"""
if not has_access(self.effective_user, 'view_courseware_with_prerequisites', self.course):
# Prerequisites have not been fulfilled.
# Therefore redirect to the Dashboard.
log.info(
u'User %d tried to view course %s '
u'without fulfilling prerequisites',
self.effective_user.id, unicode(self.course.id))
raise Redirect(reverse('dashboard'))
def _redirect_if_needed_for_course_survey(self):
"""
Check to see if there is a required survey that must be taken before
the user can access the course.
"""
if must_answer_survey(self.course, self.effective_user):
raise Redirect(reverse('course_survey', args=[unicode(self.course.id)]))
def _reset_section_to_exam_if_required(self): def _reset_section_to_exam_if_required(self):
""" """
...@@ -487,33 +433,6 @@ class CoursewareIndex(View): ...@@ -487,33 +433,6 @@ class CoursewareIndex(View):
section_context['specific_masquerade'] = self._is_masquerading_as_specific_student() section_context['specific_masquerade'] = self._is_masquerading_as_specific_student()
return section_context return section_context
def _handle_unexpected_error(self):
"""
Handle unexpected exceptions raised by View.
"""
# In production, don't want to let a 500 out for any reason
if settings.DEBUG:
raise
log.exception(
u"Error in index view: user=%s, effective_user=%s, course=%s, chapter=%s section=%s position=%s",
self.real_user,
self.effective_user,
unicode(self.course_key),
self.chapter_url_name,
self.section_url_name,
self.position,
)
try:
return render_to_response('courseware/courseware-error.html', {
'staff_access': self.is_staff,
'course': self.course
})
except:
# Let the exception propagate, relying on global config to
# at least return a nice error message
log.exception("Error while rendering courseware-error page")
raise
def render_accordion(request, course, table_of_contents): def render_accordion(request, course, table_of_contents):
""" """
......
...@@ -34,12 +34,12 @@ from xmodule.modulestore.tests.django_utils import ( ...@@ -34,12 +34,12 @@ from xmodule.modulestore.tests.django_utils import (
) )
from xmodule.modulestore.tests.factories import check_mongo_calls, CourseFactory, ItemFactory from xmodule.modulestore.tests.factories import check_mongo_calls, CourseFactory, ItemFactory
from courseware.courses import UserNotEnrolled
from nose.tools import assert_true from nose.tools import assert_true
from mock import patch, Mock, ANY, call from mock import patch, Mock, ANY, call
from openedx.core.djangoapps.course_groups.models import CourseUserGroup from openedx.core.djangoapps.course_groups.models import CourseUserGroup
from lms.djangoapps.courseware.exceptions import CourseAccessRedirect
from lms.djangoapps.teams.tests.factories import CourseTeamFactory from lms.djangoapps.teams.tests.factories import CourseTeamFactory
log = logging.getLogger(__name__) log = logging.getLogger(__name__)
...@@ -1574,7 +1574,7 @@ class EnrollmentTestCase(ForumsEnableMixin, ModuleStoreTestCase): ...@@ -1574,7 +1574,7 @@ class EnrollmentTestCase(ForumsEnableMixin, ModuleStoreTestCase):
mock_request.side_effect = make_mock_request_impl(course=self.course, text='dummy') mock_request.side_effect = make_mock_request_impl(course=self.course, text='dummy')
request = RequestFactory().get('dummy_url') request = RequestFactory().get('dummy_url')
request.user = self.student request.user = self.student
with self.assertRaises(UserNotEnrolled): with self.assertRaises(CourseAccessRedirect):
views.forum_form_discussion(request, course_id=self.course.id.to_deprecated_string()) views.forum_form_discussion(request, course_id=self.course.id.to_deprecated_string())
......
...@@ -14,6 +14,7 @@ from openedx.core.djangoapps.user_api.accounts.views import AccountViewSet ...@@ -14,6 +14,7 @@ from openedx.core.djangoapps.user_api.accounts.views import AccountViewSet
from rest_framework.exceptions import PermissionDenied from rest_framework.exceptions import PermissionDenied
from lms.djangoapps.courseware.exceptions import CourseAccessRedirect
from opaque_keys import InvalidKeyError from opaque_keys import InvalidKeyError
from opaque_keys.edx.locator import CourseKey from opaque_keys.edx.locator import CourseKey
from courseware.courses import get_course_with_access from courseware.courses import get_course_with_access
...@@ -80,6 +81,11 @@ def _get_course(course_key, user): ...@@ -80,6 +81,11 @@ def _get_course(course_key, user):
try: try:
course = get_course_with_access(user, 'load', course_key, check_if_enrolled=True) course = get_course_with_access(user, 'load', course_key, check_if_enrolled=True)
except Http404: except Http404:
# Convert 404s into CourseNotFoundErrors.
raise CourseNotFoundError("Course not found.")
except CourseAccessRedirect:
# Raise course not found if the user cannot access the course
# since it doesn't make sense to redirect an API.
raise CourseNotFoundError("Course not found.") raise CourseNotFoundError("Course not found.")
if not any([tab.type == 'discussion' and tab.is_enabled(course, user) for tab in course.tabs]): if not any([tab.type == 'discussion' and tab.is_enabled(course, user) for tab in course.tabs]):
raise DiscussionDisabledError("Discussion is disabled for the course.") raise DiscussionDisabledError("Discussion is disabled for the course.")
......
...@@ -17,6 +17,8 @@ from opaque_keys.edx.keys import CourseKey ...@@ -17,6 +17,8 @@ from opaque_keys.edx.keys import CourseKey
from courseware.access import has_access from courseware.access import has_access
from util.file import store_uploaded_file from util.file import store_uploaded_file
from courseware.courses import get_course_with_access, get_course_overview_with_access, get_course_by_id from courseware.courses import get_course_with_access, get_course_overview_with_access, get_course_by_id
from lms.djangoapps.courseware.exceptions import CourseAccessRedirect
import django_comment_client.settings as cc_settings import django_comment_client.settings as cc_settings
from django_comment_common.signals import ( from django_comment_common.signals import (
thread_created, thread_created,
...@@ -778,6 +780,9 @@ def users(request, course_id): ...@@ -778,6 +780,9 @@ def users(request, course_id):
except Http404: except Http404:
# course didn't exist, or requesting user does not have access to it. # course didn't exist, or requesting user does not have access to it.
return JsonError(status=404) return JsonError(status=404)
except CourseAccessRedirect:
# user does not have access to the course.
return JsonError(status=404)
try: try:
username = request.GET['username'] username = request.GET['username']
......
...@@ -22,8 +22,8 @@ from provider.oauth2.models import Client ...@@ -22,8 +22,8 @@ from provider.oauth2.models import Client
from edxnotes.exceptions import EdxNotesParseError, EdxNotesServiceUnavailable from edxnotes.exceptions import EdxNotesParseError, EdxNotesServiceUnavailable
from edxnotes.plugins import EdxNotesTab from edxnotes.plugins import EdxNotesTab
from courseware.views.views import get_current_child
from courseware.access import has_access from courseware.access import has_access
from courseware.courses import get_current_child
from openedx.core.lib.token_utils import JwtBuilder from openedx.core.lib.token_utils import JwtBuilder
from student.models import anonymous_id_for_user from student.models import anonymous_id_for_user
from util.date_utils import get_default_time_display from util.date_utils import get_default_time_display
......
...@@ -13,9 +13,9 @@ from rest_framework.response import Response ...@@ -13,9 +13,9 @@ from rest_framework.response import Response
from courseware.access import has_access from courseware.access import has_access
from lms.djangoapps.ccx.utils import prep_course_for_grading from lms.djangoapps.ccx.utils import prep_course_for_grading
from lms.djangoapps.courseware import courses from lms.djangoapps.courseware import courses
from lms.djangoapps.courseware.exceptions import CourseAccessRedirect
from lms.djangoapps.grades.api.serializers import GradingPolicySerializer from lms.djangoapps.grades.api.serializers import GradingPolicySerializer
from lms.djangoapps.grades.new.course_grade_factory import CourseGradeFactory from lms.djangoapps.grades.new.course_grade_factory import CourseGradeFactory
from openedx.core.lib.api.authentication import OAuth2AuthenticationAllowInactiveUser
from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes
from student.roles import CourseStaffRole from student.roles import CourseStaffRole
...@@ -47,15 +47,17 @@ class GradeViewMixin(DeveloperErrorViewMixin): ...@@ -47,15 +47,17 @@ class GradeViewMixin(DeveloperErrorViewMixin):
user, user,
access_action, access_action,
course_key, course_key,
check_if_enrolled=True check_if_enrolled=True,
) )
except Http404: except Http404:
log.info('Course with ID "%s" not found', course_key_string) log.info('Course with ID "%s" not found', course_key_string)
return self.make_error_response( except CourseAccessRedirect:
status_code=status.HTTP_404_NOT_FOUND, log.info('User %s does not have access to course with ID "%s"', user.username, course_key_string)
developer_message='The user, the course or both do not exist.', return self.make_error_response(
error_code='user_or_course_does_not_exist' status_code=status.HTTP_404_NOT_FOUND,
) developer_message='The user, the course or both do not exist.',
error_code='user_or_course_does_not_exist',
)
def _get_effective_user(self, request, course): def _get_effective_user(self, request, course):
""" """
......
...@@ -5,8 +5,11 @@ import functools ...@@ -5,8 +5,11 @@ import functools
from rest_framework import status from rest_framework import status
from rest_framework.response import Response from rest_framework.response import Response
from django.http import Http404
from lms.djangoapps.courseware.courses import get_course_with_access from lms.djangoapps.courseware.courses import get_course_with_access
from lms.djangoapps.courseware.courseware_access_exception import CoursewareAccessException from lms.djangoapps.courseware.courseware_access_exception import CoursewareAccessException
from lms.djangoapps.courseware.exceptions import CourseAccessRedirect
from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.keys import CourseKey
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
...@@ -39,6 +42,9 @@ def mobile_course_access(depth=0): ...@@ -39,6 +42,9 @@ def mobile_course_access(depth=0):
) )
except CoursewareAccessException as error: except CoursewareAccessException as error:
return Response(data=error.to_json(), status=status.HTTP_404_NOT_FOUND) return Response(data=error.to_json(), status=status.HTTP_404_NOT_FOUND)
except CourseAccessRedirect as error:
# Raise a 404 if the user does not have course access
raise Http404
return func(self, request, course=course, *args, **kwargs) return func(self, request, course=course, *args, **kwargs)
return _wrapper return _wrapper
......
...@@ -13,10 +13,10 @@ from opaque_keys.edx.keys import UsageKey ...@@ -13,10 +13,10 @@ from opaque_keys.edx.keys import UsageKey
from opaque_keys import InvalidKeyError from opaque_keys import InvalidKeyError
from courseware.access import is_mobile_available_for_user from courseware.access import is_mobile_available_for_user
from courseware.courses import get_current_child
from courseware.model_data import FieldDataCache from courseware.model_data import FieldDataCache
from courseware.module_render import get_module_for_descriptor from courseware.module_render import get_module_for_descriptor
from courseware.views.index import save_positions_recursively_up from courseware.views.index import save_positions_recursively_up
from courseware.views.views import get_current_child
from student.models import CourseEnrollment, User from student.models import CourseEnrollment, User
from xblock.fields import Scope from xblock.fields import Scope
......
...@@ -177,6 +177,8 @@ class SurveyAnswer(TimeStampedModel): ...@@ -177,6 +177,8 @@ class SurveyAnswer(TimeStampedModel):
Returns whether a user has any answers for a given SurveyForm for a course Returns whether a user has any answers for a given SurveyForm for a course
This can be used to determine if a user has taken a CourseSurvey. This can be used to determine if a user has taken a CourseSurvey.
""" """
if user.is_anonymous():
return False
return SurveyAnswer.objects.filter(form=form, user=user).exists() return SurveyAnswer.objects.filter(form=form, user=user).exists()
@classmethod @classmethod
......
...@@ -83,7 +83,7 @@ class SurveyViewsTests(ModuleStoreTestCase): ...@@ -83,7 +83,7 @@ class SurveyViewsTests(ModuleStoreTestCase):
# is the SurveyForm html present in the HTML response? # is the SurveyForm html present in the HTML response?
self.assertIn(self.test_form, resp.content) self.assertIn(self.test_form, resp.content)
def test_unautneticated_survey_postback(self): def test_unauthenticated_survey_postback(self):
""" """
Asserts that an anonymous user cannot answer a survey Asserts that an anonymous user cannot answer a survey
""" """
......
...@@ -25,6 +25,10 @@ def must_answer_survey(course_descriptor, user): ...@@ -25,6 +25,10 @@ def must_answer_survey(course_descriptor, user):
if not is_survey_required_for_course(course_descriptor): if not is_survey_required_for_course(course_descriptor):
return False return False
# anonymous users do not need to answer the survey
if user.is_anonymous():
return False
# this will throw exception if not found, but a non existing survey name will # this will throw exception if not found, but a non existing survey name will
# be trapped in the above is_survey_required_for_course() method # be trapped in the above is_survey_required_for_course() method
survey = SurveyForm.get(course_descriptor.course_survey_name) survey = SurveyForm.get(course_descriptor.course_survey_name)
......
...@@ -1180,7 +1180,7 @@ MIDDLEWARE_CLASSES = ( ...@@ -1180,7 +1180,7 @@ MIDDLEWARE_CLASSES = (
'django.middleware.clickjacking.XFrameOptionsMiddleware', 'django.middleware.clickjacking.XFrameOptionsMiddleware',
# to redirected unenrolled students to the course info page # to redirected unenrolled students to the course info page
'courseware.middleware.RedirectUnenrolledMiddleware', 'courseware.middleware.RedirectMiddleware',
'course_wiki.middleware.WikiAccessMiddleware', 'course_wiki.middleware.WikiAccessMiddleware',
......
...@@ -20,7 +20,11 @@ from openedx.core.djangolib.markup import HTML ...@@ -20,7 +20,11 @@ from openedx.core.djangolib.markup import HTML
<div class="page-header-secondary"> <div class="page-header-secondary">
<div class="form-actions"> <div class="form-actions">
<a class="btn action-resume-course" href="${reverse('courseware', kwargs={'course_id': unicode(course.id.to_deprecated_string())})}"> <a class="btn action-resume-course" href="${reverse('courseware', kwargs={'course_id': unicode(course.id.to_deprecated_string())})}">
${_("Resume Course")} % if has_visited_course:
${_("Resume Course")}
% else:
${_("Start Course")}
% endif
</a> </a>
<a class="btn action-show-bookmarks" href="${reverse('openedx.course_bookmarks.home', args=[course.id])}"> <a class="btn action-show-bookmarks" href="${reverse('openedx.course_bookmarks.home', args=[course.id])}">
${_("Bookmarks")} ${_("Bookmarks")}
......
...@@ -45,7 +45,7 @@ from django.utils.translation import ugettext as _ ...@@ -45,7 +45,7 @@ from django.utils.translation import ugettext as _
<b>${ _("Resume Course") }</b> <b>${ _("Resume Course") }</b>
<span class="icon fa fa-arrow-circle-right" aria-hidden="true"></span> <span class="icon fa fa-arrow-circle-right" aria-hidden="true"></span>
</span> </span>
%endif % endif
</a> </a>
</li> </li>
% endfor % endfor
......
...@@ -9,7 +9,7 @@ from django.utils.decorators import method_decorator ...@@ -9,7 +9,7 @@ from django.utils.decorators import method_decorator
from django.views.decorators.cache import cache_control from django.views.decorators.cache import cache_control
from django.views.decorators.csrf import ensure_csrf_cookie from django.views.decorators.csrf import ensure_csrf_cookie
from courseware.courses import get_course_with_access from courseware.courses import get_course_with_access, get_last_accessed_courseware
from lms.djangoapps.courseware.views.views import CourseTabView from lms.djangoapps.courseware.views.views import CourseTabView
from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.keys import CourseKey
from openedx.core.djangoapps.plugin_api.views import EdxFragmentView from openedx.core.djangoapps.plugin_api.views import EdxFragmentView
...@@ -53,11 +53,15 @@ class CourseHomeFragmentView(EdxFragmentView): ...@@ -53,11 +53,15 @@ class CourseHomeFragmentView(EdxFragmentView):
# Render the outline as a fragment # Render the outline as a fragment
outline_fragment = CourseOutlineFragmentView().render_to_fragment(request, course_id=course_id, **kwargs) outline_fragment = CourseOutlineFragmentView().render_to_fragment(request, course_id=course_id, **kwargs)
# Get the last accessed courseware
last_accessed_url, __ = get_last_accessed_courseware(course, request, request.user)
# Render the course home fragment # Render the course home fragment
context = { context = {
'csrf': csrf(request)['csrf_token'], 'csrf': csrf(request)['csrf_token'],
'course': course, 'course': course,
'outline_fragment': outline_fragment, 'outline_fragment': outline_fragment,
'has_visited_course': last_accessed_url is not None,
'disable_courseware_js': True, 'disable_courseware_js': True,
'uses_pattern_library': True, 'uses_pattern_library': True,
} }
......
...@@ -5,8 +5,7 @@ Views to show a course outline. ...@@ -5,8 +5,7 @@ Views to show a course outline.
from django.core.context_processors import csrf from django.core.context_processors import csrf
from django.template.loader import render_to_string from django.template.loader import render_to_string
from courseware.courses import get_course_with_access from courseware.courses import get_course_with_access, get_last_accessed_courseware
from lms.djangoapps.courseware.views.views import get_last_accessed_courseware
from lms.djangoapps.course_api.blocks.api import get_blocks from lms.djangoapps.course_api.blocks.api import get_blocks
from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.keys import CourseKey
from openedx.core.djangoapps.plugin_api.views import EdxFragmentView from openedx.core.djangoapps.plugin_api.views import EdxFragmentView
......
...@@ -665,7 +665,6 @@ class TestRecommenderFileUploading(TestRecommender): ...@@ -665,7 +665,6 @@ class TestRecommenderFileUploading(TestRecommender):
url = self.get_handler_url(event_name) url = self.get_handler_url(event_name)
resp = self.client.post(url, {'file': f_handler}) resp = self.client.post(url, {'file': f_handler})
self.assertEqual(resp.status_code, test_case['status']) self.assertEqual(resp.status_code, test_case['status'])
self.assert_request_status_code(200, self.course_url)
@data( @data(
{ {
......
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