diff --git a/lms/djangoapps/course_wiki/tests/test_access.py b/lms/djangoapps/course_wiki/tests/test_access.py index aca2223d13..5d56c228d8 100644 --- a/lms/djangoapps/course_wiki/tests/test_access.py +++ b/lms/djangoapps/course_wiki/tests/test_access.py @@ -4,6 +4,7 @@ Tests for wiki permissions from django.contrib.auth.models import Group from student.tests.factories import UserFactory +from xmodule.modulestore.django import loc_mapper from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase @@ -24,21 +25,37 @@ class TestWikiAccessBase(ModuleStoreTestCase): self.wiki = get_or_create_root() - self.course_math101 = CourseFactory.create(org='org', number='math101', display_name='Course') - self.course_math101_staff = [ - InstructorFactory(course=self.course_math101.location), - StaffFactory(course=self.course_math101.location) - ] + self.course_math101 = CourseFactory.create(org='org', number='math101', display_name='Course', metadata={'use_unique_wiki_id': 'false'}) + self.course_math101_staff = self.create_staff_for_course(self.course_math101) wiki_math101 = self.create_urlpath(self.wiki, course_wiki_slug(self.course_math101)) wiki_math101_page = self.create_urlpath(wiki_math101, 'Child') wiki_math101_page_page = self.create_urlpath(wiki_math101_page, 'Grandchild') self.wiki_math101_pages = [wiki_math101, wiki_math101_page, wiki_math101_page_page] + self.course_math101b = CourseFactory.create(org='org', number='math101b', display_name='Course', metadata={'use_unique_wiki_id': 'true'}) + self.course_math101b_staff = self.create_staff_for_course(self.course_math101b) + + wiki_math101b = self.create_urlpath(self.wiki, course_wiki_slug(self.course_math101b)) + wiki_math101b_page = self.create_urlpath(wiki_math101b, 'Child') + wiki_math101b_page_page = self.create_urlpath(wiki_math101b_page, 'Grandchild') + self.wiki_math101b_pages = [wiki_math101b, wiki_math101b_page, wiki_math101b_page_page] + def create_urlpath(self, parent, slug): """Creates an article at /parent/slug and returns its URLPath""" return URLPath.create_article(parent, slug, title=slug) + def create_staff_for_course(self, course): + """Creates and returns users with instructor and staff access to course.""" + + course_locator = loc_mapper().translate_location(course.id, course.location) + return [ + InstructorFactory(course=course.location), # Creates instructor_org/number/run role name + StaffFactory(course=course.location), # Creates staff_org/number/run role name + InstructorFactory(course=course_locator), # Creates instructor_org.number.run role name + StaffFactory(course=course_locator), # Creates staff_org.number.run role name + ] + @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) class TestWikiAccess(TestWikiAccessBase): @@ -47,21 +64,15 @@ class TestWikiAccess(TestWikiAccessBase): super(TestWikiAccess, self).setUp() self.course_310b = CourseFactory.create(org='org', number='310b', display_name='Course') - self.course_310b_staff = [ - InstructorFactory(course=self.course_310b.location), - StaffFactory(course=self.course_310b.location) - ] - self.course_310b_ = CourseFactory.create(org='org', number='310b_', display_name='Course') - self.course_310b__staff = [ - InstructorFactory(course=self.course_310b_.location), - StaffFactory(course=self.course_310b_.location) - ] + self.course_310b_staff = self.create_staff_for_course(self.course_310b) + self.course_310b2 = CourseFactory.create(org='org', number='310b_', display_name='Course') + self.course_310b2_staff = self.create_staff_for_course(self.course_310b2) self.wiki_310b = self.create_urlpath(self.wiki, course_wiki_slug(self.course_310b)) - self.wiki_310b_ = self.create_urlpath(self.wiki, course_wiki_slug(self.course_310b_)) + self.wiki_310b2 = self.create_urlpath(self.wiki, course_wiki_slug(self.course_310b2)) def test_no_one_is_root_wiki_staff(self): - all_course_staff = self.course_math101_staff + self.course_310b_staff + self.course_310b__staff + all_course_staff = self.course_math101_staff + self.course_310b_staff + self.course_310b2_staff for course_staff in all_course_staff: self.assertFalse(user_is_article_course_staff(course_staff, self.wiki.article)) @@ -70,6 +81,10 @@ class TestWikiAccess(TestWikiAccessBase): for course_staff in self.course_math101_staff: self.assertTrue(user_is_article_course_staff(course_staff, page.article)) + for page in self.wiki_math101b_pages: + for course_staff in self.course_math101b_staff: + self.assertTrue(user_is_article_course_staff(course_staff, page.article)) + def test_settings(self): for page in self.wiki_math101_pages: for course_staff in self.course_math101_staff: @@ -79,15 +94,27 @@ class TestWikiAccess(TestWikiAccessBase): self.assertTrue(settings.CAN_ASSIGN(page.article, course_staff)) self.assertTrue(settings.CAN_ASSIGN_OWNER(page.article, course_staff)) + for page in self.wiki_math101b_pages: + for course_staff in self.course_math101b_staff: + self.assertTrue(settings.CAN_DELETE(page.article, course_staff)) + self.assertTrue(settings.CAN_MODERATE(page.article, course_staff)) + self.assertTrue(settings.CAN_CHANGE_PERMISSIONS(page.article, course_staff)) + self.assertTrue(settings.CAN_ASSIGN(page.article, course_staff)) + self.assertTrue(settings.CAN_ASSIGN_OWNER(page.article, course_staff)) + def test_other_course_staff_is_not_course_wiki_staff(self): + for page in self.wiki_math101_pages: + for course_staff in self.course_math101b_staff: + self.assertFalse(user_is_article_course_staff(course_staff, page.article)) + for page in self.wiki_math101_pages: for course_staff in self.course_310b_staff: self.assertFalse(user_is_article_course_staff(course_staff, page.article)) for course_staff in self.course_310b_staff: - self.assertFalse(user_is_article_course_staff(course_staff, self.wiki_310b_.article)) + self.assertFalse(user_is_article_course_staff(course_staff, self.wiki_310b2.article)) - for course_staff in self.course_310b__staff: + for course_staff in self.course_310b2_staff: self.assertFalse(user_is_article_course_staff(course_staff, self.wiki_310b.article)) @@ -114,10 +141,7 @@ class TestWikiAccessForNumericalCourseNumber(TestWikiAccessBase): super(TestWikiAccessForNumericalCourseNumber, self).setUp() self.course_200 = CourseFactory.create(org='org', number='200', display_name='Course') - self.course_200_staff = [ - InstructorFactory(course=self.course_200.location), - StaffFactory(course=self.course_200.location) - ] + self.course_200_staff = self.create_staff_for_course(self.course_200) wiki_200 = self.create_urlpath(self.wiki, course_wiki_slug(self.course_200)) wiki_200_page = self.create_urlpath(wiki_200, 'Child') @@ -138,10 +162,7 @@ class TestWikiAccessForOldFormatCourseStaffGroups(TestWikiAccessBase): self.course_math101c = CourseFactory.create(org='org', number='math101c', display_name='Course') Group.objects.get_or_create(name='instructor_math101c') - self.course_math101c_staff = [ - InstructorFactory(course=self.course_math101c.location), - StaffFactory(course=self.course_math101c.location) - ] + self.course_math101c_staff = self.create_staff_for_course(self.course_math101c) wiki_math101c = self.create_urlpath(self.wiki, course_wiki_slug(self.course_math101c)) wiki_math101c_page = self.create_urlpath(wiki_math101c, 'Child') diff --git a/lms/djangoapps/course_wiki/utils.py b/lms/djangoapps/course_wiki/utils.py index 2fb5412d35..9f9956a7b2 100644 --- a/lms/djangoapps/course_wiki/utils.py +++ b/lms/djangoapps/course_wiki/utils.py @@ -3,7 +3,8 @@ Utility functions for course_wiki. """ from django.core.exceptions import ObjectDoesNotExist - +from xmodule import modulestore +import courseware def user_is_article_course_staff(user, article): """ @@ -20,24 +21,24 @@ def user_is_article_course_staff(user, article): so this will return True. """ - course_slug = article_course_wiki_root_slug(article) + wiki_slug = article_course_wiki_root_slug(article) - if course_slug is None: + if wiki_slug is None: return False - user_groups = user.groups.all() - # The wiki expects article slugs to contain at least one non-digit so if # the course number is just a number the course wiki root slug is set to # be '_'. This means slug '202_' can belong to either # course numbered '202_' or '202' and so we need to consider both. - if user_is_staff_on_course_number(user_groups, course_slug): + courses = modulestore.django.modulestore().get_courses_for_wiki(wiki_slug) + if any(courseware.access.has_access(user, course, 'staff', course.course_id) for course in courses): return True - if (course_slug.endswith('_') and slug_is_numerical(course_slug[:-1]) and - user_is_staff_on_course_number(user_groups, course_slug[:-1])): - return True + if (wiki_slug.endswith('_') and slug_is_numerical(wiki_slug[:-1])): + courses = modulestore.django.modulestore().get_courses_for_wiki(wiki_slug[:-1]) + if any(courseware.access.has_access(user, course, 'staff', course.course_id) for course in courses): + return True return False @@ -64,28 +65,6 @@ def course_wiki_slug(course): return slug -def user_is_staff_on_course_number(user_groups, course_number): - """Returns whether the groups contain a staff group for the course number""" - - # Course groups have format 'instructor_' and 'staff_' where - # course_id = org/course_number/run. So check if user's groups contain a group - # whose name starts with 'instructor_' or 'staff_' and contains '/course_number/'. - course_number_fragment = '/{0}/'.format(course_number) - if [group for group in user_groups if (group.name.startswith(('instructor_', 'staff_')) and - course_number_fragment in group.name)]: - return True - - # Old course groups had format 'instructor_' and 'staff_' - # Check if user's groups contain either of these. - old_instructor_group_name = 'instructor_{0}'.format(course_number) - old_staff_group_name = 'staff_{0}'.format(course_number) - if [group for group in user_groups if (group.name == old_instructor_group_name or - group.name == old_staff_group_name)]: - return True - - return False - - def article_course_wiki_root_slug(article): """ We assume the second level ancestor is the course wiki root. Examples: