From c8ed59ac61d7604e4a3345685ec8cfd8ec606db0 Mon Sep 17 00:00:00 2001 From: Peter Fogg Date: Thu, 23 Jul 2015 11:44:25 -0400 Subject: [PATCH] Redirect to dashboard from non-live courses. TNL-2693 --- common/djangoapps/student/views.py | 8 +++++ common/test/acceptance/pages/lms/dashboard.py | 11 ++++++ common/test/acceptance/tests/lms/test_lms.py | 34 +++++++++++++++++++ .../courseware/tests/test_course_info.py | 21 ++++++++++++ lms/djangoapps/courseware/views.py | 22 ++++++++++-- lms/static/js/views/message_banner.js | 10 ++++-- lms/static/lms/js/build.js | 1 + lms/templates/dashboard.html | 11 ++++++ .../fields/message_banner.underscore | 4 +-- 9 files changed, 114 insertions(+), 8 deletions(-) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 1dc601ad20..3f62b8876a 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -662,8 +662,16 @@ def dashboard(request): user, course_org_filter, org_filter_out_set ) + if 'notlive' in request.GET: + redirect_message = _("The course you are looking for does not start until {0}.").format( + request.GET['notlive'] + ) + else: + redirect_message = '' + context = { 'enrollment_message': enrollment_message, + 'redirect_message': redirect_message, 'course_enrollments': course_enrollments, 'course_optouts': course_optouts, 'message': message, diff --git a/common/test/acceptance/pages/lms/dashboard.py b/common/test/acceptance/pages/lms/dashboard.py index cba42d2154..0e96e013a7 100644 --- a/common/test/acceptance/pages/lms/dashboard.py +++ b/common/test/acceptance/pages/lms/dashboard.py @@ -48,6 +48,17 @@ class DashboardPage(PageObject): return self.q(css='h3.course-title > a').map(_get_course_name).results + @property + def banner_text(self): + """ + Return the text of the banner on top of the page, or None if + the banner is not present. + """ + message = self.q(css='div.wrapper-msg') + if message.present: + return message.text[0] + return None + def get_enrollment_mode(self, course_name): """Get the enrollment mode for a given course on the dashboard. diff --git a/common/test/acceptance/tests/lms/test_lms.py b/common/test/acceptance/tests/lms/test_lms.py index 9d0ae381c2..ca0a6a260e 100644 --- a/common/test/acceptance/tests/lms/test_lms.py +++ b/common/test/acceptance/tests/lms/test_lms.py @@ -3,6 +3,7 @@ End-to-end tests for the LMS. """ +from datetime import datetime from flaky import flaky from textwrap import dedent from unittest import skip @@ -17,6 +18,7 @@ from ..helpers import ( select_option_by_value, element_has_text ) +from ...pages.lms import BASE_URL from ...pages.lms.account_settings import AccountSettingsPage from ...pages.lms.auto_auth import AutoAuthPage from ...pages.lms.create_mode import ModeCreationPage @@ -1152,3 +1154,35 @@ class EntranceExamTest(UniqueCourseTest): css_selector=entrance_exam_link_selector, text='Entrance Exam' )) + + +@attr('shard_5') +class NotLiveRedirectTest(UniqueCourseTest): + """ + Test that a banner is shown when the user is redirected to + the dashboard from a non-live course. + """ + + def setUp(self): + """Create a course that isn't live yet and enroll for it.""" + super(NotLiveRedirectTest, self).setUp() + CourseFixture( + self.course_info['org'], self.course_info['number'], + self.course_info['run'], self.course_info['display_name'], + start_date=datetime(year=2099, month=1, day=1) + ).install() + AutoAuthPage(self.browser, course_id=self.course_id).visit() + + def test_redirect_banner(self): + """ + Navigate to the course info page, then check that we're on the + dashboard page with the appropriate message. + """ + url = BASE_URL + "/courses/" + self.course_id + "/" + 'info' + self.browser.get(url) + page = DashboardPage(self.browser) + page.wait_for_page() + self.assertIn( + 'The course you are looking for does not start until', + page.banner_text + ) diff --git a/lms/djangoapps/courseware/tests/test_course_info.py b/lms/djangoapps/courseware/tests/test_course_info.py index 857c4f4f82..6d1f3b63fe 100644 --- a/lms/djangoapps/courseware/tests/test_course_info.py +++ b/lms/djangoapps/courseware/tests/test_course_info.py @@ -3,10 +3,13 @@ Test the course_info xblock """ import mock from nose.plugins.attrib import attr +from urllib import urlencode +from django.conf import settings from django.core.urlresolvers import reverse from opaque_keys.edx.locations import SlashSeparatedCourseKey +from util.date_utils import strftime_localized from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_CLOSED_MODULESTORE from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory @@ -62,6 +65,24 @@ class CourseInfoTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): ).exists() self.assertFalse(enrollment_exists) + @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.setup_user() + self.enroll(self.course) + url = reverse('info', args=[unicode(self.course.id)]) + response = self.client.get(url) + start_date = strftime_localized(self.course.start, 'SHORT_DATE') + self.assertRedirects(response, '{0}?{1}'.format(reverse('dashboard'), urlencode({'notlive': start_date}))) + + def test_nonexistent_course(self): + self.setup_user() + url = reverse('info', args=['not/a/course']) + response = self.client.get(url) + self.assertEqual(response.status_code, 404) + @attr('shard_1') class CourseInfoTestCaseXML(LoginEnrollmentTestCase, ModuleStoreTestCase): diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 2b8a4f1285..398b5bb465 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -31,8 +31,9 @@ from markupsafe import escape from courseware import grades from courseware.access import has_access, in_preview_mode, _adjust_start_date_for_beta_testers +from courseware.access_response import StartDateError from courseware.courses import ( - get_courses, get_course, + get_courses, get_course, get_course_by_id, get_studio_url, get_course_with_access, sort_by_announcement, sort_by_start_date, @@ -60,6 +61,7 @@ from open_ended_grading.views import StaffGradingTab, PeerGradingTab, OpenEndedG from student.models import UserTestGroup, CourseEnrollment from student.views import is_course_blocked from util.cache import cache, cache_if_anonymous +from util.date_utils import strftime_localized from xblock.fragment import Fragment from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem @@ -674,9 +676,23 @@ def course_info(request, course_id): Assumes the course_id is in a valid format. """ course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) - with modulestore().bulk_operations(course_key): - course = get_course_with_access(request.user, 'load', 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 = urllib.urlencode({'notlive': start_date}) + return redirect('{0}?{1}'.format(reverse('dashboard'), params)) + # Otherwise, give a 404 to avoid leaking info about access + # control. + raise Http404("Course not found.") + staff_access = has_access(request.user, 'staff', course) masquerade, user = setup_masquerade(request, course_key, staff_access, reset_masquerade_data=True) diff --git a/lms/static/js/views/message_banner.js b/lms/static/js/views/message_banner.js index 87e36fafa0..133adac8eb 100644 --- a/lms/static/js/views/message_banner.js +++ b/lms/static/js/views/message_banner.js @@ -6,7 +6,11 @@ var MessageBannerView = Backbone.View.extend({ - initialize: function () { + initialize: function (options) { + if (_.isUndefined(options)) { + options = {}; + } + this.options = _.defaults(options, {urgency: 'high', type: ''}); this.template = _.template($('#message_banner-tpl').text()); }, @@ -14,9 +18,9 @@ if (_.isUndefined(this.message) || _.isNull(this.message)) { this.$el.html(''); } else { - this.$el.html(this.template({ + this.$el.html(this.template(_.extend(this.options, { message: this.message - })); + }))); } return this; }, diff --git a/lms/static/lms/js/build.js b/lms/static/lms/js/build.js index 35b31ace59..33b1903289 100644 --- a/lms/static/lms/js/build.js +++ b/lms/static/lms/js/build.js @@ -27,6 +27,7 @@ 'js/student_account/views/account_settings_factory', 'js/student_account/views/finish_auth_factory', 'js/student_profile/views/learner_profile_factory', + 'js/views/message_banner', 'teams/js/teams_tab_factory' ]), diff --git a/lms/templates/dashboard.html b/lms/templates/dashboard.html index ed5d369a42..b3c53bb6a6 100644 --- a/lms/templates/dashboard.html +++ b/lms/templates/dashboard.html @@ -7,6 +7,7 @@ import third_party_auth from third_party_auth import pipeline from microsite_configuration import microsite from django.core.urlresolvers import reverse +import json %> <% @@ -24,6 +25,9 @@ from django.core.urlresolvers import reverse + % endfor % for template_name in ["dashboard_search_item", "dashboard_search_results", "search_loading", "search_error"]: @@ -49,6 +53,13 @@ from django.core.urlresolvers import reverse DashboardSearchFactory(); % endif + % if redirect_message: + <%static:require_module module_name="js/views/message_banner" class_name="MessageBannerView"> + var banner = new MessageBannerView({urgency: 'low', type: 'warning'}); + $('#content').prepend(banner.$el); + banner.showMessage(${json.dumps(redirect_message)}) + + % endif
diff --git a/lms/templates/fields/message_banner.underscore b/lms/templates/fields/message_banner.underscore index c2cd316ccf..cf6c22e65d 100644 --- a/lms/templates/fields/message_banner.underscore +++ b/lms/templates/fields/message_banner.underscore @@ -1,4 +1,4 @@ -
+
@@ -6,4 +6,4 @@
-
\ No newline at end of file +