Allow promoting project milestones to group milestones

This commit is contained in:
Felipe Artur 2017-10-31 15:03:52 +00:00 committed by Sean McGivern
parent bd33a8290a
commit 2f8577d45e
10 changed files with 230 additions and 13 deletions

View file

@ -2,13 +2,13 @@ class Projects::MilestonesController < Projects::ApplicationController
include MilestoneActions
before_action :check_issuables_available!
before_action :milestone, only: [:edit, :update, :destroy, :show, :merge_requests, :participants, :labels]
before_action :milestone, only: [:edit, :update, :destroy, :show, :merge_requests, :participants, :labels, :promote]
# Allow read any milestone
before_action :authorize_read_milestone!
# Allow admin milestone
before_action :authorize_admin_milestone!, except: [:index, :show, :merge_requests, :participants, :labels]
before_action :authorize_admin_milestone!, except: [:index, :show, :merge_requests, :participants, :labels, :promote]
respond_to :html
@ -69,6 +69,14 @@ class Projects::MilestonesController < Projects::ApplicationController
end
end
def promote
promoted_milestone = Milestones::PromoteService.new(project, current_user).execute(milestone)
flash[:notice] = "Milestone has been promoted to group milestone."
redirect_to group_milestone_path(project.group, promoted_milestone.iid)
rescue Milestones::PromoteService::PromoteMilestoneError => error
redirect_to milestone, alert: error.message
end
def destroy
return access_denied! unless can?(current_user, :admin_milestone, @project)

View file

@ -0,0 +1,80 @@
module Milestones
class PromoteService < Milestones::BaseService
PromoteMilestoneError = Class.new(StandardError)
def execute(milestone)
check_project_milestone!(milestone)
Milestone.transaction do
# Destroy all milestones with same title across projects
destroy_old_milestones(milestone)
group_milestone = clone_project_milestone(milestone)
move_children_to_group_milestone(group_milestone)
# Just to be safe
unless group_milestone.valid?
raise_error(group_milestone.errors.full_messages.to_sentence)
end
group_milestone
end
end
private
def milestone_ids_for_merge(group_milestone)
# Pluck need to be used here instead of select so the array of ids
# is persistent after old milestones gets deleted.
@milestone_ids_for_merge ||= begin
search_params = { title: group_milestone.title, project_ids: group_project_ids, state: 'all' }
milestones = MilestonesFinder.new(search_params).execute
milestones.pluck(:id)
end
end
def move_children_to_group_milestone(group_milestone)
milestone_ids_for_merge(group_milestone).in_groups_of(100) do |milestone_ids|
update_children(group_milestone, milestone_ids)
end
end
def check_project_milestone!(milestone)
raise_error('Only project milestones can be promoted.') unless milestone.project_milestone?
end
def clone_project_milestone(milestone)
params = milestone.slice(:title, :description, :start_date, :due_date, :state_event)
create_service = CreateService.new(group, current_user, params)
create_service.execute
end
def update_children(group_milestone, milestone_ids)
issues = Issue.where(project_id: group_project_ids, milestone_id: milestone_ids)
merge_requests = MergeRequest.where(source_project_id: group_project_ids, milestone_id: milestone_ids)
[issues, merge_requests].each do |issuable_collection|
issuable_collection.update_all(milestone_id: group_milestone.id)
end
end
def group
@group ||= parent.group || raise_error('Project does not belong to a group.')
end
def destroy_old_milestones(group_milestone)
Milestone.where(id: milestone_ids_for_merge(group_milestone)).destroy_all
end
def group_project_ids
@group_project_ids ||= group.projects.map(&:id)
end
def raise_error(message)
raise PromoteMilestoneError, "Promotion failed - #{message}"
end
end
end

View file

@ -23,14 +23,18 @@
= milestone_date_range(@milestone)
.milestone-buttons
- if can?(current_user, :admin_milestone, @project)
= link_to edit_project_milestone_path(@project, @milestone), class: "btn btn-grouped btn-nr" do
Edit
- if @project.group
= link_to promote_project_milestone_path(@milestone.project, @milestone), title: "Promote to Group Milestone", class: 'btn btn-grouped', data: { confirm: "Promoting this milestone will make it available for all projects inside the group. Existing project milestones with the same name will be merged. Are you sure?", toggle: "tooltip" }, method: :post do
Promote
- if @milestone.active?
= link_to 'Close milestone', project_milestone_path(@project, @milestone, milestone: {state_event: :close }), method: :put, class: "btn btn-close btn-nr btn-grouped"
- else
= link_to 'Reopen milestone', project_milestone_path(@project, @milestone, milestone: {state_event: :activate }), method: :put, class: "btn btn-reopen btn-nr btn-grouped"
= link_to edit_project_milestone_path(@project, @milestone), class: "btn btn-grouped btn-nr" do
Edit
= link_to project_milestone_path(@project, @milestone), data: { confirm: 'Are you sure?' }, method: :delete, class: "btn btn-grouped btn-danger" do
Delete
@ -40,6 +44,7 @@
.detail-page-description.milestone-detail
%h2.title
= markdown_field(@milestone, :title)
%div
- if @milestone.description.present?
.description

View file

@ -49,6 +49,13 @@
= link_to edit_project_milestone_path(milestone.project, milestone), class: "btn btn-xs btn-grouped" do
Edit
\
- if @project.group
= link_to promote_project_milestone_path(milestone.project, milestone), title: "Promote to Group Milestone", class: 'btn btn-xs btn-grouped', data: { confirm: "Promoting this milestone will make it available for all projects inside the group. Existing project milestones with the same name will be merged. Are you sure?", toggle: "tooltip" }, method: :post do
Promote
= link_to 'Close Milestone', project_milestone_path(@project, milestone, milestone: {state_event: :close }), method: :put, remote: true, class: "btn btn-xs btn-close btn-grouped"
= link_to project_milestone_path(milestone.project, milestone), data: { confirm: 'Are you sure?' }, method: :delete, class: "btn btn-xs btn-remove btn-grouped" do
Delete

View file

@ -0,0 +1,5 @@
---
title: Allow promoting project milestones to group milestones
merge_request:
author:
type: added

View file

@ -293,6 +293,7 @@ constraints(ProjectUrlConstrainer.new) do
resources :milestones, constraints: { id: /\d+/ } do
member do
post :promote
put :sort_issues
put :sort_merge_requests
get :merge_requests

View file

@ -29,7 +29,8 @@ In addition to that you will be able to filter issues or merge requests by group
## Milestone promotion
You will be able to promote a project milestone to a group milestone [in the future](https://gitlab.com/gitlab-org/gitlab-ce/issues/35833).
Project milestones can be promoted to group milestones if its project belongs to a group. When a milestone is promoted all other milestones across the group projects with the same title will be merged into it, which means all milestone's children like issues, merge requests and boards will be moved into the new promoted milestone.
The promote button can be found in the milestone view or milestones list.
## Special milestone filters

View file

@ -86,4 +86,32 @@ describe Projects::MilestonesController do
expect(last_note).to eq('removed milestone')
end
end
describe '#promote' do
context 'promotion succeeds' do
before do
group = create(:group)
group.add_developer(user)
milestone.project.update(namespace: group)
end
it 'shows group milestone' do
post :promote, namespace_id: project.namespace.id, project_id: project.id, id: milestone.iid
group_milestone = assigns(:milestone)
expect(response).to redirect_to(group_milestone_path(project.group, group_milestone.iid))
expect(flash[:notice]).to eq('Milestone has been promoted to group milestone.')
end
end
context 'promotion fails' do
it 'shows project milestone' do
post :promote, namespace_id: project.namespace.id, project_id: project.id, id: milestone.iid
expect(response).to redirect_to(project_milestone_path(project, milestone))
expect(flash[:alert]).to eq('Promotion failed - Project does not belong to a group.')
end
end
end
end

View file

@ -426,18 +426,23 @@ describe 'project routing' do
end
end
# project_milestones GET /:project_id/milestones(.:format) milestones#index
# POST /:project_id/milestones(.:format) milestones#create
# new_project_milestone GET /:project_id/milestones/new(.:format) milestones#new
# edit_project_milestone GET /:project_id/milestones/:id/edit(.:format) milestones#edit
# project_milestone GET /:project_id/milestones/:id(.:format) milestones#show
# PUT /:project_id/milestones/:id(.:format) milestones#update
# DELETE /:project_id/milestones/:id(.:format) milestones#destroy
# project_milestones GET /:project_id/milestones(.:format) milestones#index
# POST /:project_id/milestones(.:format) milestones#create
# new_project_milestone GET /:project_id/milestones/new(.:format) milestones#new
# edit_project_milestone GET /:project_id/milestones/:id/edit(.:format) milestones#edit
# project_milestone GET /:project_id/milestones/:id(.:format) milestones#show
# PUT /:project_id/milestones/:id(.:format) milestones#update
# DELETE /:project_id/milestones/:id(.:format) milestones#destroy
# promote_project_milestone POST /:project_id/milestones/:id/promote milestones#promote
describe Projects::MilestonesController, 'routing' do
it_behaves_like 'RESTful project resources' do
let(:controller) { 'milestones' }
let(:actions) { [:index, :create, :new, :edit, :show, :update] }
end
it 'to #promote' do
expect(post('/gitlab/gitlabhq/milestones/1/promote')).to route_to('projects/milestones#promote', namespace_id: 'gitlab', project_id: 'gitlabhq', id: "1")
end
end
# project_labels GET /:project_id/labels(.:format) labels#index

View file

@ -0,0 +1,77 @@
require 'spec_helper'
describe Milestones::PromoteService do
let(:group) { create(:group) }
let(:project) { create(:project, namespace: group) }
let(:user) { create(:user) }
let(:milestone_title) { 'project milestone' }
let(:milestone) { create(:milestone, project: project, title: milestone_title) }
let(:service) { described_class.new(project, user) }
describe '#execute' do
before do
group.add_master(user)
end
context 'validations' do
it 'raises error if milestone does not belong to a project' do
allow(milestone).to receive(:project_milestone?).and_return(false)
expect { service.execute(milestone) }.to raise_error(described_class::PromoteMilestoneError)
end
it 'raises error if project does not belong to a group' do
project.update(namespace: user.namespace)
expect { service.execute(milestone) }.to raise_error(described_class::PromoteMilestoneError)
end
end
context 'without duplicated milestone titles across projects' do
it 'promotes project milestone to group milestone' do
promoted_milestone = service.execute(milestone)
expect(promoted_milestone).to be_group_milestone
end
it 'sets issuables with new promoted milestone' do
issue = create(:issue, milestone: milestone, project: project)
merge_request = create(:merge_request, milestone: milestone, source_project: project)
promoted_milestone = service.execute(milestone)
expect(promoted_milestone).to be_group_milestone
expect(issue.reload.milestone).to eq(promoted_milestone)
expect(merge_request.reload.milestone).to eq(promoted_milestone)
end
end
context 'with duplicated milestone titles across projects' do
let(:project_2) { create(:project, namespace: group) }
let!(:milestone_2) { create(:milestone, project: project_2, title: milestone_title) }
it 'deletes project milestones with the same title' do
promoted_milestone = service.execute(milestone)
expect(promoted_milestone).to be_group_milestone
expect(promoted_milestone).to be_valid
expect(Milestone.exists?(milestone.id)).to be_falsy
expect(Milestone.exists?(milestone_2.id)).to be_falsy
end
it 'sets all issuables with new promoted milestone' do
issue = create(:issue, milestone: milestone, project: project)
issue_2 = create(:issue, milestone: milestone_2, project: project_2)
merge_request = create(:merge_request, milestone: milestone, source_project: project)
merge_request_2 = create(:merge_request, milestone: milestone_2, source_project: project_2)
promoted_milestone = service.execute(milestone)
expect(issue.reload.milestone).to eq(promoted_milestone)
expect(issue_2.reload.milestone).to eq(promoted_milestone)
expect(merge_request.reload.milestone).to eq(promoted_milestone)
expect(merge_request_2.reload.milestone).to eq(promoted_milestone)
end
end
end
end