diff --git a/lms/djangoapps/ccx/plugins.py b/lms/djangoapps/ccx/plugins.py index 5408dbdaf7..c081f50c50 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): @@ -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) 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