Fix N+1 in milestone show page

The issue is that using becomes means a new object is instantiated all
the time, which means we throw away the route preload for the namespace

In fact, we don't even need to pass in the namespace for the polymorphic
route, the issuable_path helper seems to be able to figure out the
correct path just from the project.

Adds test for n+1 for group milestone show page. We are loading the
route for the namespace for each issue for the milestone.

Also, use the same link for both issue links

Also, removes @project_namespace definition as nothing use it anymore

app/controllers/projects/milestones_controller.rb

@@ -24,7 +24,6 @@ class Projects::MilestonesController < Projects::ApplicationController
 
     respond_to do |format|
       format.html do
-        @project_namespace = @project.namespace.becomes(Namespace)
         # We need to show group milestones in the JSON response
         # so that people can filter by and assign group milestones,
         # but we don't need to show them on the project milestones page itself.
@@ -47,8 +46,6 @@ class Projects::MilestonesController < Projects::ApplicationController
   end
 
   def show
-    @project_namespace = @project.namespace.becomes(Namespace)
-
     respond_to do |format|
       format.html
     end

app/views/shared/milestones/_issuable.html.haml

 -# @project is present when viewing Project's milestone
 - project = @project || issuable.project
-- namespace = @project_namespace || project.namespace.becomes(Namespace)
 - labels = issuable.labels
 - assignees = issuable.assignees
-- base_url_args = [namespace, project]
+- base_url_args = [project]
 - issuable_type_args = base_url_args + [issuable.class.table_name]
 - issuable_url_args = base_url_args + [issuable]
 
@@ -17,7 +16,7 @@
       = confidential_icon(issuable)
     = link_to issuable.title, issuable_url_args, title: issuable.title
   .issuable-detail
-    = link_to [namespace, project, issuable], class: 'issue-link' do
+    = link_to issuable_url_args, class: 'issue-link' do
       %span.issuable-number= issuable.to_reference
 
     - labels.each do |label|

changelogs/unreleased/group_milestones_n_1.yml

+---
+title: Fix N+1 in Group milestone view
+merge_request: 26051
+author:
+type: performance

spec/requests/groups/milestones_controller_spec.rb

@@ -32,5 +32,25 @@ describe Groups::MilestonesController do
         expect(milestones.map {|x| x['title']}).not_to include(private_milestone.title)
       end
     end
+
+    describe 'GET #show' do
+      let(:milestone) { create(:milestone, group: public_group) }
+      let(:show_path) { group_milestone_path(public_group, milestone) }
+
+      it 'avoids N+1 database queries' do
+        projects = create_list(:project, 3, :public, :merge_requests_enabled, :issues_enabled, group: public_group)
+        projects.each do |project|
+          create_list(:issue, 2, milestone: milestone, project: project)
+        end
+        control = ActiveRecord::QueryRecorder.new(skip_cached: false) { get show_path }
+
+        projects = create_list(:project, 3, :public, :merge_requests_enabled, :issues_enabled, group: public_group)
+        projects.each do |project|
+          create_list(:issue, 2, milestone: milestone, project: project)
+        end
+
+        expect { get show_path }.not_to exceed_all_query_limit(control)
+      end
+    end
   end
 end