Merge branch 'release'
Conflicts: cms/djangoapps/contentstore/tests/test_course_listing.py common/djangoapps/student/management/commands/create_random_users.py common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py lms/djangoapps/bulk_email/forms.py lms/djangoapps/courseware/tests/test_video_handlers.py lms/djangoapps/courseware/views.py lms/djangoapps/instructor/management/commands/openended_post.py lms/djangoapps/instructor/management/commands/openended_stats.py lms/djangoapps/instructor/tests/test_spoc_gradebook.py
This commit is contained in:
@@ -3,20 +3,22 @@ Unit tests for getting the list of courses for a user through iterating all cour
|
||||
by reversing group name formats.
|
||||
"""
|
||||
import random
|
||||
from chrono import Timer
|
||||
|
||||
from django.contrib.auth.models import Group
|
||||
from chrono import Timer
|
||||
from mock import patch, Mock
|
||||
|
||||
from django.test import RequestFactory
|
||||
|
||||
from contentstore.views.course import _accessible_courses_list, _accessible_courses_list_from_groups
|
||||
from contentstore.utils import delete_course_and_groups, reverse_course_url
|
||||
from contentstore.tests.utils import AjaxEnabledTestClient
|
||||
from student.tests.factories import UserFactory
|
||||
from student.roles import CourseInstructorRole, CourseStaffRole
|
||||
from xmodule.modulestore.exceptions import ItemNotFoundError
|
||||
from student.roles import CourseInstructorRole, CourseStaffRole, GlobalStaff
|
||||
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
|
||||
from xmodule.modulestore.tests.factories import CourseFactory
|
||||
from opaque_keys.edx.locations import SlashSeparatedCourseKey
|
||||
from xmodule.modulestore.django import modulestore
|
||||
from xmodule.error_module import ErrorDescriptor
|
||||
|
||||
TOTAL_COURSES_COUNT = 500
|
||||
USER_COURSES_COUNT = 50
|
||||
@@ -32,8 +34,11 @@ class TestCourseListing(ModuleStoreTestCase):
|
||||
"""
|
||||
super(TestCourseListing, self).setUp()
|
||||
# create and log in a staff user.
|
||||
self.user = UserFactory(is_staff=True) # pylint: disable=no-member
|
||||
# create and log in a non-staff user
|
||||
self.user = UserFactory()
|
||||
self.factory = RequestFactory()
|
||||
self.request = self.factory.get('/course')
|
||||
self.request.user = self.user
|
||||
self.client = AjaxEnabledTestClient()
|
||||
self.client.login(username=self.user.username, password='test')
|
||||
|
||||
@@ -64,38 +69,74 @@ class TestCourseListing(ModuleStoreTestCase):
|
||||
"""
|
||||
Test getting courses with new access group format e.g. 'instructor_edx.course.run'
|
||||
"""
|
||||
request = self.factory.get('/course/')
|
||||
request.user = self.user
|
||||
|
||||
course_location = SlashSeparatedCourseKey('Org1', 'Course1', 'Run1')
|
||||
self._create_course_with_access_groups(course_location, self.user)
|
||||
|
||||
# get courses through iterating all courses
|
||||
courses_list = _accessible_courses_list(request)
|
||||
courses_list = _accessible_courses_list(self.request)
|
||||
self.assertEqual(len(courses_list), 1)
|
||||
|
||||
# get courses by reversing group name formats
|
||||
courses_list_by_groups = _accessible_courses_list_from_groups(request)
|
||||
courses_list_by_groups = _accessible_courses_list_from_groups(self.request)
|
||||
self.assertEqual(len(courses_list_by_groups), 1)
|
||||
# check both course lists have same courses
|
||||
self.assertEqual(courses_list, courses_list_by_groups)
|
||||
|
||||
def test_errored_course_global_staff(self):
|
||||
"""
|
||||
Test the course list for global staff when get_course returns an ErrorDescriptor
|
||||
"""
|
||||
GlobalStaff().add_users(self.user)
|
||||
|
||||
course_key = SlashSeparatedCourseKey('Org1', 'Course1', 'Run1')
|
||||
self._create_course_with_access_groups(course_key, self.user)
|
||||
|
||||
with patch('xmodule.modulestore.mongo.base.MongoKeyValueStore', Mock(side_effect=Exception)):
|
||||
self.assertIsInstance(modulestore().get_course(course_key), ErrorDescriptor)
|
||||
|
||||
# get courses through iterating all courses
|
||||
courses_list = _accessible_courses_list(self.request)
|
||||
self.assertEqual(courses_list, [])
|
||||
|
||||
# get courses by reversing group name formats
|
||||
courses_list_by_groups = _accessible_courses_list_from_groups(self.request)
|
||||
self.assertEqual(courses_list_by_groups, [])
|
||||
|
||||
def test_errored_course_regular_access(self):
|
||||
"""
|
||||
Test the course list for regular staff when get_course returns an ErrorDescriptor
|
||||
"""
|
||||
GlobalStaff().remove_users(self.user)
|
||||
CourseStaffRole(SlashSeparatedCourseKey('Non', 'Existent', 'Course')).add_users(self.user)
|
||||
|
||||
course_key = SlashSeparatedCourseKey('Org1', 'Course1', 'Run1')
|
||||
self._create_course_with_access_groups(course_key, self.user)
|
||||
|
||||
with patch('xmodule.modulestore.mongo.base.MongoKeyValueStore', Mock(side_effect=Exception)):
|
||||
self.assertIsInstance(modulestore().get_course(course_key), ErrorDescriptor)
|
||||
|
||||
# get courses through iterating all courses
|
||||
courses_list = _accessible_courses_list(self.request)
|
||||
self.assertEqual(courses_list, [])
|
||||
|
||||
# get courses by reversing group name formats
|
||||
courses_list_by_groups = _accessible_courses_list_from_groups(self.request)
|
||||
self.assertEqual(courses_list_by_groups, [])
|
||||
self.assertEqual(courses_list, courses_list_by_groups)
|
||||
|
||||
def test_get_course_list_with_invalid_course_location(self):
|
||||
"""
|
||||
Test getting courses with invalid course location (course deleted from modulestore).
|
||||
"""
|
||||
request = self.factory.get('/course')
|
||||
request.user = self.user
|
||||
|
||||
course_key = SlashSeparatedCourseKey('Org', 'Course', 'Run')
|
||||
self._create_course_with_access_groups(course_key, self.user)
|
||||
|
||||
# get courses through iterating all courses
|
||||
courses_list = _accessible_courses_list(request)
|
||||
courses_list = _accessible_courses_list(self.request)
|
||||
self.assertEqual(len(courses_list), 1)
|
||||
|
||||
# get courses by reversing group name formats
|
||||
courses_list_by_groups = _accessible_courses_list_from_groups(request)
|
||||
courses_list_by_groups = _accessible_courses_list_from_groups(self.request)
|
||||
self.assertEqual(len(courses_list_by_groups), 1)
|
||||
# check both course lists have same courses
|
||||
self.assertEqual(courses_list, courses_list_by_groups)
|
||||
@@ -106,25 +147,15 @@ class TestCourseListing(ModuleStoreTestCase):
|
||||
CourseInstructorRole(course_key).add_users(self.user)
|
||||
|
||||
# test that get courses through iterating all courses now returns no course
|
||||
courses_list = _accessible_courses_list(request)
|
||||
courses_list = _accessible_courses_list(self.request)
|
||||
self.assertEqual(len(courses_list), 0)
|
||||
|
||||
# now test that get courses by reversing group name formats gives 'ItemNotFoundError'
|
||||
with self.assertRaises(ItemNotFoundError):
|
||||
_accessible_courses_list_from_groups(request)
|
||||
|
||||
def test_course_listing_performance(self):
|
||||
"""
|
||||
Create large number of courses and give access of some of these courses to the user and
|
||||
compare the time to fetch accessible courses for the user through traversing all courses and
|
||||
reversing django groups
|
||||
"""
|
||||
# create and log in a non-staff user
|
||||
self.user = UserFactory()
|
||||
request = self.factory.get('/course')
|
||||
request.user = self.user
|
||||
self.client.login(username=self.user.username, password='test')
|
||||
|
||||
# create list of random course numbers which will be accessible to the user
|
||||
user_course_ids = random.sample(range(TOTAL_COURSES_COUNT), USER_COURSES_COUNT)
|
||||
|
||||
@@ -141,22 +172,22 @@ class TestCourseListing(ModuleStoreTestCase):
|
||||
|
||||
# time the get courses by iterating through all courses
|
||||
with Timer() as iteration_over_courses_time_1:
|
||||
courses_list = _accessible_courses_list(request)
|
||||
courses_list = _accessible_courses_list(self.request)
|
||||
self.assertEqual(len(courses_list), USER_COURSES_COUNT)
|
||||
|
||||
# time again the get courses by iterating through all courses
|
||||
with Timer() as iteration_over_courses_time_2:
|
||||
courses_list = _accessible_courses_list(request)
|
||||
courses_list = _accessible_courses_list(self.request)
|
||||
self.assertEqual(len(courses_list), USER_COURSES_COUNT)
|
||||
|
||||
# time the get courses by reversing django groups
|
||||
with Timer() as iteration_over_groups_time_1:
|
||||
courses_list = _accessible_courses_list_from_groups(request)
|
||||
courses_list = _accessible_courses_list_from_groups(self.request)
|
||||
self.assertEqual(len(courses_list), USER_COURSES_COUNT)
|
||||
|
||||
# time again the get courses by reversing django groups
|
||||
with Timer() as iteration_over_groups_time_2:
|
||||
courses_list = _accessible_courses_list_from_groups(request)
|
||||
courses_list = _accessible_courses_list_from_groups(self.request)
|
||||
self.assertEqual(len(courses_list), USER_COURSES_COUNT)
|
||||
|
||||
# test that the time taken by getting courses through reversing django groups is lower then the time
|
||||
@@ -169,21 +200,15 @@ class TestCourseListing(ModuleStoreTestCase):
|
||||
Test getting courses with same id but with different name case. Then try to delete one of them and
|
||||
check that it is properly deleted and other one is accessible
|
||||
"""
|
||||
# create and log in a non-staff user
|
||||
self.user = UserFactory()
|
||||
request = self.factory.get('/course')
|
||||
request.user = self.user
|
||||
self.client.login(username=self.user.username, password='test')
|
||||
|
||||
course_location_caps = SlashSeparatedCourseKey('Org', 'COURSE', 'Run')
|
||||
self._create_course_with_access_groups(course_location_caps, self.user)
|
||||
|
||||
# get courses through iterating all courses
|
||||
courses_list = _accessible_courses_list(request)
|
||||
courses_list = _accessible_courses_list(self.request)
|
||||
self.assertEqual(len(courses_list), 1)
|
||||
|
||||
# get courses by reversing group name formats
|
||||
courses_list_by_groups = _accessible_courses_list_from_groups(request)
|
||||
courses_list_by_groups = _accessible_courses_list_from_groups(self.request)
|
||||
self.assertEqual(len(courses_list_by_groups), 1)
|
||||
# check both course lists have same courses
|
||||
self.assertEqual(courses_list, courses_list_by_groups)
|
||||
@@ -193,22 +218,22 @@ class TestCourseListing(ModuleStoreTestCase):
|
||||
self._create_course_with_access_groups(course_location_camel, self.user)
|
||||
|
||||
# test that get courses through iterating all courses returns both courses
|
||||
courses_list = _accessible_courses_list(request)
|
||||
courses_list = _accessible_courses_list(self.request)
|
||||
self.assertEqual(len(courses_list), 2)
|
||||
|
||||
# test that get courses by reversing group name formats returns both courses
|
||||
courses_list_by_groups = _accessible_courses_list_from_groups(request)
|
||||
courses_list_by_groups = _accessible_courses_list_from_groups(self.request)
|
||||
self.assertEqual(len(courses_list_by_groups), 2)
|
||||
|
||||
# now delete first course (course_location_caps) and check that it is no longer accessible
|
||||
delete_course_and_groups(course_location_caps, commit=True)
|
||||
|
||||
# test that get courses through iterating all courses now returns one course
|
||||
courses_list = _accessible_courses_list(request)
|
||||
courses_list = _accessible_courses_list(self.request)
|
||||
self.assertEqual(len(courses_list), 1)
|
||||
|
||||
# test that get courses by reversing group name formats also returns one course
|
||||
courses_list_by_groups = _accessible_courses_list_from_groups(request)
|
||||
courses_list_by_groups = _accessible_courses_list_from_groups(self.request)
|
||||
self.assertEqual(len(courses_list_by_groups), 1)
|
||||
|
||||
# now check that deleted course is not accessible
|
||||
@@ -220,3 +245,29 @@ class TestCourseListing(ModuleStoreTestCase):
|
||||
outline_url = reverse_course_url('course_handler', course_location_camel)
|
||||
response = self.client.get(outline_url, HTTP_ACCEPT='application/json')
|
||||
self.assertEqual(response.status_code, 200)
|
||||
|
||||
def test_course_listing_errored_deleted_courses(self):
|
||||
"""
|
||||
Create good courses, courses that won't load, and deleted courses which still have
|
||||
roles. Test course listing.
|
||||
"""
|
||||
course_location = SlashSeparatedCourseKey('testOrg', 'testCourse', 'RunBabyRun')
|
||||
self._create_course_with_access_groups(course_location, self.user)
|
||||
|
||||
course_location = SlashSeparatedCourseKey('testOrg', 'doomedCourse', 'RunBabyRun')
|
||||
self._create_course_with_access_groups(course_location, self.user)
|
||||
modulestore().delete_course(course_location)
|
||||
|
||||
course_location = SlashSeparatedCourseKey('testOrg', 'erroredCourse', 'RunBabyRun')
|
||||
course = self._create_course_with_access_groups(course_location, self.user)
|
||||
course_db_record = modulestore()._find_one(course.location)
|
||||
course_db_record.setdefault('metadata', {}).get('tabs', []).append({"type": "wiko", "name": "Wiki" })
|
||||
modulestore().collection.update(
|
||||
{'_id': course_db_record['_id']},
|
||||
{'$set': {
|
||||
'metadata.tabs': course_db_record['metadata']['tabs'],
|
||||
}},
|
||||
)
|
||||
|
||||
courses_list = _accessible_courses_list_from_groups(self.request)
|
||||
self.assertEqual(len(courses_list), 1, courses_list)
|
||||
|
||||
@@ -163,6 +163,9 @@ def _accessible_courses_list(request):
|
||||
"""
|
||||
Get courses to which this user has access
|
||||
"""
|
||||
if isinstance(course, ErrorDescriptor):
|
||||
return False
|
||||
|
||||
if GlobalStaff().has_user(request.user):
|
||||
return course.location.course != 'templates'
|
||||
|
||||
@@ -189,9 +192,9 @@ def _accessible_courses_list_from_groups(request):
|
||||
course_key = course_access.course_id
|
||||
if course_key not in courses_list:
|
||||
course = modulestore('direct').get_course(course_key)
|
||||
if course is None:
|
||||
raise ItemNotFoundError(course_key)
|
||||
courses_list[course_key] = course
|
||||
if course is not None and not isinstance(course, ErrorDescriptor):
|
||||
# ignore deleted or errored courses
|
||||
courses_list[course_key] = course
|
||||
|
||||
return courses_list.values()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user