Merge pull request #9020 from edx/peter-fogg/404-not-live

Redirect to dashboard from non-live courses.

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,

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.
 

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
+        )

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):

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)
 

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;
             },

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'
         ]),
 

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
 <script type="text/template" id="${template_name}-tpl">
   <%static:include path="dashboard/${template_name}.underscore" />
 </script>
+<script type="text/template" id="message_banner-tpl">
+  <%static:include path="fields/message_banner.underscore" />
+</script>
 % 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();
     </%static:require_module>
   % 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)})
+    </%static:require_module>
+  % endif
 </%block>
 
 <div class="dashboard-notifications" tabindex="-1">

lms/templates/fields/message_banner.underscore

-<div class="wrapper-msg urgency-high">
+<div class="wrapper-msg urgency-<%= urgency %> <%= type %>">
     <div class="msg">
         <div class="msg-content">
             <div class="copy">