Skip to content

Commit 5de2b7a

Browse files
committed
squash!: Apply suggestions
1 parent 7638992 commit 5de2b7a

2 files changed

Lines changed: 30 additions & 9 deletions

File tree

openedx_authz/rest_api/v1/views.py

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -593,7 +593,7 @@ def get_queryset(self) -> QuerySet:
593593
class AdminConsoleScopesAPIView(generics.ListAPIView):
594594
"""
595595
API view for listing scopes
596-
This API is used on the filters and assigna roles functionality on the Admin Console.
596+
This API is used on the filters and assign roles functionality on the Admin Console.
597597
598598
**Endpoints**
599599
@@ -604,7 +604,7 @@ class AdminConsoleScopesAPIView(generics.ListAPIView):
604604
- search (Optional): Search term to filter scopes by display name
605605
- page (Optional): Page number for pagination
606606
- page_size (Optional): Number of items per page
607-
- management_permission_only (Optional): Filter scopes either by only the ones to wich the user has "manage team"
607+
- management_permission_only (Optional): Filter scopes either by only the ones to which the user has "manage team"
608608
permissions (if true), or just "view team" permissions.
609609
610610
**Response Format**
@@ -654,7 +654,7 @@ class AdminConsoleScopesAPIView(generics.ListAPIView):
654654

655655
def get_serializer_context(self):
656656
context = super().get_serializer_context()
657-
context["org_map"] = {org.short_name: org for org in Organization.objects.all()}
657+
context["org_map"] = Organization.objects.filter(active=True).in_bulk(field_name="short_name")
658658
return context
659659

660660
def _get_courses_queryset(self, allowed_ids: set | None = None, search: str = "") -> QuerySet:
@@ -682,9 +682,10 @@ def _get_libraries_queryset(self, allowed_pairs: set | None = None, search: str
682682
"""
683683
qs = ContentLibrary.objects
684684
if allowed_pairs is not None:
685-
qs = qs.filter(
686-
reduce(operator.or_, (Q(org__short_name=org, slug=slug) for org, slug in allowed_pairs), Q())
687-
)
685+
if not allowed_pairs:
686+
qs = qs.none()
687+
else:
688+
qs = qs.filter(reduce(operator.or_, (Q(org__short_name=org, slug=slug) for org, slug in allowed_pairs)))
688689
if search:
689690
qs = qs.filter(learning_package__title__icontains=search)
690691
return qs.annotate(

openedx_authz/tests/rest_api/test_views.py

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -957,9 +957,12 @@ def setUp(self):
957957
def stub_get_libraries_queryset(_, allowed_pairs=None, search=""): # pylint: disable=unused-argument
958958
qs = ContentLibrary.objects
959959
if allowed_pairs is not None:
960-
qs = qs.filter(
961-
reduce(operator.or_, (Q(org__short_name=org, slug=slug) for org, slug in allowed_pairs), Q())
962-
)
960+
if not allowed_pairs:
961+
qs = qs.none()
962+
else:
963+
qs = qs.filter(
964+
reduce(operator.or_, (Q(org__short_name=org, slug=slug) for org, slug in allowed_pairs))
965+
)
963966
return qs.annotate(
964967
scope_id=Cast("slug", output_field=CharField()),
965968
org_name=Cast("org__short_name", output_field=CharField()),
@@ -1214,6 +1217,23 @@ def test_manage_permission_filters_libraries_for_non_staff(self):
12141217
self.assertIn("lib:Org3:LIB3", external_keys)
12151218
self.assertNotIn(self.LIBRARY_ORG1, external_keys)
12161219

1220+
def test_empty_allowed_library_pairs_returns_no_libraries(self):
1221+
"""When a non-staff user has no allowed libraries, no libraries are returned.
1222+
1223+
Regression test: an empty allowed_pairs set must not bypass the filter
1224+
and return all libraries (reduce with Q() default was a no-op).
1225+
"""
1226+
# regular_9 has no library permissions, only a course role.
1227+
user = User.objects.get(username="regular_9")
1228+
self.client.force_authenticate(user=user)
1229+
self.build_qs_patcher.stop()
1230+
1231+
response = self.client.get(self.url, {"type": "library"})
1232+
1233+
self.build_qs_patcher.start()
1234+
self.assertEqual(response.status_code, status.HTTP_200_OK)
1235+
self.assertEqual(response.data["count"], 0)
1236+
12171237
def test_manage_permission_only_uses_manage_permission(self):
12181238
"""management_permission_only=true calls get_admin_manage_permission, not get_admin_view_permission."""
12191239
user = User.objects.get(username="regular_1")

0 commit comments

Comments
 (0)