diff --git a/common/djangoapps/student/roles.py b/common/djangoapps/student/roles.py index 9da258e19b..01252f742f 100644 --- a/common/djangoapps/student/roles.py +++ b/common/djangoapps/student/roles.py @@ -168,7 +168,7 @@ class CourseRole(GroupBasedRole): else: groupnames.append('{0}_{1}'.format(role, course_context)) try: - locator = loc_mapper().translate_location(course_context, self.location, False, False) + locator = loc_mapper().translate_location(course_context, self.location, True, True) groupnames.append('{0}_{1}'.format(role, locator.package_id)) except (InvalidLocationError, ItemNotFoundError): # if it's never been mapped, the auth won't be via the Locator syntax diff --git a/common/djangoapps/student/tests/test_roles.py b/common/djangoapps/student/tests/test_roles.py index d7ec715426..cd20d16cf6 100644 --- a/common/djangoapps/student/tests/test_roles.py +++ b/common/djangoapps/student/tests/test_roles.py @@ -8,7 +8,9 @@ from xmodule.modulestore import Location from courseware.tests.factories import UserFactory, StaffFactory, InstructorFactory from student.tests.factories import AnonymousUserFactory -from student.roles import GlobalStaff, CourseRole +from student.roles import GlobalStaff, CourseRole, CourseStaffRole +from xmodule.modulestore.django import loc_mapper +from xmodule.modulestore.locator import BlockUsageLocator class RolesTestCase(TestCase): @@ -45,3 +47,40 @@ class RolesTestCase(TestCase): self.assertTrue(CourseRole("role", lowercase_loc).has_user(uppercase_user)) self.assertTrue(CourseRole("role", uppercase_loc).has_user(uppercase_user)) + def test_course_role(self): + """ + Test that giving a user a course role enables access appropriately + """ + course_locator = loc_mapper().translate_location( + self.course.course_id, self.course, add_entry_if_missing=True + ) + self.assertFalse( + CourseStaffRole(course_locator).has_user(self.student), + "Student has premature access to {}".format(unicode(course_locator)) + ) + self.assertFalse( + CourseStaffRole(self.course).has_user(self.student), + "Student has premature access to {}".format(self.course.url()) + ) + CourseStaffRole(course_locator).add_users(self.student) + self.assertTrue( + CourseStaffRole(course_locator).has_user(self.student), + "Student doesn't have access to {}".format(unicode(course_locator)) + ) + self.assertTrue( + CourseStaffRole(self.course).has_user(self.student), + "Student doesn't have access to {}".format(unicode(self.course.url())) + ) + # now try accessing something internal to the course + vertical_locator = BlockUsageLocator( + package_id=course_locator.package_id, branch='published', block_id='madeup' + ) + vertical_location = self.course.replace(category='vertical', name='madeuptoo') + self.assertTrue( + CourseStaffRole(vertical_locator).has_user(self.student), + "Student doesn't have access to {}".format(unicode(vertical_locator)) + ) + self.assertTrue( + CourseStaffRole(vertical_location, course_context=self.course.course_id).has_user(self.student), + "Student doesn't have access to {}".format(unicode(vertical_location.url())) + )