diff --git a/app/models/event.rb b/app/models/event.rb index 0997b056c6a2322b1b4ce3a16061e0b0184ddce4..8a79100de5a3c357b1ed44c755bf647012fdaae8 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -48,7 +48,18 @@ class Event < ActiveRecord::Base belongs_to :author, class_name: "User" belongs_to :project - belongs_to :target, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations + + belongs_to :target, -> { + # If the association for "target" defines an "author" association we want to + # eager-load this so Banzai & friends don't end up performing N+1 queries to + # get the authors of notes, issues, etc. + if reflections['events'].active_record.reflect_on_association(:author) + includes(:author) + else + self + end + }, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations + has_one :push_event_payload # Callbacks diff --git a/changelogs/unreleased/conditionally-eager-load-event-target-authors.yml b/changelogs/unreleased/conditionally-eager-load-event-target-authors.yml new file mode 100644 index 0000000000000000000000000000000000000000..a5f1a958fa8c2572a04fc3ddc302bb25bd4ee182 --- /dev/null +++ b/changelogs/unreleased/conditionally-eager-load-event-target-authors.yml @@ -0,0 +1,5 @@ +--- +title: Eager load event target authors whenever possible +merge_request: +author: +type: performance diff --git a/spec/features/dashboard/activity_spec.rb b/spec/features/dashboard/activity_spec.rb index bd115785646f647ba796dc349362c4106c89ad66..a74a8aac2b2dbad35e5469e4251820ee5931ec20 100644 --- a/spec/features/dashboard/activity_spec.rb +++ b/spec/features/dashboard/activity_spec.rb @@ -24,6 +24,7 @@ feature 'Dashboard > Activity' do end let(:note) { create(:note, project: project, noteable: merge_request) } + let(:milestone) { create(:milestone, :active, project: project, title: '1.0') } let!(:push_event) do event = create(:push_event, project: project, author: user) @@ -54,6 +55,10 @@ feature 'Dashboard > Activity' do create(:event, :commented, project: project, target: note, author: user) end + let!(:milestone_event) do + create(:event, :closed, project: project, target: milestone, author: user) + end + before do project.add_master(user) @@ -68,6 +73,7 @@ feature 'Dashboard > Activity' do expect(page).to have_content('accepted') expect(page).to have_content('closed') expect(page).to have_content('commented on') + expect(page).to have_content('closed milestone') end end @@ -107,6 +113,7 @@ feature 'Dashboard > Activity' do expect(page).not_to have_content('accepted') expect(page).to have_content('closed') expect(page).not_to have_content('commented on') + expect(page).to have_content('closed milestone') end end diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index e999192940cecf148a79f2715484003764974952..67f49348acb80e234937c7be991de6eac1527ff8 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -347,6 +347,22 @@ describe Event do end end + describe '#target' do + it 'eager loads the author of an event target' do + create(:closed_issue_event) + + events = described_class.preload(:target).all.to_a + count = ActiveRecord::QueryRecorder + .new { events.first.target.author }.count + + # This expectation exists to make sure the test doesn't pass when the + # author is for some reason not loaded at all. + expect(events.first.target.author).to be_an_instance_of(User) + + expect(count).to be_zero + end + end + def create_push_event(project, user) event = create(:push_event, project: project, author: user)