Added link to bulk assign issues to MR author. (Issue #18876)
This commit is contained in:
parent
1584dc02d4
commit
492b4332a4
9 changed files with 202 additions and 1 deletions
|
@ -55,6 +55,7 @@ v 8.13.0 (unreleased)
|
||||||
- Fix broken repository 500 errors in project list
|
- Fix broken repository 500 errors in project list
|
||||||
- Fix Pipeline list commit column width should be adjusted
|
- Fix Pipeline list commit column width should be adjusted
|
||||||
- Close todos when accepting merge requests via the API !6486 (tonygambone)
|
- Close todos when accepting merge requests via the API !6486 (tonygambone)
|
||||||
|
- Ability to batch assign issues relating to a merge request to the author. !5725 (jamedjo)
|
||||||
- Changed Slack service user referencing from full name to username (Sebastian Poxhofer)
|
- Changed Slack service user referencing from full name to username (Sebastian Poxhofer)
|
||||||
- Add Container Registry on/off status to Admin Area !6638 (the-undefined)
|
- Add Container Registry on/off status to Admin Area !6638 (the-undefined)
|
||||||
- Grouped pipeline dropdown is a scrollable container
|
- Grouped pipeline dropdown is a scrollable container
|
||||||
|
|
|
@ -10,7 +10,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
|
||||||
before_action :module_enabled
|
before_action :module_enabled
|
||||||
before_action :merge_request, only: [
|
before_action :merge_request, only: [
|
||||||
:edit, :update, :show, :diffs, :commits, :conflicts, :builds, :pipelines, :merge, :merge_check,
|
:edit, :update, :show, :diffs, :commits, :conflicts, :builds, :pipelines, :merge, :merge_check,
|
||||||
:ci_status, :toggle_subscription, :cancel_merge_when_build_succeeds, :remove_wip, :resolve_conflicts
|
:ci_status, :toggle_subscription, :cancel_merge_when_build_succeeds, :remove_wip, :resolve_conflicts, :assign_related_issues
|
||||||
]
|
]
|
||||||
before_action :validates_merge_request, only: [:show, :diffs, :commits, :builds, :pipelines]
|
before_action :validates_merge_request, only: [:show, :diffs, :commits, :builds, :pipelines]
|
||||||
before_action :define_show_vars, only: [:show, :diffs, :commits, :conflicts, :builds, :pipelines]
|
before_action :define_show_vars, only: [:show, :diffs, :commits, :conflicts, :builds, :pipelines]
|
||||||
|
@ -355,6 +355,25 @@ class Projects::MergeRequestsController < Projects::ApplicationController
|
||||||
render layout: false
|
render layout: false
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def assign_related_issues
|
||||||
|
result = MergeRequests::AssignIssuesService.new(project, current_user, merge_request: @merge_request).execute
|
||||||
|
|
||||||
|
respond_to do |format|
|
||||||
|
format.html do
|
||||||
|
case result[:count]
|
||||||
|
when 0
|
||||||
|
flash[:error] = "Failed to assign you issues related to the merge request"
|
||||||
|
when 1
|
||||||
|
flash[:notice] = "1 issue has been assigned to you"
|
||||||
|
else
|
||||||
|
flash[:notice] = "#{result[:count]} issues have been assigned to you"
|
||||||
|
end
|
||||||
|
|
||||||
|
redirect_to(merge_request_path(@merge_request))
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
def ci_status
|
def ci_status
|
||||||
pipeline = @merge_request.pipeline
|
pipeline = @merge_request.pipeline
|
||||||
if pipeline
|
if pipeline
|
||||||
|
|
|
@ -72,6 +72,19 @@ module MergeRequestsHelper
|
||||||
)
|
)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def mr_assign_issues_link
|
||||||
|
issues = MergeRequests::AssignIssuesService.new(@project,
|
||||||
|
current_user,
|
||||||
|
merge_request: @merge_request,
|
||||||
|
closes_issues: mr_closes_issues
|
||||||
|
).assignable_issues
|
||||||
|
path = assign_related_issues_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)
|
||||||
|
if issues.present?
|
||||||
|
pluralize_this_issue = issues.count > 1 ? "these issues" : "this issue"
|
||||||
|
link_to "Assign yourself to #{pluralize_this_issue}", path, method: :post
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
def source_branch_with_namespace(merge_request)
|
def source_branch_with_namespace(merge_request)
|
||||||
branch = link_to(merge_request.source_branch, namespace_project_commits_path(merge_request.source_project.namespace, merge_request.source_project, merge_request.source_branch))
|
branch = link_to(merge_request.source_branch, namespace_project_commits_path(merge_request.source_project.namespace, merge_request.source_project, merge_request.source_branch))
|
||||||
|
|
||||||
|
|
35
app/services/merge_requests/assign_issues_service.rb
Normal file
35
app/services/merge_requests/assign_issues_service.rb
Normal file
|
@ -0,0 +1,35 @@
|
||||||
|
module MergeRequests
|
||||||
|
class AssignIssuesService < BaseService
|
||||||
|
def assignable_issues
|
||||||
|
@assignable_issues ||= begin
|
||||||
|
if current_user == merge_request.author
|
||||||
|
closes_issues.
|
||||||
|
reject { |issue| issue.assignee_id? }.
|
||||||
|
select { |issue| can?(current_user, :admin_issue, issue) }
|
||||||
|
else
|
||||||
|
[]
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def execute
|
||||||
|
assignable_issues.each do |issue|
|
||||||
|
Issues::UpdateService.new(issue.project, current_user, assignee_id: current_user.id).execute(issue)
|
||||||
|
end
|
||||||
|
|
||||||
|
{
|
||||||
|
count: assignable_issues.count
|
||||||
|
}
|
||||||
|
end
|
||||||
|
|
||||||
|
private
|
||||||
|
|
||||||
|
def merge_request
|
||||||
|
params[:merge_request]
|
||||||
|
end
|
||||||
|
|
||||||
|
def closes_issues
|
||||||
|
@closes_issues ||= params[:closes_issues] || merge_request.closes_issues(current_user)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
|
@ -35,3 +35,4 @@
|
||||||
Accepting this merge request will close #{"issue".pluralize(mr_closes_issues.size)}
|
Accepting this merge request will close #{"issue".pluralize(mr_closes_issues.size)}
|
||||||
= succeed '.' do
|
= succeed '.' do
|
||||||
!= markdown issues_sentence(mr_closes_issues), pipeline: :gfm, author: @merge_request.author
|
!= markdown issues_sentence(mr_closes_issues), pipeline: :gfm, author: @merge_request.author
|
||||||
|
= mr_assign_issues_link
|
||||||
|
|
|
@ -277,6 +277,7 @@ resources :namespaces, path: '/', constraints: { id: /[a-zA-Z.0-9_\-]+/ }, only:
|
||||||
post :remove_wip
|
post :remove_wip
|
||||||
get :diff_for_path
|
get :diff_for_path
|
||||||
post :resolve_conflicts
|
post :resolve_conflicts
|
||||||
|
post :assign_related_issues
|
||||||
end
|
end
|
||||||
|
|
||||||
collection do
|
collection do
|
||||||
|
|
|
@ -708,4 +708,35 @@ describe Projects::MergeRequestsController do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe 'POST assign_related_issues' do
|
||||||
|
let(:issue1) { create(:issue, project: project) }
|
||||||
|
let(:issue2) { create(:issue, project: project) }
|
||||||
|
|
||||||
|
def post_assign_issues
|
||||||
|
merge_request.update!(description: "Closes #{issue1.to_reference} and #{issue2.to_reference}",
|
||||||
|
author: user,
|
||||||
|
source_branch: 'feature',
|
||||||
|
target_branch: 'master')
|
||||||
|
|
||||||
|
post :assign_related_issues,
|
||||||
|
namespace_id: project.namespace.to_param,
|
||||||
|
project_id: project.to_param,
|
||||||
|
id: merge_request.iid
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'shows a flash message on success' do
|
||||||
|
post_assign_issues
|
||||||
|
|
||||||
|
expect(flash[:notice]).to eq '2 issues have been assigned to you'
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'correctly pluralizes flash message on success' do
|
||||||
|
issue2.update!(assignee: user)
|
||||||
|
|
||||||
|
post_assign_issues
|
||||||
|
|
||||||
|
expect(flash[:notice]).to eq '1 issue has been assigned to you'
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
51
spec/features/merge_requests/assign_issues_spec.rb
Normal file
51
spec/features/merge_requests/assign_issues_spec.rb
Normal file
|
@ -0,0 +1,51 @@
|
||||||
|
require 'rails_helper'
|
||||||
|
|
||||||
|
feature 'Merge request issue assignment', js: true, feature: true do
|
||||||
|
let(:user) { create(:user) }
|
||||||
|
let(:project) { create(:project, :public) }
|
||||||
|
let(:issue1) { create(:issue, project: project) }
|
||||||
|
let(:issue2) { create(:issue, project: project) }
|
||||||
|
let(:merge_request) { create(:merge_request, :simple, source_project: project, author: user, description: "fixes #{issue1.to_reference} and #{issue2.to_reference}") }
|
||||||
|
let(:service) { MergeRequests::AssignIssuesService.new(merge_request, user, user, project) }
|
||||||
|
|
||||||
|
before do
|
||||||
|
project.team << [user, :developer]
|
||||||
|
end
|
||||||
|
|
||||||
|
def visit_merge_request(current_user = nil)
|
||||||
|
login_as(current_user || user)
|
||||||
|
visit namespace_project_merge_request_path(project.namespace, project, merge_request)
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'logged in as author' do
|
||||||
|
scenario 'updates related issues' do
|
||||||
|
visit_merge_request
|
||||||
|
click_link "Assign yourself to these issues"
|
||||||
|
|
||||||
|
expect(page).to have_content "2 issues have been assigned to you"
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'returns user to the merge request' do
|
||||||
|
visit_merge_request
|
||||||
|
click_link "Assign yourself to these issues"
|
||||||
|
|
||||||
|
expect(page).to have_content merge_request.description
|
||||||
|
end
|
||||||
|
|
||||||
|
it "doesn't display if related issues are already assigned" do
|
||||||
|
[issue1, issue2].each { |issue| issue.update!(assignee: user) }
|
||||||
|
|
||||||
|
visit_merge_request
|
||||||
|
|
||||||
|
expect(page).not_to have_content "Assign yourself"
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'not MR author' do
|
||||||
|
it "doesn't not show assignment link" do
|
||||||
|
visit_merge_request(create(:user))
|
||||||
|
|
||||||
|
expect(page).not_to have_content "Assign yourself"
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
49
spec/services/merge_requests/assign_issues_service_spec.rb
Normal file
49
spec/services/merge_requests/assign_issues_service_spec.rb
Normal file
|
@ -0,0 +1,49 @@
|
||||||
|
require 'spec_helper'
|
||||||
|
|
||||||
|
describe MergeRequests::AssignIssuesService, services: true do
|
||||||
|
let(:user) { create(:user) }
|
||||||
|
let(:project) { create(:project, :public) }
|
||||||
|
let(:issue) { create(:issue, project: project) }
|
||||||
|
let(:merge_request) { create(:merge_request, :simple, source_project: project, author: user, description: "fixes #{issue.to_reference}") }
|
||||||
|
let(:service) { described_class.new(project, user, merge_request: merge_request) }
|
||||||
|
|
||||||
|
before do
|
||||||
|
project.team << [user, :developer]
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'finds unassigned issues fixed in merge request' do
|
||||||
|
expect(service.assignable_issues.map(&:id)).to include(issue.id)
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'ignores issues already assigned to any user' do
|
||||||
|
issue.update!(assignee: create(:user))
|
||||||
|
|
||||||
|
expect(service.assignable_issues).to be_empty
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'ignores issues the user cannot update assignee on' do
|
||||||
|
project.team.truncate
|
||||||
|
|
||||||
|
expect(service.assignable_issues).to be_empty
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'ignores all issues unless current_user is merge_request.author' do
|
||||||
|
merge_request.update!(author: create(:user))
|
||||||
|
|
||||||
|
expect(service.assignable_issues).to be_empty
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'accepts precomputed data for closes_issues' do
|
||||||
|
issue2 = create(:issue, project: project)
|
||||||
|
service2 = described_class.new(project,
|
||||||
|
user,
|
||||||
|
merge_request: merge_request,
|
||||||
|
closes_issues: [issue, issue2])
|
||||||
|
|
||||||
|
expect(service2.assignable_issues.count).to eq 2
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'assigns these to the merge request owner' do
|
||||||
|
expect { service.execute }.to change { issue.reload.assignee }.to(user)
|
||||||
|
end
|
||||||
|
end
|
Loading…
Reference in a new issue