Merge branch 'fix-n-plus-1-queries-projects' into 'master'

Fix some N+1 queries related to Admin Dashboard, User Dashboards and Activity Stream

Closes #55106

See merge request gitlab-org/gitlab-ce!23034

10 files changed, 60 insertions, 6 deletions

diff --git a/app/controllers/dashboard/projects_controller.rb b/app/controllers/dashboard/projects_controller.rb
index 57e612d89d3..f073b6de444 100644
--- a/app/controllers/dashboard/projects_controller.rb
+++ b/app/controllers/dashboard/projects_controller.rb
@@ -56,7 +56,7 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController
     projects = ProjectsFinder
                 .new(params: finder_params, current_user: current_user)
                 .execute
-                .includes(:route, :creator, namespace: [:route, :owner])
+                .includes(:route, :creator, :group, namespace: [:route, :owner])
                 .page(finder_params[:page])
 
     prepare_projects_for_rendering(projects)
diff --git a/app/controllers/explore/projects_controller.rb b/app/controllers/explore/projects_controller.rb
index 7ecbc32cf4e..778fdda8dbd 100644
--- a/app/controllers/explore/projects_controller.rb
+++ b/app/controllers/explore/projects_controller.rb
@@ -57,7 +57,7 @@ class Explore::ProjectsController < Explore::ApplicationController
   def load_projects
     projects = ProjectsFinder.new(current_user: current_user, params: params)
                  .execute
-                 .includes(:route, namespace: :route)
+                 .includes(:route, :creator, :group, namespace: [:route, :owner])
                  .page(params[:page])
                  .without_count
 
diff --git a/app/helpers/events_helper.rb b/app/helpers/events_helper.rb
index 3ce2398f1de..1371e9993b4 100644
--- a/app/helpers/events_helper.rb
+++ b/app/helpers/events_helper.rb
@@ -161,6 +161,10 @@ module EventsHelper
       project_commit_url(event.project, event.note_target, anchor: dom_id(event.target))
     elsif event.project_snippet_note?
       project_snippet_url(event.project, event.note_target, anchor: dom_id(event.target))
+    elsif event.issue_note?
+      project_issue_url(event.project, id: event.note_target, anchor: dom_id(event.target))
+    elsif event.merge_request_note?
+      project_merge_request_url(event.project, id: event.note_target, anchor: dom_id(event.target))
     else
       polymorphic_url([event.project.namespace.becomes(Namespace),
                        event.project, event.note_target],
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index 87aebe415c8..7c8557a1a8a 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -2,7 +2,7 @@
 
 module ProjectsHelper
   def link_to_project(project)
-    link_to [project.namespace.becomes(Namespace), project], title: h(project.name) do
+    link_to namespace_project_path(namespace_id: project.namespace, id: project), title: h(project.name) do
       title = content_tag(:span, project.name, class: 'project-name')
 
       if project.namespace
diff --git a/app/models/event.rb b/app/models/event.rb
index 2e690f8c013..2ceef412af5 100644
--- a/app/models/event.rb
+++ b/app/models/event.rb
@@ -87,7 +87,7 @@ class Event < ActiveRecord::Base
   scope :with_associations, -> do
     # We're using preload for "push_event_payload" as otherwise the association
     # 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)
   end
 
diff --git a/app/models/note.rb b/app/models/note.rb
index a6ae4f58ac4..17c7d97fa0a 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -131,7 +131,7 @@ class Note < ActiveRecord::Base
   scope :with_associations, -> do
     # FYI noteable cannot be loaded for LegacyDiffNote for commits
     includes(:author, :noteable, :updated_by,
-             project: [:project_members, { group: [:group_members] }])
+             project: [:project_members, :namespace, { group: [:group_members] }])
   end
   scope :with_metadata, -> { includes(:system_note_metadata) }
 
diff --git a/app/views/admin/dashboard/index.html.haml b/app/views/admin/dashboard/index.html.haml
index 7ac79cc77f5..6756299cf43 100644
--- a/app/views/admin/dashboard/index.html.haml
+++ b/app/views/admin/dashboard/index.html.haml
@@ -174,7 +174,7 @@
             %h4 Latest projects
             - @projects.each do |project|
               %p
-                = link_to project.full_name, [:admin, project.namespace.becomes(Namespace), project], class: 'str-truncated-60'
+                = link_to project.full_name, admin_project_path(project), class: 'str-truncated-60'
                 %span.light.float-right
                   #{time_ago_with_tooltip(project.created_at)}
       .col-md-4
diff --git a/changelogs/unreleased/fix-n-plus-1-queries-projects.yml b/changelogs/unreleased/fix-n-plus-1-queries-projects.yml
new file mode 100644
index 00000000000..cb625784267
--- /dev/null
+++ b/changelogs/unreleased/fix-n-plus-1-queries-projects.yml
@@ -0,0 +1,6 @@
+---
+title: Fix some N+1 queries related to Admin Dashboard, User Dashboards and Activity
+  Stream
+merge_request: 23034
+author:
+type: performance
diff --git a/spec/helpers/events_helper_spec.rb b/spec/helpers/events_helper_spec.rb
index 8d0679e5699..3d15306d4d2 100644
--- a/spec/helpers/events_helper_spec.rb
+++ b/spec/helpers/events_helper_spec.rb
@@ -84,4 +84,36 @@ describe EventsHelper do
       expect(helper.event_feed_url(event)).to eq(push_event_feed_url(event))
     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
+      event.target = create(:note_on_commit, note: '+1 from me')
+
+      expect(subject).to eq("#{project_base_url}/commit/#{event.target.commit_id}#note_#{event.target.id}")
+    end
+
+    it 'returns a project snippet note url' do
+      event.target = create(:note, :on_snippet, note: 'keep going')
+
+      expect(subject).to eq("#{project_base_url}/snippets/#{event.note_target.id}#note_#{event.target.id}")
+    end
+
+    it 'returns a project issue url' do
+      event.target = create(:note_on_issue, note: 'nice work')
+
+      expect(subject).to eq("#{project_base_url}/issues/#{event.note_target.iid}#note_#{event.target.id}")
+    end
+
+    it 'returns a merge request url' do
+      event.target = create(:note_on_merge_request, note: 'LGTM!')
+
+      expect(subject).to eq("#{project_base_url}/merge_requests/#{event.note_target.iid}#note_#{event.target.id}")
+    end
+  end
 end
diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb
index a857b7646b2..486416c3370 100644
--- a/spec/helpers/projects_helper_spec.rb
+++ b/spec/helpers/projects_helper_spec.rb
@@ -229,6 +229,18 @@ describe ProjectsHelper do
     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')
+    end
+  end
+
   describe '#link_to_member_avatar' do
     let(:user) { build_stubbed(:user) }
     let(:expected) { double }