Merge pull request #11124 from mitocw/fix/aq/progress_of_student_ccx

Allowed CCX coaches to see individual students progress from grade book

lms/djangoapps/ccx/custom_exception.py

+"""
+This file has custom exceptions for ccx
+"""
+
+
+class CCXUserValidationException(Exception):
+    """
+    Custom Exception for validation of users in CCX
+    """
+    pass
+
+
+class CCXLocatorValidationException(Exception):
+    """
+    Custom Exception to validate CCX locator
+    """
+    pass

lms/djangoapps/ccx/utils.py

@@ -29,17 +29,11 @@ from student.roles import CourseCcxCoachRole
 
 from lms.djangoapps.ccx.models import CustomCourseForEdX
 from lms.djangoapps.ccx.overrides import get_override_for_ccx
+from lms.djangoapps.ccx.custom_exception import CCXUserValidationException
 
 log = logging.getLogger("edx.ccx")
 
 
-class CCXUserValidationException(Exception):
-    """
-    Custom Exception for validation of users in CCX
-    """
-    pass
-
-
 def get_ccx_from_ccx_locator(course_id):
     """ helper function to allow querying ccx fields from templates """
     ccx_id = getattr(course_id, 'ccx', None)

lms/djangoapps/courseware/access.py

@@ -39,6 +39,7 @@ from student import auth
 from student.models import CourseEnrollmentAllowed
 from student.roles import (
     CourseBetaTesterRole,
+    CourseCcxCoachRole,
     CourseInstructorRole,
     CourseStaffRole,
     GlobalStaff,
@@ -62,9 +63,41 @@ from courseware.access_response import (
 )
 from courseware.access_utils import adjust_start_date, check_start_date, debug, ACCESS_GRANTED, ACCESS_DENIED
 
+from lms.djangoapps.ccx.custom_exception import CCXLocatorValidationException
+from lms.djangoapps.ccx.models import CustomCourseForEdX
+
 log = logging.getLogger(__name__)
 
 
+def has_ccx_coach_role(user, course_key):
+    """
+    Check if user is a coach on this ccx.
+
+    Arguments:
+        user (User): the user whose descriptor access we are checking.
+        course_key (CCXLocator): Key to CCX.
+
+    Returns:
+        bool: whether user is a coach on this ccx or not.
+    """
+    if hasattr(course_key, 'ccx'):
+        ccx_id = course_key.ccx
+        role = CourseCcxCoachRole(course_key)
+
+        if role.has_user(user):
+            list_ccx = CustomCourseForEdX.objects.filter(
+                course_id=course_key.to_course_locator(),
+                coach=user
+            )
+            if list_ccx.exists():
+                coach_ccx = list_ccx[0]
+                return str(coach_ccx.id) == ccx_id
+    else:
+        raise CCXLocatorValidationException("Invalid CCX key. To verify that "
+                                            "user is a coach on CCX, you must provide key to CCX")
+    return False
+
+
 def has_access(user, action, obj, course_key=None):
     """
     Check whether a user has the access to do action on obj.  Handles any magic

lms/djangoapps/courseware/tests/test_access.py

@@ -8,6 +8,9 @@ import itertools
 import pytz
 
 from django.contrib.auth.models import User
+from ccx_keys.locator import CCXLocator
+from django.http import Http404
+from django.test.client import RequestFactory
 from django.core.urlresolvers import reverse
 from django.test import TestCase
 from mock import Mock, patch
@@ -24,9 +27,14 @@ from courseware.tests.factories import (
     StaffFactory,
     UserFactory,
 )
+import courseware.views as views
 from courseware.tests.helpers import LoginEnrollmentTestCase
+from edxmako.tests import mako_middleware_process_request
 from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
+from student.models import CourseEnrollment
+from student.roles import CourseCcxCoachRole
 from student.tests.factories import (
+    AdminFactory,
     AnonymousUserFactory,
     CourseEnrollmentAllowedFactory,
     CourseEnrollmentFactory,
@@ -37,7 +45,11 @@ from xmodule.course_module import (
     CATALOG_VISIBILITY_NONE,
 )
 from xmodule.modulestore.tests.factories import CourseFactory
-from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+from xmodule.modulestore.tests.django_utils import (
+    ModuleStoreTestCase,
+    SharedModuleStoreTestCase,
+    TEST_DATA_SPLIT_MODULESTORE
+)
 
 from util.milestones_helpers import (
     set_prerequisite_courses,
@@ -45,9 +57,98 @@ from util.milestones_helpers import (
     seed_milestone_relationship_types,
 )
 
+from lms.djangoapps.ccx.models import CustomCourseForEdX
+
 # pylint: disable=protected-access
 
 
+class CoachAccessTestCaseCCX(SharedModuleStoreTestCase, LoginEnrollmentTestCase):
+    """
+    Test if user is coach on ccx.
+    """
+    MODULESTORE = TEST_DATA_SPLIT_MODULESTORE
+
+    @classmethod
+    def setUpClass(cls):
+        """
+        Set up course for tests
+        """
+        super(CoachAccessTestCaseCCX, cls).setUpClass()
+        cls.course = CourseFactory.create()
+
+    def setUp(self):
+        """
+        Set up tests
+        """
+        super(CoachAccessTestCaseCCX, self).setUp()
+
+        # Create ccx coach account
+        self.coach = AdminFactory.create(password="test")
+        self.client.login(username=self.coach.username, password="test")
+
+        # assign role to coach
+        role = CourseCcxCoachRole(self.course.id)
+        role.add_users(self.coach)
+        self.request_factory = RequestFactory()
+
+    def make_ccx(self):
+        """
+        create ccx
+        """
+        ccx = CustomCourseForEdX(
+            course_id=self.course.id,
+            coach=self.coach,
+            display_name="Test CCX"
+        )
+        ccx.save()
+
+        ccx_locator = CCXLocator.from_course_locator(self.course.id, unicode(ccx.id))
+        role = CourseCcxCoachRole(ccx_locator)
+        role.add_users(self.coach)
+        CourseEnrollment.enroll(self.coach, ccx_locator)
+        return ccx_locator
+
+    def test_has_ccx_coach_role(self):
+        """
+        Assert that user has coach access on ccx.
+        """
+        ccx_locator = self.make_ccx()
+
+        # user have access as coach on ccx
+        self.assertTrue(access.has_ccx_coach_role(self.coach, ccx_locator))
+
+        # user dont have access as coach on ccx
+        self.setup_user()
+        self.assertFalse(access.has_ccx_coach_role(self.user, ccx_locator))
+
+    def test_access_student_progress_ccx(self):
+        """
+        Assert that only a coach can see progress of student.
+        """
+        ccx_locator = self.make_ccx()
+        student = UserFactory()
+
+        # Enroll user
+        CourseEnrollment.enroll(student, ccx_locator)
+
+        # Test for access of a coach
+        request = self.request_factory.get(reverse('about_course', args=[unicode(ccx_locator)]))
+        request.user = self.coach
+        mako_middleware_process_request(request)
+        resp = views.progress(request, course_id=unicode(ccx_locator), student_id=student.id)
+        self.assertEqual(resp.status_code, 200)
+
+        # Assert access of a student
+        request = self.request_factory.get(reverse('about_course', args=[unicode(ccx_locator)]))
+        request.user = student
+        mako_middleware_process_request(request)
+
+        with self.assertRaises(Http404) as context:
+            views.progress(request, course_id=unicode(ccx_locator), student_id=self.coach.id)
+
+        self.assertIsNotNone(context.exception)
+
+
 @attr('shard_1')
 @ddt.ddt
 class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase):

lms/djangoapps/courseware/views.py

@@ -33,7 +33,7 @@ from rest_framework import status
 import newrelic.agent
 
 from courseware import grades
-from courseware.access import has_access, _adjust_start_date_for_beta_testers
+from courseware.access import has_access, has_ccx_coach_role, _adjust_start_date_for_beta_testers
 from courseware.access_response import StartDateError
 from courseware.access_utils import in_preview_mode
 from courseware.courses import (
@@ -97,6 +97,7 @@ from util.views import ensure_valid_course_key
 from eventtracking import tracker
 import analytics
 from courseware.url_helpers import get_redirect_url
+from lms.djangoapps.ccx.custom_exception import CCXLocatorValidationException
 
 from lang_pref import LANGUAGE_KEY
 from openedx.core.djangoapps.user_api.preferences.api import get_user_preference
@@ -918,7 +919,7 @@ def course_about(request, course_id):
 def progress(request, course_id, student_id=None):
     """ Display the progress page. """
 
-    course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
+    course_key = CourseKey.from_string(course_id)
 
     with modulestore().bulk_operations(course_key):
         return _progress(request, course_key, student_id)
@@ -940,13 +941,19 @@ def _progress(request, course_key, student_id):
         return redirect(reverse('course_survey', args=[unicode(course.id)]))
 
     staff_access = bool(has_access(request.user, 'staff', course))
+    try:
+        coach_access = has_ccx_coach_role(request.user, course_key)
+    except CCXLocatorValidationException:
+        coach_access = False
+
+    has_access_on_students_profiles = staff_access or coach_access
 
     if student_id is None or student_id == request.user.id:
         # always allowed to see your own profile
         student = request.user
     else:
         # Requesting access to a different student's profile
-        if not staff_access:
+        if not has_access_on_students_profiles:
             raise Http404
         try:
             student = User.objects.get(id=student_id)