squash!: Filter by active users

Author: rodmgwgu

openedx_authz/api/utils.py

@@ -27,7 +27,7 @@ def get_user_map(usernames: list[str]) -> dict[str, User]:
         dict[str, User]: Dictionary mapping each username to its corresponding User object.
             Only users that exist in the database are included in the returned dictionary.
     """
-    users = User.objects.filter(username__in=usernames).select_related("profile")
+    users = User.objects.filter(username__in=usernames, is_active=True).select_related("profile")
     return {user.username: user for user in users}
 
 

openedx_authz/tests/rest_api/test_views.py

@@ -26,17 +26,6 @@
 User = get_user_model()
 
 
-def get_user_map_without_profile(usernames: list[str]) -> dict[str, User]:
-    """
-    Test version of ``get_user_map`` that doesn't use select_related('profile').
-
-    The generic Django User model doesn't have a profile relation,
-    so we override this in tests to avoid FieldError.
-    """
-    users = User.objects.filter(username__in=usernames)
-    return {user.username: user for user in users}
-
-
 class ViewTestMixin(BaseRolesTestCase):
     """Mixin providing common test utilities for view tests."""
 
@@ -322,11 +311,6 @@ def setUp(self):
         super().setUp()
         self.client.force_authenticate(user=self.admin_user)
         self.url = reverse("openedx_authz:role-user-list")
-        self.get_user_map_patcher = patch(
-            "openedx_authz.rest_api.v1.views.get_user_map",
-            side_effect=get_user_map_without_profile,
-        )
-        self.get_user_map_patcher.start()
 
     @data(
         # All users
@@ -1076,12 +1060,6 @@ def setUp(self):
         """Set up test fixtures."""
         super().setUp()
         self.url = reverse("openedx_authz:user-list")
-        self.get_user_map_patcher = patch(
-            "openedx_authz.api.utils.get_user_map",
-            side_effect=get_user_map_without_profile,
-        )
-        self.get_user_map_patcher.start()
-        self.addCleanup(self.get_user_map_patcher.stop)
 
     # -------------------------------------------------------------------- #
     # Visibility: calling user only sees assignments it has view access to #
@@ -1340,16 +1318,6 @@ class TestTeamMemberAssignmentsAPIView(ViewTestMixin):
           (requires at least one VIEW_LIBRARY_TEAM or COURSES_VIEW_COURSE_TEAM permission).
     """
 
-    def setUp(self):
-        """Set up test fixtures."""
-        super().setUp()
-        self.get_user_map_patcher = patch(
-            "openedx_authz.api.utils.get_user_map",
-            side_effect=get_user_map_without_profile,
-        )
-        self.get_user_map_patcher.start()
-        self.addCleanup(self.get_user_map_patcher.stop)
-
     def _url(self, username: str) -> str:
         return reverse("openedx_authz:user-assignment-list", kwargs={"username": username})
 
@@ -2102,12 +2070,6 @@ def setUp(self):
         """Set up test fixtures."""
         super().setUp()
         self.url = reverse("openedx_authz:assignment-list")
-        self.get_user_map_patcher = patch(
-            "openedx_authz.api.utils.get_user_map",
-            side_effect=get_user_map_without_profile,
-        )
-        self.get_user_map_patcher.start()
-        self.addCleanup(self.get_user_map_patcher.stop)
 
     # -------------------------------------------------------------------- #
     # Visibility: calling user only sees assignments it has view access to #
@@ -2585,6 +2547,44 @@ def test_combined_scope_and_search(self):
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         self.assertEqual(response.data["count"], 2)
 
+    # ------------------------------------------------------------------ #
+    # Active user filtering                                              #
+    # ------------------------------------------------------------------ #
+
+    def test_inactive_users_excluded_from_results(self):
+        """Role assignments for inactive users are not included in results.
+
+        Deactivating a user (is_active=False) should remove their role assignments
+        from the response, even though the assignments still exist in the database.
+        Superadmin entries are also excluded for inactive staff/superusers.
+
+        Expected result:
+            - Returns 200 OK.
+            - The inactive user's assignments do not appear in the results.
+            - The total count decreases by the number of assignments the inactive user had.
+        """
+        # Baseline: admin_1 (staff) sees all 14 rows (3 superadmin + 11 role assignments)
+        baseline_response = self.client.get(self.url)
+        self.assertEqual(baseline_response.status_code, status.HTTP_200_OK)
+        baseline_count = baseline_response.data["count"]
+
+        # Deactivate regular_1, who has 1 role assignment in lib:Org1:LIB1
+        inactive_user = User.objects.get(username="regular_1")
+        inactive_user.is_active = False
+        inactive_user.save()
+        try:
+            response = self.client.get(self.url)
+
+            self.assertEqual(response.status_code, status.HTTP_200_OK)
+            # regular_1 had 1 role assignment → count should drop by 1
+            self.assertEqual(response.data["count"], baseline_count - 1)
+            # Confirm regular_1 is not in the results
+            usernames = {item["username"] for item in response.data["results"]}
+            self.assertNotIn("regular_1", usernames)
+        finally:
+            inactive_user.is_active = True
+            inactive_user.save()
+
 
 @ddt
 class TestAssignmentsAPIViewPermissions(ViewTestMixin):
@@ -2638,12 +2638,6 @@ def setUp(self):
         """Set up test fixtures."""
         super().setUp()
         self.url = reverse("openedx_authz:assignment-list")
-        self.get_user_map_patcher = patch(
-            "openedx_authz.api.utils.get_user_map",
-            side_effect=get_user_map_without_profile,
-        )
-        self.get_user_map_patcher.start()
-        self.addCleanup(self.get_user_map_patcher.stop)
 
     # ------------------------------------------------------------------ #
     # Superadmin caller                                                  #

openedx_authz/tests/stubs/migrations/0002_userprofile.py

@@ -0,0 +1,38 @@
+"""Migration to add the UserProfile stub model."""
+
+import django.db.models.deletion
+from django.conf import settings
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("stubs", "0001_initial"),
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name="UserProfile",
+            fields=[
+                (
+                    "id",
+                    models.BigAutoField(
+                        auto_created=True,
+                        primary_key=True,
+                        serialize=False,
+                        verbose_name="ID",
+                    ),
+                ),
+                ("name", models.CharField(blank=True, default="", max_length=255)),
+                (
+                    "user",
+                    models.OneToOneField(
+                        on_delete=django.db.models.deletion.CASCADE,
+                        related_name="profile",
+                        to=settings.AUTH_USER_MODEL,
+                    ),
+                ),
+            ],
+        ),
+    ]

openedx_authz/tests/stubs/models.py

@@ -13,6 +13,26 @@
 from organizations.models import Organization
 
 
+class UserProfile(models.Model):
+    """Stub model mimicking the Open edX UserProfile for testing purposes.
+
+    Provides the ``profile`` reverse relation that ``select_related('profile')``
+    expects on the User model, along with the ``name`` field used by serializers.
+
+    .. no_pii:
+    """
+
+    user = models.OneToOneField(
+        settings.AUTH_USER_MODEL,
+        on_delete=models.CASCADE,
+        related_name="profile",
+    )
+    name = models.CharField(max_length=255, blank=True, default="")
+
+    def __str__(self):
+        return f"UserProfile({self.user.username})"
+
+
 class ContentLibraryManager(models.Manager):
     """Manager for ContentLibrary model with helper methods."""