Support branch creation from confidential issue
Accept a `confidential_issue_project_id` param which will be used for the system note target. This also includes some refactoring on the spec to use shared examples.
This commit is contained in:
parent
09454b702a
commit
ac3de494bd
4 changed files with 172 additions and 135 deletions
|
@ -64,7 +64,7 @@ class Projects::BranchesController < Projects::ApplicationController
|
|||
success = (result[:status] == :success)
|
||||
|
||||
if params[:issue_iid] && success
|
||||
issue = IssuesFinder.new(current_user, project_id: @project.id).find_by(iid: params[:issue_iid])
|
||||
issue = IssuesFinder.new(current_user, project_id: (confidential_issue_project || @project).id).find_by(iid: params[:issue_iid])
|
||||
SystemNoteService.new_issue_branch(issue, @project, current_user, branch_name) if issue
|
||||
end
|
||||
|
||||
|
@ -162,4 +162,15 @@ class Projects::BranchesController < Projects::ApplicationController
|
|||
@branches = Kaminari.paginate_array(@branches).page(params[:page])
|
||||
end
|
||||
end
|
||||
|
||||
def confidential_issue_project
|
||||
return unless Feature.enabled?(:create_confidential_merge_request, @project)
|
||||
return if params[:confidential_issue_project_id].blank?
|
||||
|
||||
confidential_issue_project = Project.find(params[:confidential_issue_project_id])
|
||||
|
||||
return unless can?(current_user, :push_code, confidential_issue_project)
|
||||
|
||||
confidential_issue_project
|
||||
end
|
||||
end
|
||||
|
|
|
@ -103,6 +103,75 @@ describe Projects::BranchesController do
|
|||
}
|
||||
end
|
||||
|
||||
context 'confidential_issue_project_id is present' do
|
||||
let(:confidential_issue_project) { create(:project) }
|
||||
|
||||
def create_branch_with_confidential_issue_project
|
||||
post(
|
||||
:create,
|
||||
params: {
|
||||
namespace_id: project.namespace,
|
||||
project_id: project,
|
||||
branch_name: branch,
|
||||
confidential_issue_project_id: confidential_issue_project.id,
|
||||
issue_iid: issue.iid
|
||||
}
|
||||
)
|
||||
end
|
||||
|
||||
context 'create_confidential_merge_request feature is enabled' do
|
||||
before do
|
||||
stub_feature_flags(create_confidential_merge_request: true)
|
||||
end
|
||||
|
||||
context 'user cannot push code to issue project' do
|
||||
let(:issue) { create(:issue, project: confidential_issue_project) }
|
||||
|
||||
it 'does not post a system note' do
|
||||
expect(SystemNoteService).not_to receive(:new_issue_branch)
|
||||
|
||||
create_branch_with_confidential_issue_project
|
||||
end
|
||||
end
|
||||
|
||||
context 'user can push code to issue project' do
|
||||
before do
|
||||
confidential_issue_project.add_developer(user)
|
||||
end
|
||||
|
||||
context 'issue is under the specified project' do
|
||||
let(:issue) { create(:issue, project: confidential_issue_project) }
|
||||
|
||||
it 'posts a system note' do
|
||||
expect(SystemNoteService).to receive(:new_issue_branch).with(issue, confidential_issue_project, user, "1-feature-branch", branch_project: project)
|
||||
|
||||
create_branch_with_confidential_issue_project
|
||||
end
|
||||
end
|
||||
|
||||
context 'issue is not under the specified project' do
|
||||
it 'does not post a system note' do
|
||||
expect(SystemNoteService).not_to receive(:new_issue_branch)
|
||||
|
||||
create_branch_with_confidential_issue_project
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'create_confidential_merge_request feature is disabled' do
|
||||
before do
|
||||
stub_feature_flags(create_confidential_merge_request: false)
|
||||
end
|
||||
|
||||
it 'posts a system note on project' do
|
||||
expect(SystemNoteService).to receive(:new_issue_branch).with(issue, project, user, "1-feature-branch", branch_project: project)
|
||||
|
||||
create_branch_with_confidential_issue_project
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'repository-less project' do
|
||||
let(:project) { create :project }
|
||||
|
||||
|
|
|
@ -3,6 +3,8 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe Projects::IssuesController do
|
||||
include ProjectForksHelper
|
||||
|
||||
let(:project) { create(:project) }
|
||||
let(:user) { create(:user) }
|
||||
let(:issue) { create(:issue, project: project) }
|
||||
|
@ -1130,6 +1132,7 @@ describe Projects::IssuesController do
|
|||
end
|
||||
|
||||
describe 'POST create_merge_request' do
|
||||
let(:target_project_id) { nil }
|
||||
let(:project) { create(:project, :repository, :public) }
|
||||
|
||||
before do
|
||||
|
@ -1163,13 +1166,42 @@ describe Projects::IssuesController do
|
|||
expect(response).to have_gitlab_http_status(404)
|
||||
end
|
||||
|
||||
context 'target_project_id is set' do
|
||||
let(:target_project) { fork_project(project, user, repository: true) }
|
||||
let(:target_project_id) { target_project.id }
|
||||
|
||||
context 'create_confidential_merge_request feature is enabled' do
|
||||
before do
|
||||
stub_feature_flags(create_confidential_merge_request: true)
|
||||
end
|
||||
|
||||
it 'creates a new merge request' do
|
||||
expect { create_merge_request }.to change(target_project.merge_requests, :count).by(1)
|
||||
end
|
||||
end
|
||||
|
||||
context 'create_confidential_merge_request feature is disabled' do
|
||||
before do
|
||||
stub_feature_flags(create_confidential_merge_request: false)
|
||||
end
|
||||
|
||||
it 'creates a new merge request' do
|
||||
expect { create_merge_request }.to change(project.merge_requests, :count).by(1)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def create_merge_request
|
||||
post :create_merge_request, params: {
|
||||
namespace_id: project.namespace.to_param,
|
||||
project_id: project.to_param,
|
||||
id: issue.to_param
|
||||
},
|
||||
format: :json
|
||||
post(
|
||||
:create_merge_request,
|
||||
params: {
|
||||
namespace_id: project.namespace.to_param,
|
||||
project_id: project.to_param,
|
||||
id: issue.to_param,
|
||||
target_project_id: target_project_id
|
||||
},
|
||||
format: :json
|
||||
)
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
@ -13,15 +13,20 @@ describe MergeRequests::CreateFromIssueService do
|
|||
let(:custom_source_branch) { 'custom-source-branch' }
|
||||
|
||||
subject(:service) { described_class.new(project, user, service_params) }
|
||||
subject(:service_with_custom_source_branch) { described_class.new(project, user, service_params.merge(branch_name: custom_source_branch)) }
|
||||
subject(:service_with_custom_source_branch) { described_class.new(project, user, branch_name: custom_source_branch, **service_params) }
|
||||
|
||||
before do
|
||||
project.add_developer(user)
|
||||
end
|
||||
|
||||
describe '#execute' do
|
||||
context 'no target_project_id specified' do
|
||||
let(:service_params) { { issue_iid: issue.iid } }
|
||||
shared_examples_for 'a service that creates a merge request from an issue' do
|
||||
it 'returns an error when user can not create merge request on target project' do
|
||||
result = described_class.new(project, create(:user), service_params).execute
|
||||
|
||||
expect(result[:status]).to eq(:error)
|
||||
expect(result[:message]).to eq('Not allowed to create merge request')
|
||||
end
|
||||
|
||||
it 'returns an error with invalid issue iid' do
|
||||
result = described_class.new(project, user, issue_iid: -1).execute
|
||||
|
@ -30,40 +35,20 @@ describe MergeRequests::CreateFromIssueService do
|
|||
expect(result[:message]).to eq('Invalid issue iid')
|
||||
end
|
||||
|
||||
it "inherits labels" do
|
||||
issue.assign_attributes(label_ids: label_ids)
|
||||
|
||||
result = service.execute
|
||||
|
||||
expect(result[:merge_request].label_ids).to eq(label_ids)
|
||||
end
|
||||
|
||||
it "inherits milestones" do
|
||||
result = service.execute
|
||||
|
||||
expect(result[:merge_request].milestone_id).to eq(milestone_id)
|
||||
end
|
||||
|
||||
it 'delegates the branch creation to CreateBranchService' do
|
||||
expect_any_instance_of(CreateBranchService).to receive(:execute).once.and_call_original
|
||||
|
||||
service.execute
|
||||
end
|
||||
|
||||
it 'creates a branch based on issue title' do
|
||||
service.execute
|
||||
|
||||
expect(project.repository.branch_exists?(issue.to_branch_name)).to be_truthy
|
||||
expect(target_project.repository.branch_exists?(issue.to_branch_name)).to be_truthy
|
||||
end
|
||||
|
||||
it 'creates a branch using passed name' do
|
||||
service_with_custom_source_branch.execute
|
||||
|
||||
expect(project.repository.branch_exists?(custom_source_branch)).to be_truthy
|
||||
expect(target_project.repository.branch_exists?(custom_source_branch)).to be_truthy
|
||||
end
|
||||
|
||||
it 'creates the new_merge_request system note' do
|
||||
expect(SystemNoteService).to receive(:new_merge_request).with(issue, project, user, instance_of(MergeRequest))
|
||||
expect(SystemNoteService).to receive(:new_merge_request).with(issue, target_project, user, instance_of(MergeRequest))
|
||||
|
||||
service.execute
|
||||
end
|
||||
|
@ -71,19 +56,13 @@ describe MergeRequests::CreateFromIssueService do
|
|||
it 'creates the new_issue_branch system note when the branch could be created but the merge_request cannot be created' do
|
||||
expect_any_instance_of(MergeRequest).to receive(:valid?).at_least(:once).and_return(false)
|
||||
|
||||
expect(SystemNoteService).to receive(:new_issue_branch).with(issue, project, user, issue.to_branch_name)
|
||||
expect(SystemNoteService).to receive(:new_issue_branch).with(issue, target_project, user, issue.to_branch_name)
|
||||
|
||||
service.execute
|
||||
end
|
||||
|
||||
it 'creates a merge request' do
|
||||
expect { service.execute }.to change(project.merge_requests, :count).by(1)
|
||||
end
|
||||
|
||||
it 'sets the merge request title to: "WIP: Resolves "$issue-title"' do
|
||||
result = service.execute
|
||||
|
||||
expect(result[:merge_request].title).to eq("WIP: Resolve \"#{issue.title}\"")
|
||||
expect { service.execute }.to change(target_project.merge_requests, :count).by(1)
|
||||
end
|
||||
|
||||
it 'sets the merge request author to current user' do
|
||||
|
@ -107,7 +86,7 @@ describe MergeRequests::CreateFromIssueService do
|
|||
it 'sets the merge request target branch to the project default branch' do
|
||||
result = service.execute
|
||||
|
||||
expect(result[:merge_request].target_branch).to eq(project.default_branch)
|
||||
expect(result[:merge_request].target_branch).to eq(target_project.default_branch)
|
||||
end
|
||||
|
||||
it 'executes quick actions if the build service sets them in the description' do
|
||||
|
@ -123,7 +102,7 @@ describe MergeRequests::CreateFromIssueService do
|
|||
end
|
||||
|
||||
context 'when ref branch is set' do
|
||||
subject { described_class.new(project, user, service_params.merge(ref: 'feature')).execute }
|
||||
subject { described_class.new(project, user, ref: 'feature', **service_params).execute }
|
||||
|
||||
it 'sets the merge request source branch to the new issue branch' do
|
||||
expect(subject[:merge_request].source_branch).to eq(issue.to_branch_name)
|
||||
|
@ -134,25 +113,59 @@ describe MergeRequests::CreateFromIssueService do
|
|||
end
|
||||
|
||||
context 'when ref branch does not exist' do
|
||||
subject { described_class.new(project, user, service_params.merge(ref: 'no-such-branch')).execute }
|
||||
subject { described_class.new(project, user, ref: 'no-such-branch', **service_params).execute }
|
||||
|
||||
it 'creates a merge request' do
|
||||
expect { subject }.to change(project.merge_requests, :count).by(1)
|
||||
expect { subject }.to change(target_project.merge_requests, :count).by(1)
|
||||
end
|
||||
|
||||
it 'sets the merge request target branch to the project default branch' do
|
||||
expect(subject[:merge_request].target_branch).to eq(project.default_branch)
|
||||
expect(subject[:merge_request].target_branch).to eq(target_project.default_branch)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'no target_project_id specified' do
|
||||
let(:service_params) { { issue_iid: issue.iid } }
|
||||
let(:target_project) { project }
|
||||
|
||||
it_behaves_like 'a service that creates a merge request from an issue'
|
||||
|
||||
it "inherits labels" do
|
||||
issue.assign_attributes(label_ids: label_ids)
|
||||
|
||||
result = service.execute
|
||||
|
||||
expect(result[:merge_request].label_ids).to eq(label_ids)
|
||||
end
|
||||
|
||||
it "inherits milestones" do
|
||||
result = service.execute
|
||||
|
||||
expect(result[:merge_request].milestone_id).to eq(milestone_id)
|
||||
end
|
||||
|
||||
it 'sets the merge request title to: "WIP: Resolves "$issue-title"' do
|
||||
result = service.execute
|
||||
|
||||
expect(result[:merge_request].title).to eq("WIP: Resolve \"#{issue.title}\"")
|
||||
end
|
||||
end
|
||||
|
||||
context 'target_project_id is specified' do
|
||||
let(:service_params) { { issue_iid: issue.iid, target_project_id: target_project.id } }
|
||||
|
||||
context 'target project is not a fork of the project' do
|
||||
let(:target_project) { create(:project, :repository) }
|
||||
|
||||
it 'returns an error about not finding the project' do
|
||||
result = service.execute
|
||||
|
||||
expect(result[:status]).to eq(:error)
|
||||
expect(result[:message]).to eq('Project not found')
|
||||
end
|
||||
|
||||
it 'does not create merge request' do
|
||||
expect { service.execute }.to change(target_project.merge_requests, :count).by(0)
|
||||
end
|
||||
|
@ -161,101 +174,13 @@ describe MergeRequests::CreateFromIssueService do
|
|||
context 'target project is a fork of project project' do
|
||||
let(:target_project) { fork_project(project, user, repository: true) }
|
||||
|
||||
it 'creates a branch based on issue title' do
|
||||
service.execute
|
||||
|
||||
expect(target_project.repository.branch_exists?(issue.to_branch_name)).to be_truthy
|
||||
end
|
||||
|
||||
it 'creates a branch using passed name' do
|
||||
service_with_custom_source_branch.execute
|
||||
|
||||
expect(target_project.repository.branch_exists?(custom_source_branch)).to be_truthy
|
||||
end
|
||||
|
||||
it 'creates the new_merge_request system note' do
|
||||
expect(SystemNoteService).to receive(:new_merge_request).with(issue, target_project, user, instance_of(MergeRequest))
|
||||
|
||||
service.execute
|
||||
end
|
||||
|
||||
it 'creates the new_issue_branch system note when the branch could be created but the merge_request cannot be created' do
|
||||
expect_any_instance_of(MergeRequest).to receive(:valid?).at_least(:once).and_return(false)
|
||||
|
||||
expect(SystemNoteService).to receive(:new_issue_branch).with(issue, target_project, user, issue.to_branch_name)
|
||||
|
||||
service.execute
|
||||
end
|
||||
|
||||
it 'creates a merge request' do
|
||||
expect { service.execute }.to change(target_project.merge_requests, :count).by(1)
|
||||
end
|
||||
it_behaves_like 'a service that creates a merge request from an issue'
|
||||
|
||||
it 'sets the merge request title to: "WIP: $issue-branch-name' do
|
||||
result = service.execute
|
||||
|
||||
expect(result[:merge_request].title).to eq("WIP: #{issue.to_branch_name.titleize.humanize}")
|
||||
end
|
||||
|
||||
it 'sets the merge request author to current user' do
|
||||
result = service.execute
|
||||
|
||||
expect(result[:merge_request].author).to eq(user)
|
||||
end
|
||||
|
||||
it 'sets the merge request source branch to the new issue branch' do
|
||||
result = service.execute
|
||||
|
||||
expect(result[:merge_request].source_branch).to eq(issue.to_branch_name)
|
||||
end
|
||||
|
||||
it 'sets the merge request source branch to the passed branch name' do
|
||||
result = service_with_custom_source_branch.execute
|
||||
|
||||
expect(result[:merge_request].source_branch).to eq(custom_source_branch)
|
||||
end
|
||||
|
||||
it 'sets the merge request target branch to the project default branch' do
|
||||
result = service.execute
|
||||
|
||||
expect(result[:merge_request].target_branch).to eq(project.default_branch)
|
||||
end
|
||||
|
||||
it 'executes quick actions if the build service sets them in the description' do
|
||||
allow(service).to receive(:merge_request).and_wrap_original do |m, *args|
|
||||
m.call(*args).tap do |merge_request|
|
||||
merge_request.description = "/assign #{user.to_reference}"
|
||||
end
|
||||
end
|
||||
|
||||
result = service.execute
|
||||
|
||||
expect(result[:merge_request].assignees).to eq([user])
|
||||
end
|
||||
|
||||
context 'when ref branch is set' do
|
||||
subject { described_class.new(project, user, service_params.merge(ref: 'feature')).execute }
|
||||
|
||||
it 'sets the merge request source branch to the new issue branch' do
|
||||
expect(subject[:merge_request].source_branch).to eq(issue.to_branch_name)
|
||||
end
|
||||
|
||||
it 'sets the merge request target branch to the ref branch' do
|
||||
expect(subject[:merge_request].target_branch).to eq('feature')
|
||||
end
|
||||
|
||||
context 'when ref branch does not exist' do
|
||||
subject { described_class.new(project, user, service_params.merge(ref: 'no-such-branch')).execute }
|
||||
|
||||
it 'creates a merge request' do
|
||||
expect { subject }.to change(target_project.merge_requests, :count).by(1)
|
||||
end
|
||||
|
||||
it 'sets the merge request target branch to the project default branch' do
|
||||
expect(subject[:merge_request].target_branch).to eq(target_project.default_branch)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in a new issue