Commit ad97bebf authored by Zeger-Jan van de Weg's avatar Zeger-Jan van de Weg Committed by Zeger-Jan van de Weg

Enhance new branch button on an issue

parent 228007df
...@@ -27,6 +27,12 @@ class Projects::BranchesController < Projects::ApplicationController ...@@ -27,6 +27,12 @@ class Projects::BranchesController < Projects::ApplicationController
result = CreateBranchService.new(project, current_user). result = CreateBranchService.new(project, current_user).
execute(branch_name, ref) execute(branch_name, ref)
if params[:issue_id]
issue = Issue.where(id: params[:issue_id], project: @project).limit(1).first
SystemNoteService.new_issue_branch(issue, @project, current_user, branch_name)
end
if result[:status] == :success if result[:status] == :success
@branch = result[:branch] @branch = result[:branch]
redirect_to namespace_project_tree_path(@project.namespace, @project, redirect_to namespace_project_tree_path(@project.namespace, @project,
......
...@@ -95,7 +95,7 @@ class Issue < ActiveRecord::Base ...@@ -95,7 +95,7 @@ class Issue < ActiveRecord::Base
end end
def related_branches def related_branches
self.project.repository.branch_names.select { |branch| /\A#{iid}-/ =~ branch } self.project.repository.branch_names.select { |branch| branch.start_with? "#{iid}-" }
end end
# Reset issue events cache # Reset issue events cache
...@@ -126,12 +126,13 @@ class Issue < ActiveRecord::Base ...@@ -126,12 +126,13 @@ class Issue < ActiveRecord::Base
end end
def to_branch_name def to_branch_name
"#{iid}-#{title.parameterize}"[0,25] "#{iid}-#{title.parameterize}"
end end
def new_branch_button?(current_user) def can_be_worked_on?(current_user)
!self.closed? && !self.closed? &&
referenced_merge_requests(current_user).empty? && !self.project.forked? &&
related_branches.empty? referenced_merge_requests(current_user).none? { |mr| mr.closes_issue?(self) } &&
related_branches.empty?
end end
end end
...@@ -51,11 +51,15 @@ module MergeRequests ...@@ -51,11 +51,15 @@ module MergeRequests
# be interpreted as the use wants to close that issue on this project # be interpreted as the use wants to close that issue on this project
# Pattern example: 112-fix-mep-mep # Pattern example: 112-fix-mep-mep
# Will lead to appending `Closes #112` to the description # Will lead to appending `Closes #112` to the description
if merge_request.source_branch =~ /\A\d+-/ if match = merge_request.source_branch.match(/\A(\d+)-/)
closes_issue = "Closes ##{Regexp.last_match(0)[0...-1]}" iid = match[1]
closes_issue.prepend("\n") if merge_request.description.present? closes_issue = "Closes ##{iid}"
merge_request.description << closes_issue if merge_request.description.present?
merge_request.description << closes_issue.prepend("\n")
else
merge_request.description = closes_issue
end
end end
merge_request merge_request
......
...@@ -207,6 +207,15 @@ class SystemNoteService ...@@ -207,6 +207,15 @@ class SystemNoteService
create_note(noteable: noteable, project: project, author: author, note: body) create_note(noteable: noteable, project: project, author: author, note: body)
end end
# Called when a branch is created from the 'new branch' button on a issue
# Example note text:
#
# "Started branch `201-issue-branch-button`"
def self.new_issue_branch(issue, project, author, branch)
body = "Started branch `#{branch}`"
create_note(noteable: issue, project: project, author: author, note: body)
end
# Called when a Mentionable references a Noteable # Called when a Mentionable references a Noteable
# #
# noteable - Noteable object being referenced # noteable - Noteable object being referenced
......
- if current_user && can?(current_user, :push_code, @project) && @issue.new_branch_button?(current_user) - if current_user && can?(current_user, :push_code, @project) && @issue.can_be_worked_on?(current_user)
.pull-right .pull-right
= link_to namespace_project_branches_path(@project.namespace, @project, branch_name: @issue.to_branch_name), method: :post, class: 'btn' do = link_to namespace_project_branches_path(@project.namespace, @project, branch_name: @issue.to_branch_name, issue_id: @issue.id), method: :post, class: 'btn', title: @issue.to_branch_name do
= icon('code-fork') = icon('code-fork')
New Branch New Branch
...@@ -17,49 +17,70 @@ describe Projects::BranchesController do ...@@ -17,49 +17,70 @@ describe Projects::BranchesController do
describe "POST create" do describe "POST create" do
render_views render_views
before do context "on creation of a new branch" do
post :create, before do
namespace_id: project.namespace.to_param, post :create,
project_id: project.to_param, namespace_id: project.namespace.to_param,
branch_name: branch, project_id: project.to_param,
ref: ref branch_name: branch,
end ref: ref
end
context "valid branch name, valid source" do context "valid branch name, valid source" do
let(:branch) { "merge_branch" } let(:branch) { "merge_branch" }
let(:ref) { "master" } let(:ref) { "master" }
it 'redirects' do it 'redirects' do
expect(subject). expect(subject).
to redirect_to("/#{project.path_with_namespace}/tree/merge_branch") to redirect_to("/#{project.path_with_namespace}/tree/merge_branch")
end
end end
end
context "invalid branch name, valid ref" do context "invalid branch name, valid ref" do
let(:branch) { "<script>alert('merge');</script>" } let(:branch) { "<script>alert('merge');</script>" }
let(:ref) { "master" } let(:ref) { "master" }
it 'redirects' do it 'redirects' do
expect(subject). expect(subject).
to redirect_to("/#{project.path_with_namespace}/tree/alert('merge');") to redirect_to("/#{project.path_with_namespace}/tree/alert('merge');")
end
end end
end
context "valid branch name, invalid ref" do context "valid branch name, invalid ref" do
let(:branch) { "merge_branch" } let(:branch) { "merge_branch" }
let(:ref) { "<script>alert('ref');</script>" } let(:ref) { "<script>alert('ref');</script>" }
it { is_expected.to render_template('new') } it { is_expected.to render_template('new') }
end end
context "invalid branch name, invalid ref" do
let(:branch) { "<script>alert('merge');</script>" }
let(:ref) { "<script>alert('ref');</script>" }
it { is_expected.to render_template('new') }
end
context "invalid branch name, invalid ref" do context "valid branch name with encoded slashes" do
let(:branch) { "<script>alert('merge');</script>" } let(:branch) { "feature%2Ftest" }
let(:ref) { "<script>alert('ref');</script>" } let(:ref) { "<script>alert('ref');</script>" }
it { is_expected.to render_template('new') } it { is_expected.to render_template('new') }
it { project.repository.branch_names.include?('feature/test') }
end
end end
context "valid branch name with encoded slashes" do describe "created from the new branch button on issues" do
let(:branch) { "feature%2Ftest" } let(:branch) { "1-feature-branch" }
let(:ref) { "<script>alert('ref');</script>" } let!(:issue) { create(:issue) }
it { is_expected.to render_template('new') }
it { project.repository.branch_names.include?('feature/test')} before do
post :create,
namespace_id: project.namespace.to_param,
project_id: project.to_param,
branch_name: branch,
issue_id: issue.id
end
it 'redirects' do
expect(subject).
to redirect_to("/#{project.path_with_namespace}/tree/1-feature-branch")
end
end end
end end
......
...@@ -144,10 +144,6 @@ describe Issue, models: true do ...@@ -144,10 +144,6 @@ describe Issue, models: true do
describe "#to_branch_name" do describe "#to_branch_name" do
let(:issue) { build(:issue, title: 'a' * 30) } let(:issue) { build(:issue, title: 'a' * 30) }
it "is expected not to exceed 25 chars" do
expect(issue.to_branch_name.length).to eq 25
end
it "starts with the issue iid" do it "starts with the issue iid" do
expect(issue.to_branch_name).to match /\A#{issue.iid}-a+\z/ expect(issue.to_branch_name).to match /\A#{issue.iid}-a+\z/
end end
......
...@@ -280,6 +280,18 @@ describe SystemNoteService, services: true do ...@@ -280,6 +280,18 @@ describe SystemNoteService, services: true do
end end
end end
describe '.new_issue_branch' do
subject { described_class.new_issue_branch(noteable, project, author, "1-mepmep") }
it_behaves_like 'a system note'
context 'when a branch is created from the new branch button' do
it 'sets the note text' do
expect(subject.note).to eq 'Started branch 1-mepmep'
end
end
end
describe '.cross_reference' do describe '.cross_reference' do
subject { described_class.cross_reference(noteable, mentioner, author) } subject { described_class.cross_reference(noteable, mentioner, author) }
......
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