Commit cf2e3ff6 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'new-branch-button-issue' into 'master'

New branch button on issues

Creates a button which creates a branch for you, referencing the original issue in the branch name. When creating the MR the text `Closes #iid` is appended to the description field.

The button; with styling

![Screenshot_from_2016-02-13_17-28-54](/uploads/9595fbfef88c10a499829beaa3db11c1/Screenshot_from_2016-02-13_17-28-54.png)

Links to the related branches on the issue

![Screenshot_from_2016-02-13_17-31-48](/uploads/ee7a78754eb5e2389f8671771bb59af9/Screenshot_from_2016-02-13_17-31-48.png)

Styled like the MR links

![Screenshot_from_2016-02-13_17-31-53](/uploads/c60130333a650a16f51b014aeb1458c3/Screenshot_from_2016-02-13_17-31-53.png)


Please provide input on the following; the CI repo on the GDK had the following happening with the current way I implemented the matching:

![Screenshot_from_2016-02-14_08-57-19](/uploads/35e7a3687a0fa801aba1ad66305ab2ff/Screenshot_from_2016-02-14_08-57-19.png)

Closes #3886 

cc @DouweM 


See merge request !2808
parents 4874c549 93374066
...@@ -4,6 +4,7 @@ v 8.6.0 (unreleased) ...@@ -4,6 +4,7 @@ v 8.6.0 (unreleased)
- Bump gitlab_git to 9.0.3 (Stan Hu) - Bump gitlab_git to 9.0.3 (Stan Hu)
- Support Golang subpackage fetching (Stan Hu) - Support Golang subpackage fetching (Stan Hu)
- Bump Capybara gem to 2.6.2 (Stan Hu) - Bump Capybara gem to 2.6.2 (Stan Hu)
- New branch button appears on issues where applicable
- Contributions to forked projects are included in calendar - Contributions to forked projects are included in calendar
- Improve the formatting for the user page bio (Connor Shea) - Improve the formatting for the user page bio (Connor Shea)
- Removed the default password from the initial admin account created during - Removed the default password from the initial admin account created during
......
...@@ -49,7 +49,7 @@ form.edit-issue { ...@@ -49,7 +49,7 @@ form.edit-issue {
margin: 0; margin: 0;
} }
.merge-requests-title { .merge-requests-title, .related-branches-title {
font-size: 16px; font-size: 16px;
font-weight: 600; font-weight: 600;
} }
......
...@@ -9,7 +9,7 @@ class Projects::BranchesController < Projects::ApplicationController ...@@ -9,7 +9,7 @@ class Projects::BranchesController < Projects::ApplicationController
@sort = params[:sort] || 'name' @sort = params[:sort] || 'name'
@branches = @repository.branches_sorted_by(@sort) @branches = @repository.branches_sorted_by(@sort)
@branches = Kaminari.paginate_array(@branches).page(params[:page]).per(PER_PAGE) @branches = Kaminari.paginate_array(@branches).page(params[:page]).per(PER_PAGE)
@max_commits = @branches.reduce(0) do |memo, branch| @max_commits = @branches.reduce(0) do |memo, branch|
diverging_commit_counts = repository.diverging_commit_counts(branch) diverging_commit_counts = repository.diverging_commit_counts(branch)
[memo, diverging_commit_counts[:behind], diverging_commit_counts[:ahead]].max [memo, diverging_commit_counts[:behind], diverging_commit_counts[:ahead]].max
...@@ -23,11 +23,15 @@ class Projects::BranchesController < Projects::ApplicationController ...@@ -23,11 +23,15 @@ class Projects::BranchesController < Projects::ApplicationController
def create def create
branch_name = sanitize(strip_tags(params[:branch_name])) branch_name = sanitize(strip_tags(params[:branch_name]))
branch_name = Addressable::URI.unescape(branch_name) branch_name = Addressable::URI.unescape(branch_name)
ref = sanitize(strip_tags(params[:ref]))
ref = Addressable::URI.unescape(ref)
result = CreateBranchService.new(project, current_user). result = CreateBranchService.new(project, current_user).
execute(branch_name, ref) execute(branch_name, ref)
if params[:issue_iid]
issue = @project.issues.find_by(iid: params[:issue_iid])
SystemNoteService.new_issue_branch(issue, @project, current_user, branch_name) if issue
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,
...@@ -49,4 +53,15 @@ class Projects::BranchesController < Projects::ApplicationController ...@@ -49,4 +53,15 @@ class Projects::BranchesController < Projects::ApplicationController
format.js { render status: status[:return_code] } format.js { render status: status[:return_code] }
end end
end end
private
def ref
if params[:ref]
ref_escaped = sanitize(strip_tags(params[:ref]))
Addressable::URI.unescape(ref_escaped)
else
@project.default_branch
end
end
end end
...@@ -65,6 +65,7 @@ class Projects::IssuesController < Projects::ApplicationController ...@@ -65,6 +65,7 @@ class Projects::IssuesController < Projects::ApplicationController
@notes = @issue.notes.nonawards.with_associations.fresh @notes = @issue.notes.nonawards.with_associations.fresh
@noteable = @issue @noteable = @issue
@merge_requests = @issue.referenced_merge_requests(current_user) @merge_requests = @issue.referenced_merge_requests(current_user)
@related_branches = @issue.related_branches - @merge_requests.map(&:source_branch)
respond_with(@issue) respond_with(@issue)
end end
......
...@@ -87,11 +87,21 @@ class Issue < ActiveRecord::Base ...@@ -87,11 +87,21 @@ class Issue < ActiveRecord::Base
end end
def referenced_merge_requests(current_user = nil) def referenced_merge_requests(current_user = nil)
Gitlab::ReferenceExtractor.lazily do @referenced_merge_requests ||= {}
[self, *notes].flat_map do |note| @referenced_merge_requests[current_user] ||= begin
note.all_references(current_user).merge_requests Gitlab::ReferenceExtractor.lazily do
end [self, *notes].flat_map do |note|
end.sort_by(&:iid) note.all_references(current_user).merge_requests
end
end.sort_by(&:iid).uniq
end
end
def related_branches
return [] if self.project.empty_repo?
self.project.repository.branch_names.select do |branch|
branch =~ /\A#{iid}-(?!\d+-stable)/i
end
end end
# Reset issue events cache # Reset issue events cache
...@@ -120,4 +130,15 @@ class Issue < ActiveRecord::Base ...@@ -120,4 +130,15 @@ class Issue < ActiveRecord::Base
note.all_references(current_user).merge_requests note.all_references(current_user).merge_requests
end.uniq.select { |mr| mr.open? && mr.closes_issue?(self) } end.uniq.select { |mr| mr.open? && mr.closes_issue?(self) }
end end
def to_branch_name
"#{iid}-#{title.parameterize}"
end
def can_be_worked_on?(current_user)
!self.closed? &&
!self.project.forked? &&
self.related_branches.empty? &&
self.closed_by_merge_requests(current_user).empty?
end
end end
...@@ -47,6 +47,21 @@ module MergeRequests ...@@ -47,6 +47,21 @@ module MergeRequests
merge_request.title = merge_request.source_branch.titleize.humanize merge_request.title = merge_request.source_branch.titleize.humanize
end end
# When your branch name starts with an iid followed by a dash this pattern will
# be interpreted as the use wants to close that issue on this project
# Pattern example: 112-fix-mep-mep
# Will lead to appending `Closes #112` to the description
if match = merge_request.source_branch.match(/\A(\d+)-/)
iid = match[1]
closes_issue = "Closes ##{iid}"
if merge_request.description.present?
merge_request.description << closes_issue.prepend("\n")
else
merge_request.description = closes_issue
end
end
merge_request merge_request
end end
......
...@@ -207,6 +207,18 @@ class SystemNoteService ...@@ -207,6 +207,18 @@ 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)
h = Gitlab::Application.routes.url_helpers
link = h.namespace_project_compare_url(project.namespace, project, from: project.default_branch, to: branch)
body = "Started branch [`#{branch}`](#{link})"
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 @merge_requests.any? - if @merge_requests.any?
%h2.merge-requests-title %h2.merge-requests-title
= pluralize(@merge_requests.count, 'Related Merge Request') = pluralize(@merge_requests.count, 'Related Merge Request')
%ul.unstyled-list %ul.unstyled-list
......
- if current_user && can?(current_user, :push_code, @project) && @issue.can_be_worked_on?(current_user)
.pull-right
= link_to namespace_project_branches_path(@project.namespace, @project, branch_name: @issue.to_branch_name, issue_iid: @issue.iid), method: :post, class: 'btn', title: @issue.to_branch_name do
= icon('code-fork')
New Branch
- if @related_branches.any?
%h2.related-branches-title
= pluralize(@related_branches.count, 'Related Branch')
%ul.unstyled-list
- @related_branches.each do |branch|
%li
- sha = @project.repository.find_branch(branch).target
- ci_commit = @project.ci_commit(sha) if sha
- if ci_commit
%span.related-branch-ci-status
= render_ci_status(ci_commit)
%span.related-branch-info
%strong
= link_to namespace_project_compare_path(@project.namespace, @project, from: @project.default_branch, to: branch), class: "label-branch" do
= branch
...@@ -70,8 +70,10 @@ ...@@ -70,8 +70,10 @@
.merge-requests .merge-requests
= render 'merge_requests' = render 'merge_requests'
= render 'related_branches'
.content-block.content-block-small .content-block.content-block-small
= render 'new_branch'
= render 'votes/votes_block', votable: @issue = render 'votes/votes_block', votable: @issue
.row .row
......
...@@ -17,49 +17,79 @@ describe Projects::BranchesController do ...@@ -17,49 +17,79 @@ 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
context "invalid branch name, valid ref" do
let(:branch) { "<script>alert('merge');</script>" }
let(:ref) { "master" }
it 'redirects' do
expect(subject).
to redirect_to("/#{project.path_with_namespace}/tree/alert('merge');")
end
end
context "valid branch name, invalid ref" do
let(:branch) { "merge_branch" }
let(:ref) { "<script>alert('ref');</script>" }
it { is_expected.to render_template('new') }
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 "valid branch name with encoded slashes" do
let(:branch) { "feature%2Ftest" }
let(:ref) { "<script>alert('ref');</script>" }
it { is_expected.to render_template('new') }
it { project.repository.branch_names.include?('feature/test') }
end end
end end
context "invalid branch name, valid ref" do describe "created from the new branch button on issues" do
let(:branch) { "<script>alert('merge');</script>" } let(:branch) { "1-feature-branch" }
let(:ref) { "master" } let!(:issue) { create(:issue, project: project) }
it 'redirects' do it 'redirects' do
post :create,
namespace_id: project.namespace.to_param,
project_id: project.to_param,
branch_name: branch,
issue_iid: issue.iid
expect(subject). expect(subject).
to redirect_to("/#{project.path_with_namespace}/tree/alert('merge');") to redirect_to("/#{project.path_with_namespace}/tree/1-feature-branch")
end end
end
context "valid branch name, invalid ref" do it 'posts a system note' do
let(:branch) { "merge_branch" } expect(SystemNoteService).to receive(:new_issue_branch).with(issue, project, user, "1-feature-branch")
let(:ref) { "<script>alert('ref');</script>" }
it { is_expected.to render_template('new') }
end
context "invalid branch name, invalid ref" do post :create,
let(:branch) { "<script>alert('merge');</script>" } namespace_id: project.namespace.to_param,
let(:ref) { "<script>alert('ref');</script>" } project_id: project.to_param,
it { is_expected.to render_template('new') } branch_name: branch,
end issue_iid: issue.iid
end
context "valid branch name with encoded slashes" do
let(:branch) { "feature%2Ftest" }
let(:ref) { "<script>alert('ref');</script>" }
it { is_expected.to render_template('new') }
it { project.repository.branch_names.include?('feature/test')}
end end
end end
......
require 'rails_helper'
feature 'Start new branch from an issue', feature: true do
let!(:project) { create(:project) }
let!(:issue) { create(:issue, project: project) }
let!(:user) { create(:user)}
context "for team members" do
before do
project.team << [user, :master]
login_as(user)
end
it 'shown the new branch button', js: false do
visit namespace_project_issue_path(project.namespace, project, issue)
expect(page).to have_link "New Branch"
end
context "when there is a referenced merge request" do
let(:note) do
create(:note, :on_issue, :system, project: project,
note: "mentioned in !#{referenced_mr.iid}")
end
let(:referenced_mr) do
create(:merge_request, :simple, source_project: project, target_project: project,
description: "Fixes ##{issue.iid}")
end
before do
issue.notes << note
visit namespace_project_issue_path(project.namespace, project, issue)
end
it "hides the new branch button", js: true do
expect(page).not_to have_link "New Branch"
expect(page).to have_content /1 Related Merge Request/
end
end
end
context "for visiters" do
it 'no button is shown', js: false do
visit namespace_project_issue_path(project.namespace, project, issue)
expect(page).not_to have_link "New Branch"
end
end
end
...@@ -130,6 +130,15 @@ describe Issue, models: true do ...@@ -130,6 +130,15 @@ describe Issue, models: true do
end end
end end
describe '#related_branches' do
it "should " do
allow(subject.project.repository).to receive(:branch_names).
and_return(["mpempe", "#{subject.iid}mepmep", subject.to_branch_name])
expect(subject.related_branches).to eq [subject.to_branch_name]
end
end
it_behaves_like 'an editable mentionable' do it_behaves_like 'an editable mentionable' do
subject { create(:issue) } subject { create(:issue) }
...@@ -140,4 +149,12 @@ describe Issue, models: true do ...@@ -140,4 +149,12 @@ describe Issue, models: true do
it_behaves_like 'a Taskable' do it_behaves_like 'a Taskable' do
let(:subject) { create :issue } let(:subject) { create :issue }
end end
describe "#to_branch_name" do
let(:issue) { build(:issue, title: 'a' * 30) }
it "starts with the issue iid" do
expect(issue.to_branch_name).to match /\A#{issue.iid}-a+\z/
end
end
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 match /\AStarted 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