Skip to content

E
edx-platform
Commit 22c8e4c6 by Robert Raposa
Options

Redirect Course Home for course that hasn't started. 

Includes the following:
- Move the redirect logic for before course that hasn't started to
share between Course Info and Course Home.
- Add audit comments for Course Info vs Course Home
- Other minor clean-up.

LEARNER-613
parent 13e06a37
Showing with 146 additions and 71 deletions
lms/djangoapps/courseware/courses.py
......@@ -6,16 +6,10 @@ import logging
from collections import defaultdict
from datetime import datetime
import pytz
from django.conf import settings
from django.core.urlresolvers import reverse
from django.http import Http404
from fs.errors import ResourceNotFoundError
from opaque_keys.edx.keys import UsageKey
from path import Path as path
import branding
import pytz
from courseware.access import has_access
from courseware.access_response import StartDateError
from courseware.date_summary import (
CourseEndDate,
CourseStartDate,
......@@ -25,13 +19,20 @@ from courseware.date_summary import (
)
from courseware.model_data import FieldDataCache
from courseware.module_render import get_module, get_module_for_descriptor
from django.conf import settings
from django.core.urlresolvers import reverse
from django.http import Http404, QueryDict
from edxmako.shortcuts import render_to_string
from fs.errors import ResourceNotFoundError
from lms.djangoapps.courseware.courseware_access_exception import CoursewareAccessException
from lms.djangoapps.courseware.exceptions import CourseAccessRedirect
from opaque_keys.edx.keys import UsageKey
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
from path import Path as path
from static_replace import replace_static_urls
from student.models import CourseEnrollment
from util.date_utils import strftime_localized
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.exceptions import ItemNotFoundError
from xmodule.x_module import STUDENT_VIEW
......@@ -121,6 +122,16 @@ def check_course_access(course, user, action, check_if_enrolled=False):
access_response = has_access(user, action, course, course.id)
if not access_response:
# Redirect if StartDateError
if isinstance(access_response, StartDateError):
start_date = strftime_localized(course.start, 'SHORT_DATE')
params = QueryDict(mutable=True)
params['notlive'] = start_date
raise CourseAccessRedirect('{dashboard_url}?{params}'.format(
dashboard_url=reverse('dashboard'),
params=params.urlencode()
))
# Deliberately return a non-specific error message to avoid
# leaking info about access control settings
raise CoursewareAccessException(access_response)
......
lms/djangoapps/courseware/tests/test_course_info.py
......@@ -8,11 +8,10 @@ from django.conf import settings
from django.core.urlresolvers import reverse
from django.http import QueryDict
from django.test.utils import override_settings
from nose.plugins.attrib import attr
from pyquery import PyQuery as pq
from lms.djangoapps.ccx.tests.factories import CcxFactory
from nose.plugins.attrib import attr
from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration
from pyquery import PyQuery as pq
from student.models import CourseEnrollment
from student.tests.factories import AdminFactory
from util.date_utils import strftime_localized
......@@ -58,6 +57,7 @@ class CourseInfoTestCase(LoginEnrollmentTestCase, SharedModuleStoreTestCase):
resp = self.client.get(url)
self.assertNotIn("You are not currently enrolled in this course", resp.content)
# TODO: LEARNER-611: If this is only tested under Course Info, does this need to move?
@mock.patch('openedx.features.enterprise_support.api.get_enterprise_consent_url')
def test_redirection_missing_enterprise_consent(self, mock_get_url):
"""
......@@ -120,7 +120,7 @@ class CourseInfoTestCase(LoginEnrollmentTestCase, SharedModuleStoreTestCase):
self.assertRedirects(response, expected_url)
@mock.patch.dict(settings.FEATURES, {'DISABLE_START_DATES': False})
@mock.patch("courseware.views.views.strftime_localized")
@mock.patch("util.date_utils.strftime_localized")
def test_non_live_course_other_language(self, mock_strftime_localized):
"""Ensure that a user accessing a non-live course sees a redirect to
the student dashboard, not a 404, even if the localized date is unicode
......@@ -385,7 +385,7 @@ class SelfPacedCourseInfoTestCase(LoginEnrollmentTestCase, SharedModuleStoreTest
self.assertEqual(resp.status_code, 200)
def test_num_queries_instructor_paced(self):
self.fetch_course_info_with_queries(self.instructor_paced_course, 26, 4)
self.fetch_course_info_with_queries(self.instructor_paced_course, 26, 3)
def test_num_queries_self_paced(self):
self.fetch_course_info_with_queries(self.self_paced_course, 26, 4)
self.fetch_course_info_with_queries(self.self_paced_course, 26, 3)
lms/djangoapps/courseware/tests/test_view_authentication.py
......@@ -60,7 +60,7 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
Check that non-staff don't have access to dark urls.
"""
names = ['courseware', 'instructor_dashboard', 'progress']
names = ['courseware', 'progress']
urls = self._reverse_urls(names, course)
urls.extend([
reverse('book', kwargs={'course_id': course.id.to_deprecated_string(),
......@@ -68,7 +68,11 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
for index, __ in enumerate(course.textbooks)
])
for url in urls:
self.assert_request_status_code(404, url)
self.assert_request_status_code(302, url)
self.assert_request_status_code(
404, reverse('instructor_dashboard', kwargs={'course_id': course.id.to_deprecated_string()})
)
def _check_staff(self, course):
"""
......@@ -97,7 +101,7 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
'student_id': self.enrolled_user.id,
}
)
self.assert_request_status_code(404, url)
self.assert_request_status_code(302, url)
# The courseware url should redirect, not 200
url = self._reverse_urls(['courseware'], course)[0]
......@@ -351,9 +355,9 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
self.enroll(self.test_course, True)
# should now be able to get to everything for self.course
self._check_staff(self.course)
self._check_non_staff_light(self.test_course)
self._check_non_staff_dark(self.test_course)
self._check_staff(self.course)
@patch.dict('courseware.access.settings.FEATURES', {'DISABLE_START_DATES': False})
def test_dark_launch_global_staff(self):
......
lms/djangoapps/courseware/views/views.py
......@@ -8,36 +8,10 @@ from collections import OrderedDict, namedtuple
from datetime import datetime, timedelta
import analytics
import waffle
from django.conf import settings
from django.contrib.auth.decorators import login_required
from django.contrib.auth.models import AnonymousUser, User
from django.core.context_processors import csrf
from django.core.exceptions import PermissionDenied
from django.core.urlresolvers import reverse
from django.db import transaction
from django.db.models import Q
from django.http import Http404, HttpResponse, HttpResponseBadRequest, HttpResponseForbidden, QueryDict
from django.shortcuts import redirect
from django.utils.decorators import method_decorator
from django.utils.text import slugify
from django.utils.timezone import UTC
from django.utils.translation import ugettext as _
from django.views.decorators.cache import cache_control
from django.views.decorators.csrf import ensure_csrf_cookie
from django.views.decorators.http import require_GET, require_http_methods, require_POST
from django.views.generic import View
from ipware.ip import get_ip
from markupsafe import escape
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey, UsageKey
from pytz import utc
from rest_framework import status
from web_fragments.fragment import Fragment
import shoppingcart
import survey.utils
import survey.views
import waffle
from certificates import api as certs_api
from certificates.models import CertificateStatuses
from commerce.utils import EcommerceService
......@@ -64,9 +38,28 @@ from courseware.model_data import FieldDataCache
from courseware.models import BaseStudentModuleHistory, StudentModule
from courseware.url_helpers import get_redirect_url
from courseware.user_state_client import DjangoXBlockUserStateClient
from django.conf import settings
from django.contrib.auth.decorators import login_required
from django.contrib.auth.models import AnonymousUser, User
from django.core.context_processors import csrf
from django.core.exceptions import PermissionDenied
from django.core.urlresolvers import reverse
from django.db import transaction
from django.db.models import Q
from django.http import Http404, HttpResponse, HttpResponseBadRequest, HttpResponseForbidden, QueryDict
from django.shortcuts import redirect
from django.utils.decorators import method_decorator
from django.utils.text import slugify
from django.utils.timezone import UTC
from django.utils.translation import ugettext as _
from django.views.decorators.cache import cache_control
from django.views.decorators.csrf import ensure_csrf_cookie
from django.views.decorators.http import require_GET, require_http_methods, require_POST
from django.views.generic import View
from edxmako.shortcuts import marketing_link, render_to_response, render_to_string
from enrollment.api import add_enrollment
from eventtracking import tracker
from ipware.ip import get_ip
from lms.djangoapps.ccx.custom_exception import CCXLocatorValidationException
from lms.djangoapps.ccx.utils import prep_course_for_grading
from lms.djangoapps.courseware.exceptions import CourseAccessRedirect, Redirect
......@@ -74,6 +67,9 @@ from lms.djangoapps.grades.new.course_grade_factory import CourseGradeFactory
from lms.djangoapps.instructor.enrollment import uses_shib
from lms.djangoapps.instructor.views.api import require_global_staff
from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification
from markupsafe import escape
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey, UsageKey
from openedx.core.djangoapps.catalog.utils import get_programs, get_programs_with_type
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from openedx.core.djangoapps.credit.api import (
......@@ -91,6 +87,8 @@ from openedx.features.course_experience import UNIFIED_COURSE_TAB_FLAG, course_h
from openedx.features.course_experience.course_tools import CourseToolsPluginManager
from openedx.features.course_experience.views.course_dates import CourseDatesFragmentView
from openedx.features.enterprise_support.api import data_sharing_consent_required
from pytz import utc
from rest_framework import status
from shoppingcart.utils import is_shopping_cart_enabled
from student.models import CourseEnrollment, UserTestGroup
from survey.utils import must_answer_survey
......@@ -99,6 +97,7 @@ from util.date_utils import strftime_localized
from util.db import outer_atomic
from util.milestones_helpers import get_prerequisite_courses_display
from util.views import _record_feedback_in_zendesk, ensure_valid_course_key, ensure_valid_usage_key
from web_fragments.fragment import Fragment
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem
from xmodule.tabs import CourseTabList
......@@ -233,6 +232,8 @@ def course_info(request, course_id):
Assumes the course_id is in a valid format.
"""
# TODO: LEARNER-611: This can be deleted with Course Info removal. The new
# Course Home is using its own processing of last accessed.
def get_last_accessed_courseware(course, request, user):
"""
Returns the courseware module URL that the user last accessed, or None if it cannot be found.
......@@ -262,29 +263,13 @@ def course_info(request, course_id):
return redirect(reverse(course_home_url_name(course_key), args=[course_id]))
with modulestore().bulk_operations(course_key):
course = get_course_by_id(course_key, depth=2)
access_response = has_access(request.user, 'load', course, course_key)
if not access_response:
# The user doesn't have access to the course. If they're
# denied permission due to the course not being live yet,
# redirect to the dashboard page.
if isinstance(access_response, StartDateError):
start_date = strftime_localized(course.start, 'SHORT_DATE')
params = QueryDict(mutable=True)
params['notlive'] = start_date
return redirect('{dashboard_url}?{params}'.format(
dashboard_url=reverse('dashboard'),
params=params.urlencode()
))
# Otherwise, give a 404 to avoid leaking info about access
# control.
raise Http404("Course not found.")
course = get_course_with_access(request.user, 'load', course_key)
staff_access = has_access(request.user, 'staff', course)
masquerade, user = setup_masquerade(request, course_key, staff_access, reset_masquerade_data=True)
# LEARNER-612: CCX redirect handled by new Course Home (DONE)
# TODO: LEARNER-1697: Transition banner messages to new Course Home.
# if user is not enrolled in a course then app will show enroll/get register link inside course info page.
user_is_enrolled = CourseEnrollment.is_enrolled(user, course.id)
show_enroll_banner = request.user.is_authenticated() and not user_is_enrolled
......@@ -294,20 +279,24 @@ def course_info(request, course_id):
if not user_is_enrolled and not can_self_enroll_in_course(course_key):
return redirect(reverse('dashboard'))
# TODO: LEARNER-1865: Handle prereqs and course survey in new Course Home.
# Redirect the user if they are not yet allowed to view this course
check_access_to_course(request, course)
# LEARNER-170: Entrance exam is handled by new Course Outline. (DONE)
# If the user needs to take an entrance exam to access this course, then we'll need
# to send them to that specific course module before allowing them into other areas
if not user_can_skip_entrance_exam(user, course):
return redirect(reverse('courseware', args=[unicode(course.id)]))
# TODO: LEARNER-611: Remove deprecated course.bypass_home.
# If the user is coming from the dashboard and bypass_home setting is set,
# redirect them straight to the courseware page.
is_from_dashboard = reverse('dashboard') in request.META.get('HTTP_REFERER', [])
if course.bypass_home and is_from_dashboard:
return redirect(reverse('courseware', args=[course_id]))
# TODO: LEARNER-1697: Transition handling of enroll links in new Course Home.
# 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])
......@@ -318,7 +307,9 @@ def course_info(request, course_id):
dates_fragment = None
if request.user.is_authenticated():
# TODO: LEARNER-611: Remove enable_course_home_improvements
if SelfPacedConfiguration.current().enable_course_home_improvements:
# Shared code with the new Course Home (DONE)
dates_fragment = CourseDatesFragmentView().render_to_fragment(request, course_id=course_id)
# This local import is due to the circularity of lms and openedx references.
......@@ -326,9 +317,7 @@ def course_info(request, course_id):
# as plugins, and to avoid the direct import.
from openedx.features.course_experience.views.course_reviews import CourseReviewsModuleFragmentView
# Decide whether or not to show the reviews link in the course tools bar
show_reviews_link = CourseReviewsModuleFragmentView.is_configured()
# Shared code with the new Course Home (DONE)
# Get the course tools enabled for this user and course
course_tools = CourseToolsPluginManager.get_enabled_course_tools(request, course_key)
......@@ -346,7 +335,6 @@ def course_info(request, course_id):
'user_is_enrolled': user_is_enrolled,
'dates_fragment': dates_fragment,
'url_to_enroll': url_to_enroll,
'show_reviews_link': show_reviews_link,
'course_tools': course_tools,
# TODO: (Experimental Code). See https://openedx.atlassian.net/wiki/display/RET/2.+In-course+Verification+Prompts
......@@ -358,9 +346,11 @@ def course_info(request, course_id):
# Get the URL of the user's last position in order to display the 'where you were last' message
context['resume_course_url'] = None
# TODO: LEARNER-611: Remove enable_course_home_improvements
if SelfPacedConfiguration.current().enable_course_home_improvements:
context['resume_course_url'] = get_last_accessed_courseware(course, request, user)
# LEARNER-981/LEARNER-837: Hide masquerade as necessary in Course Home (DONE)
if not is_course_open_for_learner(user, course):
# Disable student view button if user is staff and
# course is not yet visible to students.
......@@ -1697,6 +1687,7 @@ def check_access_to_course(request, course):
"""
Raises Redirect exceptions if the user does not have course access.
"""
# TODO: LEARNER-1865: Handle prereqs in new Course Home.
# Redirect to the dashboard if not all prerequisites have been met
if not has_access(request.user, 'view_courseware_with_prerequisites', course):
log.info(
......@@ -1705,6 +1696,7 @@ def check_access_to_course(request, course):
request.user.id, unicode(course.id))
raise CourseAccessRedirect(reverse('dashboard'))
# TODO: LEARNER-1865: Handle course surveys in new Course Home.
# Redirect if the user must answer a survey before entering the course.
if must_answer_survey(course, request.user):
raise CourseAccessRedirect(reverse('course_survey', args=[unicode(course.id)]))
openedx/features/course_experience/tests/views/test_course_home.py
# coding=utf-8
"""
Tests for the course home page.
"""
import ddt
import mock
from courseware.tests.factories import StaffFactory
from django.conf import settings
from django.core.urlresolvers import reverse
from django.http import QueryDict
from django.utils.http import urlquote_plus
from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES, override_waffle_flag
from openedx.features.course_experience import SHOW_REVIEWS_TOOL_FLAG, UNIFIED_COURSE_TAB_FLAG
from student.models import CourseEnrollment
from student.tests.factories import UserFactory
from util.date_utils import strftime_localized
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.tests.django_utils import CourseUserType, SharedModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls
......@@ -28,12 +32,25 @@ QUERY_COUNT_TABLE_BLACKLIST = WAFFLE_TABLES
def course_home_url(course):
"""
Returns the URL for the course's home page
Returns the URL for the course's home page.
Arguments:
course (CourseDescriptor): The course being tested.
"""
return course_home_url_from_string(unicode(course.id))
def course_home_url_from_string(course_key_string):
"""
Returns the URL for the course's home page.
Arguments:
course_key_string (String): The course key as string.
"""
return reverse(
'openedx.course_experience.course_home',
kwargs={
'course_id': unicode(course.id),
'course_id': course_key_string,
}
)
......@@ -211,3 +228,54 @@ class TestCourseHomePageAccess(CourseHomePageTestCase):
url = course_home_url(self.course)
response = self.client.get(url)
self.assertContains(response, '/login?next={url}'.format(url=urlquote_plus(url)))
@mock.patch.dict(settings.FEATURES, {'DISABLE_START_DATES': False})
def test_non_live_course(self):
"""
Ensure that a user accessing a non-live course sees a redirect to
the student dashboard, not a 404.
"""
self.user = self.create_user_for_course(self.course, CourseUserType.ENROLLED)
url = course_home_url(self.course)
response = self.client.get(url)
start_date = strftime_localized(self.course.start, 'SHORT_DATE')
expected_params = QueryDict(mutable=True)
expected_params['notlive'] = start_date
expected_url = '{url}?{params}'.format(
url=reverse('dashboard'),
params=expected_params.urlencode()
)
self.assertRedirects(response, expected_url)
@mock.patch.dict(settings.FEATURES, {'DISABLE_START_DATES': False})
@mock.patch("util.date_utils.strftime_localized")
def test_non_live_course_other_language(self, mock_strftime_localized):
"""
Ensure that a user accessing a non-live course sees a redirect to
the student dashboard, not a 404, even if the localized date is unicode
"""
self.user = self.create_user_for_course(self.course, CourseUserType.ENROLLED)
fake_unicode_start_time = u"üñîçø∂é_ßtå®t_tîµé"
mock_strftime_localized.return_value = fake_unicode_start_time
url = course_home_url(self.course)
response = self.client.get(url)
expected_params = QueryDict(mutable=True)
expected_params['notlive'] = fake_unicode_start_time
expected_url = u'{url}?{params}'.format(
url=reverse('dashboard'),
params=expected_params.urlencode()
)
self.assertRedirects(response, expected_url)
def test_nonexistent_course(self):
"""
Ensure a non-existent course results in a 404.
"""
self.user = self.create_user_for_course(self.course, CourseUserType.ANONYMOUS)
url = course_home_url_from_string('not/a/course')
response = self.client.get(url)
self.assertEqual(response.status_code, 404)
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