diff --git a/cms/djangoapps/contentstore/tests/test_course_listing.py b/cms/djangoapps/contentstore/tests/test_course_listing.py index d2fc51411482..990eff83c922 100644 --- a/cms/djangoapps/contentstore/tests/test_course_listing.py +++ b/cms/djangoapps/contentstore/tests/test_course_listing.py @@ -35,7 +35,6 @@ 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 openedx.core.djangolib.testing.utils import AUTHZ_TABLES 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 from xmodule.modulestore.tests.factories import CourseFactory # lint-amnesty, pylint: disable=wrong-import-order @@ -43,8 +42,6 @@ TOTAL_COURSES_COUNT = 10 USER_COURSES_COUNT = 1 -QUERY_COUNT_TABLE_IGNORELIST = WAFFLE_TABLES + AUTHZ_TABLES - @ddt.ddt class TestCourseListing(ModuleStoreTestCase): @@ -306,10 +303,10 @@ def test_course_listing_performance(self): courses_list, __ = _accessible_courses_list_from_groups(self.request) self.assertEqual(len(courses_list), USER_COURSES_COUNT) - with self.assertNumQueries(2, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): + with self.assertNumQueries(1, table_ignorelist=WAFFLE_TABLES): _accessible_courses_list_from_groups(self.request) - with self.assertNumQueries(2, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): + with self.assertNumQueries(2, table_ignorelist=WAFFLE_TABLES): _accessible_courses_iter_for_tests(self.request) def test_course_listing_errored_deleted_courses(self): diff --git a/cms/djangoapps/contentstore/views/library.py b/cms/djangoapps/contentstore/views/library.py index bcc0ffe84842..92e4329c2f94 100644 --- a/cms/djangoapps/contentstore/views/library.py +++ b/cms/djangoapps/contentstore/views/library.py @@ -73,23 +73,24 @@ def _user_can_create_library_for_org(user, org=None): elif user.is_staff: return True elif settings.FEATURES.get('ENABLE_CREATOR_GROUP', False): + org_filter_params = {} + if org: + org_filter_params['org'] = org is_course_creator = get_course_creator_status(user) == 'granted' - if is_course_creator: - return True - - has_org_staff_role = OrgStaffRole().has_org_for_user(user, org) - if has_org_staff_role: - return True - - has_course_staff_role = UserBasedRole(user=user, role=CourseStaffRole.ROLE).has_courses_with_role(org) - if has_course_staff_role: - return True - - has_course_admin_role = UserBasedRole(user=user, role=CourseInstructorRole.ROLE).has_courses_with_role(org) - if has_course_admin_role: - return True - - return False + has_org_staff_role = OrgStaffRole().get_orgs_for_user(user).filter(**org_filter_params).exists() + has_course_staff_role = ( + UserBasedRole(user=user, role=CourseStaffRole.ROLE) + .courses_with_role() + .filter(**org_filter_params) + .exists() + ) + has_course_admin_role = ( + UserBasedRole(user=user, role=CourseInstructorRole.ROLE) + .courses_with_role() + .filter(**org_filter_params) + .exists() + ) + return is_course_creator or has_org_staff_role or has_course_staff_role or has_course_admin_role else: # EDUCATOR-1924: DISABLE_LIBRARY_CREATION overrides DISABLE_COURSE_CREATION, if present. disable_library_creation = settings.FEATURES.get('DISABLE_LIBRARY_CREATION', None) diff --git a/common/djangoapps/student/role_helpers.py b/common/djangoapps/student/role_helpers.py index ee58efab9ee4..64ed5cc17efb 100644 --- a/common/djangoapps/student/role_helpers.py +++ b/common/djangoapps/student/role_helpers.py @@ -14,7 +14,7 @@ ) from openedx.core.lib.cache_utils import request_cached from common.djangoapps.student.roles import ( - AuthzCompatCourseAccessRole, + CourseAccessRole, CourseBetaTesterRole, CourseInstructorRole, CourseStaffRole, @@ -66,7 +66,7 @@ def get_role_cache(user: User) -> RoleCache: @request_cached() -def get_course_roles(user: User) -> list[AuthzCompatCourseAccessRole]: +def get_course_roles(user: User) -> list[CourseAccessRole]: """ Returns a list of all course-level roles that this user has. diff --git a/common/djangoapps/student/roles.py b/common/djangoapps/student/roles.py index 40e4ebe970eb..971433c9c523 100644 --- a/common/djangoapps/student/roles.py +++ b/common/djangoapps/student/roles.py @@ -4,23 +4,16 @@ """ +from collections import defaultdict import logging from abc import ABCMeta, abstractmethod -from collections import defaultdict from contextlib import contextmanager -from dataclasses import dataclass from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user -from common.djangoapps.student.signals.signals import emit_course_access_role_added, emit_course_access_role_removed from opaque_keys.edx.django.models import CourseKeyField -from opaque_keys.edx.keys import CourseKey -from opaque_keys.edx.locator import CourseLocator -from openedx_authz.api import users as authz_api -from openedx_authz.constants import roles as authz_roles -from common.djangoapps.student.models import CourseAccessRole from openedx.core.lib.cache_utils import get_cache -from openedx.core.toggles import enable_authz_course_authoring +from common.djangoapps.student.models import CourseAccessRole log = logging.getLogger(__name__) @@ -34,46 +27,6 @@ ROLE_CACHE_UNGROUPED_ROLES__KEY = 'ungrouped' -def get_authz_role_from_legacy_role(legacy_role: str) -> str: - return authz_roles.LEGACY_COURSE_ROLE_EQUIVALENCES.get(legacy_role, None) - - -def get_legacy_role_from_authz_role(authz_role: str) -> str: - return next((k for k, v in authz_roles.LEGACY_COURSE_ROLE_EQUIVALENCES.items() if v == authz_role), None) - - -def authz_add_role(user: User, authz_role: str, course_key: str): - """ - Add a user's role in a course if not already added. - Args: - user (User): The user whose role is being changed. - authz_role (str): The new authorization role to assign (authz role, not legacy). - course_key (str): The course key where the role change is taking effect. - """ - course_locator = CourseLocator.from_string(course_key) - - # Check if the user is not already assigned this role for this course - existing_assignments = authz_api.get_user_role_assignments_in_scope( - user_external_key=user.username, - scope_external_key=course_key - ) - existing_roles = [existing_role.external_key - for existing_assignment in existing_assignments - for existing_role in existing_assignment.roles] - - if authz_role in existing_roles: - return - - # Assign new role - authz_api.assign_role_to_user_in_scope( - user_external_key=user.username, - role_external_key=authz_role, - scope_external_key=course_key - ) - legacy_role = get_legacy_role_from_authz_role(authz_role) - emit_course_access_role_added(user, course_locator, course_locator.org, legacy_role) - - def register_access_role(cls): """ Decorator that allows access roles to be registered within the roles module and referenced by their @@ -117,43 +70,6 @@ def get_role_cache_key_for_course(course_key=None): return str(course_key) if course_key else ROLE_CACHE_UNGROUPED_ROLES__KEY -@dataclass(frozen=True) -class AuthzCompatCourseAccessRole: - """ - Generic data class for storing CourseAccessRole-compatible data - to be used inside BulkRoleCache and RoleCache. - This allows the cache to store both legacy and openedx-authz compatible roles - """ - user_id: int - username: str - org: str - course_id: str # Course key - role: str - - -def get_authz_compat_course_access_roles_for_user(user: User) -> set[AuthzCompatCourseAccessRole]: - """ - Retrieve all CourseAccessRole objects for a given user and convert them to AuthzCompatCourseAccessRole objects. - """ - compat_role_assignments = set() - assignments = authz_api.get_user_role_assignments(user_external_key=user.username) - for assignment in assignments: - for role in assignment.roles: - legacy_role = get_legacy_role_from_authz_role(authz_role=role.external_key) - course_key = assignment.scope.external_key - parsed_key = CourseKey.from_string(course_key) - org = parsed_key.org - compat_role = AuthzCompatCourseAccessRole( - user_id=user.id, - username=user.username, - org=org, - course_id=course_key, - role=legacy_role - ) - compat_role_assignments.add(compat_role) - return compat_role_assignments - - class BulkRoleCache: # lint-amnesty, pylint: disable=missing-class-docstring """ This class provides a caching mechanism for roles grouped by users and courses, @@ -182,29 +98,13 @@ def prefetch(cls, users): # lint-amnesty, pylint: disable=missing-function-docs roles_by_user = defaultdict(lambda: defaultdict(set)) get_cache(cls.CACHE_NAMESPACE)[cls.CACHE_KEY] = roles_by_user - # Legacy roles for role in CourseAccessRole.objects.filter(user__in=users).select_related('user'): user_id = role.user.id course_id = get_role_cache_key_for_course(role.course_id) # Add role to the set in roles_by_user[user_id][course_id] user_roles_set_for_course = roles_by_user[user_id][course_id] - compat_role = AuthzCompatCourseAccessRole( - user_id=role.user.id, - username=role.user.username, - org=role.org, - course_id=role.course_id, - role=role.role - ) - user_roles_set_for_course.add(compat_role) - - # openedx-authz roles - for user in users: - compat_roles = get_authz_compat_course_access_roles_for_user(user) - for role in compat_roles: - course_id = get_role_cache_key_for_course(role.course_id) - user_roles_set_for_course = roles_by_user[user.id][course_id] - user_roles_set_for_course.add(compat_role) + user_roles_set_for_course.add(role) users_without_roles = [u for u in users if u.id not in roles_by_user] for user in users_without_roles: @@ -217,7 +117,7 @@ def get_user_roles(cls, user): class RoleCache: """ - A cache of the AuthzCompatCourseAccessRoles held by a particular user. + A cache of the CourseAccessRoles held by a particular user. Internal data structures should be accessed by getter and setter methods; don't use `_roles_by_course_id` or `_roles` directly. _roles_by_course_id: This is the data structure as saved in the RequestCache. @@ -234,35 +134,18 @@ def __init__(self, user): self._roles_by_course_id = BulkRoleCache.get_user_roles(user) except KeyError: self._roles_by_course_id = {} - - # openedx-authz compatibility implementation - compat_roles = get_authz_compat_course_access_roles_for_user(user) - for compat_role in compat_roles: - course_id = get_role_cache_key_for_course(compat_role.course_id) - if not self._roles_by_course_id.get(course_id): - self._roles_by_course_id[course_id] = set() - self._roles_by_course_id[course_id].add(compat_role) - - # legacy implementation roles = CourseAccessRole.objects.filter(user=user).all() for role in roles: course_id = get_role_cache_key_for_course(role.course_id) if not self._roles_by_course_id.get(course_id): self._roles_by_course_id[course_id] = set() - compat_role = AuthzCompatCourseAccessRole( - user_id=user.id, - username=user.username, - org=role.org, - course_id=role.course_id, - role=role.role - ) - self._roles_by_course_id[course_id].add(compat_role) + self._roles_by_course_id[course_id].add(role) self._roles = set() for roles_for_course in self._roles_by_course_id.values(): self._roles.update(roles_for_course) @staticmethod - def get_roles(role: str) -> set[str]: + def get_roles(role): """ Return the roles that should have the same permissions as the specified role. """ @@ -386,33 +269,13 @@ def has_user(self, user, check_user_activation=True): return user._roles.has_role(self._role_name, self.course_key, self.org) - def _authz_add_users(self, users): - """ - Add the supplied django users to this role. - AuthZ compatibility layer - """ - role = get_authz_role_from_legacy_role(self.ROLE) - # silently ignores anonymous and inactive users so that any that are - # legit get updated. - for user in users: - if user.is_authenticated and user.is_active: - authz_add_role( - user=user, - authz_role=role, - course_key=str(self.course_key), - ) - if hasattr(user, '_roles'): - del user._roles - - def _legacy_add_users(self, users): + def add_users(self, *users): """ Add the supplied django users to this role. - legacy implementation """ # silently ignores anonymous and inactive users so that any that are # legit get updated. - from common.djangoapps.student.models import \ - CourseAccessRole # lint-amnesty, pylint: disable=redefined-outer-name, reimported + from common.djangoapps.student.models import CourseAccessRole # lint-amnesty, pylint: disable=redefined-outer-name, reimported for user in users: if user.is_authenticated and user.is_active: CourseAccessRole.objects.get_or_create( @@ -421,38 +284,9 @@ def _legacy_add_users(self, users): if hasattr(user, '_roles'): del user._roles - def add_users(self, *users): - """ - Add the supplied django users to this role. - """ - if enable_authz_course_authoring(self.course_key): - self._authz_add_users(users) - else: - self._legacy_add_users(users) - - def _authz_remove_users(self, users): - """ - Remove the supplied django users from this role. - AuthZ compatibility layer - """ - usernames = [user.username for user in users] - role = get_authz_role_from_legacy_role(self.ROLE) - course_key_str = str(self.course_key) - course_locator = CourseLocator.from_string(course_key_str) - authz_api.batch_unassign_role_from_users( - users=usernames, - role_external_key=role, - scope_external_key=course_key_str - ) - for user in users: - emit_course_access_role_removed(user, course_locator, course_locator.org, self.ROLE) - if hasattr(user, '_roles'): - del user._roles - - def _legacy_remove_users(self, users): + def remove_users(self, *users): """ Remove the supplied django users from this role. - legacy implementation """ entries = CourseAccessRole.objects.filter( user__in=users, role=self._role_name, org=self.org, course_id=self.course_key @@ -462,33 +296,9 @@ def _legacy_remove_users(self, users): if hasattr(user, '_roles'): del user._roles - def remove_users(self, *users): - """ - Remove the supplied django users from this role. - """ - if enable_authz_course_authoring(self.course_key): - self._authz_remove_users(users) - else: - self._legacy_remove_users(users) - - def _authz_users_with_role(self): - """ - Return a django QuerySet for all of the users with this role - AuthZ compatibility layer - """ - role = get_authz_role_from_legacy_role(self.ROLE) - users_data = authz_api.get_users_for_role_in_scope( - role_external_key=role, - scope_external_key=str(self.course_key) - ) - usernames = [user_data.username for user_data in users_data] - entries = User.objects.filter(username__in=usernames) - return entries - - def _legacy_users_with_role(self): + def users_with_role(self): """ Return a django QuerySet for all of the users with this role - legacy implementation """ # Org roles don't query by CourseKey, so use CourseKeyField.Empty for that query if self.course_key is None: @@ -500,63 +310,12 @@ def _legacy_users_with_role(self): ) return entries - def users_with_role(self): - """ - Return a django QuerySet for all of the users with this role - """ - if enable_authz_course_authoring(self.course_key): - return self._authz_users_with_role() - else: - return self._legacy_users_with_role() - - def _authz_get_orgs_for_user(self, user) -> list[str]: - """ - Returns a list of org short names for the user with given role. - AuthZ compatibility layer - """ - # TODO: This will be implemented on Milestone 1 - # of the Authz for Course Authoring project - return [] - - def _legacy_get_orgs_for_user(self, user) -> list[str]: - """ - Returns a list of org short names for the user with given role. - legacy implementation - """ - return list(CourseAccessRole.objects.filter(user=user, role=self._role_name).values_list('org', flat=True)) - def get_orgs_for_user(self, user): """ Returns a list of org short names for the user with given role. """ - if enable_authz_course_authoring(self.course_key): - return self._authz_get_orgs_for_user(user) - else: - return self._legacy_get_orgs_for_user(user) + return CourseAccessRole.objects.filter(user=user, role=self._role_name).values_list('org', flat=True) - def has_org_for_user(self, user: User, org: str | None = None) -> bool: - """ - Checks whether a user has a specific role within an org. - - Arguments: - user: user to check against access to role - org: optional org to check against access to role, - if not specified, will return True if the user has access to at least one org - """ - if enable_authz_course_authoring(self.course_key): - orgs_with_role = self.get_orgs_for_user(user) - if org: - return org in orgs_with_role - return len(orgs_with_role) > 0 - else: - # Use ORM query directly for performance - filter_params = { - 'user': user, - 'role': self._role_name - } - if org: - filter_params['org'] = org - return CourseAccessRole.objects.filter(**filter_params).exists() class CourseRole(RoleBase): """ @@ -570,25 +329,9 @@ def __init__(self, role, course_key): super().__init__(role, course_key.org, course_key) @classmethod - def _authz_course_group_already_exists(cls, course_key): # lint-amnesty, pylint: disable=bad-classmethod-argument - # AuthZ compatibility layer - return len(authz_api.get_all_user_role_assignments_in_scope(scope_external_key=str(course_key))) > 0 - - @classmethod - def _legacy_course_group_already_exists(cls, course_key): # lint-amnesty, pylint: disable=bad-classmethod-argument - # Legacy implementation + def course_group_already_exists(self, course_key): # lint-amnesty, pylint: disable=bad-classmethod-argument return CourseAccessRole.objects.filter(org=course_key.org, course_id=course_key).exists() - @classmethod - def course_group_already_exists(cls, course_key): # lint-amnesty, pylint: disable=bad-classmethod-argument - """ - Returns whether role assignations for a course already exist - """ - if enable_authz_course_authoring(course_key): - return cls._authz_course_group_already_exists(course_key) - else: - return cls._legacy_course_group_already_exists(course_key) - def __repr__(self): return f'<{self.__class__.__name__}: course_key={self.course_key}>' @@ -776,18 +519,9 @@ def add_course(self, *course_keys): Grant this object's user the object's role for the supplied courses """ if self.user.is_authenticated and self.user.is_active: - authz_role = get_authz_role_from_legacy_role(self.role) for course_key in course_keys: - if enable_authz_course_authoring(course_key): - # AuthZ compatibility layer - authz_add_role( - user=self.user, - authz_role=authz_role, - course_key=str(course_key), - ) - else: - entry = CourseAccessRole(user=self.user, role=self.role, course_id=course_key, org=course_key.org) - entry.save() + entry = CourseAccessRole(user=self.user, role=self.role, course_id=course_key, org=course_key.org) + entry.save() if hasattr(self.user, '_roles'): del self.user._roles else: @@ -797,102 +531,18 @@ def remove_courses(self, *course_keys): """ Remove the supplied courses from this user's configured role. """ - # CourseAccessRoles for courses managed by AuthZ should already be removed, so always doing this is ok entries = CourseAccessRole.objects.filter(user=self.user, role=self.role, course_id__in=course_keys) entries.delete() - # Execute bulk delete on AuthZ - role = get_authz_role_from_legacy_role(self.role) - for course_key in course_keys: - course_key_str = str(course_key) - success = authz_api.unassign_role_from_user( - user_external_key=self.user.username, - role_external_key=role, - scope_external_key=course_key_str - ) - if success: - course_locator = CourseLocator.from_string(course_key_str) - emit_course_access_role_removed(self.user, course_locator, course_locator.org, self.role) - if hasattr(self.user, '_roles'): del self.user._roles - def courses_with_role(self) -> set[AuthzCompatCourseAccessRole]: + def courses_with_role(self): """ - Return a set of AuthzCompatCourseAccessRole for all of the courses with this user x (or derived from x) role. + Return a django QuerySet for all of the courses with this user x (or derived from x) role. You can access + any of these properties on each result record: + * user (will be self.user--thus uninteresting) + * org + * course_id + * role (will be self.role--thus uninteresting) """ - roles = RoleCache.get_roles(self.role) - legacy_assignments = CourseAccessRole.objects.filter(role__in=roles, user=self.user) - - # Get all assignments for a user to a role - new_authz_roles = [get_authz_role_from_legacy_role(role) for role in roles] - all_authz_user_assignments = authz_api.get_user_role_assignments( - user_external_key=self.user.username - ) - - all_assignments = set() - - for legacy_assignment in legacy_assignments: - for role in roles: - all_assignments.add(AuthzCompatCourseAccessRole( - user_id=self.user.id, - username=self.user.username, - org=legacy_assignment.org, - course_id=legacy_assignment.course_id, - role=role - )) - - for assignment in all_authz_user_assignments: - for role in assignment.roles: - if role.external_key not in new_authz_roles: - continue - legacy_role = get_legacy_role_from_authz_role(authz_role=role.external_key) - course_key = assignment.scope.external_key - parsed_key = CourseKey.from_string(course_key) - org = parsed_key.org - all_assignments.add(AuthzCompatCourseAccessRole( - user_id=self.user.id, - username=self.user.username, - org=org, - course_id=course_key, - role=legacy_role - )) - - return all_assignments - - def has_courses_with_role(self, org: str | None = None) -> bool: - """ - Return whether this user has any courses with this role and optional org (or derived roles) - - Arguments: - org (str): Optional org to filter by - """ - roles = RoleCache.get_roles(self.role) - # First check if we have any legacy assignment with an optimized ORM query - filter_params = { - 'user': self.user, - 'role__in': roles - } - if org: - filter_params['org'] = org - has_legacy_assignments = CourseAccessRole.objects.filter(**filter_params).exists() - if has_legacy_assignments: - return True - - # Then check for authz assignments - new_authz_roles = [get_authz_role_from_legacy_role(role) for role in roles] - all_authz_user_assignments = authz_api.get_user_role_assignments( - user_external_key=self.user.username - ) - - for assignment in all_authz_user_assignments: - for role in assignment.roles: - if role.external_key not in new_authz_roles: - continue - if org is None: - # There is at least one assignment, short circuit - return True - course_key = assignment.scope.external_key - parsed_key = CourseKey.from_string(course_key) - if org == parsed_key.org: - return True - return False + return CourseAccessRole.objects.filter(role__in=RoleCache.get_roles(self.role), user=self.user) diff --git a/common/djangoapps/student/tests/test_roles.py b/common/djangoapps/student/tests/test_roles.py index 48e4e36ed0ea..92dcf1957626 100644 --- a/common/djangoapps/student/tests/test_roles.py +++ b/common/djangoapps/student/tests/test_roles.py @@ -6,17 +6,12 @@ import ddt from django.contrib.auth.models import Permission from django.test import TestCase -from edx_toggles.toggles.testutils import override_waffle_flag from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import LibraryLocator -from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory -from openedx_authz.engine.enforcer import AuthzEnforcer - from common.djangoapps.student.admin import CourseAccessRoleHistoryAdmin from common.djangoapps.student.models import CourseAccessRoleHistory, User from common.djangoapps.student.roles import ( - AuthzCompatCourseAccessRole, CourseAccessRole, CourseBetaTesterRole, CourseInstructorRole, @@ -37,10 +32,8 @@ ) from common.djangoapps.student.role_helpers import get_course_roles, has_staff_roles from common.djangoapps.student.tests.factories import AnonymousUserFactory, InstructorFactory, StaffFactory, UserFactory -from openedx.core.toggles import AUTHZ_COURSE_AUTHORING_FLAG -@ddt.ddt class RolesTestCase(TestCase): """ Tests of student.roles @@ -48,10 +41,8 @@ class RolesTestCase(TestCase): def setUp(self): super().setUp() - self._seed_database_with_policies() self.course_key = CourseKey.from_string('course-v1:course-v1:edX+toy+2012_Fall') self.course_loc = self.course_key.make_usage_key('course', '2012_Fall') - self.course = CourseOverviewFactory.create(id=self.course_key) self.anonymous_user = AnonymousUserFactory() self.student = UserFactory() self.global_staff = UserFactory(is_staff=True) @@ -59,67 +50,37 @@ def setUp(self): self.course_instructor = InstructorFactory(course_key=self.course_key) self.orgs = ["Marvel", "DC"] - @classmethod - def _seed_database_with_policies(cls): - """Seed the database with policies from the policy file for openedx_authz tests. - - This simulates the one-time database seeding that would happen - during application deployment, separate from the runtime policy loading. - """ - import pkg_resources - from openedx_authz.engine.utils import migrate_policy_between_enforcers - import casbin - - global_enforcer = AuthzEnforcer.get_enforcer() - global_enforcer.load_policy() - model_path = pkg_resources.resource_filename("openedx_authz.engine", "config/model.conf") - policy_path = pkg_resources.resource_filename("openedx_authz.engine", "config/authz.policy") - - migrate_policy_between_enforcers( - source_enforcer=casbin.Enforcer(model_path, policy_path), - target_enforcer=global_enforcer, - ) - global_enforcer.clear_policy() # Clear to simulate fresh start for each test - - @ddt.data(True, False) - def test_global_staff(self, authz_enabled): - with override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=authz_enabled): - assert not GlobalStaff().has_user(self.student) - assert not GlobalStaff().has_user(self.course_staff) - assert not GlobalStaff().has_user(self.course_instructor) - assert GlobalStaff().has_user(self.global_staff) - - @ddt.data(True, False) - def test_has_staff_roles(self, authz_enabled): - with override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=authz_enabled): - assert has_staff_roles(self.global_staff, self.course_key) - assert has_staff_roles(self.course_staff, self.course_key) - assert has_staff_roles(self.course_instructor, self.course_key) - assert not has_staff_roles(self.student, self.course_key) - - @ddt.data(True, False) - def test_get_course_roles(self, authz_enabled): - with override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=authz_enabled): - assert not list(get_course_roles(self.student)) - assert not list(get_course_roles(self.global_staff)) - assert list(get_course_roles(self.course_staff)) == [ - AuthzCompatCourseAccessRole( - user_id=self.course_staff.id, - username=self.course_staff.username, - course_id=self.course_key, - org=self.course_key.org, - role=CourseStaffRole.ROLE, - ) - ] - assert list(get_course_roles(self.course_instructor)) == [ - AuthzCompatCourseAccessRole( - user_id=self.course_instructor.id, - username=self.course_instructor.username, - course_id=self.course_key, - org=self.course_key.org, - role=CourseInstructorRole.ROLE, - ) - ] + def test_global_staff(self): + assert not GlobalStaff().has_user(self.student) + assert not GlobalStaff().has_user(self.course_staff) + assert not GlobalStaff().has_user(self.course_instructor) + assert GlobalStaff().has_user(self.global_staff) + + def test_has_staff_roles(self): + assert has_staff_roles(self.global_staff, self.course_key) + assert has_staff_roles(self.course_staff, self.course_key) + assert has_staff_roles(self.course_instructor, self.course_key) + assert not has_staff_roles(self.student, self.course_key) + + def test_get_course_roles(self): + assert not list(get_course_roles(self.student)) + assert not list(get_course_roles(self.global_staff)) + assert list(get_course_roles(self.course_staff)) == [ + CourseAccessRole( + user=self.course_staff, + course_id=self.course_key, + org=self.course_key.org, + role=CourseStaffRole.ROLE, + ) + ] + assert list(get_course_roles(self.course_instructor)) == [ + CourseAccessRole( + user=self.course_instructor, + course_id=self.course_key, + org=self.course_key.org, + role=CourseInstructorRole.ROLE, + ) + ] def test_group_name_case_sensitive(self): uppercase_course_id = "ORG/COURSE/NAME" @@ -139,22 +100,20 @@ def test_group_name_case_sensitive(self): assert not CourseRole(role, lowercase_course_key).has_user(uppercase_user) assert CourseRole(role, uppercase_course_key).has_user(uppercase_user) - @ddt.data(True, False) - def test_course_role(self, authz_enabled): + def test_course_role(self): """ Test that giving a user a course role enables access appropriately """ - with override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=authz_enabled): - assert not CourseStaffRole(self.course_key).has_user(self.student), \ - f'Student has premature access to {self.course_key}' - CourseStaffRole(self.course_key).add_users(self.student) - assert CourseStaffRole(self.course_key).has_user(self.student), \ - f"Student doesn't have access to {str(self.course_key)}" + assert not CourseStaffRole(self.course_key).has_user(self.student), \ + f'Student has premature access to {self.course_key}' + CourseStaffRole(self.course_key).add_users(self.student) + assert CourseStaffRole(self.course_key).has_user(self.student), \ + f"Student doesn't have access to {str(self.course_key)}" - # remove access and confirm - CourseStaffRole(self.course_key).remove_users(self.student) - assert not CourseStaffRole(self.course_key).has_user(self.student), \ - f'Student still has access to {self.course_key}' + # remove access and confirm + CourseStaffRole(self.course_key).remove_users(self.student) + assert not CourseStaffRole(self.course_key).has_user(self.student), \ + f'Student still has access to {self.course_key}' def test_org_role(self): """ @@ -199,30 +158,26 @@ def test_org_and_course_roles(self): assert not CourseInstructorRole(self.course_key).has_user(self.student), \ f"Student doesn't have access to {str(self.course_key)}" - @ddt.data(True, False) - def test_get_user_for_role(self, authz_enabled): + def test_get_user_for_role(self): """ test users_for_role """ - with override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=authz_enabled): - role = CourseStaffRole(self.course_key) - role.add_users(self.student) - assert len(role.users_with_role()) > 0 + role = CourseStaffRole(self.course_key) + role.add_users(self.student) + assert len(role.users_with_role()) > 0 - @ddt.data(True, False) - def test_add_users_doesnt_add_duplicate_entry(self, authz_enabled): + def test_add_users_doesnt_add_duplicate_entry(self): """ Tests that calling add_users multiple times before a single call to remove_users does not result in the user remaining in the group. """ - with override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=authz_enabled): - role = CourseStaffRole(self.course_key) - role.add_users(self.student) - assert role.has_user(self.student) - # Call add_users a second time, then remove just once. - role.add_users(self.student) - role.remove_users(self.student) - assert not role.has_user(self.student) + role = CourseStaffRole(self.course_key) + role.add_users(self.student) + assert role.has_user(self.student) + # Call add_users a second time, then remove just once. + role.add_users(self.student) + role.remove_users(self.student) + assert not role.has_user(self.student) def test_get_orgs_for_user(self): """ diff --git a/lms/djangoapps/ccx/tests/test_field_override_performance.py b/lms/djangoapps/ccx/tests/test_field_override_performance.py index 1e8e71c5d6ae..a7128495eb59 100644 --- a/lms/djangoapps/ccx/tests/test_field_override_performance.py +++ b/lms/djangoapps/ccx/tests/test_field_override_performance.py @@ -8,7 +8,6 @@ from unittest import mock import ddt -from openedx.core.djangolib.testing.utils import AUTHZ_TABLES import pytest from ccx_keys.locator import CCXLocator from django.conf import settings @@ -35,7 +34,7 @@ from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES from openedx.features.content_type_gating.models import ContentTypeGatingConfig -QUERY_COUNT_TABLE_IGNORELIST = WAFFLE_TABLES + AUTHZ_TABLES +QUERY_COUNT_TABLE_IGNORELIST = WAFFLE_TABLES @mock.patch.dict( @@ -235,7 +234,7 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase): __test__ = True # TODO: decrease query count as part of REVO-28 - QUERY_COUNT = 36 + QUERY_COUNT = 34 TEST_DATA = { ('no_overrides', 1, True, False): (QUERY_COUNT, 2), diff --git a/lms/djangoapps/course_api/blocks/tests/test_api.py b/lms/djangoapps/course_api/blocks/tests/test_api.py index 47d0b62fa6e6..93e400d4a804 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_api.py +++ b/lms/djangoapps/course_api/blocks/tests/test_api.py @@ -10,7 +10,6 @@ from common.djangoapps.student.tests.factories import UserFactory from openedx.core.djangoapps.content.block_structure.api import clear_course_from_cache -from openedx.core.djangolib.testing.utils import AUTHZ_TABLES from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.factories import SampleCourseFactory, check_mongo_calls # lint-amnesty, pylint: disable=wrong-import-order @@ -18,8 +17,6 @@ from ..api import get_blocks -QUERY_COUNT_TABLE_IGNORELIST = AUTHZ_TABLES - class TestGetBlocks(SharedModuleStoreTestCase): """ @@ -199,7 +196,7 @@ def _get_blocks(self, course, expected_mongo_queries, expected_sql_queries): get_blocks on the given course. """ with check_mongo_calls(expected_mongo_queries): - with self.assertNumQueries(expected_sql_queries, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): + with self.assertNumQueries(expected_sql_queries): get_blocks(self.request, course.location, self.user) @@ -215,11 +212,11 @@ def test_query_counts_cached(self, store_type): self._get_blocks( course, expected_mongo_queries=0, - expected_sql_queries=16, + expected_sql_queries=14, ) @ddt.data( - (ModuleStoreEnum.Type.split, 2, 26), + (ModuleStoreEnum.Type.split, 2, 24), ) @ddt.unpack def test_query_counts_uncached(self, store_type, expected_mongo_queries, num_sql_queries): diff --git a/lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py b/lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py index 8ebd2477deef..5efea79c1986 100644 --- a/lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py +++ b/lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py @@ -16,14 +16,10 @@ import openedx.core.djangoapps.content.block_structure.api as bs_api from openedx.core.djangoapps.content.block_structure.transformers import BlockStructureTransformers from openedx.core.djangoapps.course_apps.toggles import EXAMS_IDA -from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES -from openedx.core.djangolib.testing.utils import AUTHZ_TABLES from openedx.core.lib.gating import api as gating_api from ..milestones import MilestonesAndSpecialExamsTransformer -QUERY_COUNT_TABLE_IGNORELIST = WAFFLE_TABLES + AUTHZ_TABLES - @ddt.ddt @patch.dict('django.conf.settings.FEATURES', {'ENABLE_SPECIAL_EXAMS': True}) @@ -175,7 +171,7 @@ def test_gated(self, gated_block_ref, gating_block_ref, expected_blocks_before_c # get data back. This would happen as a part of publishing in a production system. bs_api.update_course_in_cache(self.course.id) - with self.assertNumQueries(6, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): + with self.assertNumQueries(4): self.get_blocks_and_check_against_expected(self.user, expected_blocks_before_completion) # clear the request cache to simulate a new request @@ -188,7 +184,7 @@ def test_gated(self, gated_block_ref, gating_block_ref, expected_blocks_before_c self.user, ) - with self.assertNumQueries(4, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): + with self.assertNumQueries(4): self.get_blocks_and_check_against_expected(self.user, self.ALL_BLOCKS_EXCEPT_SPECIAL) def test_staff_access(self): diff --git a/lms/djangoapps/courseware/tests/test_access.py b/lms/djangoapps/courseware/tests/test_access.py index 8500c3fc2d7e..4e97e09c83f1 100644 --- a/lms/djangoapps/courseware/tests/test_access.py +++ b/lms/djangoapps/courseware/tests/test_access.py @@ -7,7 +7,6 @@ import itertools from unittest.mock import Mock, patch -from openedx.core.djangolib.testing.utils import AUTHZ_TABLES import pytest import ddt import pytz @@ -71,7 +70,7 @@ ) from crum import set_current_request -QUERY_COUNT_TABLE_IGNORELIST = WAFFLE_TABLES + AUTHZ_TABLES +QUERY_COUNT_TABLE_IGNORELIST = WAFFLE_TABLES # pylint: disable=protected-access @@ -880,15 +879,15 @@ def test_course_catalog_access_num_queries_no_enterprise(self, user_attr_name, a if user_attr_name == 'user_staff' and action == 'see_exists': # always checks staff role, and if the course has started, check the duration configuration if course_attr_name == 'course_started': - num_queries = 4 + num_queries = 2 else: - num_queries = 3 + num_queries = 1 elif user_attr_name == 'user_normal' and action == 'see_exists': if course_attr_name == 'course_started': - num_queries = 6 + num_queries = 4 else: # checks staff role and enrollment data - num_queries = 4 + num_queries = 2 elif user_attr_name == 'user_anonymous' and action == 'see_exists': if course_attr_name == 'course_started': num_queries = 1 @@ -897,7 +896,7 @@ def test_course_catalog_access_num_queries_no_enterprise(self, user_attr_name, a else: # if the course has started, check the duration configuration if action == 'see_exists' and course_attr_name == 'course_started': - num_queries = 5 + num_queries = 3 else: num_queries = 0 @@ -951,17 +950,17 @@ def test_course_catalog_access_num_queries_enterprise(self, user_attr_name, cour if user_attr_name == 'user_staff': if course_attr_name == 'course_started': # read: CourseAccessRole + django_comment_client.Role - num_queries = 4 + num_queries = 2 else: # read: CourseAccessRole + EnterpriseCourseEnrollment - num_queries = 4 + num_queries = 2 elif user_attr_name == 'user_normal': if course_attr_name == 'course_started': # read: CourseAccessRole + django_comment_client.Role + FBEEnrollmentExclusion + CourseMode - num_queries = 6 + num_queries = 4 else: # read: CourseAccessRole + CourseEnrollmentAllowed + EnterpriseCourseEnrollment - num_queries = 5 + num_queries = 3 elif user_attr_name == 'user_anonymous': if course_attr_name == 'course_started': # read: CourseMode diff --git a/lms/djangoapps/courseware/tests/test_model_data.py b/lms/djangoapps/courseware/tests/test_model_data.py index 577edf831739..6c763d57b338 100644 --- a/lms/djangoapps/courseware/tests/test_model_data.py +++ b/lms/djangoapps/courseware/tests/test_model_data.py @@ -4,7 +4,6 @@ import json from functools import partial from unittest.mock import Mock, patch -from openedx.core.djangolib.testing.utils import AUTHZ_TABLES, FilteredQueryCountMixin import pytest from django.db import connections, DatabaseError @@ -14,7 +13,6 @@ from xblock.fields import BlockScope, Scope, ScopeIds from common.djangoapps.student.tests.factories import UserFactory -from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES from lms.djangoapps.courseware.model_data import DjangoKeyValueStore, FieldDataCache, InvalidScopeError from lms.djangoapps.courseware.models import ( StudentModule, @@ -29,8 +27,6 @@ from lms.djangoapps.courseware.tests.factories import StudentPrefsFactory from lms.djangoapps.courseware.tests.factories import UserStateSummaryFactory -QUERY_COUNT_TABLE_IGNORELIST = WAFFLE_TABLES + AUTHZ_TABLES - def mock_field(scope, name): field = Mock() @@ -243,7 +239,7 @@ def test_set_many_failure(self): assert exception_context.value.saved_field_names == [] -class TestMissingStudentModule(FilteredQueryCountMixin, TestCase): # lint-amnesty, pylint: disable=missing-class-docstring +class TestMissingStudentModule(TestCase): # lint-amnesty, pylint: disable=missing-class-docstring # Tell Django to clean out all databases, not just default databases = set(connections) @@ -280,9 +276,7 @@ def test_set_field_in_missing_student_module(self): # on the StudentModule). # Django 1.8 also has a number of other BEGIN and SAVESTATE queries. with self.assertNumQueries(4, using='default'): - with self.assertNumQueries(2, - using='student_module_history', - table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): + with self.assertNumQueries(2, using='student_module_history'): self.kvs.set(user_state_key('a_field'), 'a_value') assert 1 == sum(len(cache) for cache in self.field_data_cache.cache.values()) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index d96edb07c674..52eb044e932d 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -89,7 +89,7 @@ from openedx.core.djangoapps.credit.api import set_credit_requirements from openedx.core.djangoapps.credit.models import CreditCourse, CreditProvider from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES -from openedx.core.djangolib.testing.utils import AUTHZ_TABLES, get_mock_request +from openedx.core.djangolib.testing.utils import get_mock_request from openedx.core.djangoapps.video_config.toggles import PUBLIC_VIDEO_SHARE from openedx.core.lib.url_utils import quote_slashes from openedx.features.content_type_gating.models import ContentTypeGatingConfig @@ -108,7 +108,7 @@ from openedx.features.enterprise_support.api import add_enterprise_customer_to_session from enterprise.api.v1.serializers import EnterpriseCustomerSerializer -QUERY_COUNT_TABLE_IGNORELIST = WAFFLE_TABLES + AUTHZ_TABLES +QUERY_COUNT_TABLE_IGNORELIST = WAFFLE_TABLES FEATURES_WITH_DISABLE_HONOR_CERTIFICATE = settings.FEATURES.copy() FEATURES_WITH_DISABLE_HONOR_CERTIFICATE['DISABLE_HONOR_CERTIFICATES'] = True @@ -1283,8 +1283,8 @@ def test_view_certificate_link(self): self.assertContains(resp, "earned a certificate for this course.") @ddt.data( - (True, 56), - (False, 56), + (True, 54), + (False, 54), ) @ddt.unpack def test_progress_queries_paced_courses(self, self_paced, query_count): @@ -1299,7 +1299,7 @@ def test_progress_queries(self): ContentTypeGatingConfig.objects.create(enabled=True, enabled_as_of=datetime(2018, 1, 1)) self.setup_course() with self.assertNumQueries( - 56, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST + 54, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST ), check_mongo_calls(2): self._get_progress_page() diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index 8adb98302f69..8f32991272e2 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -156,8 +156,8 @@ def test_block_structure_created_only_once(self): assert mock_block_structure_create.call_count == 1 @ddt.data( - (ModuleStoreEnum.Type.split, 1, 47, True), - (ModuleStoreEnum.Type.split, 1, 47, False), + (ModuleStoreEnum.Type.split, 1, 42, True), + (ModuleStoreEnum.Type.split, 1, 42, False), ) @ddt.unpack def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, create_multiple_subsections): @@ -168,7 +168,7 @@ def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, creat self._apply_recalculate_subsection_grade() @ddt.data( - (ModuleStoreEnum.Type.split, 1, 47), + (ModuleStoreEnum.Type.split, 1, 42), ) @ddt.unpack def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls): @@ -256,7 +256,7 @@ def test_problem_block_with_restricted_access(self, mock_subsection_signal): UserPartition.scheme_extensions = None @ddt.data( - (ModuleStoreEnum.Type.split, 1, 47), + (ModuleStoreEnum.Type.split, 1, 42), ) @ddt.unpack def test_persistent_grades_on_course(self, default_store, num_mongo_queries, num_sql_queries): diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index 1447a66e9b87..d8dc0ecd0a53 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -16,7 +16,6 @@ from unittest.mock import ANY, MagicMock, Mock, patch import ddt -from openedx.core.djangolib.testing.utils import AUTHZ_TABLES import pytest import unicodecsv from django.conf import settings @@ -86,8 +85,6 @@ }) USE_ON_DISK_GRADE_REPORT = 'lms.djangoapps.instructor_task.tasks_helper.grades.use_on_disk_grade_reporting' -QUERY_COUNT_TABLE_IGNORELIST = AUTHZ_TABLES - class InstructorGradeReportTestCase(TestReportMixin, InstructorTaskCourseTestCase): """ Base class for grade report tests. """ @@ -414,7 +411,7 @@ def test_query_counts(self): with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'): with check_mongo_calls(2): - with self.assertNumQueries(48, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): + with self.assertNumQueries(46): CourseGradeReport.generate(None, None, course.id, {}, 'graded') def test_inactive_enrollments(self): @@ -2218,7 +2215,7 @@ def test_certificate_generation_for_students(self): 'failed': 0, 'skipped': 2 } - with self.assertNumQueries(69, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): + with self.assertNumQueries(61): self.assertCertificatesGenerated(task_input, expected_results) @ddt.data( diff --git a/lms/djangoapps/teams/tests/test_serializers.py b/lms/djangoapps/teams/tests/test_serializers.py index 0a6adf6c4f41..384039bd661c 100644 --- a/lms/djangoapps/teams/tests/test_serializers.py +++ b/lms/djangoapps/teams/tests/test_serializers.py @@ -10,14 +10,10 @@ from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory from lms.djangoapps.teams.serializers import BulkTeamCountTopicSerializer, MembershipSerializer, TopicSerializer from lms.djangoapps.teams.tests.factories import CourseTeamFactory, CourseTeamMembershipFactory -from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES -from openedx.core.djangolib.testing.utils import AUTHZ_TABLES from openedx.core.lib.teams_config import TeamsConfig from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.factories import CourseFactory # lint-amnesty, pylint: disable=wrong-import-order -QUERY_COUNT_TABLE_IGNORELIST = WAFFLE_TABLES + AUTHZ_TABLES - class SerializerTestCase(SharedModuleStoreTestCase): """ @@ -79,9 +75,7 @@ def test_topic_with_no_team_count(self): Verifies that the `TopicSerializer` correctly displays a topic with a team count of 0, and that it takes a known number of SQL queries. """ - with self.assertNumQueries( - 3, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST - ): # 2 split modulestore MySQL queries, 1 for Teams + with self.assertNumQueries(3): # 2 split modulestore MySQL queries, 1 for Teams serializer = TopicSerializer( self.course.teamsets[0].cleaned_data, context={'course_id': self.course.id}, @@ -97,9 +91,7 @@ def test_topic_with_team_count(self): CourseTeamFactory.create( course_id=self.course.id, topic_id=self.course.teamsets[0].teamset_id ) - with self.assertNumQueries( - 3, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST - ): # 2 split modulestore MySQL queries, 1 for Teams + with self.assertNumQueries(3): # 2 split modulestore MySQL queries, 1 for Teams serializer = TopicSerializer( self.course.teamsets[0].cleaned_data, context={'course_id': self.course.id}, @@ -118,9 +110,7 @@ def test_scoped_within_course(self): ) CourseTeamFactory.create(course_id=self.course.id, topic_id=duplicate_topic['id']) CourseTeamFactory.create(course_id=second_course.id, topic_id=duplicate_topic['id']) - with self.assertNumQueries( - 3, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST - ): # 2 split modulestore MySQL queries, 1 for Teams + with self.assertNumQueries(3): # 2 split modulestore MySQL queries, 1 for Teams serializer = TopicSerializer( self.course.teamsets[0].cleaned_data, context={'course_id': self.course.id}, @@ -173,7 +163,7 @@ def assert_serializer_output(self, topics, num_teams_per_topic, num_queries): """ Verify that the serializer produced the expected topics. """ - with self.assertNumQueries(num_queries, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): + with self.assertNumQueries(num_queries): page = Paginator( self.course.teams_configuration.cleaned_data['teamsets'], self.PAGE_SIZE, @@ -213,7 +203,7 @@ def test_topics_with_no_team_counts(self): query. """ topics = self.setup_topics(teams_per_topic=0) - self.assert_serializer_output(topics, num_teams_per_topic=0, num_queries=4) + self.assert_serializer_output(topics, num_teams_per_topic=0, num_queries=2) def test_topics_with_team_counts(self): """ @@ -222,7 +212,7 @@ def test_topics_with_team_counts(self): """ teams_per_topic = 10 topics = self.setup_topics(teams_per_topic=teams_per_topic) - self.assert_serializer_output(topics, num_teams_per_topic=teams_per_topic, num_queries=4) + self.assert_serializer_output(topics, num_teams_per_topic=teams_per_topic, num_queries=2) def test_subset_of_topics(self): """ @@ -231,7 +221,7 @@ def test_subset_of_topics(self): """ teams_per_topic = 10 topics = self.setup_topics(num_topics=self.NUM_TOPICS, teams_per_topic=teams_per_topic) - self.assert_serializer_output(topics, num_teams_per_topic=teams_per_topic, num_queries=4) + self.assert_serializer_output(topics, num_teams_per_topic=teams_per_topic, num_queries=2) def test_scoped_within_course(self): """Verify that team counts are scoped within a course.""" @@ -245,7 +235,7 @@ def test_scoped_within_course(self): }), ) CourseTeamFactory.create(course_id=second_course.id, topic_id=duplicate_topic['id']) - self.assert_serializer_output(first_course_topics, num_teams_per_topic=teams_per_topic, num_queries=4) + self.assert_serializer_output(first_course_topics, num_teams_per_topic=teams_per_topic, num_queries=2) def _merge_dicts(self, first, second): """Convenience method to merge two dicts in a single expression""" @@ -261,9 +251,7 @@ def assert_serializer_output(self, topics, num_teams_per_topic, num_queries): request = RequestFactory().get('/api/team/v0/topics') request.user = self.user - with self.assertNumQueries( - num_queries + 2, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST - ): # num_queries on teams tables, plus 2 split modulestore queries + with self.assertNumQueries(num_queries + 2): # num_queries on teams tables, plus 2 split modulestore queries serializer = self.serializer( topics, context={ @@ -281,4 +269,4 @@ def test_no_topics(self): with no topics. """ self.course.teams_configuration = TeamsConfig({'topics': []}) - self.assert_serializer_output([], num_teams_per_topic=0, num_queries=3) + self.assert_serializer_output([], num_teams_per_topic=0, num_queries=1) diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py index b40ad5d55156..0e78b963103b 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py @@ -514,12 +514,12 @@ def test_create_taxonomy(self, user_attr: str, expected_status: int) -> None: @ddt.data( ('staff', 11), - ("content_creatorA", 23), - ("library_staffA", 23), - ("library_userA", 23), - ("instructorA", 23), - ("course_instructorA", 23), - ("course_staffA", 23), + ("content_creatorA", 22), + ("library_staffA", 22), + ("library_userA", 22), + ("instructorA", 22), + ("course_instructorA", 22), + ("course_staffA", 22), ) @ddt.unpack def test_list_taxonomy_query_count(self, user_attr: str, expected_queries: int): @@ -1947,16 +1947,16 @@ def test_get_copied_tags(self): ('staff', 'courseA', 8), ('staff', 'libraryA', 8), ('staff', 'collection_key', 8), - ("content_creatorA", 'courseA', 18, False), - ("content_creatorA", 'libraryA', 18, False), - ("content_creatorA", 'collection_key', 18, False), - ("library_staffA", 'libraryA', 18, False), # Library users can only view objecttags, not change them? - ("library_staffA", 'collection_key', 18, False), - ("library_userA", 'libraryA', 18, False), - ("library_userA", 'collection_key', 18, False), - ("instructorA", 'courseA', 18), - ("course_instructorA", 'courseA', 18), - ("course_staffA", 'courseA', 18), + ("content_creatorA", 'courseA', 17, False), + ("content_creatorA", 'libraryA', 17, False), + ("content_creatorA", 'collection_key', 17, False), + ("library_staffA", 'libraryA', 17, False), # Library users can only view objecttags, not change them? + ("library_staffA", 'collection_key', 17, False), + ("library_userA", 'libraryA', 17, False), + ("library_userA", 'collection_key', 17, False), + ("instructorA", 'courseA', 17), + ("course_instructorA", 'courseA', 17), + ("course_staffA", 'courseA', 17), ) @ddt.unpack def test_object_tags_query_count( diff --git a/openedx/core/djangoapps/embargo/tests/test_api.py b/openedx/core/djangoapps/embargo/tests/test_api.py index a07edeb2486a..4ebb3999f925 100644 --- a/openedx/core/djangoapps/embargo/tests/test_api.py +++ b/openedx/core/djangoapps/embargo/tests/test_api.py @@ -19,8 +19,7 @@ from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, mixed_store_config -from openedx.core.djangolib.testing.utils import AUTHZ_TABLES, skip_unless_lms -from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES +from openedx.core.djangolib.testing.utils import skip_unless_lms from common.djangoapps.student.tests.factories import UserFactory from common.djangoapps.student.roles import ( GlobalStaff, CourseRole, OrgRole, @@ -36,7 +35,6 @@ from .. import api as embargo_api from ..exceptions import InvalidAccessPoint -QUERY_COUNT_TABLE_IGNORELIST = WAFFLE_TABLES + AUTHZ_TABLES MODULESTORE_CONFIG = mixed_store_config(settings.COMMON_TEST_DATA_ROOT, {}) @@ -177,10 +175,10 @@ def test_caching(self): # (restricted course, but pass all the checks) # This is the worst case, so it will hit all of the # caching code. - with self.assertNumQueries(5, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): + with self.assertNumQueries(3): embargo_api.check_course_access(self.course.id, user=self.user, ip_addresses=['0.0.0.0']) - with self.assertNumQueries(0, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): + with self.assertNumQueries(0): embargo_api.check_course_access(self.course.id, user=self.user, ip_addresses=['0.0.0.0']) def test_caching_no_restricted_courses(self): diff --git a/openedx/core/djangoapps/enrollments/tests/test_data.py b/openedx/core/djangoapps/enrollments/tests/test_data.py index 41bdfff62a50..491be2cc8cdc 100644 --- a/openedx/core/djangoapps/enrollments/tests/test_data.py +++ b/openedx/core/djangoapps/enrollments/tests/test_data.py @@ -12,7 +12,6 @@ from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.course_modes.tests.factories import CourseModeFactory -from common.djangoapps.student.roles import AuthzCompatCourseAccessRole from openedx.core.djangoapps.enrollments import data from openedx.core.djangoapps.enrollments.errors import ( CourseEnrollmentClosedError, @@ -388,15 +387,8 @@ def test_get_roles(self): expected_role = CourseAccessRoleFactory.create( course_id=self.course.id, user=self.user, role="SuperCoolTestRole", ) - expected_role_compat = AuthzCompatCourseAccessRole( - user_id=expected_role.user.id, - username=expected_role.user.username, - org=expected_role.org, - course_id=expected_role.course_id, - role=expected_role.role, - ) roles = data.get_user_roles(self.user.username) - assert roles == {expected_role_compat} + assert roles == {expected_role} def test_get_roles_no_roles(self): """Get roles for a user who has no roles""" diff --git a/openedx/core/djangoapps/schedules/tests/test_resolvers.py b/openedx/core/djangoapps/schedules/tests/test_resolvers.py index 9608769f2c49..20536abcbd6f 100644 --- a/openedx/core/djangoapps/schedules/tests/test_resolvers.py +++ b/openedx/core/djangoapps/schedules/tests/test_resolvers.py @@ -34,11 +34,8 @@ ) from openedx.core.djangoapps.schedules.tests.factories import ScheduleConfigFactory from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory, SiteFactory -from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES from openedx.core.djangolib.testing.utils import CacheIsolationMixin, skip_unless_lms -QUERY_COUNT_TABLE_IGNORELIST = WAFFLE_TABLES - class SchedulesResolverTestMixin(CacheIsolationMixin): """ @@ -279,7 +276,7 @@ def create_resolver(self, user_start_date_offset=8): def test_schedule_context(self): resolver = self.create_resolver() # using this to make sure the select_related stays intact - with self.assertNumQueries(22, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): + with self.assertNumQueries(26): sc = resolver.get_schedules() schedules = list(sc) apple_logo_url = 'http://email-media.s3.amazonaws.com/edX/2021/store_apple_229x78.jpg' diff --git a/openedx/core/djangolib/testing/utils.py b/openedx/core/djangolib/testing/utils.py index 20003462064c..3beb63586b3f 100644 --- a/openedx/core/djangolib/testing/utils.py +++ b/openedx/core/djangolib/testing/utils.py @@ -25,13 +25,6 @@ from openedx.core.lib import ensure_cms, ensure_lms -# Used to ignore queries against authz tables when using assertNumQueries in FilteredQueryCountMixin -AUTHZ_TABLES = [ - "casbin_rule", - "openedx_authz_policycachecontrol", - "django_migrations", -] - class CacheIsolationMixin: """ @@ -189,9 +182,9 @@ def is_unfiltered_query(query): if self.table_ignorelist: for table in self.table_ignorelist: # SQL contains the following format for columns: - # "table_name"."column_name" or table_name.column_name. - # The regex ensures there is no "." before the name to avoid matching columns. - if re.search(fr'[^."]"?{table}"?', query['sql']): + # "table_name"."column_name". The regex ensures there is no + # "." before the name to avoid matching columns. + if re.search(fr'[^.]"{table}"', query['sql']): return False return True diff --git a/openedx/envs/common.py b/openedx/envs/common.py index 240f41f901ae..9665ec372e5b 100644 --- a/openedx/envs/common.py +++ b/openedx/envs/common.py @@ -2103,7 +2103,7 @@ def add_optional_apps(optional_apps, installed_apps): # .. toggle_warning: Not production-ready until relevant subtask https://github.com/openedx/edx-platform/issues/34827 is done. # .. toggle_creation_date: 2024-11-10 # .. toggle_target_removal_date: 2025-06-01 -USE_EXTRACTED_VIDEO_BLOCK = False +USE_EXTRACTED_VIDEO_BLOCK = True ############################## Marketing Site ############################## diff --git a/openedx/features/content_type_gating/tests/test_models.py b/openedx/features/content_type_gating/tests/test_models.py index 53d94439fd6c..adb9bd6737df 100644 --- a/openedx/features/content_type_gating/tests/test_models.py +++ b/openedx/features/content_type_gating/tests/test_models.py @@ -16,16 +16,13 @@ from openedx.core.djangoapps.config_model_utils.models import Provenance from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory -from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES -from openedx.core.djangolib.testing.utils import AUTHZ_TABLES, CacheIsolationTestCase, FilteredQueryCountMixin +from openedx.core.djangolib.testing.utils import CacheIsolationTestCase from openedx.features.content_type_gating.models import ContentTypeGatingConfig from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory -QUERY_COUNT_TABLE_IGNORELIST = WAFFLE_TABLES + AUTHZ_TABLES - @ddt.ddt -class TestContentTypeGatingConfig(FilteredQueryCountMixin, CacheIsolationTestCase): # pylint: disable=missing-class-docstring +class TestContentTypeGatingConfig(CacheIsolationTestCase): # pylint: disable=missing-class-docstring ENABLED_CACHES = ['default'] @@ -74,9 +71,9 @@ def test_enabled_for_enrollment( user = self.user course_key = self.course_overview.id - query_count = 9 + query_count = 7 - with self.assertNumQueries(query_count, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): + with self.assertNumQueries(query_count): enabled = ContentTypeGatingConfig.enabled_for_enrollment( user=user, course_key=course_key, diff --git a/openedx/features/course_duration_limits/tests/test_models.py b/openedx/features/course_duration_limits/tests/test_models.py index 5fdbc08d5b70..f596f3534489 100644 --- a/openedx/features/course_duration_limits/tests/test_models.py +++ b/openedx/features/course_duration_limits/tests/test_models.py @@ -18,15 +18,12 @@ from openedx.core.djangoapps.config_model_utils.models import Provenance from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory -from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES -from openedx.core.djangolib.testing.utils import AUTHZ_TABLES, CacheIsolationTestCase, FilteredQueryCountMixin +from openedx.core.djangolib.testing.utils import CacheIsolationTestCase from openedx.features.course_duration_limits.models import CourseDurationLimitConfig -QUERY_COUNT_TABLE_IGNORELIST = WAFFLE_TABLES + AUTHZ_TABLES - @ddt.ddt -class TestCourseDurationLimitConfig(FilteredQueryCountMixin, CacheIsolationTestCase): +class TestCourseDurationLimitConfig(CacheIsolationTestCase): """ Tests of CourseDurationLimitConfig """ @@ -77,9 +74,9 @@ def test_enabled_for_enrollment( user = self.user course_key = self.course_overview.id # lint-amnesty, pylint: disable=unused-variable - query_count = 9 + query_count = 7 - with self.assertNumQueries(query_count, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): + with self.assertNumQueries(query_count): enabled = CourseDurationLimitConfig.enabled_for_enrollment(user, self.course_overview) assert (not enrolled_before_enabled) == enabled diff --git a/openedx/features/course_experience/tests/views/test_course_updates.py b/openedx/features/course_experience/tests/views/test_course_updates.py index aff8700bc44e..6ca701f14677 100644 --- a/openedx/features/course_experience/tests/views/test_course_updates.py +++ b/openedx/features/course_experience/tests/views/test_course_updates.py @@ -8,12 +8,11 @@ from zoneinfo import ZoneInfo from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES -from openedx.core.djangolib.testing.utils import AUTHZ_TABLES from openedx.features.content_type_gating.models import ContentTypeGatingConfig from openedx.features.course_experience.tests import BaseCourseUpdatesTestCase from xmodule.modulestore.tests.factories import check_mongo_calls # lint-amnesty, pylint: disable=wrong-import-order -QUERY_COUNT_TABLE_IGNORELIST = WAFFLE_TABLES + AUTHZ_TABLES +QUERY_COUNT_TABLE_IGNORELIST = WAFFLE_TABLES def course_updates_url(course): @@ -50,7 +49,7 @@ def test_queries(self): # Fetch the view and verify that the query counts haven't changed # TODO: decrease query count as part of REVO-28 - with self.assertNumQueries(54, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): + with self.assertNumQueries(52, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): with check_mongo_calls(3): url = course_updates_url(self.course) self.client.get(url) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 7fefaa75a5e7..184dcb8c64af 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -823,7 +823,7 @@ openedx-atlas==0.7.0 # enterprise-integrated-channels # openedx-authz # openedx-forum -openedx-authz==0.23.0 +openedx-authz==0.22.0 # via -r requirements/edx/kernel.in openedx-calc==4.0.3 # via diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 9f902c00ee1a..cc323c983ee0 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1373,7 +1373,7 @@ openedx-atlas==0.7.0 # enterprise-integrated-channels # openedx-authz # openedx-forum -openedx-authz==0.23.0 +openedx-authz==0.22.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index aa212395650b..b2806fceb113 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -1001,7 +1001,7 @@ openedx-atlas==0.7.0 # enterprise-integrated-channels # openedx-authz # openedx-forum -openedx-authz==0.23.0 +openedx-authz==0.22.0 # via -r requirements/edx/base.txt openedx-calc==4.0.3 # via diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index afe1379b1774..46605bbb9656 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -1050,7 +1050,7 @@ openedx-atlas==0.7.0 # enterprise-integrated-channels # openedx-authz # openedx-forum -openedx-authz==0.23.0 +openedx-authz==0.22.0 # via -r requirements/edx/base.txt openedx-calc==4.0.3 # via