fix: UnboundLocalError in BulkRoleCache.prefetch for authz compat roles

common/djangoapps/student/roles.py

@@ -329,7 +329,7 @@ def prefetch(cls, users):  # pylint: disable=missing-function-docstring
             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:

common/djangoapps/student/tests/test_roles.py

@@ -7,6 +7,7 @@
 import ddt
 from django.contrib.auth.models import Permission
 from django.test import TestCase
+from edx_django_utils.cache import RequestCache
 from edx_toggles.toggles.testutils import override_waffle_flag
 from opaque_keys.edx.keys import CourseKey
 from opaque_keys.edx.locator import LibraryLocator
@@ -32,6 +33,7 @@
 from common.djangoapps.student.roles import (
     ROLE_CACHE_UNGROUPED_ROLES__KEY,
     AuthzCompatCourseAccessRole,
+    BulkRoleCache,
     CourseAccessRole,
     CourseBetaTesterRole,
     CourseCreatorRole,
@@ -618,6 +620,91 @@ def test_role_cache_roles_by_course_id(self):
         assert roles_dict.get('course-v1:edX+toy2+2013_Fall').pop().course_id.course == 'toy2'
 
 
+class BulkRoleCachePrefetchTestCase(TestCase):
+    """
+    Regression tests for BulkRoleCache.prefetch mixing legacy CourseAccessRole rows
+    with openedx-authz compat roles. The authz loop used to add the previous
+    iteration's `compat_role` (left over from the legacy loop above it) instead of
+    its own `role`, raising UnboundLocalError when no legacy role existed at all,
+    and silently caching the wrong role when one did.
+
+    These mock get_authz_compat_course_access_roles_for_user directly rather than
+    assigning real AuthZ roles, since that goes through the global casbin enforcer,
+    whose in-memory policy isn't rolled back between tests the way the database is.
+
+    prefetch() also stores its result in a RequestCache, which outside of an actual
+    request (as here) isn't cleared automatically between tests, so it's cleared in
+    tearDown to avoid leaking cached roles into unrelated tests that construct a
+    RoleCache for a user this class never prefetched.
+    """
+
+    def setUp(self):
+        super().setUp()
+        self.course_key = CourseKey.from_string('course-v1:edX+toy+2012_Fall')
+
+    def tearDown(self):
+        super().tearDown()
+        RequestCache(BulkRoleCache.CACHE_NAMESPACE).clear()
+
+    def test_prefetch_with_only_authz_roles_does_not_crash(self):
+        """
+        A user with an authz compat role and no legacy CourseAccessRole entry used to
+        crash prefetch with UnboundLocalError, since compat_role was never assigned.
+        """
+        user = UserFactory()
+        authz_role = AuthzCompatCourseAccessRole(
+            user_id=user.id,
+            username=user.username,
+            course_id=str(self.course_key),
+            org=self.course_key.org,
+            role='staff',
+        )
+
+        with patch(
+            'common.djangoapps.student.roles.get_authz_compat_course_access_roles_for_user',
+            return_value={authz_role},
+        ):
+            BulkRoleCache.prefetch([user])
+
+        cached_roles = BulkRoleCache.get_user_roles(user)
+        assert cached_roles[get_role_cache_key_for_course(self.course_key)] == {authz_role}
+
+    def test_prefetch_with_legacy_and_authz_roles_for_different_users(self):
+        """
+        Each user's authz compat role must be cached as itself, not as whichever
+        legacy CourseAccessRole happened to be the last one scanned by the loop above.
+        """
+        legacy_user = UserFactory()
+        CourseAccessRole.objects.create(
+            user=legacy_user, org=self.course_key.org, course_id=self.course_key, role='staff',
+        )
+
+        authz_user = UserFactory()
+        authz_role = AuthzCompatCourseAccessRole(
+            user_id=authz_user.id,
+            username=authz_user.username,
+            course_id=str(self.course_key),
+            org=self.course_key.org,
+            role='instructor',
+        )
+
+        def fake_authz_roles(user):
+            return {authz_role} if user.id == authz_user.id else set()
+
+        with patch(
+            'common.djangoapps.student.roles.get_authz_compat_course_access_roles_for_user',
+            side_effect=fake_authz_roles,
+        ):
+            BulkRoleCache.prefetch([legacy_user, authz_user])
+
+        course_cache_key = get_role_cache_key_for_course(self.course_key)
+        legacy_roles = BulkRoleCache.get_user_roles(legacy_user)[course_cache_key]
+        authz_roles = BulkRoleCache.get_user_roles(authz_user)[course_cache_key]
+
+        assert {r.role for r in legacy_roles} == {'staff'}
+        assert authz_roles == {authz_role}
+
+
 class CourseAccessRoleHistoryTest(TestCase):
     """
     Tests for the CourseAccessRoleHistory model and associated signals/admin actions.