diff options
author | Douwe Maan <douwe@gitlab.com> | 2018-01-04 16:46:40 +0000 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2018-01-04 16:46:40 +0000 |
commit | d83c4049de46b44c2ac8140345d74fb8f0487370 (patch) | |
tree | cf921f107a7c677535bf1f52dfec7ee9c7b6b7d4 | |
parent | 7d3e6d09021b677c4765a58540d1278bc3948322 (diff) | |
parent | dac51ace521d7b2b2a5a5bb19167a8690ead242e (diff) | |
download | gitlab-ce-d83c4049de46b44c2ac8140345d74fb8f0487370.tar.gz |
Merge branch 'conditionally-eager-load-event-target-authors' into 'master'
Eager load event target authors whenever possible
Closes #41618
See merge request gitlab-org/gitlab-ce!16199
-rw-r--r-- | app/models/event.rb | 13 | ||||
-rw-r--r-- | changelogs/unreleased/conditionally-eager-load-event-target-authors.yml | 5 | ||||
-rw-r--r-- | spec/features/dashboard/activity_spec.rb | 7 | ||||
-rw-r--r-- | spec/models/event_spec.rb | 16 |
4 files changed, 40 insertions, 1 deletions
diff --git a/app/models/event.rb b/app/models/event.rb index 0997b056c6a..8a79100de5a 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 00000000000..a5f1a958fa8 --- /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 bd115785646..a74a8aac2b2 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 e999192940c..67f49348acb 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) |