summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFatih Acet <acetfatih@gmail.com>2018-12-20 12:14:33 +0100
committerFatih Acet <acetfatih@gmail.com>2018-12-20 13:33:09 +0100
commite9ef02096be859e31c155174fe2784d8a7ba73e3 (patch)
treec839c271e35cd1474801ff20d90bf9be3a2135bf
parentf7ac8041f7ba3639085c26b4185eda00fd54a6e7 (diff)
downloadgitlab-ce-ccr/49289_milestone_link.tar.gz
Add project milestone link to dashboard milestonesccr/49289_milestone_link
One of the steps to deprecate dashboard milestones. Links do dashboard milestone are replaced with links for each project milestone
-rw-r--r--app/assets/stylesheets/framework/issue_box.scss5
-rw-r--r--app/assets/stylesheets/pages/milestone.scss3
-rw-r--r--app/controllers/groups/milestones_controller.rb21
-rw-r--r--app/helpers/milestones_helper.rb15
-rw-r--r--app/models/concerns/milestoneish.rb2
-rw-r--r--app/models/dashboard_group_milestone.rb20
-rw-r--r--app/models/dashboard_milestone.rb10
-rw-r--r--app/models/global_milestone.rb116
-rw-r--r--app/models/group_milestone.rb29
-rw-r--r--app/models/milestone.rb6
-rw-r--r--app/views/dashboard/milestones/_milestone.html.haml2
-rw-r--r--app/views/shared/milestones/_milestone.html.haml14
-rw-r--r--app/views/shared/milestones/_top.html.haml11
-rw-r--r--changelogs/unreleased/ccr-49289_milestone_link.yml5
-rw-r--r--spec/controllers/dashboard/milestones_controller_spec.rb2
-rw-r--r--spec/controllers/groups/milestones_controller_spec.rb2
-rw-r--r--spec/features/groups/milestone_spec.rb5
-rw-r--r--spec/features/groups/milestones_sorting_spec.rb1
-rw-r--r--spec/models/global_milestone_spec.rb119
-rw-r--r--spec/models/group_milestone_spec.rb27
20 files changed, 243 insertions, 172 deletions
diff --git a/app/assets/stylesheets/framework/issue_box.scss b/app/assets/stylesheets/framework/issue_box.scss
index a66604e56ff..e51f230a680 100644
--- a/app/assets/stylesheets/framework/issue_box.scss
+++ b/app/assets/stylesheets/framework/issue_box.scss
@@ -45,9 +45,4 @@
&.status-box-upcoming {
background: $gl-text-color-secondary;
}
-
- &.status-box-milestone {
- color: $gl-text-color;
- background: $gray-darker;
- }
}
diff --git a/app/assets/stylesheets/pages/milestone.scss b/app/assets/stylesheets/pages/milestone.scss
index 1e92582d6d9..94bf32945fc 100644
--- a/app/assets/stylesheets/pages/milestone.scss
+++ b/app/assets/stylesheets/pages/milestone.scss
@@ -1,3 +1,5 @@
+$status-box-line-height: 26px;
+
.issues-sortable-list .str-truncated {
max-width: 90%;
}
@@ -38,6 +40,7 @@
font-size: $tooltip-font-size;
margin-top: 0;
margin-right: $gl-padding-4;
+ line-height: $status-box-line-height;
@include media-breakpoint-down(xs) {
line-height: unset;
diff --git a/app/controllers/groups/milestones_controller.rb b/app/controllers/groups/milestones_controller.rb
index b42116b0f36..868deea3f01 100644
--- a/app/controllers/groups/milestones_controller.rb
+++ b/app/controllers/groups/milestones_controller.rb
@@ -43,14 +43,7 @@ class Groups::MilestonesController < Groups::ApplicationController
def update
# Keep this compatible with legacy group milestones where we have to update
# all projects milestones states at once.
- if @milestone.legacy_group_milestone?
- update_params = milestone_params.select { |key| key == "state_event" }
- milestones = @milestone.milestones
- else
- update_params = milestone_params
- milestones = [@milestone]
- end
-
+ milestones, update_params = get_milestones_for_update
milestones.each do |milestone|
Milestones::UpdateService.new(milestone.parent, current_user, update_params).execute(milestone)
end
@@ -71,6 +64,14 @@ class Groups::MilestonesController < Groups::ApplicationController
private
+ def get_milestones_for_update
+ if @milestone.legacy_group_milestone?
+ [@milestone.milestones, legacy_milestone_params]
+ else
+ [[@milestone], milestone_params]
+ end
+ end
+
def authorize_admin_milestones!
return render_404 unless can?(current_user, :admin_milestone, group)
end
@@ -79,6 +80,10 @@ class Groups::MilestonesController < Groups::ApplicationController
params.require(:milestone).permit(:title, :description, :start_date, :due_date, :state_event)
end
+ def legacy_milestone_params
+ params.require(:milestone).permit(:state_event)
+ end
+
def milestone_path
if @milestone.legacy_group_milestone?
group_milestone_path(group, @milestone.safe_title, title: @milestone.title)
diff --git a/app/helpers/milestones_helper.rb b/app/helpers/milestones_helper.rb
index 9666080092b..c212e1804dc 100644
--- a/app/helpers/milestones_helper.rb
+++ b/app/helpers/milestones_helper.rb
@@ -237,12 +237,15 @@ module MilestonesHelper
end
end
- def group_or_dashboard_milestone_path(milestone)
- if milestone.group_milestone?
- group_milestone_path(milestone.group, milestone.iid, milestone: { title: milestone.title })
- else
- dashboard_milestone_path(milestone.safe_title, title: milestone.title)
- end
+ def group_or_project_milestone_path(milestone)
+ params =
+ if milestone.group_milestone?
+ { milestone: { title: milestone.title } }
+ else
+ { title: milestone.title }
+ end
+
+ milestone_path(milestone.milestone, params)
end
def can_admin_project_milestones?
diff --git a/app/models/concerns/milestoneish.rb b/app/models/concerns/milestoneish.rb
index e44a069b730..055ffe04646 100644
--- a/app/models/concerns/milestoneish.rb
+++ b/app/models/concerns/milestoneish.rb
@@ -42,7 +42,7 @@ module Milestoneish
def issues_visible_to_user(user)
memoize_per_user(user, :issues_visible_to_user) do
IssuesFinder.new(user, issues_finder_params)
- .execute.preload(:assignees).where(milestone_id: milestoneish_ids)
+ .execute.preload(:assignees).where(milestone_id: milestoneish_id)
end
end
diff --git a/app/models/dashboard_group_milestone.rb b/app/models/dashboard_group_milestone.rb
index 32e8104125c..c06823184c2 100644
--- a/app/models/dashboard_group_milestone.rb
+++ b/app/models/dashboard_group_milestone.rb
@@ -7,7 +7,7 @@ class DashboardGroupMilestone < GlobalMilestone
override :initialize
def initialize(milestone)
- super(milestone.title, Array(milestone))
+ super
@group_name = milestone.group.full_name
end
@@ -19,22 +19,4 @@ class DashboardGroupMilestone < GlobalMilestone
.active
.map { |m| new(m) }
end
-
- override :group_milestone?
- def group_milestone?
- @first_milestone.group_milestone?
- end
-
- override :milestoneish_ids
- def milestoneish_ids
- milestones.map(&:id)
- end
-
- def group
- @first_milestone.group
- end
-
- def iid
- @first_milestone.iid
- end
end
diff --git a/app/models/dashboard_milestone.rb b/app/models/dashboard_milestone.rb
index 96bc8090b81..9b377b70e5b 100644
--- a/app/models/dashboard_milestone.rb
+++ b/app/models/dashboard_milestone.rb
@@ -1,11 +1,15 @@
# frozen_string_literal: true
class DashboardMilestone < GlobalMilestone
- def issues_finder_params
- { authorized_only: true }
+ attr_reader :project_name
+
+ def initialize(milestone)
+ super
+
+ @project_name = milestone.project.full_name
end
- def dashboard_milestone?
+ def project_milestone?
true
end
end
diff --git a/app/models/global_milestone.rb b/app/models/global_milestone.rb
index 085ffd16c6a..4e82f3fed27 100644
--- a/app/models/global_milestone.rb
+++ b/app/models/global_milestone.rb
@@ -3,69 +3,78 @@
class GlobalMilestone
include Milestoneish
- EPOCH = DateTime.parse('1970-01-01')
STATE_COUNT_HASH = { opened: 0, closed: 0, all: 0 }.freeze
- attr_accessor :title, :milestones
+ attr_reader :milestone
alias_attribute :name, :title
+ delegate :title, :state, :due_date, :start_date, :participants, :project, :group, :expires_at, :closed?, :iid, :group_milestone?, :safe_title, :milestoneish_id, to: :milestone
+
+ def to_hash
+ {
+ name: title,
+ title: title,
+ group_name: group&.full_name,
+ project_name: project&.full_name
+ }
+ end
+
def for_display
- @first_milestone
+ @milestone
end
def self.build_collection(projects, params)
- params =
- { project_ids: projects.map(&:id), state: params[:state] }
-
- child_milestones = MilestonesFinder.new(params).execute # rubocop: disable CodeReuse/Finder
-
- milestones = child_milestones.select(:id, :title).group_by(&:title).map do |title, grouped|
- milestones_relation = Milestone.where(id: grouped.map(&:id))
- new(title, milestones_relation)
- end
+ items = Milestone.of_projects(projects)
+ .reorder_by_due_date_asc
+ .order_by_name_asc
- milestones.sort_by { |milestone| milestone.due_date || EPOCH }
+ Milestone.filter_by_state(items, params[:state]).map { |m| new(m) }
end
+ # necessary for legacy milestones
def self.build(projects, title)
- child_milestones = Milestone.of_projects(projects).where(title: title)
- return if child_milestones.blank?
+ milestones = Milestone.of_projects(projects).where(title: title)
+ return if milestones.blank?
- new(title, child_milestones)
+ new(milestones.first)
end
- def self.count_by_state(milestones_by_state_and_title, state)
- milestones_by_state_and_title.count do |(milestone_state, _), _|
- milestone_state == state
+ def self.states_count(projects, group = nil)
+ legacy_group_milestones_count = legacy_group_milestone_states_count(projects)
+ group_milestones_count = group_milestones_states_count(group)
+
+ legacy_group_milestones_count.merge(group_milestones_count) do |k, legacy_group_milestones_count, group_milestones_count|
+ legacy_group_milestones_count + group_milestones_count
end
end
- private_class_method :count_by_state
- def initialize(title, milestones)
- @title = title
- @name = title
- @milestones = milestones
- @first_milestone = milestones.find {|m| m.description.present? } || milestones.first
- end
+ def self.group_milestones_states_count(group)
+ return STATE_COUNT_HASH unless group
- def milestoneish_ids
- milestones.select(:id)
- end
+ counts_by_state = Milestone.of_groups(group).count_by_state
- def safe_title
- @title.to_slug.normalize.to_s
+ {
+ opened: counts_by_state['active'] || 0,
+ closed: counts_by_state['closed'] || 0,
+ all: counts_by_state.values.sum
+ }
end
- def projects
- @projects ||= Project.for_milestones(milestoneish_ids)
- end
+ def self.legacy_group_milestone_states_count(projects)
+ return STATE_COUNT_HASH unless projects
- def state
- milestones.each do |milestone|
- return 'active' if milestone.state != 'closed'
- end
+ # We need to reorder(nil) on the projects, because the controller passes them in sorted.
+ relation = Milestone.of_projects(projects.reorder(nil)).count_by_state
- 'closed'
+ {
+ opened: relation['active'] || 0,
+ closed: relation['closed'] || 0,
+ all: relation.values.sum
+ }
+ end
+
+ def initialize(milestone)
+ @milestone = milestone
end
def active?
@@ -77,37 +86,14 @@ class GlobalMilestone
end
def issues
- @issues ||= Issue.of_milestones(milestoneish_ids).includes(:project, :assignees, :labels)
+ @issues ||= Issue.of_milestones(milestone).includes(:project, :assignees, :labels)
end
def merge_requests
- @merge_requests ||= MergeRequest.of_milestones(milestoneish_ids).includes(:target_project, :assignee, :labels)
- end
-
- def participants
- @participants ||= milestones.map(&:participants).flatten.uniq
+ @merge_requests ||= MergeRequest.of_milestones(milestone).includes(:target_project, :assignee, :labels)
end
def labels
- @labels ||= GlobalLabel.build_collection(milestones.includes(:labels).map(&:labels).flatten)
- .sort_by!(&:title)
- end
-
- def due_date
- return @due_date if defined?(@due_date)
-
- @due_date =
- if @milestones.all? { |x| x.due_date == @milestones.first.due_date }
- @milestones.first.due_date
- end
- end
-
- def start_date
- return @start_date if defined?(@start_date)
-
- @start_date =
- if @milestones.all? { |x| x.start_date == @milestones.first.start_date }
- @milestones.first.start_date
- end
+ @labels ||= GlobalLabel.build_collection(milestone.labels).sort_by!(&:title)
end
end
diff --git a/app/models/group_milestone.rb b/app/models/group_milestone.rb
index 9dfaebacc83..a58537de319 100644
--- a/app/models/group_milestone.rb
+++ b/app/models/group_milestone.rb
@@ -1,18 +1,35 @@
# frozen_string_literal: true
# Group Milestones are milestones that can be shared among many projects within the same group
class GroupMilestone < GlobalMilestone
- attr_accessor :group
+ attr_reader :group, :milestones
def self.build_collection(group, projects, params)
- super(projects, params).each do |milestone|
- milestone.group = group
+ params =
+ { state: params[:state] }
+
+ project_milestones = Milestone.of_projects(projects)
+ child_milestones = Milestone.filter_by_state(project_milestones, params[:state])
+ grouped_milestones = child_milestones.group_by(&:title)
+
+ grouped_milestones.map do |title, grouped|
+ new(title, grouped, group)
end
end
def self.build(group, projects, title)
- super(projects, title).tap do |milestone|
- milestone&.group = group
- end
+ child_milestones = Milestone.of_projects(projects).where(title: title)
+ return if child_milestones.blank?
+
+ new(title, child_milestones, group)
+ end
+
+ def initialize(title, milestones, group)
+ @milestones = milestones
+ @group = group
+ end
+
+ def milestone
+ @milestone ||= milestones.find { |m| m.description.present? } || milestones.first
end
def issues_finder_params
diff --git a/app/models/milestone.rb b/app/models/milestone.rb
index 3cc8e2c44bb..db56533dea5 100644
--- a/app/models/milestone.rb
+++ b/app/models/milestone.rb
@@ -94,6 +94,10 @@ class Milestone < ActiveRecord::Base
end
end
+ def count_by_state
+ reorder(nil).group(:state).count
+ end
+
def predefined?(milestone)
milestone == Any ||
milestone == None ||
@@ -215,7 +219,7 @@ class Milestone < ActiveRecord::Base
self.title
end
- def milestoneish_ids
+ def milestoneish_id
id
end
diff --git a/app/views/dashboard/milestones/_milestone.html.haml b/app/views/dashboard/milestones/_milestone.html.haml
index b876d6fd1f3..89212eb6bf9 100644
--- a/app/views/dashboard/milestones/_milestone.html.haml
+++ b/app/views/dashboard/milestones/_milestone.html.haml
@@ -1,5 +1,5 @@
= render 'shared/milestones/milestone',
- milestone_path: group_or_dashboard_milestone_path(milestone),
+ milestone_path: group_or_project_milestone_path(milestone),
issues_path: issues_dashboard_path(milestone_title: milestone.title),
merge_requests_path: merge_requests_dashboard_path(milestone_title: milestone.title),
milestone: milestone,
diff --git a/app/views/shared/milestones/_milestone.html.haml b/app/views/shared/milestones/_milestone.html.haml
index ed7fefba56d..40b8374848e 100644
--- a/app/views/shared/milestones/_milestone.html.haml
+++ b/app/views/shared/milestones/_milestone.html.haml
@@ -1,5 +1,5 @@
- dashboard = local_assigns[:dashboard]
-- custom_dom_id = dom_id(milestone.try(:milestones) ? milestone.milestones.first : milestone)
+- custom_dom_id = dom_id(milestone.try(:milestone) ? milestone.milestone : milestone)
- milestone_type = milestone.group_milestone? ? 'Group Milestone' : 'Project Milestone'
%li{ class: "milestone milestone-#{milestone.closed? ? 'closed' : 'open'}", id: custom_dom_id }
@@ -21,10 +21,12 @@
= milestone.group.full_name
- if milestone.legacy_group_milestone?
.projects
- - milestone.milestones.each do |milestone|
- = link_to milestone_path(milestone) do
- %span.label-badge.label-badge-blue.d-inline-block.append-bottom-5
- = dashboard ? milestone.project.full_name : milestone.project.name
+ - link_to milestone_path(milestone.milestone) do
+ %span.label-badge.label-badge-blue.d-inline-block.append-bottom-5
+ = dashboard ? milestone.project.full_name : milestone.project.name
+ - if milestone.project
+ .label-badge.label-badge-gray.d-inline-block
+ = milestone.project.full_name
.col-sm-4.milestone-progress
= milestone_progress_bar(milestone)
@@ -58,5 +60,5 @@
- else
= link_to 'Close Milestone', group_milestone_route(milestone, {state_event: :close }), method: :put, class: "btn btn-sm btn-grouped btn-close"
- if dashboard
- .status-box.status-box-milestone
+ .label-badge.label-badge-gray
= milestone_type
diff --git a/app/views/shared/milestones/_top.html.haml b/app/views/shared/milestones/_top.html.haml
index 0499b04a482..55b1c14022f 100644
--- a/app/views/shared/milestones/_top.html.haml
+++ b/app/views/shared/milestones/_top.html.haml
@@ -62,20 +62,19 @@
%th Open issues
%th State
%th Due date
- - milestone.milestones.each do |ms|
%tr
%td
- - project_name = group ? ms.project.name : ms.project.full_name
- = link_to project_name, project_milestone_path(ms.project, ms)
+ - project_name = group ? milestone.project.name : milestone.project.full_name
+ = link_to project_name, milestone_path(milestone.milestone)
%td
- = ms.issues_visible_to_user(current_user).opened.count
+ = milestone.milestone.issues_visible_to_user(current_user).opened.count
%td
- - if ms.closed?
+ - if milestone.closed?
Closed
- else
Open
%td
- = ms.expires_at
+ = milestone.expires_at
- elsif milestone.group_milestone?
%br
View
diff --git a/changelogs/unreleased/ccr-49289_milestone_link.yml b/changelogs/unreleased/ccr-49289_milestone_link.yml
new file mode 100644
index 00000000000..14c09752a24
--- /dev/null
+++ b/changelogs/unreleased/ccr-49289_milestone_link.yml
@@ -0,0 +1,5 @@
+---
+title: Add project milestone link
+merge_request: 22552
+author:
+type: added
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