Merge pull request #11546 from mitocw/feature/aq/allow_staff_view_coach_dashboard_mitocw#125

Allowed staff/admin of course to view ccx coach dashboard

lms/djangoapps/ccx/plugins.py

@@ -7,6 +7,7 @@ from django.utils.translation import ugettext_noop
 
 from xmodule.tabs import CourseTab
 from student.roles import CourseCcxCoachRole
+from courseware.access import has_access
 
 
 class CcxCourseTab(CourseTab):
@@ -24,9 +25,12 @@ class CcxCourseTab(CourseTab):
         """
         Returns true if CCX has been enabled and the specified user is a coach
         """
-        if not user:
-            return True
         if not settings.FEATURES.get('CUSTOM_COURSES_EDX', False) or not course.enable_ccx:
+            # If ccx is not enable do not show ccx coach tab.
             return False
+        if has_access(user, 'staff', course) or has_access(user, 'instructor', course):
+            # if user is staff or instructor then he can always see ccx coach tab.
+            return True
+        # check if user has coach access.
         role = CourseCcxCoachRole(course.id)
         return role.has_user(user)

lms/djangoapps/ccx/tests/test_views.py

@@ -23,7 +23,11 @@ from django.test import RequestFactory
 from edxmako.shortcuts import render_to_response
 from request_cache.middleware import RequestCache
 from opaque_keys.edx.keys import CourseKey
-from student.roles import CourseCcxCoachRole
+from student.roles import (
+    CourseCcxCoachRole,
+    CourseInstructorRole,
+    CourseStaffRole
+)
 from student.models import (
     CourseEnrollment,
     CourseEnrollmentAllowed,
@@ -116,6 +120,75 @@ def setup_students_and_grades(context):
                     )
 
 
+class TestAdminAccessCoachDashboard(CcxTestCase, LoginEnrollmentTestCase):
+    """
+    Tests for Custom Courses views.
+    """
+    MODULESTORE = TEST_DATA_SPLIT_MODULESTORE
+
+    def make_staff(self):
+        """
+        create staff user
+        """
+        staff = AdminFactory.create(password="test")
+        role = CourseStaffRole(self.course.id)
+        role.add_users(staff)
+
+        return staff
+
+    def make_instructor(self):
+        """
+        create staff instructor
+        """
+        instructor = AdminFactory.create(password="test")
+        role = CourseInstructorRole(self.course.id)
+        role.add_users(instructor)
+
+        return instructor
+
+    def test_staff_access_coach_dashboard(self):
+        """
+        User is staff, should access coach dashboard.
+        """
+        staff = self.make_staff()
+        self.client.login(username=staff.username, password="test")
+        self.make_coach()
+        ccx = self.make_ccx()
+        url = reverse(
+            'ccx_coach_dashboard',
+            kwargs={'course_id': CCXLocator.from_course_locator(self.course.id, ccx.id)})
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 200)
+
+    def test_instructor_access_coach_dashboard(self):
+        """
+        User is instructor, should access coach dashboard.
+        """
+        instructor = self.make_instructor()
+        self.client.login(username=instructor.username, password="test")
+        self.make_coach()
+        ccx = self.make_ccx()
+        url = reverse(
+            'ccx_coach_dashboard',
+            kwargs={'course_id': CCXLocator.from_course_locator(self.course.id, ccx.id)})
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 200)
+
+    def test_forbidden_user_access_coach_dashboard(self):
+        """
+        Assert user with no access must not see dashboard.
+        """
+        user = UserFactory.create(password="test")
+        self.client.login(username=user.username, password="test")
+        self.make_coach()
+        ccx = self.make_ccx()
+        url = reverse(
+            'ccx_coach_dashboard',
+            kwargs={'course_id': CCXLocator.from_course_locator(self.course.id, ccx.id)})
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 403)
+
+
 @attr('shard_1')
 @ddt.ddt
 class TestCoachDashboard(CcxTestCase, LoginEnrollmentTestCase):
@@ -164,7 +237,12 @@ class TestCoachDashboard(CcxTestCase, LoginEnrollmentTestCase):
         """
         User is not a coach, should get Forbidden response.
         """
+        self.make_coach()
         ccx = self.make_ccx()
+
+        # create session of non-coach user
+        user = UserFactory.create(password="test")
+        self.client.login(username=user.username, password="test")
         url = reverse(
             'ccx_coach_dashboard',
             kwargs={'course_id': CCXLocator.from_course_locator(self.course.id, ccx.id)})

lms/djangoapps/ccx/views.py

@@ -26,6 +26,7 @@ from django.views.decorators.cache import cache_control
 from django.views.decorators.csrf import ensure_csrf_cookie
 from django.contrib.auth.models import User
 
+from courseware.access import has_access
 from courseware.courses import get_course_by_id
 
 from courseware.field_overrides import disable_overrides
@@ -85,20 +86,25 @@ def coach_dashboard(view):
             ccx = CustomCourseForEdX.objects.get(pk=ccx_id)
             course_key = ccx.course_id
 
-        role = CourseCcxCoachRole(course_key)
-        if not role.has_user(request.user):
-            return HttpResponseForbidden(
-                _('You must be a CCX Coach to access this view.'))
-
         course = get_course_by_id(course_key, depth=None)
-
-        # if there is a ccx, we must validate that it is the ccx for this coach
-        if ccx is not None:
-            coach_ccx = get_ccx_by_ccx_id(course, request.user, ccx.id)
-            if coach_ccx is None:
-                return HttpResponseForbidden(
-                    _('You must be the coach for this ccx to access this view')
-                )
+        is_staff = has_access(request.user, 'staff', course)
+        is_instructor = has_access(request.user, 'instructor', course)
+
+        if is_staff or is_instructor:
+            # if user is staff or instructor then he can view ccx coach dashboard.
+            return view(request, course, ccx)
+        else:
+            role = CourseCcxCoachRole(course_key)
+            if not role.has_user(request.user):
+                return HttpResponseForbidden(_('You must be a CCX Coach to access this view.'))
+
+            # if there is a ccx, we must validate that it is the ccx for this coach
+            if ccx is not None:
+                coach_ccx = get_ccx_by_ccx_id(course, request.user, ccx.id)
+                if coach_ccx is None:
+                    return HttpResponseForbidden(
+                        _('You must be the coach for this ccx to access this view')
+                    )
 
         return view(request, course, ccx)
     return wrapper