From 89bd9b7968ac89bdb2d7ed8f889fb2a93aa6e944 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Tue, 14 Jan 2025 15:11:37 -0500 Subject: [PATCH] fix(auditLogs): correctly serialize audit logs from deleted users (#5418) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### đŸ“Ŗ Summary Fixes a 500 error from the various audit log endpoints when there are actions by deleted users. ### 📖 Description Return empty user and username fields in the response if the user was deleted after the log was created. This applies to `/api/v2/audit-logs`, `api/v2/assets//history`, and `api/v2/project-history-logs`. ### 💭 Notes Small fix in the serializer. Also updates the ProjectHistoryLog serializer to inherit from the AuditLogSerializer so we don't have to duplicate the method fields. ### 👀 Preview steps Bug template: 1. ℹī¸ have a super user account and a project 2. Create a new user (user1) and give them the `Edit Form` permission on the project. 3. Log in as user1 and make an edit to the project. 4. Log out user1 and log back in as the super user 5. Delete user1. You can do this from the admin page if you delete the user from the User list, then from the Trash Bin. 6. Go to: a. `api/v2/audit-logs` b. `api/v2/project-history-logs` c. `api/v2/assets//history` 7. 🔴 [on main] All will return a 500 error (`AttributeError: 'NoneType' object has no attribute 'username'`) 8. đŸŸĸ [on PR] The endpoint will return the expected logs. For all user1's actions, the user and username fields will be empty. but the user_uid should still refer to the old user. --- kobo/apps/audit_log/serializers.py | 17 ++-------------- .../tests/api/v2/test_api_audit_log.py | 20 +++++++++++++++++++ 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/kobo/apps/audit_log/serializers.py b/kobo/apps/audit_log/serializers.py index 5307018584..4130e47279 100644 --- a/kobo/apps/audit_log/serializers.py +++ b/kobo/apps/audit_log/serializers.py @@ -45,7 +45,7 @@ def get_date_created(self, audit_log): return audit_log.date_created.strftime('%Y-%m-%dT%H:%M:%SZ') def get_username(self, audit_log): - return audit_log.user.username + return getattr(audit_log.user, 'username', None) class AccessLogSerializer(serializers.Serializer): @@ -66,14 +66,7 @@ def get_date_created(self, audit_log): return audit_log['date_created'].strftime('%Y-%m-%dT%H:%M:%SZ') -class ProjectHistoryLogSerializer(serializers.ModelSerializer): - user = serializers.HyperlinkedRelatedField( - queryset=get_user_model().objects.all(), - lookup_field='username', - view_name='user-kpi-detail', - ) - date_created = serializers.SerializerMethodField() - username = serializers.SerializerMethodField() +class ProjectHistoryLogSerializer(AuditLogSerializer): class Meta: model = ProjectHistoryLog @@ -94,9 +87,3 @@ class Meta: 'metadata', 'date_created', ) - - def get_date_created(self, audit_log): - return audit_log.date_created.strftime('%Y-%m-%dT%H:%M:%SZ') - - def get_username(self, audit_log): - return audit_log.user.username diff --git a/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py b/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py index ff73c25cd1..b0a0d9f9d0 100644 --- a/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py +++ b/kobo/apps/audit_log/tests/api/v2/test_api_audit_log.py @@ -263,6 +263,26 @@ def test_filter_list(self): assert response.data['count'] == 1 assert response.data['results'] == expected + def test_view_log_from_deleted_user(self): + someuser = get_user_model().objects.get(username='someuser') + date_created = timezone.now().strftime('%Y-%m-%dT%H:%M:%SZ') + AuditLog.objects.create( + user=someuser, + app_label='foo', + model_name='bar', + object_id=1, + date_created=date_created, + action=AuditAction.UPDATE, + log_type=AuditType.DATA_EDITING, + ) + someuser.delete() + self.login_user(username='adminuser', password='pass') + response = self.client.get(self.url) + assert response.status_code == status.HTTP_200_OK + assert response.data['count'] == 1 + assert response.data['results'][0]['username'] is None + assert response.data['results'][0]['user'] is None + class ApiAccessLogTestCase(BaseAuditLogTestCase):