From e1035c7bd7383b6a596d689447fd8c0d4c05885f Mon Sep 17 00:00:00 2001 From: Amir Qayyum Khan Date: Fri, 15 Jan 2016 02:16:17 +0500 Subject: [PATCH 1/2] Allowed staff/admin of course to view ccx coach dashboard --- lms/djangoapps/ccx/plugins.py | 3 + lms/djangoapps/ccx/tests/test_views.py | 80 +++++++++++++++++++++++++- lms/djangoapps/ccx/views.py | 30 ++++++---- 3 files changed, 100 insertions(+), 13 deletions(-) diff --git a/lms/djangoapps/ccx/plugins.py b/lms/djangoapps/ccx/plugins.py index 5408dbdaf7..f01c2899be 100644 --- a/lms/djangoapps/ccx/plugins.py +++ b/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): @@ -28,5 +29,7 @@ class CcxCourseTab(CourseTab): return True if not settings.FEATURES.get('CUSTOM_COURSES_EDX', False) or not course.enable_ccx: return False + if has_access(user, 'staff', course) or has_access(user, 'instructor', course): + return True role = CourseCcxCoachRole(course.id) return role.has_user(user) diff --git a/lms/djangoapps/ccx/tests/test_views.py b/lms/djangoapps/ccx/tests/test_views.py index e40e9df734..fd3c636d8e 100644 --- a/lms/djangoapps/ccx/tests/test_views.py +++ b/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)}) diff --git a/lms/djangoapps/ccx/views.py b/lms/djangoapps/ccx/views.py index 5b9aae0b6f..3a6c3da7e7 100644 --- a/lms/djangoapps/ccx/views.py +++ b/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) + is_staff = has_access(request.user, 'staff', course) + is_instructor = has_access(request.user, 'instructor', course) - # 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') - ) + 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 From 8be8ac8fbf77ba89f95d0273d0ae11a053ec94d6 Mon Sep 17 00:00:00 2001 From: Amir Qayyum Khan Date: Thu, 25 Feb 2016 17:02:04 +0500 Subject: [PATCH 2/2] Refactored ccx coach tab plugin --- lms/djangoapps/ccx/plugins.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/ccx/plugins.py b/lms/djangoapps/ccx/plugins.py index f01c2899be..c081f50c50 100644 --- a/lms/djangoapps/ccx/plugins.py +++ b/lms/djangoapps/ccx/plugins.py @@ -25,11 +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)