Commit 5a29a304 authored by Jacopo's avatar Jacopo

Shows new branch/mr button even when branch exists

parent 674702e0
...@@ -84,20 +84,21 @@ export default class CreateMergeRequestDropdown { ...@@ -84,20 +84,21 @@ export default class CreateMergeRequestDropdown {
if (data.can_create_branch) { if (data.can_create_branch) {
this.available(); this.available();
this.enable(); this.enable();
this.updateBranchName(data.suggested_branch_name);
if (!this.droplabInitialized) { if (!this.droplabInitialized) {
this.droplabInitialized = true; this.droplabInitialized = true;
this.initDroplab(); this.initDroplab();
this.bindEvents(); this.bindEvents();
} }
} else if (data.has_related_branch) { } else {
this.hide(); this.hide();
} }
}) })
.catch(() => { .catch(() => {
this.unavailable(); this.unavailable();
this.disable(); this.disable();
Flash('Failed to check if a new branch can be created.'); Flash(__('Failed to check related branches.'));
}); });
} }
...@@ -409,13 +410,16 @@ export default class CreateMergeRequestDropdown { ...@@ -409,13 +410,16 @@ export default class CreateMergeRequestDropdown {
this.unavailableButton.classList.remove('hide'); this.unavailableButton.classList.remove('hide');
} }
updateBranchName(suggestedBranchName) {
this.branchInput.value = suggestedBranchName;
this.updateCreatePaths('branch', suggestedBranchName);
}
updateInputState(target, ref, result) { updateInputState(target, ref, result) {
// target - 'branch' or 'ref' - which the input field we are searching a ref for. // target - 'branch' or 'ref' - which the input field we are searching a ref for.
// ref - string - what a user typed. // ref - string - what a user typed.
// result - string - what has been found on backend. // result - string - what has been found on backend.
const pathReplacement = `$1${ref}`;
// If a found branch equals exact the same text a user typed, // If a found branch equals exact the same text a user typed,
// that means a new branch cannot be created as it already exists. // that means a new branch cannot be created as it already exists.
if (ref === result) { if (ref === result) {
...@@ -426,18 +430,12 @@ export default class CreateMergeRequestDropdown { ...@@ -426,18 +430,12 @@ export default class CreateMergeRequestDropdown {
this.refIsValid = true; this.refIsValid = true;
this.refInput.dataset.value = ref; this.refInput.dataset.value = ref;
this.showAvailableMessage('ref'); this.showAvailableMessage('ref');
this.createBranchPath = this.createBranchPath.replace(this.regexps.ref.createBranchPath, this.updateCreatePaths(target, ref);
pathReplacement);
this.createMrPath = this.createMrPath.replace(this.regexps.ref.createMrPath,
pathReplacement);
} }
} else if (target === 'branch') { } else if (target === 'branch') {
this.branchIsValid = true; this.branchIsValid = true;
this.showAvailableMessage('branch'); this.showAvailableMessage('branch');
this.createBranchPath = this.createBranchPath.replace(this.regexps.branch.createBranchPath, this.updateCreatePaths(target, ref);
pathReplacement);
this.createMrPath = this.createMrPath.replace(this.regexps.branch.createMrPath,
pathReplacement);
} else { } else {
this.refIsValid = false; this.refIsValid = false;
this.refInput.dataset.value = ref; this.refInput.dataset.value = ref;
...@@ -457,4 +455,15 @@ export default class CreateMergeRequestDropdown { ...@@ -457,4 +455,15 @@ export default class CreateMergeRequestDropdown {
this.disableCreateAction(); this.disableCreateAction();
} }
} }
// target - 'branch' or 'ref'
// ref - string - the new value to use as branch or ref
updateCreatePaths(target, ref) {
const pathReplacement = `$1${ref}`;
this.createBranchPath = this.createBranchPath.replace(this.regexps[target].createBranchPath,
pathReplacement);
this.createMrPath = this.createMrPath.replace(this.regexps[target].createMrPath,
pathReplacement);
}
} }
...@@ -134,11 +134,11 @@ class Projects::IssuesController < Projects::ApplicationController ...@@ -134,11 +134,11 @@ class Projects::IssuesController < Projects::ApplicationController
def can_create_branch def can_create_branch
can_create = current_user && can_create = current_user &&
can?(current_user, :push_code, @project) && can?(current_user, :push_code, @project) &&
@issue.can_be_worked_on?(current_user) @issue.can_be_worked_on?
respond_to do |format| respond_to do |format|
format.json do format.json do
render json: { can_create_branch: can_create, has_related_branch: @issue.has_related_branch? } render json: { can_create_branch: can_create, suggested_branch_name: @issue.suggested_branch_name }
end end
end end
end end
...@@ -177,7 +177,7 @@ class Projects::IssuesController < Projects::ApplicationController ...@@ -177,7 +177,7 @@ class Projects::IssuesController < Projects::ApplicationController
end end
def authorize_create_merge_request! def authorize_create_merge_request!
render_404 unless can?(current_user, :push_code, @project) && @issue.can_be_worked_on?(current_user) render_404 unless can?(current_user, :push_code, @project) && @issue.can_be_worked_on?
end end
def render_issue_json def render_issue_json
......
...@@ -194,6 +194,15 @@ class Issue < ActiveRecord::Base ...@@ -194,6 +194,15 @@ class Issue < ActiveRecord::Base
branches_with_iid - branches_with_merge_request branches_with_iid - branches_with_merge_request
end end
def suggested_branch_name
return to_branch_name unless project.repository.branch_exists?(to_branch_name)
index = 2
index += 1 while project.repository.branch_exists?("#{to_branch_name}-#{index}")
"#{to_branch_name}-#{index}"
end
# Returns boolean if a related branch exists for the current issue # Returns boolean if a related branch exists for the current issue
# ignores merge requests branchs # ignores merge requests branchs
def has_related_branch? def has_related_branch?
...@@ -248,11 +257,8 @@ class Issue < ActiveRecord::Base ...@@ -248,11 +257,8 @@ class Issue < ActiveRecord::Base
end end
end end
def can_be_worked_on?(current_user) def can_be_worked_on?
!self.closed? && !self.closed? && !self.project.forked?
!self.project.forked? &&
self.related_branches(current_user).empty? &&
self.closed_by_merge_requests(current_user).empty?
end end
# Returns `true` if the current issue can be viewed by either a logged in User # Returns `true` if the current issue can be viewed by either a logged in User
......
---
title: Show new branch/mr button even when branch exists
merge_request: 17712
author: Jacopo Beschi @jacopo-beschi
type: added
...@@ -92,6 +92,7 @@ describe('Issue', function() { ...@@ -92,6 +92,7 @@ describe('Issue', function() {
function mockCanCreateBranch(canCreateBranch) { function mockCanCreateBranch(canCreateBranch) {
mock.onGet(/(.*)\/can_create_branch$/).reply(200, { mock.onGet(/(.*)\/can_create_branch$/).reply(200, {
can_create_branch: canCreateBranch, can_create_branch: canCreateBranch,
suggested_branch_name: 'foo-99',
}); });
} }
......
...@@ -376,6 +376,48 @@ describe Issue do ...@@ -376,6 +376,48 @@ describe Issue do
end end
end end
describe '#suggested_branch_name' do
let(:repository) { double }
subject { build(:issue) }
before do
allow(subject.project).to receive(:repository).and_return(repository)
end
context '#to_branch_name does not exists' do
before do
allow(repository).to receive(:branch_exists?).and_return(false)
end
it 'returns #to_branch_name' do
expect(subject.suggested_branch_name).to eq(subject.to_branch_name)
end
end
context '#to_branch_name exists not ending with -index' do
before do
allow(repository).to receive(:branch_exists?).and_return(true)
allow(repository).to receive(:branch_exists?).with(/#{subject.to_branch_name}-\d/).and_return(false)
end
it 'returns #to_branch_name ending with -2' do
expect(subject.suggested_branch_name).to eq("#{subject.to_branch_name}-2")
end
end
context '#to_branch_name exists ending with -index' do
before do
allow(repository).to receive(:branch_exists?).and_return(true)
allow(repository).to receive(:branch_exists?).with("#{subject.to_branch_name}-3").and_return(false)
end
it 'returns #to_branch_name ending with max index + 1' do
expect(subject.suggested_branch_name).to eq("#{subject.to_branch_name}-3")
end
end
end
describe '#has_related_branch?' do describe '#has_related_branch?' do
let(:issue) { create(:issue, title: "Blue Bell Knoll") } let(:issue) { create(:issue, title: "Blue Bell Knoll") }
subject { issue.has_related_branch? } subject { issue.has_related_branch? }
...@@ -425,6 +467,27 @@ describe Issue do ...@@ -425,6 +467,27 @@ describe Issue do
end end
end end
describe '#can_be_worked_on?' do
let(:project) { build(:project) }
subject { build(:issue, :opened, project: project) }
context 'is closed' do
subject { build(:issue, :closed) }
it { is_expected.not_to be_can_be_worked_on }
end
context 'project is forked' do
before do
allow(project).to receive(:forked?).and_return(true)
end
it { is_expected.not_to be_can_be_worked_on }
end
it { is_expected.to be_can_be_worked_on }
end
describe '#participants' do describe '#participants' do
context 'using a public project' do context 'using a public project' do
let(:project) { create(:project, :public) } let(:project) { create(:project, :public) }
......
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