Merge branch '220523-refactor-audit-controllers' into 'master'

Consolidate audit event variables in rendered views

Closes #220523

See merge request gitlab-org/gitlab!39600

ee/app/controllers/admin/audit_logs_controller.rb

 # frozen_string_literal: true
 
 class Admin::AuditLogsController < Admin::ApplicationController
+  include Gitlab::Utils::StrongMemoize
   include AuditEvents::EnforcesValidDateParams
   include AuditEvents::AuditLogsParams
   include AuditEvents::Sortable
@@ -13,8 +14,8 @@ class Admin::AuditLogsController < Admin::ApplicationController
   PER_PAGE = 25
 
   def index
-    @events = audit_log_events
-    @table_events = AuditEventSerializer.new.represent(@events)
+    @is_last_page = events.last_page?
+    @events = AuditEventSerializer.new.represent(events)
 
     @entity = case audit_logs_params[:entity_type]
               when 'User'
@@ -30,13 +31,19 @@ class Admin::AuditLogsController < Admin::ApplicationController
 
   private
 
-  def audit_log_events
+  def events
+    strong_memoize(:events) do
       level = Gitlab::Audit::Levels::Instance.new
-    events = AuditLogFinder.new(level: level, params: audit_logs_params).execute
-    events = events.page(params[:page]).per(PER_PAGE).without_count
+      events = AuditLogFinder
+        .new(level: level, params: audit_logs_params)
+        .execute
+        .page(params[:page])
+        .per(PER_PAGE)
+        .without_count
 
       Gitlab::Audit::Events::Preloader.preload!(events)
     end
+  end
 
   def check_license_admin_audit_log_available!
     render_404 unless License.feature_available?(:admin_audit_log)

ee/app/controllers/groups/audit_events_controller.rb

 # frozen_string_literal: true
 
 class Groups::AuditEventsController < Groups::ApplicationController
+  include Gitlab::Utils::StrongMemoize
   include AuditEvents::EnforcesValidDateParams
   include AuditEvents::AuditLogsParams
   include AuditEvents::Sortable
@@ -14,21 +15,29 @@ class Groups::AuditEventsController < Groups::ApplicationController
   layout 'group_settings'
 
   def index
-    level = Gitlab::Audit::Levels::Group.new(group: group)
-    # This is an interim change until we have proper API support within Audit Events
-    audit_params = transform_author_entity_type(audit_logs_params)
+    @is_last_page = events.last_page?
+    @events = AuditEventSerializer.new.represent(events)
+  end
+
+  private
 
+  def events
+    strong_memoize(:events) do
+      level = Gitlab::Audit::Levels::Group.new(group: group)
       events = AuditLogFinder
         .new(level: level, params: audit_params)
         .execute
         .page(params[:page])
         .without_count
 
-    @events = Gitlab::Audit::Events::Preloader.preload!(events)
-    @table_events = AuditEventSerializer.new.represent(@events)
+      Gitlab::Audit::Events::Preloader.preload!(events)
+    end
   end
 
-  private
+  def audit_params
+    # This is an interim change until we have proper API support within Audit Events
+    transform_author_entity_type(audit_logs_params)
+  end
 
   def transform_author_entity_type(params)
     return params unless params[:entity_type] == 'Author'

ee/app/controllers/projects/audit_events_controller.rb

 # frozen_string_literal: true
 
 class Projects::AuditEventsController < Projects::ApplicationController
+  include Gitlab::Utils::StrongMemoize
   include LicenseHelper
   include AuditEvents::EnforcesValidDateParams
   include AuditEvents::AuditLogsParams
@@ -12,24 +13,32 @@ class Projects::AuditEventsController < Projects::ApplicationController
   layout 'project_settings'
 
   def index
-    level = Gitlab::Audit::Levels::Project.new(project: project)
-    # This is an interim change until we have proper API support within Audit Events
-    audit_params = transform_author_entity_type(audit_logs_params)
+    @is_last_page = events.last_page?
+    @events = AuditEventSerializer.new.represent(events)
+  end
+
+  private
+
+  def check_audit_events_available!
+    render_404 unless @project.feature_available?(:audit_events) || LicenseHelper.show_promotions?(current_user)
+  end
 
+  def events
+    strong_memoize(:events) do
+      level = Gitlab::Audit::Levels::Project.new(project: project)
       events = AuditLogFinder
         .new(level: level, params: audit_params)
         .execute
         .page(params[:page])
         .without_count
 
-    @events = Gitlab::Audit::Events::Preloader.preload!(events)
-    @table_events = AuditEventSerializer.new.represent(@events)
+      Gitlab::Audit::Events::Preloader.preload!(events)
+    end
   end
 
-  private
-
-  def check_audit_events_available!
-    render_404 unless @project.feature_available?(:audit_events) || LicenseHelper.show_promotions?(current_user)
+  def audit_params
+    # This is an interim change until we have proper API support within Audit Events
+    transform_author_entity_type(audit_logs_params)
   end
 
   def transform_author_entity_type(params)

ee/app/views/admin/audit_logs/index.html.haml

 - page_title _('Audit Log')
 
 #js-audit-log-app{ data: { form_path: admin_audit_logs_path,
-  events: @table_events.to_json,
-  is_last_page: @events.last_page?.to_json,
+  events: @events.to_json,
+  is_last_page: @is_last_page.to_json,
   filter_qa_selector: 'admin_audit_log_filter',
   table_qa_selector: 'admin_audit_log_table',
   filter_token_options: admin_audit_event_tokens.to_json } }

ee/app/views/groups/audit_events/index.html.haml

@@ -4,8 +4,8 @@
 %p.light= _('Group-level events in %{group_name} (no project-level events)') % { group_name: @group.name }
 
 #js-group-audit-events-app{ data: { form_path: group_audit_events_path(@group),
-  events: @table_events.to_json,
-  is_last_page: @events.last_page?.to_json,
+  events: @events.to_json,
+  is_last_page: @is_last_page.to_json,
   filter_qa_selector: 'group_audit_log_filter',
   table_qa_selector: 'group_audit_log_table',
   filter_token_options: group_audit_event_tokens(@group.id).to_json } }

ee/app/views/projects/audit_events/index.html.haml

@@ -8,8 +8,8 @@
     = _('Events in %{project_path}') % { project_path: @project.full_path }
 
   #js-project-audit-events-app{ data: { form_path: project_audit_events_path(@project),
-  events: @table_events.to_json,
-  is_last_page: @events.last_page?.to_json,
+  events: @events.to_json,
+  is_last_page: @is_last_page.to_json,
   filter_qa_selector: 'project_audit_log_filter',
   table_qa_selector: 'project_audit_log_table',
   filter_token_options: project_audit_event_tokens(@project.full_path).to_json } }

ee/spec/controllers/admin/audit_logs_controller_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe Admin::AuditLogsController do
-  let(:admin) { create(:admin) }
+  let_it_be(:admin) { create(:admin) }
 
   describe 'GET #index' do
     before do
@@ -19,9 +19,12 @@ RSpec.describe Admin::AuditLogsController do
         it 'paginates audit events, without casting a count query' do
           create(:user_audit_event, created_at: 5.days.ago)
 
+          serializer = instance_spy(AuditEventSerializer)
+          allow(AuditEventSerializer).to receive(:new).and_return(serializer)
+
           get :index, params: { 'entity_type': 'User' }
 
-          expect(assigns(:events)).to be_kind_of(Kaminari::PaginatableWithoutCount)
+          expect(serializer).to have_received(:represent).with(kind_of(Kaminari::PaginatableWithoutCount))
         end
       end
 

ee/spec/controllers/groups/audit_events_controller_spec.rb

@@ -3,9 +3,10 @@
 require 'spec_helper'
 
 RSpec.describe Groups::AuditEventsController do
-  let(:user) { create(:user) }
-  let(:owner) { create(:user) }
-  let(:group) { create(:group, :private) }
+  let_it_be(:user) { create(:user) }
+  let_it_be(:owner) { create(:user) }
+  let_it_be(:group) { create(:group, :private) }
+  let_it_be(:events) { create_list(:group_audit_event, 5, entity_id: group.id) }
 
   describe 'GET #index' do
     let(:sort) { nil }
@@ -74,12 +75,11 @@ RSpec.describe Groups::AuditEventsController do
               it 'orders by id descending' do
                 request
 
-                expect(assigns(:events)).to eq(group.audit_events.order(id: :desc))
-              end
-            end
+                actual_event_ids = assigns(:events).map { |event| event[:id] }
+                expected_event_ids = events.map(&:id).reverse
 
-            before do
-              create_list(:group_audit_event, 5, entity_id: group.id)
+                expect(actual_event_ids).to eq(expected_event_ids)
+              end
             end
 
             context 'when no sort order is specified' do
@@ -98,7 +98,10 @@ RSpec.describe Groups::AuditEventsController do
               it 'orders by id ascending' do
                 request
 
-                expect(assigns(:events)).to eq(group.audit_events.order(id: :asc))
+                actual_event_ids = assigns(:events).map { |event| event[:id] }
+                expected_event_ids = events.map(&:id)
+
+                expect(actual_event_ids).to eq(expected_event_ids)
               end
             end
 
@@ -110,10 +113,19 @@ RSpec.describe Groups::AuditEventsController do
           end
 
           context 'pagination' do
+            it 'sets instance variables' do
+              request
+
+              expect(assigns(:is_last_page)).to be(true)
+            end
+
             it 'paginates audit events, without casting a count query' do
+              serializer = instance_spy(AuditEventSerializer)
+              allow(AuditEventSerializer).to receive(:new).and_return(serializer)
+
               request
 
-              expect(assigns(:events)).to be_kind_of(Kaminari::PaginatableWithoutCount)
+              expect(serializer).to have_received(:represent).with(kind_of(Kaminari::PaginatableWithoutCount))
             end
           end
         end

ee/spec/controllers/projects/audit_events_controller_spec.rb

@@ -3,9 +3,10 @@
 require 'spec_helper'
 
 RSpec.describe Projects::AuditEventsController do
-  let(:user) { create(:user) }
-  let(:maintainer) { create(:user) }
-  let(:project) { create(:project, :private) }
+  let_it_be(:user) { create(:user) }
+  let_it_be(:maintainer) { create(:user) }
+  let_it_be(:project) { create(:project, :private) }
+  let_it_be(:events) { create_list(:project_audit_event, 5, entity_id: project.id) }
 
   describe 'GET #index' do
     let(:sort) { nil }
@@ -73,12 +74,11 @@ RSpec.describe Projects::AuditEventsController do
             it 'orders by id descending' do
               request
 
-              expect(assigns(:events)).to eq(project.audit_events.order(id: :desc))
-            end
-          end
+              actual_event_ids = assigns(:events).map { |event| event[:id] }
+              expected_event_ids = events.map(&:id).reverse
 
-          before do
-            create_list(:project_audit_event, 5, entity_id: project.id)
+              expect(actual_event_ids).to eq(expected_event_ids)
+            end
           end
 
           context 'when no sort order is specified' do
@@ -97,7 +97,10 @@ RSpec.describe Projects::AuditEventsController do
             it 'orders by id ascending' do
               request
 
-              expect(assigns(:events)).to eq(project.audit_events.order(id: :asc))
+              actual_event_ids = assigns(:events).map { |event| event[:id] }
+              expected_event_ids = events.map(&:id)
+
+              expect(actual_event_ids).to eq(expected_event_ids)
             end
           end
 
@@ -110,10 +113,19 @@ RSpec.describe Projects::AuditEventsController do
       end
 
       context 'pagination' do
+        it 'sets instance variables' do
+          request
+
+          expect(assigns(:is_last_page)).to be(true)
+        end
+
         it 'paginates audit events, without casting a count query' do
+          serializer = instance_spy(AuditEventSerializer)
+          allow(AuditEventSerializer).to receive(:new).and_return(serializer)
+
           request
 
-          expect(assigns(:events)).to be_kind_of(Kaminari::PaginatableWithoutCount)
+          expect(serializer).to have_received(:represent).with(kind_of(Kaminari::PaginatableWithoutCount))
         end
       end