test: favor CourseOverview when getting course listing for groups
This commit is contained in:
committed by
Julia Eskew
parent
3322d18446
commit
c6cd064194
@@ -32,6 +32,9 @@ from common.djangoapps.student.roles import (
|
||||
UserBasedRole
|
||||
)
|
||||
from common.djangoapps.student.tests.factories import UserFactory
|
||||
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
|
||||
from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
|
||||
from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES
|
||||
from xmodule.course_module import CourseSummary # lint-amnesty, pylint: disable=wrong-import-order
|
||||
from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order
|
||||
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order
|
||||
@@ -60,17 +63,17 @@ class TestCourseListing(ModuleStoreTestCase):
|
||||
self.client = AjaxEnabledTestClient()
|
||||
self.client.login(username=self.user.username, password='test')
|
||||
|
||||
def _create_course_with_access_groups(self, course_location, user=None, store=ModuleStoreEnum.Type.split):
|
||||
def _create_course_with_access_groups(self, course_location, user=None):
|
||||
"""
|
||||
Create dummy course with 'CourseFactory' and role (instructor/staff) groups
|
||||
"""
|
||||
course = CourseFactory.create(
|
||||
CourseFactory.create(
|
||||
org=course_location.org,
|
||||
number=course_location.course,
|
||||
run=course_location.run,
|
||||
default_store=store
|
||||
run=course_location.run
|
||||
)
|
||||
self._add_role_access_to_user(user, course.id)
|
||||
course = CourseOverviewFactory.create(id=course_location, org=course_location.org)
|
||||
self._add_role_access_to_user(user, course_location)
|
||||
return course
|
||||
|
||||
def _add_role_access_to_user(self, user, course_id):
|
||||
@@ -124,7 +127,7 @@ class TestCourseListing(ModuleStoreTestCase):
|
||||
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')
|
||||
course_location = CourseLocator('Org1', 'Course1', 'Course1')
|
||||
course = self._create_course_with_access_groups(course_location, self.user)
|
||||
|
||||
# Create a ccx course key and add assign access roles to user.
|
||||
@@ -152,7 +155,10 @@ class TestCourseListing(ModuleStoreTestCase):
|
||||
|
||||
# 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_course_summaries', return_value=[mocked_ccx_course]):
|
||||
with patch(
|
||||
'openedx.core.djangoapps.content.course_overviews.models.CourseOverview.get_all_courses',
|
||||
return_value=[mocked_ccx_course],
|
||||
):
|
||||
courses_iter, __ = _accessible_courses_iter_for_tests(self.request)
|
||||
self.assertEqual(len(list(courses_iter)), 0)
|
||||
|
||||
@@ -175,7 +181,7 @@ class TestCourseListing(ModuleStoreTestCase):
|
||||
# Create few courses
|
||||
for num in range(TOTAL_COURSES_COUNT):
|
||||
course_location = self.store.make_course_key('Org', 'CreatedCourse' + str(num), 'Run')
|
||||
self._create_course_with_access_groups(course_location, self.user, default_store)
|
||||
self._create_course_with_access_groups(course_location, self.user)
|
||||
|
||||
# Fetch accessible courses list & verify their count
|
||||
courses_list_by_staff, __ = get_courses_accessible_to_user(self.request)
|
||||
@@ -195,7 +201,7 @@ class TestCourseListing(ModuleStoreTestCase):
|
||||
"""
|
||||
with self.store.default_store(store):
|
||||
course_key = self.store.make_course_key('Org', 'Course', 'Run')
|
||||
self._create_course_with_access_groups(course_key, self.user, store)
|
||||
course = self._create_course_with_access_groups(course_key, self.user)
|
||||
|
||||
# get courses through iterating all courses
|
||||
courses_iter, __ = _accessible_courses_iter_for_tests(self.request)
|
||||
@@ -220,6 +226,7 @@ class TestCourseListing(ModuleStoreTestCase):
|
||||
self.assertEqual(course_keys_in_course_list, course_keys_in_courses_list_by_groups)
|
||||
# now delete this course and re-add user to instructor group of this course
|
||||
delete_course(course_key, self.user.id)
|
||||
course.delete()
|
||||
|
||||
CourseInstructorRole(course_key).add_users(self.user)
|
||||
|
||||
@@ -239,8 +246,8 @@ class TestCourseListing(ModuleStoreTestCase):
|
||||
)
|
||||
|
||||
@ddt.data(
|
||||
(ModuleStoreEnum.Type.split, 3, 3),
|
||||
(ModuleStoreEnum.Type.mongo, 2, 2)
|
||||
(ModuleStoreEnum.Type.split, 1, 2),
|
||||
(ModuleStoreEnum.Type.mongo, 1, 2),
|
||||
)
|
||||
@ddt.unpack
|
||||
def test_course_listing_performance(self, store, courses_list_from_group_calls, courses_list_calls):
|
||||
@@ -260,9 +267,9 @@ class TestCourseListing(ModuleStoreTestCase):
|
||||
run = f'Run{number}'
|
||||
course_location = self.store.make_course_key(org, course, run)
|
||||
if number in user_course_ids:
|
||||
self._create_course_with_access_groups(course_location, self.user, store=store)
|
||||
self._create_course_with_access_groups(course_location, self.user)
|
||||
else:
|
||||
self._create_course_with_access_groups(course_location, store=store)
|
||||
self._create_course_with_access_groups(course_location)
|
||||
|
||||
# get courses by iterating through all courses
|
||||
courses_iter, __ = _accessible_courses_iter_for_tests(self.request)
|
||||
@@ -280,30 +287,23 @@ class TestCourseListing(ModuleStoreTestCase):
|
||||
courses_list, __ = _accessible_courses_list_from_groups(self.request)
|
||||
self.assertEqual(len(courses_list), USER_COURSES_COUNT)
|
||||
|
||||
# Now count the db queries
|
||||
with check_mongo_calls(courses_list_from_group_calls):
|
||||
with self.assertNumQueries(courses_list_from_group_calls, table_blacklist=WAFFLE_TABLES):
|
||||
_accessible_courses_list_from_groups(self.request)
|
||||
|
||||
with check_mongo_calls(courses_list_calls):
|
||||
list(_accessible_courses_iter_for_tests(self.request))
|
||||
# Calls:
|
||||
# 1) query old mongo
|
||||
# 2) get_more on old mongo
|
||||
# 3) query split (handled with MySQL only)
|
||||
with self.assertNumQueries(courses_list_calls, table_blacklist=WAFFLE_TABLES):
|
||||
_accessible_courses_iter_for_tests(self.request)
|
||||
|
||||
@ddt.data(ModuleStoreEnum.Type.split, ModuleStoreEnum.Type.mongo)
|
||||
def test_course_listing_errored_deleted_courses(self, store):
|
||||
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.
|
||||
"""
|
||||
with self.store.default_store(store):
|
||||
course_location = self.store.make_course_key('testOrg', 'testCourse', 'RunBabyRun')
|
||||
self._create_course_with_access_groups(course_location, self.user, store)
|
||||
course_location = CourseLocator('testOrg', 'testCourse', 'RunBabyRun')
|
||||
self._create_course_with_access_groups(course_location, self.user)
|
||||
|
||||
course_location = self.store.make_course_key('testOrg', 'doomedCourse', 'RunBabyRun')
|
||||
self._create_course_with_access_groups(course_location, self.user, store)
|
||||
self.store.delete_course(course_location, self.user.id)
|
||||
course_location = CourseLocator('doomedCourse', 'testCourse', 'RunBabyRun')
|
||||
course = self._create_course_with_access_groups(course_location, self.user)
|
||||
course.delete()
|
||||
|
||||
courses_list, __ = _accessible_courses_list_from_groups(self.request)
|
||||
self.assertEqual(len(courses_list), 1, courses_list)
|
||||
@@ -338,7 +338,7 @@ class TestCourseListing(ModuleStoreTestCase):
|
||||
# Verify fetched accessible courses list is a list of CourseSummery instances and test expacted
|
||||
# course count is returned
|
||||
self.assertEqual(len(list(courses_list)), 2)
|
||||
self.assertTrue(all(isinstance(course, CourseSummary) for course in courses_list))
|
||||
self.assertTrue(all(isinstance(course, CourseOverview) for course in courses_list))
|
||||
|
||||
def test_course_listing_with_actions_in_progress(self):
|
||||
sourse_course_key = CourseLocator('source-Org', 'source-Course', 'source-Run')
|
||||
|
||||
@@ -461,7 +461,7 @@ def _accessible_courses_iter_for_tests(request):
|
||||
|
||||
return has_studio_read_access(request.user, course.id)
|
||||
|
||||
courses = filter(course_filter, modulestore().get_course_summaries())
|
||||
courses = filter(course_filter, CourseOverview.get_all_courses())
|
||||
|
||||
in_process_course_actions = get_in_process_course_actions(request)
|
||||
return courses, in_process_course_actions
|
||||
|
||||
@@ -31,6 +31,7 @@ from common.djangoapps.course_action_state.models import CourseRerunState
|
||||
from common.djangoapps.student.auth import has_course_author_access
|
||||
from common.djangoapps.student.roles import CourseStaffRole, GlobalStaff, LibraryUserRole
|
||||
from common.djangoapps.student.tests.factories import UserFactory
|
||||
from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
|
||||
from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES
|
||||
from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order
|
||||
from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order
|
||||
@@ -341,6 +342,7 @@ class TestCourseIndexArchived(CourseTestCase):
|
||||
self.course.display_name = 'Active Course 1'
|
||||
self.ORG = self.course.location.org
|
||||
self.save_course()
|
||||
CourseOverviewFactory.create(id=self.course.id, org=self.ORG)
|
||||
|
||||
# Active course has end date set to tomorrow
|
||||
self.active_course = CourseFactory.create(
|
||||
@@ -348,6 +350,11 @@ class TestCourseIndexArchived(CourseTestCase):
|
||||
org=self.ORG,
|
||||
end=self.TOMORROW,
|
||||
)
|
||||
CourseOverviewFactory.create(
|
||||
id=self.active_course.id,
|
||||
org=self.ORG,
|
||||
end=self.TOMORROW,
|
||||
)
|
||||
|
||||
# Archived course has end date set to yesterday
|
||||
self.archived_course = CourseFactory.create(
|
||||
@@ -355,6 +362,11 @@ class TestCourseIndexArchived(CourseTestCase):
|
||||
org=self.ORG,
|
||||
end=self.YESTERDAY,
|
||||
)
|
||||
CourseOverviewFactory.create(
|
||||
id=self.archived_course.id,
|
||||
org=self.ORG,
|
||||
end=self.YESTERDAY,
|
||||
)
|
||||
|
||||
# Base user has global staff access
|
||||
self.assertTrue(GlobalStaff().has_user(self.user))
|
||||
@@ -391,8 +403,8 @@ class TestCourseIndexArchived(CourseTestCase):
|
||||
|
||||
@ddt.data(
|
||||
# Staff user has course staff access
|
||||
(True, 'staff', None, 3, 18),
|
||||
(False, 'staff', None, 3, 18),
|
||||
(True, 'staff', None, 1, 19),
|
||||
(False, 'staff', None, 1, 19),
|
||||
# Base user has global staff access
|
||||
(True, 'user', ORG, 3, 18),
|
||||
(False, 'user', ORG, 3, 18),
|
||||
|
||||
Reference in New Issue
Block a user