Merge branch '288822-audit-event-removed-user-bug' into 'master'

Fix batch query issue when primary key is -1

See merge request gitlab-org/gitlab!51716

app/models/audit_event.rb

@@ -55,15 +55,20 @@ class AuditEvent < ApplicationRecord
   end
 
   def author_name
-    lazy_author.name
+    author&.name
   end
 
   def formatted_details
     details.merge(details.slice(:from, :to).transform_values(&:to_s))
   end
 
+  def author
+    lazy_author&.itself.presence ||
+      ::Gitlab::Audit::NullAuthor.for(author_id, (self[:author_name] || details[:author_name]))
+  end
+
   def lazy_author
-    BatchLoader.for(author_id).batch(default_value: default_author_value, replace_methods: false) do |author_ids, loader|
+    BatchLoader.for(author_id).batch(replace_methods: false) do |author_ids, loader|
       User.select(:id, :name, :username).where(id: author_ids).find_each do |user|
         loader.call(user.id, user)
       end

changelogs/unreleased/288822-audit-event-removed-user-bug.yml

+---
+title: Fix batch query issue when primary key is -1
+merge_request: 51716
+author:
+type: fixed

ee/app/presenters/audit_event_presenter.rb

@@ -4,7 +4,7 @@ class AuditEventPresenter < Gitlab::View::Presenter::Simple
   presents :audit_event
 
   def author_name
-    author&.name
+    audit_event.author_name
   end
 
   def author_url
@@ -48,7 +48,7 @@ class AuditEventPresenter < Gitlab::View::Presenter::Simple
   private
 
   def author
-    @author ||= audit_event.lazy_author
+    audit_event.author
   end
 
   def entity

ee/spec/features/projects/audit_events_spec.rb

@@ -173,4 +173,22 @@ RSpec.describe 'Projects > Audit Events', :js do
 
     it_behaves_like 'audit events date filter'
   end
+
+  describe 'combined list of authenticated and unauthenticated users' do
+    let!(:audit_event_1) { create(:project_audit_event, :unauthenticated, entity_type: 'Project', entity_id: project.id, created_at: 5.days.ago) }
+    let!(:audit_event_2) { create(:project_audit_event, author_id: non_existing_record_id, entity_type: 'Project', entity_id: project.id, created_at: 3.days.ago) }
+    let!(:audit_event_3) { create(:project_audit_event, entity_type: 'Project', entity_id: project.id, created_at: Date.current) }
+
+    it 'displays the correct authors names' do
+      visit project_audit_events_path(project)
+
+      wait_for_all_requests
+
+      page.within('.audit-log-table') do
+        expect(page).to have_content('An unauthenticated user')
+        expect(page).to have_content("#{audit_event_2.author_name} (removed)")
+        expect(page).to have_content(audit_event_3.user.name)
+      end
+    end
+  end
 end

ee/spec/models/audit_event_spec.rb

@@ -277,4 +277,32 @@ RSpec.describe AuditEvent, type: :model do
       expect(event.formatted_details[:from]).to eq('false')
     end
   end
+
+  describe 'author' do
+    subject { event.author }
+
+    context 'when author exists' do
+      let_it_be(:event) { create(:project_audit_event) }
+
+      it 'returns the author object' do
+        expect(subject).to eq(User.find(event.author_id))
+      end
+    end
+
+    context 'when author is unauthenticated' do
+      let_it_be(:event) { create(:project_audit_event, :unauthenticated) }
+
+      it 'is an unauthenticated user' do
+        expect(subject).to be_a(Gitlab::Audit::UnauthenticatedAuthor)
+      end
+    end
+
+    context 'when author no longer exists' do
+      let_it_be(:event) { create(:project_audit_event, author_id: non_existing_record_id) }
+
+      it 'is a deleted user' do
+        expect(subject).to be_a(Gitlab::Audit::DeletedAuthor)
+      end
+    end
+  end
 end

spec/factories/audit_events.rb

@@ -49,6 +49,21 @@ FactoryBot.define do
       end
     end
 
+    trait :unauthenticated do
+      author_id { -1 }
+      details do
+        {
+          custom_message: 'Custom action',
+          author_name: 'An unauthenticated user',
+          target_id: target_project.id,
+          target_type: 'Project',
+          target_details: target_project.name,
+          ip_address: '127.0.0.1',
+          entity_path: target_project.full_path
+        }
+      end
+    end
+
     trait :group_event do
       transient { target_group { association(:group) } }