Merge branch 'ccr/49289_milestone_link' into 'master'

Add project milestone link

Closes #49289

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

6 files changed, 111 insertions, 45 deletions

diff --git a/spec/controllers/dashboard/milestones_controller_spec.rb b/spec/controllers/dashboard/milestones_controller_spec.rb
index 8a8cc14fd4c..c9ccd5f7c55 100644
--- a/spec/controllers/dashboard/milestones_controller_spec.rb
+++ b/spec/controllers/dashboard/milestones_controller_spec.rb
@@ -52,7 +52,7 @@ describe Dashboard::MilestonesController do
 
       expect(response).to have_gitlab_http_status(200)
       expect(json_response.size).to eq(2)
-      expect(json_response.map { |i| i["first_milestone"]["id"] }).to match_array([group_milestone.id, project_milestone.id])
+      expect(json_response.map { |i| i["name"] }).to match_array([group_milestone.name, project_milestone.name])
       expect(json_response.map { |i| i["group_name"] }.compact).to match_array(group.name)
     end
 
diff --git a/spec/controllers/groups/milestones_controller_spec.rb b/spec/controllers/groups/milestones_controller_spec.rb
index b8e1e08cff7..40d991a669c 100644
--- a/spec/controllers/groups/milestones_controller_spec.rb
+++ b/spec/controllers/groups/milestones_controller_spec.rb
@@ -64,7 +64,7 @@ describe Groups::MilestonesController do
 
     context 'when there is a title parameter' do
       it 'searches for a legacy group milestone' do
-        expect(GlobalMilestone).to receive(:build)
+        expect(GroupMilestone).to receive(:build)
         expect(Milestone).not_to receive(:find_by_iid)
 
         get :show, params: { group_id: group.to_param, id: title, title: milestone1.safe_title }
diff --git a/spec/features/groups/milestone_spec.rb b/spec/features/groups/milestone_spec.rb
index 174840794ed..d57eb87ca77 100644
--- a/spec/features/groups/milestone_spec.rb
+++ b/spec/features/groups/milestone_spec.rb
@@ -81,7 +81,7 @@ describe 'Group milestones' do
           description: 'Lorem Ipsum is simply dummy text'
         )
       end
-      let!(:active_project_milestone2) { create(:milestone, project: other_project, state: 'active', title: 'v1.0') }
+      let!(:active_project_milestone2) { create(:milestone, project: other_project, state: 'active', title: 'v1.1') }
       let!(:closed_project_milestone1) { create(:milestone, project: project, state: 'closed', title: 'v2.0') }
       let!(:closed_project_milestone2) { create(:milestone, project: other_project, state: 'closed', title: 'v2.0') }
       let!(:active_group_milestone) { create(:milestone, group: group, state: 'active', title: 'GL-113') }
@@ -104,7 +104,7 @@ describe 'Group milestones' do
         legacy_milestone = GroupMilestone.build_collection(group, group.projects, { state: 'active' }).first
 
         expect(page).to have_selector("#milestone_#{active_group_milestone.id}", count: 1)
-        expect(page).to have_selector("#milestone_#{legacy_milestone.milestones.first.id}", count: 1)
+        expect(page).to have_selector("#milestone_#{legacy_milestone.milestone.id}", count: 1)
       end
 
       it 'shows milestone detail and supports its edit' do
@@ -121,6 +121,7 @@ describe 'Group milestones' do
 
       it 'renders milestones' do
         expect(page).to have_content('v1.0')
+        expect(page).to have_content('v1.1')
         expect(page).to have_content('GL-113')
         expect(page).to have_link(
           '1 Issue',
diff --git a/spec/features/groups/milestones_sorting_spec.rb b/spec/features/groups/milestones_sorting_spec.rb
index bc226ff41c1..7bc015ea28f 100644
--- a/spec/features/groups/milestones_sorting_spec.rb
+++ b/spec/features/groups/milestones_sorting_spec.rb
@@ -42,6 +42,7 @@ describe 'Milestones sorting', :js do
 
     expect(page).to have_button('Due later')
 
+    # assert descending sorting
     within '.milestones' do
       expect(page.all('ul.content-list > li').first.text).to include('v1.0')
       expect(page.all('ul.content-list > li')[1].text).to include('v3.0')
diff --git a/spec/models/global_milestone_spec.rb b/spec/models/global_milestone_spec.rb
index b6355455c1d..62699df5611 100644
--- a/spec/models/global_milestone_spec.rb
+++ b/spec/models/global_milestone_spec.rb
@@ -65,56 +65,103 @@ describe GlobalMilestone do
       )
     end
 
-    before do
-      projects = [
+    let!(:projects) do
+      [
         project1,
         project2,
         project3
       ]
-
-      @global_milestones = described_class.build_collection(projects, {})
     end
 
-    it 'has all project milestones' do
-      expect(@global_milestones.count).to eq(2)
+    let!(:global_milestones) { described_class.build_collection(projects, {}) }
+
+    context 'when building a collection of milestones' do
+      it 'has all project milestones' do
+        expect(global_milestones.count).to eq(6)
+      end
+
+      it 'has all project milestones titles' do
+        expect(global_milestones.map(&:title)).to match_array(['Milestone v1.2', 'Milestone v1.2', 'Milestone v1.2', 'VD-123', 'VD-123', 'VD-123'])
+      end
+
+      it 'has all project milestones' do
+        expect(global_milestones.size).to eq(6)
+      end
+
+      it 'sorts collection by due date' do
+        expect(global_milestones.map(&:due_date)).to eq [milestone1_due_date, milestone1_due_date, milestone1_due_date, nil, nil, nil]
+      end
     end
 
-    it 'has all project milestones titles' do
-      expect(@global_milestones.map(&:title)).to match_array(['Milestone v1.2', 'VD-123'])
+    context 'when adding new milestones' do
+      it 'does not add more queries' do
+        control_count = ActiveRecord::QueryRecorder.new do
+          described_class.build_collection(projects, {})
+        end.count
+
+        create_list(:milestone, 3, project: project3)
+
+        expect do
+          described_class.build_collection(projects, {})
+        end.not_to exceed_all_query_limit(control_count)
+      end
     end
+  end
+
+  describe '.states_count' do
+    context 'when the projects have milestones' do
+      before do
+        create(:closed_milestone, title: 'Active Group Milestone', project: project3)
+        create(:active_milestone, title: 'Active Group Milestone', project: project1)
+        create(:active_milestone, title: 'Active Group Milestone', project: project2)
+        create(:closed_milestone, title: 'Closed Group Milestone', project: project1)
+        create(:closed_milestone, title: 'Closed Group Milestone', project: project2)
+        create(:closed_milestone, title: 'Closed Group Milestone', project: project3)
+        create(:closed_milestone, title: 'Closed Group Milestone 4', group: group)
+      end
+
+      it 'returns the quantity of global milestones and group milestones in each possible state' do
+        expected_count = { opened: 2, closed: 5, all: 7 }
 
-    it 'has all project milestones' do
-      expect(@global_milestones.map { |group_milestone| group_milestone.milestones.count }.sum).to eq(6)
+        count = described_class.states_count(Project.all, group)
+
+        expect(count).to eq(expected_count)
+      end
+
+      it 'returns the quantity of global milestones in each possible state' do
+        expected_count = { opened: 2, closed: 4, all: 6 }
+
+        count = described_class.states_count(Project.all)
+
+        expect(count).to eq(expected_count)
+      end
     end
 
-    it 'sorts collection by due date' do
-      expect(@global_milestones.map(&:due_date)).to eq [nil, milestone1_due_date]
+    context 'when the projects do not have milestones' do
+      before do
+        project1
+      end
+
+      it 'returns 0 as the quantity of global milestones in each state' do
+        expected_count = { opened: 0, closed: 0, all: 0 }
+
+        count = described_class.states_count(Project.all)
+
+        expect(count).to eq(expected_count)
+      end
     end
   end
 
   describe '#initialize' do
     let(:milestone1_project1) { create(:milestone, title: "Milestone v1.2", project: project1) }
-    let(:milestone1_project2) { create(:milestone, title: "Milestone v1.2", project: project2) }
-    let(:milestone1_project3) { create(:milestone, title: "Milestone v1.2", project: project3) }
-
-    before do
-      milestones =
-        [
-          milestone1_project1,
-          milestone1_project2,
-          milestone1_project3
-        ]
-      milestones_relation = Milestone.where(id: milestones.map(&:id))
-
-      @global_milestone = described_class.new(milestone1_project1.title, milestones_relation)
-    end
+    subject(:global_milestone) { described_class.new(milestone1_project1) }
 
     it 'has exactly one group milestone' do
-      expect(@global_milestone.title).to eq('Milestone v1.2')
+      expect(global_milestone.title).to eq('Milestone v1.2')
     end
 
     it 'has all project milestones with the same title' do
-      expect(@global_milestone.milestones.count).to eq(3)
+      expect(global_milestone.milestone).to eq(milestone1_project1)
     end
   end
 
@@ -122,7 +169,7 @@ describe GlobalMilestone do
     let(:milestone) { create(:milestone, title: "git / test", project: project1) }
 
     it 'strips out slashes and spaces' do
-      global_milestone = described_class.new(milestone.title, Milestone.where(id: milestone.id))
+      global_milestone = described_class.new(milestone)
 
       expect(global_milestone.safe_title).to eq('git-test')
     end
@@ -132,11 +179,8 @@ describe GlobalMilestone do
     context 'when at least one milestone is active' do
       it 'returns active' do
         title = 'Active Group Milestone'
-        milestones = [
-          create(:active_milestone, title: title),
-          create(:closed_milestone, title: title)
-        ]
-        global_milestone = described_class.new(title, milestones)
+
+        global_milestone = described_class.new(create(:active_milestone, title: title))
 
         expect(global_milestone.state).to eq('active')
       end
@@ -145,11 +189,8 @@ describe GlobalMilestone do
     context 'when all milestones are closed' do
       it 'returns closed' do
         title = 'Closed Group Milestone'
-        milestones = [
-          create(:closed_milestone, title: title),
-          create(:closed_milestone, title: title)
-        ]
-        global_milestone = described_class.new(title, milestones)
+
+        global_milestone = described_class.new(create(:closed_milestone, title: title))
 
         expect(global_milestone.state).to eq('closed')
       end
diff --git a/spec/models/group_milestone_spec.rb b/spec/models/group_milestone_spec.rb
index b60676afc91..fcc33cd95fe 100644
--- a/spec/models/group_milestone_spec.rb
+++ b/spec/models/group_milestone_spec.rb
@@ -20,13 +20,36 @@ describe GroupMilestone do
   end
 
   describe '.build_collection' do
-    before do
-      project_milestone
+    let(:group) { create(:group) }
+    let(:project1) { create(:project, group: group) }
+    let(:project2) { create(:project, path: 'gitlab-ci', group: group) }
+    let(:project3) { create(:project, path: 'cookbook-gitlab', group: group) }
+
+    let!(:projects) do
+      [
+          project1,
+          project2,
+          project3
+      ]
     end
 
     it 'returns array of milestones, each with group assigned' do
       milestones = described_class.build_collection(group, [project], {})
       expect(milestones).to all(have_attributes(group: group))
     end
+
+    context 'when adding new milestones' do
+      it 'does not add more queries' do
+        control_count = ActiveRecord::QueryRecorder.new do
+          described_class.build_collection(group, projects, {})
+        end.count
+
+        create(:milestone, title: 'This title', project: project1)
+
+        expect do
+          described_class.build_collection(group, projects, {})
+        end.not_to exceed_all_query_limit(control_count)
+      end
+    end
   end
 end