Commit 1ca5520b authored by Patrick Bajao's avatar Patrick Bajao

Fix issues when creating system notes

When `confidential_issue_project_id` is set and the issue is
under that project, create the a note about branch creation
in that project. If not, do nothing.

When creating `new_merge_request` system note, set the project
where the MR will be referenced from so it'll be linked to when
the MR is created in another project.
parent ac3de494
...@@ -64,8 +64,9 @@ class Projects::BranchesController < Projects::ApplicationController ...@@ -64,8 +64,9 @@ class Projects::BranchesController < Projects::ApplicationController
success = (result[:status] == :success) success = (result[:status] == :success)
if params[:issue_iid] && success if params[:issue_iid] && success
issue = IssuesFinder.new(current_user, project_id: (confidential_issue_project || @project).id).find_by(iid: params[:issue_iid]) target_project = confidential_issue_project || @project
SystemNoteService.new_issue_branch(issue, @project, current_user, branch_name) if issue issue = IssuesFinder.new(current_user, project_id: target_project.id).find_by(iid: params[:issue_iid])
SystemNoteService.new_issue_branch(issue, target_project, current_user, branch_name, branch_project: @project) if issue
end end
respond_to do |format| respond_to do |format|
......
...@@ -25,11 +25,11 @@ module MergeRequests ...@@ -25,11 +25,11 @@ module MergeRequests
new_merge_request = create(merge_request) new_merge_request = create(merge_request)
if new_merge_request.valid? if new_merge_request.valid?
SystemNoteService.new_merge_request(issue, target_project, current_user, new_merge_request) SystemNoteService.new_merge_request(issue, project, current_user, new_merge_request)
success(new_merge_request) success(new_merge_request)
else else
SystemNoteService.new_issue_branch(issue, target_project, current_user, branch_name) SystemNoteService.new_issue_branch(issue, project, current_user, branch_name, branch_project: target_project)
error(new_merge_request.errors) error(new_merge_request.errors)
end end
......
...@@ -404,8 +404,9 @@ module SystemNoteService ...@@ -404,8 +404,9 @@ module SystemNoteService
# Example note text: # Example note text:
# #
# "created branch `201-issue-branch-button`" # "created branch `201-issue-branch-button`"
def new_issue_branch(issue, project, author, branch) def new_issue_branch(issue, project, author, branch, branch_project: nil)
link = url_helpers.project_compare_path(project, from: project.default_branch, to: branch) branch_project ||= project
link = url_helpers.project_compare_path(branch_project, from: branch_project.default_branch, to: branch)
body = "created branch [`#{branch}`](#{link}) to address this issue" body = "created branch [`#{branch}`](#{link}) to address this issue"
...@@ -413,7 +414,7 @@ module SystemNoteService ...@@ -413,7 +414,7 @@ module SystemNoteService
end end
def new_merge_request(issue, project, author, merge_request) def new_merge_request(issue, project, author, merge_request)
body = "created merge request #{merge_request.to_reference} to address this issue" body = "created merge request #{merge_request.to_reference(project)} to address this issue"
create_note(NoteSummary.new(issue, project, author, body, action: 'merge')) create_note(NoteSummary.new(issue, project, author, body, action: 'merge'))
end end
......
...@@ -92,7 +92,7 @@ describe Projects::BranchesController do ...@@ -92,7 +92,7 @@ describe Projects::BranchesController do
end end
it 'posts a system note' do it 'posts a system note' do
expect(SystemNoteService).to receive(:new_issue_branch).with(issue, project, user, "1-feature-branch") expect(SystemNoteService).to receive(:new_issue_branch).with(issue, project, user, "1-feature-branch", branch_project: project)
post :create, post :create,
params: { params: {
......
...@@ -48,7 +48,7 @@ describe MergeRequests::CreateFromIssueService do ...@@ -48,7 +48,7 @@ describe MergeRequests::CreateFromIssueService do
end end
it 'creates the new_merge_request system note' do it 'creates the new_merge_request system note' do
expect(SystemNoteService).to receive(:new_merge_request).with(issue, target_project, user, instance_of(MergeRequest)) expect(SystemNoteService).to receive(:new_merge_request).with(issue, project, user, instance_of(MergeRequest))
service.execute service.execute
end end
...@@ -56,7 +56,7 @@ describe MergeRequests::CreateFromIssueService do ...@@ -56,7 +56,7 @@ 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 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_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) expect(SystemNoteService).to receive(:new_issue_branch).with(issue, project, user, issue.to_branch_name, branch_project: target_project)
service.execute service.execute
end end
......
...@@ -454,16 +454,32 @@ describe SystemNoteService do ...@@ -454,16 +454,32 @@ describe SystemNoteService do
end end
describe '.new_issue_branch' do describe '.new_issue_branch' do
subject { described_class.new_issue_branch(noteable, project, author, "1-mepmep") } let(:branch) { '1-mepmep' }
subject { described_class.new_issue_branch(noteable, project, author, branch, branch_project: branch_project) }
shared_examples_for 'a system note for new issue branch' do
it_behaves_like 'a system note' do it_behaves_like 'a system note' do
let(:action) { 'branch' } let(:action) { 'branch' }
end end
context 'when a branch is created from the new branch button' do context 'when a branch is created from the new branch button' do
it 'sets the note text' do it 'sets the note text' do
expect(subject.note).to start_with("created branch [`1-mepmep`]") expect(subject.note).to start_with("created branch [`#{branch}`]")
end
end
end
context 'branch_project is set' do
let(:branch_project) { create(:project, :repository) }
it_behaves_like 'a system note for new issue branch'
end end
context 'branch_project is not set' do
let(:branch_project) { nil }
it_behaves_like 'a system note for new issue branch'
end end
end end
...@@ -477,7 +493,7 @@ describe SystemNoteService do ...@@ -477,7 +493,7 @@ describe SystemNoteService do
end end
it 'sets the new merge request note text' do it 'sets the new merge request note text' do
expect(subject.note).to eq("created merge request #{merge_request.to_reference} to address this issue") expect(subject.note).to eq("created merge request #{merge_request.to_reference(project)} to address this issue")
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment