Merge branch 'master' into ruff-linter

cms/djangoapps/contentstore/course_group_config.py

@@ -13,11 +13,11 @@
 from common.djangoapps.util.db import MYSQL_MAX_INT, generate_int_id
 from lms.lib.utils import get_parent_unit
 from openedx.core.djangoapps.course_groups.partition_scheme import get_cohorted_user_partition
-from xmodule.partitions.partitions import MINIMUM_STATIC_PARTITION_ID, ReadOnlyUserPartitionError, UserPartition  # lint-amnesty, pylint: disable=wrong-import-order
+from xmodule.partitions.partitions import MINIMUM_UNUSED_PARTITION_ID, ReadOnlyUserPartitionError, UserPartition  # lint-amnesty, pylint: disable=wrong-import-order
 from xmodule.partitions.partitions_service import get_all_partitions_for_course  # lint-amnesty, pylint: disable=wrong-import-order
 from xmodule.split_test_block import get_split_user_partitions  # lint-amnesty, pylint: disable=wrong-import-order
 
-MINIMUM_GROUP_ID = MINIMUM_STATIC_PARTITION_ID
+MINIMUM_GROUP_ID = MINIMUM_UNUSED_PARTITION_ID
 
 RANDOM_SCHEME = "random"
 COHORT_SCHEME = "cohort"

cms/djangoapps/contentstore/rest_api/v1/views/home.py

@@ -215,4 +215,5 @@ def get(self, request: Request):
 
         library_context = get_library_context(request)
         serializer = LibraryTabSerializer(library_context)
+
         return Response(serializer.data)

cms/djangoapps/contentstore/utils.py

@@ -2251,7 +2251,7 @@ def send_course_update_notification(course_key, content, user):
     notification_data = CourseNotificationData(
         course_key=course_key,
         content_context={
-            "course_update_content": text_content,
+            "course_update_content": text_content if len(text_content.strip()) < 10 else "Click here to view",
             **extra_context,
         },
         notification_type="course_update",

cms/djangoapps/contentstore/views/course.py

@@ -52,7 +52,7 @@
     CourseStaffRole,
     GlobalStaff,
     UserBasedRole,
-    OrgStaffRole
+    OrgStaffRole,
 )
 from common.djangoapps.util.json_request import JsonResponse, JsonResponseBadRequest, expect_json
 from common.djangoapps.util.string_utils import _has_non_ascii_characters

cms/djangoapps/contentstore/views/tests/test_block.py

@@ -48,7 +48,7 @@
 )
 from xmodule.partitions.partitions import (
     ENROLLMENT_TRACK_PARTITION_ID,
-    MINIMUM_STATIC_PARTITION_ID,
+    MINIMUM_UNUSED_PARTITION_ID,
     Group,
     UserPartition,
 )
@@ -421,16 +421,16 @@ def test_get_user_partitions_and_groups(self):
         # by dynamic user partitions.
         self.course.user_partitions = [
             UserPartition(
-                id=MINIMUM_STATIC_PARTITION_ID,
+                id=MINIMUM_UNUSED_PARTITION_ID,
                 name="Random user partition",
                 scheme=UserPartition.get_scheme("random"),
                 description="Random user partition",
                 groups=[
                     Group(
-                        id=MINIMUM_STATIC_PARTITION_ID + 1, name="Group A"
+                        id=MINIMUM_UNUSED_PARTITION_ID + 1, name="Group A"
                     ),  # See note above.
                     Group(
-                        id=MINIMUM_STATIC_PARTITION_ID + 2, name="Group B"
+                        id=MINIMUM_UNUSED_PARTITION_ID + 2, name="Group B"
                     ),  # See note above.
                 ],
             ),
@@ -462,18 +462,18 @@ def test_get_user_partitions_and_groups(self):
                     ],
                 },
                 {
-                    "id": MINIMUM_STATIC_PARTITION_ID,
+                    "id": MINIMUM_UNUSED_PARTITION_ID,
                     "name": "Random user partition",
                     "scheme": "random",
                     "groups": [
                         {
-                            "id": MINIMUM_STATIC_PARTITION_ID + 1,
+                            "id": MINIMUM_UNUSED_PARTITION_ID + 1,
                             "name": "Group A",
                             "selected": False,
                             "deleted": False,
                         },
                         {
-                            "id": MINIMUM_STATIC_PARTITION_ID + 2,
+                            "id": MINIMUM_UNUSED_PARTITION_ID + 2,
                             "name": "Group B",
                             "selected": False,
                             "deleted": False,
@@ -2443,13 +2443,13 @@ def setUp(self):
         self.user = UserFactory()
 
         self.first_user_partition_group_1 = Group(
-            str(MINIMUM_STATIC_PARTITION_ID + 1), "alpha"
+            str(MINIMUM_UNUSED_PARTITION_ID + 1), "alpha"
         )
         self.first_user_partition_group_2 = Group(
-            str(MINIMUM_STATIC_PARTITION_ID + 2), "beta"
+            str(MINIMUM_UNUSED_PARTITION_ID + 2), "beta"
         )
         self.first_user_partition = UserPartition(
-            MINIMUM_STATIC_PARTITION_ID,
+            MINIMUM_UNUSED_PARTITION_ID,
             "first_partition",
             "First Partition",
             [self.first_user_partition_group_1, self.first_user_partition_group_2],
@@ -2458,16 +2458,16 @@ def setUp(self):
         # There is a test point below (test_create_groups) that purposefully wants the group IDs
         # of the 2 partitions to overlap (which is not something that normally happens).
         self.second_user_partition_group_1 = Group(
-            str(MINIMUM_STATIC_PARTITION_ID + 1), "Group 1"
+            str(MINIMUM_UNUSED_PARTITION_ID + 1), "Group 1"
         )
         self.second_user_partition_group_2 = Group(
-            str(MINIMUM_STATIC_PARTITION_ID + 2), "Group 2"
+            str(MINIMUM_UNUSED_PARTITION_ID + 2), "Group 2"
         )
         self.second_user_partition_group_3 = Group(
-            str(MINIMUM_STATIC_PARTITION_ID + 3), "Group 3"
+            str(MINIMUM_UNUSED_PARTITION_ID + 3), "Group 3"
         )
         self.second_user_partition = UserPartition(
-            MINIMUM_STATIC_PARTITION_ID + 10,
+            MINIMUM_UNUSED_PARTITION_ID + 10,
             "second_partition",
             "Second Partition",
             [
@@ -2540,10 +2540,10 @@ def test_create_groups(self):
         self.assertEqual("vertical", vertical_0.category)
         self.assertEqual("vertical", vertical_1.category)
         self.assertEqual(
-            "Group ID " + str(MINIMUM_STATIC_PARTITION_ID + 1), vertical_0.display_name
+            "Group ID " + str(MINIMUM_UNUSED_PARTITION_ID + 1), vertical_0.display_name
         )
         self.assertEqual(
-            "Group ID " + str(MINIMUM_STATIC_PARTITION_ID + 2), vertical_1.display_name
+            "Group ID " + str(MINIMUM_UNUSED_PARTITION_ID + 2), vertical_1.display_name
         )
 
         # Verify that the group_id_to_child mapping is correct.

cms/djangoapps/models/settings/course_metadata.py

@@ -22,7 +22,7 @@
 from xmodule.course_block import get_available_providers  # lint-amnesty, pylint: disable=wrong-import-order
 from xmodule.modulestore.django import modulestore  # lint-amnesty, pylint: disable=wrong-import-order
 from xmodule.modulestore.exceptions import InvalidProctoringProvider  # lint-amnesty, pylint: disable=wrong-import-order
-from xmodule.partitions.partitions import MINIMUM_STATIC_PARTITION_ID
+from xmodule.partitions.partitions import MINIMUM_UNUSED_PARTITION_ID
 from xmodule.partitions.partitions_service import get_all_partitions_for_course
 
 LOGGER = logging.getLogger(__name__)
@@ -316,7 +316,7 @@ def fill_teams_user_partitions_ids(cls, block, teams_config):
             if not team_set.user_partition_id:
                 team_set.user_partition_id = cls.get_user_partition_id(
                     block,
-                    MINIMUM_STATIC_PARTITION_ID,
+                    MINIMUM_UNUSED_PARTITION_ID,
                     MYSQL_MAX_INT,
                 )
         return TeamsConfig(

cms/lib/xblock/test/test_authoring_mixin.py

@@ -10,7 +10,7 @@
 from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory
 from xmodule.partitions.partitions import (
     ENROLLMENT_TRACK_PARTITION_ID,
-    MINIMUM_STATIC_PARTITION_ID,
+    MINIMUM_UNUSED_PARTITION_ID,
     Group,
     UserPartition
 )
@@ -78,7 +78,7 @@ def create_content_groups(self, content_groups):
         """
         # pylint: disable=attribute-defined-outside-init
         self.content_partition = UserPartition(
-            MINIMUM_STATIC_PARTITION_ID,
+            MINIMUM_UNUSED_PARTITION_ID,
             self.CONTENT_GROUPS_TITLE,
             'Contains Groups for Cohorted Courseware',
             content_groups,

common/djangoapps/student/auth.py

@@ -23,7 +23,7 @@
     OrgContentCreatorRole,
     OrgInstructorRole,
     OrgLibraryUserRole,
-    OrgStaffRole
+    OrgStaffRole,
 )
 
 # Studio permissions:
@@ -106,6 +106,7 @@ def get_user_permissions(user, course_key, org=None):
     # Staff have all permissions except EDIT_ROLES:
     if OrgStaffRole(org=org).has_user(user) or (course_key and user_has_role(user, CourseStaffRole(course_key))):
         return STUDIO_VIEW_USERS | STUDIO_EDIT_CONTENT | STUDIO_VIEW_CONTENT
+
     # Otherwise, for libraries, users can view only:
     if course_key and isinstance(course_key, LibraryLocator):
         if OrgLibraryUserRole(org=org).has_user(user) or user_has_role(user, LibraryUserRole(course_key)):

common/djangoapps/student/role_helpers.py

@@ -75,4 +75,4 @@ def get_course_roles(user: User) -> list[CourseAccessRole]:
     """
     # pylint: disable=protected-access
     role_cache = get_role_cache(user)
-    return list(role_cache._roles)
+    return list(role_cache.all_roles_set)

common/djangoapps/student/roles.py

@@ -4,9 +4,9 @@
 """
 
 
+from collections import defaultdict
 import logging
 from abc import ABCMeta, abstractmethod
-from collections import defaultdict
 from contextlib import contextmanager
 
 from django.contrib.auth.models import User  # lint-amnesty, pylint: disable=imported-auth-user
@@ -23,6 +23,9 @@
 # A mapping of roles to the roles that they inherit permissions from.
 ACCESS_ROLES_INHERITANCE = {}
 
+# The key used to store roles for a user in the cache that do not belong to a course or do not have a course id.
+ROLE_CACHE_UNGROUPED_ROLES__KEY = 'ungrouped'
+
 
 def register_access_role(cls):
     """
@@ -60,21 +63,52 @@ def strict_role_checking():
     ACCESS_ROLES_INHERITANCE.update(OLD_ACCESS_ROLES_INHERITANCE)
 
 
+def get_role_cache_key_for_course(course_key=None):
+    """
+    Get the cache key for the course key.
+    """
+    return str(course_key) if course_key else ROLE_CACHE_UNGROUPED_ROLES__KEY
+
+
 class BulkRoleCache:  # lint-amnesty, pylint: disable=missing-class-docstring
+    """
+    This class provides a caching mechanism for roles grouped by users and courses,
+    using a nested dictionary structure to optimize lookup performance. The cache structure is designed as follows:
+
+    {
+        user_id_1: {
+            course_id_1: {role1, role2, role3},  # Set of roles associated with course_id_1
+            course_id_2: {role4, role5, role6},  # Set of roles associated with course_id_2
+            [ROLE_CACHE_UNGROUPED_ROLES_KEY]: {role7, role8}  # Set of roles not tied to any specific course or library
+        },
+        user_id_2: { ... }  # Similar structure for another user
+    }
+
+    - Each top-level dictionary entry keys by `user_id` to access role data for a specific user.
+    - Nested within each user's dictionary, entries are keyed by `course_id` grouping roles by course.
+    - The special key `ROLE_CACHE_UNGROUPED_ROLES_KEY` (a constant defined above)
+        stores roles that are not associated with any specific course or library.
+    """
+
     CACHE_NAMESPACE = "student.roles.BulkRoleCache"
     CACHE_KEY = 'roles_by_user'
 
     @classmethod
     def prefetch(cls, users):  # lint-amnesty, pylint: disable=missing-function-docstring
-        roles_by_user = defaultdict(set)
+        roles_by_user = defaultdict(lambda: defaultdict(set))
         get_cache(cls.CACHE_NAMESPACE)[cls.CACHE_KEY] = roles_by_user
 
         for role in CourseAccessRole.objects.filter(user__in=users).select_related('user'):
-            roles_by_user[role.user.id].add(role)
+            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]
+            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:
-            roles_by_user[user.id] = set()
+            roles_by_user[user.id] = {}
 
     @classmethod
     def get_user_roles(cls, user):
@@ -83,15 +117,32 @@ def get_user_roles(cls, user):
 
 class RoleCache:
     """
-    A cache of the CourseAccessRoles 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.
+        It contains all roles for a user as a dict that's keyed by course_id.
+        The key ROLE_CACHE_UNGROUPED_ROLES__KEY is used for all roles
+        that are not associated with a course.
+    _roles: This is a set of all roles for a user, ungrouped. It's used for some types of
+        lookups and collected from _roles_by_course_id on initialization
+        so that it doesn't need to be recalculated.
+
     """
     def __init__(self, user):
         try:
-            self._roles = BulkRoleCache.get_user_roles(user)
+            self._roles_by_course_id = BulkRoleCache.get_user_roles(user)
         except KeyError:
-            self._roles = set(
-                CourseAccessRole.objects.filter(user=user).all()
-            )
+            self._roles_by_course_id = {}
+            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()
+                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):
@@ -100,16 +151,24 @@ def get_roles(role):
         """
         return ACCESS_ROLES_INHERITANCE.get(role, set()) | {role}
 
+    @property
+    def all_roles_set(self):
+        return self._roles
+
+    @property
+    def roles_by_course_id(self):
+        return self._roles_by_course_id
+
     def has_role(self, role, course_id, org):
         """
         Return whether this RoleCache contains a role with the specified role
         or a role that inherits from the specified role, course_id and org.
         """
+        course_id_string = get_role_cache_key_for_course(course_id)
+        course_roles = self._roles_by_course_id.get(course_id_string, [])
         return any(
-            access_role.role in self.get_roles(role) and
-            access_role.course_id == course_id and
-            access_role.org == org
-            for access_role in self._roles
+            access_role.role in self.get_roles(role) and access_role.org == org
+            for access_role in course_roles
         )