Merge pull request #12889 from mitocw/fix/aq/caoch_dashboard_navigation_issues

Fixed CCX coach dashboard access issues

cms/envs/bok_choy.env.json

@@ -77,7 +77,8 @@
         "ENABLE_CONTENT_LIBRARIES": true,
         "ENABLE_SPECIAL_EXAMS": true,
         "SHOW_LANGUAGE_SELECTOR": true,
-        "ENABLE_EXTENDED_COURSE_DETAILS": true
+        "ENABLE_EXTENDED_COURSE_DETAILS": true,
+        "CUSTOM_COURSES_EDX": true
     },
     "FEEDBACK_SUBMISSION_EMAIL": "",
     "GITHUB_REPO_ROOT": "** OVERRIDDEN **",

common/test/acceptance/pages/studio/settings_advanced.py

@@ -221,5 +221,7 @@ class AdvancedSettingsPage(CoursePage):
             'enable_subsection_gating',
             'learning_info',
             'instructor_info',
-            'create_zendesk_tickets'
+            'create_zendesk_tickets',
+            'ccx_connector',
+            'enable_ccx'
         ]

common/test/acceptance/tests/lms/test_ccx.py

@@ -21,23 +21,35 @@ class CreateCCXCoachTest(EventsTestMixin, UniqueCourseTest):
     def setUp(self):
         super(CreateCCXCoachTest, self).setUp()
         self.course_info.update({"settings": {"enable_ccx": "true"}})
-        self.course_fixture = CourseFixture(**self.course_info).install()
-        self.coach_dashboard_page = CoachDashboardPage(self.browser, self.course_id)
+        self.course_fixture = CourseFixture(**self.course_info)
+        self.course_fixture.add_advanced_settings({
+            "enable_ccx": {"value": "true"}
+        })
+        self.course_fixture.install()
 
-    def _auto_auth(self, username, email):
+        self.auto_auth(self.USERNAME, self.EMAIL)
+        self.coach_dashboard_page = self.visit_coach_dashboard()
+
+    def auto_auth(self, username, email):
         """
         Logout and login with given credentials.
         """
         AutoAuthPage(self.browser, username=username, email=email,
                      course_id=self.course_id, staff=True).visit()
 
+    def visit_coach_dashboard(self):
+        """
+        Visits the instructor dashboard.
+        """
+        coach_dashboard_page = CoachDashboardPage(self.browser, self.course_id)
+        coach_dashboard_page.visit()
+        return coach_dashboard_page
+
     def test_create_ccx(self):
         """
         Assert that ccx created.
         """
         ccx_name = "Test ccx"
-        self._auto_auth(self.USERNAME, self.EMAIL)
-        self.coach_dashboard_page.visit()
 
         self.coach_dashboard_page.fill_ccx_name_text_box(ccx_name)
         self.coach_dashboard_page.wait_for_page()

lms/djangoapps/ccx/tests/test_views.py

@@ -187,6 +187,12 @@ class TestCoachDashboard(CcxTestCase, LoginEnrollmentTestCase):
     Tests for Custom Courses views.
     """
 
+    @classmethod
+    def setUpClass(cls):
+        super(TestCoachDashboard, cls).setUpClass()
+        cls.course_disable_ccx = CourseFactory.create(enable_ccx=False)
+        cls.course_with_ccx_connect_set = CourseFactory.create(enable_ccx=True, ccx_connector="http://ccx.com")
+
     def setUp(self):
         """
         Set up tests
@@ -239,16 +245,12 @@ class TestCoachDashboard(CcxTestCase, LoginEnrollmentTestCase):
         """
         Assert that coach cannot create ccx when ``ccx_connector`` url is set.
         """
-        course = CourseFactory.create()
-        course.ccx_connector = "http://ccx.com"
-        course.save()
-        self.store.update_item(course, 0)
-        role = CourseCcxCoachRole(course.id)
+        role = CourseCcxCoachRole(self.course_with_ccx_connect_set.id)
         role.add_users(self.coach)
 
         url = reverse(
             'create_ccx',
-            kwargs={'course_id': unicode(course.id)})
+            kwargs={'course_id': unicode(self.course_with_ccx_connect_set.id)})
 
         response = self.client.get(url)
         self.assertEqual(response.status_code, 200)
@@ -311,6 +313,28 @@ class TestCoachDashboard(CcxTestCase, LoginEnrollmentTestCase):
     def test_create_multiple_ccx(self, ccx_name):
         self.test_create_ccx(ccx_name)
 
+    def test_dashboard_access_of_disabled_ccx(self):
+        """
+        User should not see coach dashboard if ccx is disbale in studio.
+        """
+        ccx = CcxFactory(course_id=self.course_disable_ccx.id, coach=self.coach)
+        url = reverse(
+            'ccx_coach_dashboard',
+            kwargs={'course_id': CCXLocator.from_course_locator(self.course_disable_ccx.id, ccx.id)})
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 404)
+
+    def test_dashboard_access_with_invalid_ccx_id(self):
+        """
+        User should not see coach dashboard if ccx id is invalid.
+        """
+        self.make_ccx()
+        url = reverse(
+            'ccx_coach_dashboard',
+            kwargs={'course_id': CCXLocator.from_course_locator(self.course_disable_ccx.id, 700)})
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 404)
+
     def test_get_date(self):
         """
         Assert that get_date returns valid date.
@@ -789,7 +813,7 @@ class TestCoachDashboardSchedule(CcxTestCase, LoginEnrollmentTestCase, ModuleSto
 
     def setUp(self):
         super(TestCoachDashboardSchedule, self).setUp()
-        self.course = course = CourseFactory.create()
+        self.course = course = CourseFactory.create(enable_ccx=True)
 
         # Create a course outline
         self.mooc_start = start = datetime.datetime(

lms/djangoapps/ccx/tests/utils.py

@@ -39,7 +39,7 @@ class CcxTestCase(SharedModuleStoreTestCase):
     @classmethod
     def setUpClass(cls):
         super(CcxTestCase, cls).setUpClass()
-        cls.course = course = CourseFactory.create()
+        cls.course = course = CourseFactory.create(enable_ccx=True)
 
         # Create a course outline
         cls.mooc_start = start = datetime.datetime(

lms/djangoapps/ccx/views.py

@@ -14,6 +14,7 @@ from cStringIO import StringIO
 from django.conf import settings
 from django.core.urlresolvers import reverse
 from django.http import (
+    Http404,
     HttpResponse,
     HttpResponseForbidden,
 )
@@ -84,14 +85,20 @@ def coach_dashboard(view):
         ccx = None
         if isinstance(course_key, CCXLocator):
             ccx_id = course_key.ccx
-            ccx = CustomCourseForEdX.objects.get(pk=ccx_id)
-            course_key = ccx.course_id
+            try:
+                ccx = CustomCourseForEdX.objects.get(pk=ccx_id)
+            except CustomCourseForEdX.DoesNotExist:
+                raise Http404
 
+        if ccx:
+            course_key = ccx.course_id
         course = get_course_by_id(course_key, depth=None)
         is_staff = has_access(request.user, 'staff', course)
         is_instructor = has_access(request.user, 'instructor', course)
 
-        if is_staff or is_instructor:
+        if not course.enable_ccx:
+            raise Http404
+        elif is_staff or is_instructor:
             # if user is staff or instructor then he can view ccx coach dashboard.
             return view(request, course, ccx)
         else: