Merge branch 'release/2.025.43'

Author: kobo-bot[bot]

kobo/apps/trash_bin/tests/test_utils.py

@@ -3,10 +3,10 @@
 
 from constance import config
 from ddt import data, ddt, unpack
-from django.db.models.signals import pre_delete
-from django.core.management import call_command
 from django.conf import settings
 from django.contrib.auth import get_user_model
+from django.core.management import call_command
+from django.db.models.signals import pre_delete
 from django.test import TestCase
 from django.utils import timezone
 from django_celery_beat.models import PeriodicTask
@@ -16,26 +16,20 @@
     AuditAction,
     AuditLog,
     AuditType,
-    ProjectHistoryLog
+    ProjectHistoryLog,
 )
 from kobo.apps.kobo_auth.shortcuts import User
 from kobo.apps.openrosa.apps.logger.models import Attachment, Instance, XForm
 from kobo.apps.openrosa.apps.logger.models.attachment import AttachmentDeleteStatus
 from kobo.apps.openrosa.apps.logger.signals import pre_delete_attachment
-
 from kpi.models import Asset
 from kpi.tests.mixins.create_asset_and_submission_mixin import AssetSubmissionTestMixin
 from ..constants import DELETE_PROJECT_STR_PREFIX, DELETE_USER_STR_PREFIX
 from ..models import TrashStatus
 from ..models.account import AccountTrash
 from ..models.attachment import AttachmentTrash
 from ..models.project import ProjectTrash
-from ..tasks import (
-    empty_account,
-    empty_attachment,
-    empty_project,
-    task_restarter,
-)
+from ..tasks import empty_account, empty_attachment, empty_project, task_restarter
 from ..utils import move_to_trash, put_back, trash_bin_task_failure
 
 
@@ -174,6 +168,7 @@ def test_remove_user(self):
         everything from their account is deleted except their username
         """
         someuser = get_user_model().objects.get(username='someuser')
+        uid = someuser.extra_details.uid
         admin = get_user_model().objects.get(username='adminuser')
         someuser.extra_details.data['name'] = 'someuser'
         someuser.extra_details.save(update_fields=['data'])
@@ -207,6 +202,7 @@ def test_remove_user(self):
 
         assert not AccountTrash.objects.filter(user=someuser).exists()
         assert before <= someuser.extra_details.date_removed <= after
+        assert someuser.extra_details.uid == uid
 
         # Ensure action is logged
         assert AuditLog.objects.filter(

kobo/apps/trash_bin/utils/account.py

@@ -2,7 +2,7 @@
 
 from django.conf import settings
 from django.contrib.auth import get_user_model
-from django.db import models, transaction
+from django.db import transaction
 from django.db.models.signals import post_delete
 from django.utils import timezone
 
@@ -86,13 +86,14 @@ def delete_account(account_trash: AccountTrash):
 
                 if account_trash.retain_placeholder:
                     audit_log_params['action'] = AuditAction.REMOVE
-                    placeholder_user = _replace_user_with_placeholder(user)
+                    placeholder_user, uid = _replace_user_with_placeholder(user)
                     # Retain removal date information
                     extra_details = placeholder_user.extra_details
                     extra_details.date_removal_requested = date_removal_requested
                     extra_details.date_removed = timezone.now()
+                    extra_details.uid = uid
                     extra_details.save(
-                        update_fields=['date_removal_requested', 'date_removed']
+                        update_fields=['date_removal_requested', 'date_removed', 'uid']
                     )
                 else:
                     audit_log_params['action'] = AuditAction.DELETE
@@ -114,7 +115,7 @@ def delete_account(account_trash: AccountTrash):
 
 def _replace_user_with_placeholder(
     user: settings.AUTH_USER_MODEL, retain_audit_logs: bool = True
-) -> settings.AUTH_USER_MODEL:
+) -> tuple[settings.AUTH_USER_MODEL, str]:
     """
     Replace a user with an inactive placeholder, which prevents others from
     registering a new account with the same username. The placeholder uses the
@@ -130,26 +131,12 @@ def _replace_user_with_placeholder(
     for field in FIELDS_TO_RETAIN:
         setattr(placeholder_user, field, getattr(user, field))
 
-    if not retain_audit_logs:
+    uid = user.extra_details.uid
+    with transaction.atomic():
         user.delete()
         placeholder_user.save()
-        return placeholder_user
 
-    audit_log_user_field = AuditLog._meta.get_field('user').remote_field
-    original_audit_log_delete_handler = audit_log_user_field.on_delete
-    with transaction.atomic():
-        try:
-            # prevent the delete() call from touching the audit logs
-            audit_log_user_field.on_delete = models.DO_NOTHING
-            # …and cause a FK violation!
-            user.delete()
-            # then resolve the violation by creating the placeholder with the
-            # same PK as the original user
-            placeholder_user.save()
-        finally:
-            audit_log_user_field.on_delete = original_audit_log_delete_handler
-
-    return placeholder_user
+    return placeholder_user, uid
 
 
 def validate_pre_deletion(account_trash: AccountTrash):

kpi/views/v2/user.py

@@ -88,7 +88,6 @@ def get_queryset(self, *args, **kwargs):
             self.queryset = (
                 self.queryset.exclude(pk=settings.ANONYMOUS_USER_ID)
                 .select_related('extra_details')
-                .annotate(assets_count=Count('assets', distinct=True))
                 .order_by('id')
             )
 
@@ -106,6 +105,28 @@ def list(self, request, *args, **kwargs):
 
         return super().list(request, *args, **kwargs)
 
+    def paginate_queryset(self, queryset):
+        if not (page := super().paginate_queryset(queryset)):
+            return None
+
+        user_ids = (user.pk for user in page)
+        counts_map = {
+            asset['owner_id']: asset['assets_count']
+            for asset in (
+                Asset.objects.filter(owner_id__in=user_ids)
+                .values('owner_id')
+                .annotate(assets_count=Count('id'))
+            )
+        }
+
+        def _page_generator():
+            # Inject and yield on-the-fly assets count
+            for user in page:
+                user.assets_count = counts_map.get(user.pk, 0)
+                yield user
+
+        return _page_generator()
+
     @action(detail=True, methods=['GET'],
             url_path=r'migrate(?:/(?P<task_id>[\d\w\-]+))?')
     def migrate(self, request, task_id: str = None, **kwargs):