kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Add project milestone link to dashboard milestones

Browse source 
One of the steps to deprecate dashboard milestones.
Links do dashboard milestone are replaced with links for each
project milestone
This commit is contained in:
Fatih Acet 2018-12-20 12:14:33 +01:00
parent f7ac8041f7
commit e9ef02096b
No known key found for this signature in database
GPG key ID: E994FE39E29B7E11
20 changed files with 246 additions and 175 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
app/assets/stylesheets/framework/issue_box.scss
View file

@ -45,9 +45,4 @@
&.status-box-upcoming {
background: $gl-text-color-secondary;
}
&.status-box-milestone {
color: $gl-text-color;
background: $gray-darker;
}
}

3
app/assets/stylesheets/pages/milestone.scss
View file

@ -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;

21
app/controllers/groups/milestones_controller.rb
View file

@ -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)

15
app/helpers/milestones_helper.rb
View file

@ -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?

2
app/models/concerns/milestoneish.rb
View file

@ -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

20
app/models/dashboard_group_milestone.rb
View file

@ -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

10
app/models/dashboard_milestone.rb
View file

@ -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

120
app/models/global_milestone.rb
View file

@ -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] }
items = Milestone.of_projects(projects)
.reorder_by_due_date_asc
.order_by_name_asc
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
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
def self.group_milestones_states_count(group)
return STATE_COUNT_HASH unless group
counts_by_state = Milestone.of_groups(group).count_by_state
{
opened: counts_by_state['active'] || 0,
closed: counts_by_state['closed'] || 0,
all: counts_by_state.values.sum
}
end
def milestoneish_ids
milestones.select(:id)
def self.legacy_group_milestone_states_count(projects)
return STATE_COUNT_HASH unless projects
# 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
{
opened: relation['active'] || 0,
closed: relation['closed'] || 0,
all: relation.values.sum
}
end
def safe_title
@title.to_slug.normalize.to_s
end
def projects
@projects ||= Project.for_milestones(milestoneish_ids)
end
def state
milestones.each do |milestone|
return 'active' if milestone.state != 'closed'
end
'closed'
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

29
app/models/group_milestone.rb
View file

@ -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

6
app/models/milestone.rb
View file

@ -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

2
app/views/dashboard/milestones/_milestone.html.haml
View file

@ -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,

14
app/views/shared/milestones/_milestone.html.haml
View file

@ -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

11
app/views/shared/milestones/_top.html.haml
View file

@ -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

5
changelogs/unreleased/ccr-49289_milestone_link.yml Normal file
View file

@ -0,0 +1,5 @@
---
title: Add project milestone link
merge_request: 22552
author:
type: added

2
spec/controllers/dashboard/milestones_controller_spec.rb
View file

@ -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

2
spec/controllers/groups/milestones_controller_spec.rb
View file

@ -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 }

5
spec/features/groups/milestone_spec.rb
View file

@ -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',

1
spec/features/groups/milestones_sorting_spec.rb
View file

@ -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')

121
spec/models/global_milestone_spec.rb
View file

@ -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 }
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 'has all project milestones' do
expect(@global_milestones.map { |group_milestone| group_milestone.milestones.count }.sum).to eq(6)
end
context 'when the projects do not have milestones' do
before do
project1
end
it 'sorts collection by due date' do
expect(@global_milestones.map(&:due_date)).to eq [nil, milestone1_due_date]
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

27
spec/models/group_milestone_spec.rb
View file

@ -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