Merge eedca41412 into c442589593

app/controllers/admin/report_notes_controller.rb

@@ -22,7 +22,7 @@ module Admin
         redirect_to after_create_redirect_path, notice: I18n.t('admin.report_notes.created_msg')
       else
         @report_notes = @report.notes.chronological.includes(:account)
-        @action_logs  = @report.history.includes(:target)
+        @action_logs  = ReportHistoryPresenter.new(@report).logs.includes(:target)
         @form         = Admin::StatusBatchAction.new
         @statuses     = @report.statuses.with_includes
 

app/controllers/admin/reports_controller.rb

@@ -14,7 +14,7 @@ module Admin
 
       @report_note  = @report.notes.new
       @report_notes = @report.notes.chronological.includes(:account)
-      @action_logs  = @report.history.includes(:target)
+      @action_logs  = ReportHistoryPresenter.new(@report).logs.includes(:target)
       @form         = Admin::StatusBatchAction.new
       @statuses     = @report.statuses.with_includes
     end

app/models/report.rb

@@ -133,32 +133,6 @@ class Report < ApplicationRecord
     id
   end
 
-  def history
-    subquery = [
-      Admin::ActionLog.where(
-        target_type: 'Report',
-        target_id: id
-      ).arel,
-
-      Admin::ActionLog.where(
-        target_type: 'Account',
-        target_id: target_account_id
-      ).arel,
-
-      Admin::ActionLog.where(
-        target_type: 'Status',
-        target_id: status_ids
-      ).arel,
-
-      Admin::ActionLog.where(
-        target_type: 'AccountWarning',
-        target_id: AccountWarning.where(report_id: id).select(:id)
-      ).arel,
-    ].reduce { |union, query| Arel::Nodes::UnionAll.new(union, query) }
-
-    Admin::ActionLog.latest.from(Arel::Nodes::As.new(subquery, Admin::ActionLog.arel_table))
-  end
-
   private
 
   def set_uri

app/presenters/report_history_presenter.rb

@@ -0,0 +1,43 @@
+# frozen_string_literal: true
+
+class ReportHistoryPresenter
+  attr_reader :report
+
+  def initialize(report)
+    @report = report
+  end
+
+  def logs
+    Admin::ActionLog
+      .latest
+      .from(Arel::Nodes::As.new(subquery, Admin::ActionLog.arel_table))
+  end
+
+  private
+
+  def subquery
+    related_logs
+      .map(&:arel)
+      .reduce { |union, query| Arel::Nodes::UnionAll.new(union, query) }
+  end
+
+  def related_logs
+    [
+      logs_for('Report', report.id),
+      logs_for('Account', report.target_account_id),
+      logs_for('Status', report.status_ids),
+      logs_for('AccountWarning', report_account_warning_ids),
+    ]
+  end
+
+  def logs_for(target_type, target_id)
+    Admin::ActionLog
+      .where(target_type:, target_id:)
+  end
+
+  def report_account_warning_ids
+    AccountWarning
+      .where(report_id: report.id)
+      .select(:id)
+  end
+end

spec/models/report_spec.rb

@@ -102,38 +102,6 @@ RSpec.describe Report do
     end
   end
 
-  describe 'history' do
-    subject(:action_logs) { report.history }
-
-    let(:report) { Fabricate(:report, target_account_id: target_account.id, status_ids: [status.id]) }
-    let(:target_account) { Fabricate(:account) }
-    let(:status) { Fabricate(:status) }
-    let(:account_warning) { Fabricate(:account_warning, report_id: report.id) }
-
-    let!(:matched_type_account_warning) { Fabricate(:action_log, target_type: 'AccountWarning', target_id: account_warning.id) }
-    let!(:matched_type_account) { Fabricate(:action_log, target_type: 'Account', target_id: report.target_account_id) }
-    let!(:matched_type_report) { Fabricate(:action_log, target_type: 'Report', target_id: report.id) }
-    let!(:matched_type_status) { Fabricate(:action_log, target_type: 'Status', target_id: status.id) }
-
-    let!(:unmatched_type_account_warning) { Fabricate(:action_log, target_type: 'AccountWarning') }
-    let!(:unmatched_type_account) { Fabricate(:action_log, target_type: 'Account') }
-    let!(:unmatched_type_report) { Fabricate(:action_log, target_type: 'Report') }
-    let!(:unmatched_type_status) { Fabricate(:action_log, target_type: 'Status') }
-
-    it 'returns expected logs' do
-      expect(action_logs)
-        .to have_attributes(count: 4)
-        .and include(matched_type_account_warning)
-        .and include(matched_type_account)
-        .and include(matched_type_report)
-        .and include(matched_type_status)
-        .and not_include(unmatched_type_account_warning)
-        .and not_include(unmatched_type_account)
-        .and not_include(unmatched_type_report)
-        .and not_include(unmatched_type_status)
-    end
-  end
-
   describe '#unresolved_siblings?' do
     subject { Fabricate :report }
 

spec/presenters/report_history_presenter_spec.rb

@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe ReportHistoryPresenter do
+  describe '#logs' do
+    subject { described_class.new(report).logs }
+
+    let(:report) { Fabricate(:report, target_account_id: target_account.id, status_ids: [status.id]) }
+    let(:target_account) { Fabricate(:account) }
+    let(:status) { Fabricate(:status) }
+    let(:account_warning) { Fabricate(:account_warning, report_id: report.id) }
+
+    let!(:matched_type_account_warning) { Fabricate(:action_log, target_type: 'AccountWarning', target_id: account_warning.id) }
+    let!(:matched_type_account) { Fabricate(:action_log, target_type: 'Account', target_id: report.target_account_id) }
+    let!(:matched_type_report) { Fabricate(:action_log, target_type: 'Report', target_id: report.id) }
+    let!(:matched_type_status) { Fabricate(:action_log, target_type: 'Status', target_id: status.id) }
+
+    let!(:unmatched_type_account_warning) { Fabricate(:action_log, target_type: 'AccountWarning') }
+    let!(:unmatched_type_account) { Fabricate(:action_log, target_type: 'Account') }
+    let!(:unmatched_type_report) { Fabricate(:action_log, target_type: 'Report') }
+    let!(:unmatched_type_status) { Fabricate(:action_log, target_type: 'Status') }
+
+    it 'returns expected log records' do
+      expect(subject)
+        .to have_attributes(count: 4)
+        .and include(matched_type_account_warning)
+        .and include(matched_type_account)
+        .and include(matched_type_report)
+        .and include(matched_type_status)
+        .and not_include(unmatched_type_account_warning)
+        .and not_include(unmatched_type_account)
+        .and not_include(unmatched_type_report)
+        .and not_include(unmatched_type_status)
+    end
+  end
+end