Commit 4f5abe43 authored by Gabriel Mazetto's avatar Gabriel Mazetto

Reduce N+1 from Activity Dashboard and Banzai

There is a combination of few strategies implemented here:

1. Few relations were eager loaded
2. Changed few polymorphic routes to specific ones so we don't have to
   use `#becomes(Namespace)` which doesn't preserve association cache
parent 1297a1bd
...@@ -161,6 +161,10 @@ module EventsHelper ...@@ -161,6 +161,10 @@ module EventsHelper
project_commit_url(event.project, event.note_target, anchor: dom_id( project_commit_url(event.project, event.note_target, anchor: dom_id(
elsif event.project_snippet_note? elsif event.project_snippet_note?
project_snippet_url(event.project, event.note_target, anchor: dom_id( project_snippet_url(event.project, event.note_target, anchor: dom_id(
elsif event.issue_note?
project_issue_url(event.project, id: event.note_target, anchor: dom_id(
elsif event.merge_request_note?
project_merge_request_url(event.project, id: event.note_target, anchor: dom_id(
else else
polymorphic_url([event.project.namespace.becomes(Namespace), polymorphic_url([event.project.namespace.becomes(Namespace),
event.project, event.note_target], event.project, event.note_target],
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
module ProjectsHelper module ProjectsHelper
def link_to_project(project) def link_to_project(project)
link_to [project.namespace.becomes(Namespace), project], title: h( do link_to namespace_project_path(namespace_id: project.namespace, id: project), title: h( do
title = content_tag(:span,, class: 'project-name') title = content_tag(:span,, class: 'project-name')
if project.namespace if project.namespace
...@@ -87,7 +87,7 @@ class Event < ActiveRecord::Base ...@@ -87,7 +87,7 @@ class Event < ActiveRecord::Base
scope :with_associations, -> do scope :with_associations, -> do
# We're using preload for "push_event_payload" as otherwise the association # We're using preload for "push_event_payload" as otherwise the association
# is not always available (depending on the query being built). # is not always available (depending on the query being built).
includes(:author, :project, project: :namespace) includes(:author, :project, project: [:project_feature, :import_data, :namespace])
.preload(:target, :push_event_payload) .preload(:target, :push_event_payload)
end end
...@@ -131,7 +131,7 @@ class Note < ActiveRecord::Base ...@@ -131,7 +131,7 @@ class Note < ActiveRecord::Base
scope :with_associations, -> do scope :with_associations, -> do
# FYI noteable cannot be loaded for LegacyDiffNote for commits # FYI noteable cannot be loaded for LegacyDiffNote for commits
includes(:author, :noteable, :updated_by, includes(:author, :noteable, :updated_by,
project: [:project_members, { group: [:group_members] }]) project: [:project_members, :namespace, { group: [:group_members] }])
end end
scope :with_metadata, -> { includes(:system_note_metadata) } scope :with_metadata, -> { includes(:system_note_metadata) }
title: Fix some N+1 queries related to Admin Dashboard, User Dashboards and Activity
merge_request: 23034
type: performance
...@@ -84,4 +84,36 @@ describe EventsHelper do ...@@ -84,4 +84,36 @@ describe EventsHelper do
expect(helper.event_feed_url(event)).to eq(push_event_feed_url(event)) expect(helper.event_feed_url(event)).to eq(push_event_feed_url(event))
end end
end end
describe '#event_note_target_url' do
let(:project) { create(:project, :public, :repository) }
let(:event) { create(:event, project: project) }
let(:project_base_url) { namespace_project_url(namespace_id: project.namespace, id: project) }
subject { helper.event_note_target_url(event) }
it 'returns a commit note url' do = create(:note_on_commit, note: '+1 from me')
expect(subject).to eq("#{project_base_url}/commit/#{}#note_#{}")
it 'returns a project snippet note url' do = create(:note, :on_snippet, note: 'keep going')
expect(subject).to eq("#{project_base_url}/snippets/#{}#note_#{}")
it 'returns a project issue url' do = create(:note_on_issue, note: 'nice work')
expect(subject).to eq("#{project_base_url}/issues/#{event.note_target.iid}#note_#{}")
it 'returns a merge request url' do = create(:note_on_merge_request, note: 'LGTM!')
expect(subject).to eq("#{project_base_url}/merge_requests/#{event.note_target.iid}#note_#{}")
end end
...@@ -229,6 +229,18 @@ describe ProjectsHelper do ...@@ -229,6 +229,18 @@ describe ProjectsHelper do
end end
end end
describe '#link_to_project' do
let(:group) { create(:group, name: 'group name with space') }
let(:project) { create(:project, group: group, name: 'project name with space') }
subject { link_to_project(project) }
it 'returns an HTML link to the project' do
expect(subject).to match(%r{/#{group.full_path}/#{project.path}})
expect(subject).to include('group name with space /')
expect(subject).to include('project name with space')
describe '#link_to_member_avatar' do describe '#link_to_member_avatar' do
let(:user) { build_stubbed(:user) } let(:user) { build_stubbed(:user) }
let(:expected) { double } let(:expected) { double }
Markdown is supported
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment