Commit d83c4049 authored by Douwe Maan's avatar Douwe Maan

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
parents 7d3e6d09 dac51ace
......@@ -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
......
---
title: Eager load event target authors whenever possible
merge_request:
author:
type: performance
......@@ -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
......
......@@ -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)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment