From e6b3f8f8950220af5bf008595920c21b3d4cd90a Mon Sep 17 00:00:00 2001 From: Awais Jibran Date: Fri, 6 May 2016 12:56:36 +0500 Subject: [PATCH 1/2] Studio Edge cannot load the Course Dashboard for CCX courses. TNL-4484 --- cms/djangoapps/contentstore/views/course.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 791db5991e..c674dfc6f2 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -412,12 +412,16 @@ def _accessible_courses_list_from_groups(request): """ List all courses available to the logged in user by reversing access group names """ + def filter_ccx(course_access): + """ CCXs cannot be edited in Studio and should not be shown in this dashboard """ + return not isinstance(course_access.course_id, CCXLocator) + courses_list = {} in_process_course_actions = [] instructor_courses = UserBasedRole(request.user, CourseInstructorRole.ROLE).courses_with_role() staff_courses = UserBasedRole(request.user, CourseStaffRole.ROLE).courses_with_role() - all_courses = instructor_courses | staff_courses + all_courses = filter(filter_ccx, instructor_courses | staff_courses) for course_access in all_courses: course_key = course_access.course_id @@ -440,9 +444,7 @@ def _accessible_courses_list_from_groups(request): # If a user has access to a course that doesn't exist, don't do anything with that course pass - # Custom Courses for edX (CCX) is an edX feature for re-using course content. - # CCXs cannot be edited in Studio (aka cms) and should not be shown in this dashboard. - if course is not None and not isinstance(course, ErrorDescriptor) and not isinstance(course.id, CCXLocator): + if course is not None and not isinstance(course, ErrorDescriptor): # ignore deleted, errored or ccx courses courses_list[course_key] = course From 38f1c200cae46a704fa063b72c8208d2d5ed14e5 Mon Sep 17 00:00:00 2001 From: Awais Jibran Date: Mon, 9 May 2016 14:26:15 +0500 Subject: [PATCH 2/2] Fixing test --- .../contentstore/tests/test_course_listing.py | 76 +++++++++++-------- 1 file changed, 46 insertions(+), 30 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_course_listing.py b/cms/djangoapps/contentstore/tests/test_course_listing.py index e2da5f83e3..0f3fa3c8ed 100644 --- a/cms/djangoapps/contentstore/tests/test_course_listing.py +++ b/cms/djangoapps/contentstore/tests/test_course_listing.py @@ -16,18 +16,28 @@ from django.test.client import Client from common.test.utils import XssTestMixin from xmodule.course_module import CourseSummary -from contentstore.views.course import (_accessible_courses_list, _accessible_courses_list_from_groups, - AccessListFallback, get_courses_accessible_to_user, - _accessible_courses_summary_list) +from contentstore.views.course import ( + _accessible_courses_list, + _accessible_courses_list_from_groups, + AccessListFallback, + get_courses_accessible_to_user, + _accessible_courses_summary_list, +) from contentstore.utils import delete_course_and_groups from contentstore.tests.utils import AjaxEnabledTestClient from student.tests.factories import UserFactory -from student.roles import CourseInstructorRole, CourseStaffRole, GlobalStaff, OrgStaffRole, OrgInstructorRole +from student.roles import ( + CourseInstructorRole, + CourseStaffRole, + GlobalStaff, + OrgStaffRole, + OrgInstructorRole, + UserBasedRole, +) from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, check_mongo_calls from xmodule.modulestore import ModuleStoreEnum from opaque_keys.edx.locations import CourseLocator -from opaque_keys.edx.keys import CourseKey from xmodule.error_module import ErrorDescriptor from course_action_state.models import CourseRerunState @@ -65,12 +75,14 @@ class TestCourseListing(ModuleStoreTestCase, XssTestMixin): run=course_location.run, default_store=store ) + self._add_role_access_to_user(user, course.id) + return course + def _add_role_access_to_user(self, user, course_id): + """ Assign access roles to user in the course. """ if user is not None: for role in [CourseInstructorRole, CourseStaffRole]: - role(course.id).add_users(user) - - return course + role(course_id).add_users(user) def tearDown(self): """ @@ -133,36 +145,40 @@ class TestCourseListing(ModuleStoreTestCase, XssTestMixin): # check both course lists have same courses self.assertEqual(courses_list, courses_list_by_groups) - def test_get_course_list_when_ccx(self): + def test_courses_list_with_ccx_courses(self): """ - Assert that courses with CCXLocator are filter in course listing. + Tests that CCX courses are filtered in course listing. """ + # Create a course and assign access roles to user. course_location = self.store.make_course_key('Org1', 'Course1', 'Run1') - self._create_course_with_access_groups(course_location, self.user) + course = self._create_course_with_access_groups(course_location, self.user) - # get courses through iterating all courses - courses_list, __ = _accessible_courses_list(self.request) + # Create a ccx course key and add assign access roles to user. + ccx_course_key = CCXLocator.from_course_locator(course.id, '1') + self._add_role_access_to_user(self.user, ccx_course_key) + + # Test that CCX courses are filtered out. + courses_list, __ = _accessible_courses_list_from_groups(self.request) self.assertEqual(len(courses_list), 1) + self.assertNotIn( + ccx_course_key, + [course.id for course in courses_list] + ) - # get courses by reversing group name formats - courses_list_by_groups, __ = _accessible_courses_list_from_groups(self.request) - self.assertEqual(len(courses_list_by_groups), 1) + # Get all courses which user has access. + instructor_courses = UserBasedRole(self.user, CourseInstructorRole.ROLE).courses_with_role() + staff_courses = UserBasedRole(self.user, CourseStaffRole.ROLE).courses_with_role() + all_courses = (instructor_courses | staff_courses) - # assert no course in listing with ccx id - ccx_course = Mock() - course_key = CourseKey.from_string('course-v1:FakeOrg+CN1+CR-FALLNEVER1') - ccx_course.id = CCXLocator.from_course_locator(course_key, u"1") - - with patch( - 'xmodule.modulestore.mixed.MixedModuleStore.get_course', - return_value=ccx_course - ), patch( - 'xmodule.modulestore.mixed.MixedModuleStore.get_courses', - Mock(return_value=[ccx_course]) - ): - courses_list, __ = _accessible_courses_list_from_groups(self.request) - self.assertEqual(len(courses_list), 0) + # Verify that CCX course exists in access but filtered by `_accessible_courses_list_from_groups`. + self.assertIn( + ccx_course_key, + [access.course_id for access in all_courses] + ) + # Verify that CCX courses are filtered out while iterating over all courses + mocked_ccx_course = Mock(id=ccx_course_key) + with patch('xmodule.modulestore.mixed.MixedModuleStore.get_courses', return_value=[mocked_ccx_course]): courses_list, __ = _accessible_courses_list(self.request) self.assertEqual(len(courses_list), 0)