kotovalexarian-likes-gitlab
/
gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Add an ability to use a custom branch name on creation from issues

This commit is contained in:
Vitaliy @blackst0ne Klachkov 2017-11-25 22:33:05 +11:00
parent acae8ddb34
commit 5bc32b65f1
15 changed files with 850 additions and 290 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

492
app/assets/javascripts/create_merge_request_dropdown.js
View File

@ -2,6 +2,7 @@
import Flash from './flash';
import DropLab from './droplab/drop_lab';
import ISetter from './droplab/plugins/input_setter';
import { __, sprintf } from './locale';
// Todo: Remove this when fixing issue in input_setter plugin
const InputSetter = Object.assign({}, ISetter);
@ -12,70 +13,63 @@ const CREATE_BRANCH = 'create-branch';
export default class CreateMergeRequestDropdown {
constructor(wrapperEl) {
this.wrapperEl = wrapperEl;
this.createMergeRequestButton = this.wrapperEl.querySelector('.js-create-merge-request');
this.dropdownToggle = this.wrapperEl.querySelector('.js-dropdown-toggle');
this.dropdownList = this.wrapperEl.querySelector('.dropdown-menu');
this.availableButton = this.wrapperEl.querySelector('.available');
this.branchInput = this.wrapperEl.querySelector('.js-branch-name');
this.branchMessage = this.wrapperEl.querySelector('.js-branch-message');
this.createMergeRequestButton = this.wrapperEl.querySelector('.js-create-merge-request');
this.createTargetButton = this.wrapperEl.querySelector('.js-create-target');
this.dropdownList = this.wrapperEl.querySelector('.dropdown-menu');
this.dropdownToggle = this.wrapperEl.querySelector('.js-dropdown-toggle');
this.refInput = this.wrapperEl.querySelector('.js-ref');
this.refMessage = this.wrapperEl.querySelector('.js-ref-message');
this.unavailableButton = this.wrapperEl.querySelector('.unavailable');
this.unavailableButtonArrow = this.unavailableButton.querySelector('.fa');
this.unavailableButtonText = this.unavailableButton.querySelector('.text');
this.createBranchPath = this.wrapperEl.dataset.createBranchPath;
this.branchCreated = false;
this.branchIsValid = true;
this.canCreatePath = this.wrapperEl.dataset.canCreatePath;
this.createBranchPath = this.wrapperEl.dataset.createBranchPath;
this.createMrPath = this.wrapperEl.dataset.createMrPath;
this.droplabInitialized = false;
this.isCreatingMergeRequest = false;
this.mergeRequestCreated = false;
this.isCreatingBranch = false;
this.branchCreated = false;
this.isCreatingMergeRequest = false;
this.isGettingRef = false;
this.mergeRequestCreated = false;
this.refDebounce = _.debounce((value, target) => this.getRef(value, target), 500);
this.refIsValid = true;
this.refsPath = this.wrapperEl.dataset.refsPath;
this.suggestedRef = this.refInput.value;
// These regexps are used to replace
// a backend generated new branch name and its source (ref)
// with user's inputs.
this.regexps = {
branch: {
createBranchPath: new RegExp('(branch_name=)(.+?)(?=&issue)'),
createMrPath: new RegExp('(branch_name=)(.+?)(?=&ref)'),
},
ref: {
createBranchPath: new RegExp('(ref=)(.+?)$'),
createMrPath: new RegExp('(ref=)(.+?)$'),
},
};
this.init();
}
init() {
this.checkAbilityToCreateBranch();
}
available() {
this.availableButton.classList.remove('hide');
this.unavailableButton.classList.add('hide');
}
unavailable() {
this.availableButton.classList.add('hide');
this.unavailableButton.classList.remove('hide');
}
enable() {
this.createMergeRequestButton.classList.remove('disabled');
this.createMergeRequestButton.removeAttribute('disabled');
this.dropdownToggle.classList.remove('disabled');
this.dropdownToggle.removeAttribute('disabled');
}
disable() {
this.createMergeRequestButton.classList.add('disabled');
this.createMergeRequestButton.setAttribute('disabled', 'disabled');
this.dropdownToggle.classList.add('disabled');
this.dropdownToggle.setAttribute('disabled', 'disabled');
}
hide() {
this.wrapperEl.classList.add('hide');
}
setUnavailableButtonState(isLoading = true) {
if (isLoading) {
this.unavailableButtonArrow.classList.add('fa-spinner', 'fa-spin');
this.unavailableButtonArrow.classList.remove('fa-exclamation-triangle');
this.unavailableButtonText.textContent = 'Checking branch availability…';
} else {
this.unavailableButtonArrow.classList.remove('fa-spinner', 'fa-spin');
this.unavailableButtonArrow.classList.add('fa-exclamation-triangle');
this.unavailableButtonText.textContent = 'New branch unavailable';
}
bindEvents() {
this.createMergeRequestButton.addEventListener('click', this.onClickCreateMergeRequestButton.bind(this));
this.createTargetButton.addEventListener('click', this.onClickCreateMergeRequestButton.bind(this));
this.branchInput.addEventListener('keyup', this.onChangeInput.bind(this));
this.dropdownToggle.addEventListener('click', this.onClickSetFocusOnBranchNameInput.bind(this));
this.refInput.addEventListener('keyup', this.onChangeInput.bind(this));
this.refInput.addEventListener('keydown', CreateMergeRequestDropdown.processTab.bind(this));
}
checkAbilityToCreateBranch() {
@ -107,60 +101,18 @@ export default class CreateMergeRequestDropdown {
});
}
initDroplab() {
this.droplab = new DropLab();
this.droplab.init(this.dropdownToggle, this.dropdownList, [InputSetter],
this.getDroplabConfig());
}
getDroplabConfig() {
return {
InputSetter: [{
input: this.createMergeRequestButton,
valueAttribute: 'data-value',
inputAttribute: 'data-action',
}, {
input: this.createMergeRequestButton,
valueAttribute: 'data-text',
}],
};
}
bindEvents() {
this.createMergeRequestButton
.addEventListener('click', this.onClickCreateMergeRequestButton.bind(this));
}
isBusy() {
return this.isCreatingMergeRequest ||
this.mergeRequestCreated ||
this.isCreatingBranch ||
this.branchCreated;
}
onClickCreateMergeRequestButton(e) {
let xhr = null;
e.preventDefault();
if (this.isBusy()) {
return;
}
if (e.target.dataset.action === CREATE_MERGE_REQUEST) {
xhr = this.createMergeRequest();
} else if (e.target.dataset.action === CREATE_BRANCH) {
xhr = this.createBranch();
}
xhr.fail(() => {
this.isCreatingMergeRequest = false;
this.isCreatingBranch = false;
});
xhr.always(() => this.enable());
this.disable();
createBranch() {
return $.ajax({
method: 'POST',
dataType: 'json',
url: this.createBranchPath,
beforeSend: () => (this.isCreatingBranch = true),
})
.done((data) => {
this.branchCreated = true;
window.location.href = data.url;
})
.fail(() => new Flash('Failed to create a branch for this issue. Please try again.'));
}
createMergeRequest() {
@ -177,17 +129,343 @@ export default class CreateMergeRequestDropdown {
.fail(() => new Flash('Failed to create Merge Request. Please try again.'));
}
createBranch() {
disable() {
this.disableCreateAction();
this.dropdownToggle.classList.add('disabled');
this.dropdownToggle.setAttribute('disabled', 'disabled');
}
disableCreateAction() {
this.createMergeRequestButton.classList.add('disabled');
this.createMergeRequestButton.setAttribute('disabled', 'disabled');
this.createTargetButton.classList.add('disabled');
this.createTargetButton.setAttribute('disabled', 'disabled');
}
enable() {
this.createMergeRequestButton.classList.remove('disabled');
this.createMergeRequestButton.removeAttribute('disabled');
this.createTargetButton.classList.remove('disabled');
this.createTargetButton.removeAttribute('disabled');
this.dropdownToggle.classList.remove('disabled');
this.dropdownToggle.removeAttribute('disabled');
}
static findByValue(objects, ref, returnFirstMatch = false) {
if (!objects || !objects.length) return false;
if (objects.indexOf(ref) > -1) return ref;
if (returnFirstMatch) return objects.find(item => new RegExp(`^${ref}`).test(item));
return false;
}
getDroplabConfig() {
return {
addActiveClassToDropdownButton: true,
InputSetter: [
{
input: this.createMergeRequestButton,
valueAttribute: 'data-value',
inputAttribute: 'data-action',
},
{
input: this.createMergeRequestButton,
valueAttribute: 'data-text',
},
{
input: this.createTargetButton,
valueAttribute: 'data-value',
inputAttribute: 'data-action',
},
{
input: this.createTargetButton,
valueAttribute: 'data-text',
},
],
};
}
static getInputSelectedText(input) {
const start = input.selectionStart;
const end = input.selectionEnd;
return input.value.substr(start, end - start);
}
getRef(ref, target = 'all') {
if (!ref) return false;
return $.ajax({
method: 'POST',
method: 'GET',
dataType: 'json',
url: this.createBranchPath,
beforeSend: () => (this.isCreatingBranch = true),
url: this.refsPath + ref,
beforeSend: () => {
this.isGettingRef = true;
},
})
.always(() => {
this.isGettingRef = false;
})
.done((data) => {
this.branchCreated = true;
window.location.href = data.url;
const branches = data[Object.keys(data)[0]];
const tags = data[Object.keys(data)[1]];
let result;
if (target === 'branch') {
result = CreateMergeRequestDropdown.findByValue(branches, ref);
} else {
result = CreateMergeRequestDropdown.findByValue(branches, ref, true) ||
CreateMergeRequestDropdown.findByValue(tags, ref, true);
this.suggestedRef = result;
}
return this.updateInputState(target, ref, result);
})
.fail(() => new Flash('Failed to create a branch for this issue. Please try again.'));
.fail(() => {
this.unavailable();
this.disable();
new Flash('Failed to get ref.');
return false;
});
}
getTargetData(target) {
return {
input: this[`${target}Input`],
message: this[`${target}Message`],
};
}
hide() {
this.wrapperEl.classList.add('hide');
}
init() {
this.checkAbilityToCreateBranch();
}
initDroplab() {
this.droplab = new DropLab();
this.droplab.init(
this.dropdownToggle,
this.dropdownList,
[InputSetter],
this.getDroplabConfig(),
);
}
inputsAreValid() {
return this.branchIsValid && this.refIsValid;
}
isBusy() {
return this.isCreatingMergeRequest ||
this.mergeRequestCreated ||
this.isCreatingBranch ||
this.branchCreated ||
this.isGettingRef;
}
onChangeInput(event) {
let target;
let value;
if (event.srcElement === this.branchInput) {
target = 'branch';
value = this.branchInput.value;
} else if (event.srcElement === this.refInput) {
target = 'ref';
value = event.srcElement.value.slice(0, event.srcElement.selectionStart) +
event.srcElement.value.slice(event.srcElement.selectionEnd);
} else {
return false;
}
if (this.isGettingRef) return false;
// `ENTER` key submits the data.
if (event.keyCode === 13 && this.inputsAreValid()) {
event.preventDefault();
return this.createMergeRequestButton.click();
}
// If the input is empty, use the original value generated by the backend.
if (!value) {
this.createBranchPath = this.wrapperEl.dataset.createBranchPath;
this.createMrPath = this.wrapperEl.dataset.createMrPath;
if (target === 'branch') {
this.branchIsValid = true;
} else {
this.refIsValid = true;
}
this.enable();
this.showAvailableMessage(target);
return true;
}
this.showCheckingMessage(target);
this.refDebounce(value, target);
return true;
}
onClickCreateMergeRequestButton(event) {
let xhr = null;
event.preventDefault();
if (this.isBusy()) {
return;
}
if (event.target.dataset.action === CREATE_MERGE_REQUEST) {
xhr = this.createMergeRequest();
} else if (event.target.dataset.action === CREATE_BRANCH) {
xhr = this.createBranch();
}
xhr.fail(() => {
this.isCreatingMergeRequest = false;
this.isCreatingBranch = false;
});
xhr.always(() => this.enable());
this.disable();
}
onClickSetFocusOnBranchNameInput() {
this.branchInput.focus();
}
// `TAB` autocompletes the source.
static processTab(event) {
if (event.keyCode !== 9 || this.isGettingRef) return;
const selectedText = CreateMergeRequestDropdown.getInputSelectedText(this.refInput);
// if nothing selected, we don't need to autocomplete anything. Do the default TAB action.
// If a user manually selected text, don't autocomplete anything. Do the default TAB action.
if (!selectedText || this.refInput.dataset.value === this.suggestedRef) return;
event.preventDefault();
window.getSelection().removeAllRanges();
}
removeMessage(target) {
const { input, message } = this.getTargetData(target);
const inputClasses = ['gl-field-error-outline', 'gl-field-success-outline'];
const messageClasses = ['gl-field-hint', 'gl-field-error-message', 'gl-field-success-message'];
inputClasses.forEach(cssClass => input.classList.remove(cssClass));
messageClasses.forEach(cssClass => message.classList.remove(cssClass));
message.style.display = 'none';
}
setUnavailableButtonState(isLoading = true) {
if (isLoading) {
this.unavailableButtonArrow.classList.add('fa-spin');
this.unavailableButtonArrow.classList.add('fa-spinner');
this.unavailableButtonArrow.classList.remove('fa-exclamation-triangle');
this.unavailableButtonText.textContent = __('Checking branch availability...');
} else {
this.unavailableButtonArrow.classList.remove('fa-spin');
this.unavailableButtonArrow.classList.remove('fa-spinner');
this.unavailableButtonArrow.classList.add('fa-exclamation-triangle');
this.unavailableButtonText.textContent = __('New branch unavailable');
}
}
showAvailableMessage(target) {
const { input, message } = this.getTargetData(target);
const text = target === 'branch' ? __('Branch name') : __('Source');
this.removeMessage(target);
input.classList.add('gl-field-success-outline');
message.classList.add('gl-field-success-message');
message.textContent = sprintf(__('%{text} is available'), { text });
message.style.display = 'inline-block';
}
showCheckingMessage(target) {
const { message } = this.getTargetData(target);
const text = target === 'branch' ? __('branch name') : __('source');
this.removeMessage(target);
message.classList.add('gl-field-hint');
message.textContent = sprintf(__('Checking %{text} availability…'), { text });
message.style.display = 'inline-block';
}
showNotAvailableMessage(target) {
const { input, message } = this.getTargetData(target);
const text = target === 'branch' ? __('Branch is already taken') : __('Source is not available');
this.removeMessage(target);
input.classList.add('gl-field-error-outline');
message.classList.add('gl-field-error-message');
message.textContent = text;
message.style.display = 'inline-block';
}
unavailable() {
this.availableButton.classList.add('hide');
this.unavailableButton.classList.remove('hide');
}
updateInputState(target, ref, result) {
// target - 'branch' or 'ref' - which the input field we are searching a ref for.
// ref - string - what a user typed.
// result - string - what has been found on backend.
const pathReplacement = `$1${ref}`;
// If a found branch equals exact the same text a user typed,
// that means a new branch cannot be created as it already exists.
if (ref === result) {
if (target === 'branch') {
this.branchIsValid = false;
this.showNotAvailableMessage('branch');
} else {
this.refIsValid = true;
this.refInput.dataset.value = ref;
this.showAvailableMessage('ref');
this.createBranchPath = this.createBranchPath.replace(this.regexps.ref.createBranchPath,
pathReplacement);
this.createMrPath = this.createMrPath.replace(this.regexps.ref.createMrPath,
pathReplacement);
}
} else if (target === 'branch') {
this.branchIsValid = true;
this.showAvailableMessage('branch');
this.createBranchPath = this.createBranchPath.replace(this.regexps.branch.createBranchPath,
pathReplacement);
this.createMrPath = this.createMrPath.replace(this.regexps.branch.createMrPath,
pathReplacement);
} else {
this.refIsValid = false;
this.refInput.dataset.value = ref;
this.disableCreateAction();
this.showNotAvailableMessage('ref');
// Show ref hint.
if (result) {
this.refInput.value = result;
this.refInput.setSelectionRange(ref.length, result.length);
}
}
if (this.inputsAreValid()) {
this.enable();
} else {
this.disableCreateAction();
}
}
}

2
app/assets/javascripts/droplab/constants.js
View File

@ -3,6 +3,7 @@ const DATA_DROPDOWN = 'data-dropdown';
const SELECTED_CLASS = 'droplab-item-selected';
const ACTIVE_CLASS = 'droplab-item-active';
const IGNORE_CLASS = 'droplab-item-ignore';
const IGNORE_HIDING_CLASS = 'droplab-item-ignore-hiding';
// Matches `{{anything}}` and `{{ everything }}`.
const TEMPLATE_REGEX = /\{\{(.+?)\}\}/g;
@ -13,4 +14,5 @@ export {
ACTIVE_CLASS,
TEMPLATE_REGEX,
IGNORE_CLASS,
IGNORE_HIDING_CLASS,
};

29
app/assets/javascripts/droplab/drop_down.js
View File

@ -1,15 +1,18 @@
import utils from './utils';
import { SELECTED_CLASS, IGNORE_CLASS } from './constants';
import { SELECTED_CLASS, IGNORE_CLASS, IGNORE_HIDING_CLASS } from './constants';
class DropDown {
constructor(list) {
constructor(list, config = {}) {
this.currentIndex = 0;
this.hidden = true;
this.list = typeof list === 'string' ? document.querySelector(list) : list;
this.items = [];
this.eventWrapper = {};
if (config.addActiveClassToDropdownButton) {
this.dropdownToggle = this.list.parentNode.querySelector('.js-dropdown-toggle');
}
this.getItems();
this.initTemplateString();
this.addEvents();
@ -42,7 +45,7 @@ class DropDown {
this.addSelectedClass(selected);
e.preventDefault();
this.hide();
if (!e.target.classList.contains(IGNORE_HIDING_CLASS)) this.hide();
const listEvent = new CustomEvent('click.dl', {
detail: {
@ -67,7 +70,20 @@ class DropDown {
addEvents() {
this.eventWrapper.clickEvent = this.clickEvent.bind(this);
this.eventWrapper.closeDropdown = this.closeDropdown.bind(this);
this.list.addEventListener('click', this.eventWrapper.clickEvent);
this.list.addEventListener('keyup', this.eventWrapper.closeDropdown);
}
closeDropdown(event) {
// `ESC` key closes the dropdown.
if (event.keyCode === 27) {
event.preventDefault();
return this.toggle();
}
return true;
}
setData(data) {
@ -110,6 +126,8 @@ class DropDown {
this.list.style.display = 'block';
this.currentIndex = 0;
this.hidden = false;
if (this.dropdownToggle) this.dropdownToggle.classList.add('active');
}
hide() {
@ -117,6 +135,8 @@ class DropDown {
this.list.style.display = 'none';
this.currentIndex = 0;
this.hidden = true;
if (this.dropdownToggle) this.dropdownToggle.classList.remove('active');
}
toggle() {
@ -128,6 +148,7 @@ class DropDown {
destroy() {
this.hide();
this.list.removeEventListener('click', this.eventWrapper.clickEvent);
this.list.removeEventListener('keyup', this.eventWrapper.closeDropdown);
}
static setImagesSrc(template) {

2
app/assets/javascripts/droplab/hook.js
View File

@ -3,7 +3,7 @@ import DropDown from './drop_down';
class Hook {
constructor(trigger, list, plugins, config) {
this.trigger = trigger;
this.list = new DropDown(list);
this.list = new DropDown(list, config);
this.type = 'Hook';
this.event = 'click';
this.plugins = plugins || [];

69
app/assets/stylesheets/pages/issues.scss
View File

@ -203,7 +203,24 @@ ul.related-merge-requests > li {
}
.create-mr-dropdown-wrap {
@include new-style-dropdown;
.branch-message,
.ref-message {
display: none;
}
.ref::selection {
color: $placeholder-text-color;
}
.dropdown {
.dropdown-menu-toggle {
min-width: 285px;
}
.dropdown-select {
width: 285px;
}
}
.btn-group:not(.hide) {
display: flex;
@ -214,15 +231,16 @@ ul.related-merge-requests > li {
flex-shrink: 0;
}
.dropdown-menu {
.create-merge-request-dropdown-menu {
width: 300px;
opacity: 1;
visibility: visible;
transform: translateY(0);
display: none;
margin-top: 4px;
}
.dropdown-toggle {
.create-merge-request-dropdown-toggle {
.fa-caret-down {
pointer-events: none;
color: inherit;
@ -230,18 +248,50 @@ ul.related-merge-requests > li {
}
}
.droplab-item-ignore {
pointer-events: auto;
}
.create-item {
cursor: pointer;
margin: 0 1px;
&:hover,
&:focus {
background-color: $dropdown-item-hover-bg;
color: $gl-text-color;
}
}
li.divider {
margin: 8px 10px;
}
li:not(.divider) {
padding: 8px 9px;
&:last-child {
padding-bottom: 8px;
}
&.droplab-item-selected {
.icon-container {
i {
visibility: visible;
}
}
.description {
display: block;
}
}
&.droplab-item-ignore {
padding-top: 8px;
}
.icon-container {
float: left;
padding-left: 6px;
i {
visibility: hidden;
@ -249,13 +299,12 @@ ul.related-merge-requests > li {
}
.description {
padding-left: 30px;
font-size: 13px;
padding-left: 22px;
}
strong {
display: block;
font-weight: $gl-font-weight-bold;
}
input,
span {
margin: 4px 0 0;
}
}
}

3
app/controllers/projects/issues_controller.rb
View File

@ -158,7 +158,8 @@ class Projects::IssuesController < Projects::ApplicationController
end
def create_merge_request
result = ::MergeRequests::CreateFromIssueService.new(project, current_user, issue_iid: issue.iid).execute
create_params = params.slice(:branch_name, :ref).merge(issue_iid: issue.iid)
result = ::MergeRequests::CreateFromIssueService.new(project, current_user, create_params).execute
if result[:status] == :success
render json: MergeRequestCreateSerializer.new.represent(result[:merge_request])

66
app/services/merge_requests/build_service.rb
View File

@ -1,6 +1,8 @@
module MergeRequests
class BuildService < MergeRequests::BaseService
def execute
@issue_iid = params.delete(:issue_iid)
self.merge_request = MergeRequest.new(params)
merge_request.compare_commits = []
merge_request.source_project = find_source_project
@ -106,37 +108,53 @@ module MergeRequests
# more than one commit in the MR
#
def assign_title_and_description
if match = source_branch.match(/\A(\d+)-/)
iid = match[1]
end
assign_title_and_description_from_single_commit
assign_title_from_issue
commits = compare_commits
if commits && commits.count == 1
commit = commits.first
merge_request.title = commit.title
merge_request.description ||= commit.description.try(:strip)
elsif iid && issue = target_project.get_issue(iid, current_user)
case issue
when Issue
merge_request.title = "Resolve \"#{issue.title}\""
when ExternalIssue
merge_request.title = "Resolve #{issue.title}"
end
merge_request.title ||= source_branch.titleize.humanize
merge_request.title = wip_title if compare_commits.empty?
append_closes_description
end
def append_closes_description
return unless issue_iid
closes_issue = "Closes ##{issue_iid}"
if description.present?
merge_request.description += closes_issue.prepend("\n\n")
else
merge_request.title = source_branch.titleize.humanize
merge_request.description = closes_issue
end
end
if iid
closes_issue = "Closes ##{iid}"
def assign_title_and_description_from_single_commit
commits = compare_commits
if description.present?
merge_request.description += closes_issue.prepend("\n\n")
else
merge_request.description = closes_issue
return unless commits&.count == 1
commit = commits.first
merge_request.title ||= commit.title
merge_request.description ||= commit.description.try(:strip)
end
def assign_title_from_issue
return unless issue
merge_request.title =
case issue
when Issue then "Resolve \"#{issue.title}\""
when ExternalIssue then "Resolve #{issue.title}"
end
end
end
merge_request.title = wip_title if commits.empty?
def issue_iid
@issue_iid ||= source_branch.match(/\A(\d+)-/).try(:[], 1)
end
def issue
@issue ||= target_project.get_issue(issue_iid, current_user)
end
end
end

24
app/services/merge_requests/create_from_issue_service.rb
View File

@ -1,7 +1,18 @@
module MergeRequests
class CreateFromIssueService < MergeRequests::CreateService
def initialize(project, user, params)
# branch - the name of new branch
# ref - the source of new branch.
@branch_name = params[:branch_name]
@issue_iid = params[:issue_iid]
@ref = params[:ref]
super(project, user)
end
def execute
return error('Invalid issue iid') unless issue_iid.present? && issue.present?
return error('Invalid issue iid') unless @issue_iid.present? && issue.present?
params[:label_ids] = issue.label_ids if issue.label_ids.any?
@ -21,20 +32,16 @@ module MergeRequests
private
def issue_iid
@isssue_iid ||= params.delete(:issue_iid)
end
def issue
@issue ||= IssuesFinder.new(current_user, project_id: project.id).find_by(iid: issue_iid)
@issue ||= IssuesFinder.new(current_user, project_id: project.id).find_by(iid: @issue_iid)
end
def branch_name
@branch_name ||= issue.to_branch_name
@branch ||= @branch_name || issue.to_branch_name
end
def ref
project.default_branch || 'master'
@ref || project.default_branch || 'master'
end
def merge_request
@ -43,6 +50,7 @@ module MergeRequests
def merge_request_params
{
issue_iid: @issue_iid,
source_project_id: project.id,
source_branch: branch_name,
target_project_id: project.id,

60
app/views/projects/issues/_new_branch.html.haml
View File

@ -1,34 +1,50 @@
- can_create_merge_request = can?(current_user, :create_merge_request, @project)
- data_action = can_create_merge_request ? 'create-mr' : 'create-branch'
- value = can_create_merge_request ? 'Create a merge request' : 'Create a branch'
- value = can_create_merge_request ? 'Create merge request' : 'Create branch'
- if can?(current_user, :push_code, @project)
.create-mr-dropdown-wrap{ data: { can_create_path: can_create_branch_project_issue_path(@project, @issue), create_mr_path: create_merge_request_project_issue_path(@project, @issue), create_branch_path: project_branches_path(@project, branch_name: @issue.to_branch_name, issue_iid: @issue.iid) } }
- can_create_path = can_create_branch_project_issue_path(@project, @issue)
- create_mr_path = create_merge_request_project_issue_path(@project, @issue, branch_name: @issue.to_branch_name, ref: @project.default_branch)
- create_branch_path = project_branches_path(@project, branch_name: @issue.to_branch_name, ref: @project.default_branch, issue_iid: @issue.iid)
- refs_path = refs_namespace_project_path(@project.namespace, @project, search: '')
.create-mr-dropdown-wrap{ data: { can_create_path: can_create_path, create_mr_path: create_mr_path, create_branch_path: create_branch_path, refs_path: refs_path } }
.btn-group.unavailable
%button.btn.btn-grouped{ type: 'button', disabled: 'disabled' }
= icon('spinner', class: 'fa-spin')
%span.text
Checking branch availability…
.btn-group.available.hide
%input.btn.js-create-merge-request.btn-inverted.btn-success{ type: 'button', value: value, data: { action: data_action } }
%button.btn.btn-inverted.dropdown-toggle.btn-inverted.btn-success.js-dropdown-toggle{ type: 'button', data: { 'dropdown-trigger' => '#create-merge-request-dropdown' } }
%button.btn.js-create-merge-request.btn-default{ type: 'button', data: { action: data_action } }
= value
%button.btn.create-merge-request-dropdown-toggle.dropdown-toggle.btn-default.js-dropdown-toggle{ type: 'button', data: { dropdown: { trigger: '#create-merge-request-dropdown' } } }
= icon('caret-down')
%ul#create-merge-request-dropdown.dropdown-menu.dropdown-menu-align-right{ data: { dropdown: true } }
%ul#create-merge-request-dropdown.create-merge-request-dropdown-menu.dropdown-menu.dropdown-menu-align-right.gl-show-field-errors{ data: { dropdown: true } }
- if can_create_merge_request
%li.droplab-item-selected{ role: 'button', data: { value: 'create-mr', 'text' => 'Create a merge request' } }
.menu-item
.icon-container
= icon('check')
.description
%strong Create a merge request
%span
Creates a merge request named after this issue, with source branch created from '#{@project.default_branch}'.
%li.divider.droplab-item-ignore
%li{ class: [!can_create_merge_request && 'droplab-item-selected'], role: 'button', data: { value: 'create-branch', 'text' => 'Create a branch' } }
.menu-item
.icon-container
= icon('check')
.description
%strong Create a branch
%span
Creates a branch named after this issue, from '#{@project.default_branch}'.
%li.create-item.droplab-item-selected.droplab-item-ignore-hiding{ role: 'button', data: { value: 'create-mr', text: 'Create merge request' } }
.menu-item.droplab-item-ignore-hiding
.icon-container.droplab-item-ignore-hiding= icon('check')
.description.droplab-item-ignore-hiding Create merge request and branch
%li.create-item.droplab-item-ignore-hiding{ class: [!can_create_merge_request && 'droplab-item-selected'], role: 'button', data: { value: 'create-branch', text: 'Create branch' } }
.menu-item.droplab-item-ignore-hiding
.icon-container.droplab-item-ignore-hiding= icon('check')
.description.droplab-item-ignore-hiding Create branch
%li.divider
%li.droplab-item-ignore
Branch name
%input.js-branch-name.form-control.droplab-item-ignore{ type: 'text', placeholder: "#{@issue.to_branch_name}", value: "#{@issue.to_branch_name}" }
%span.js-branch-message.branch-message.droplab-item-ignore
%li.droplab-item-ignore
Source (branch or tag)
%input.js-ref.ref.form-control.droplab-item-ignore{ type: 'text', placeholder: "#{@project.default_branch}", value: "#{@project.default_branch}", data: { value: "#{@project.default_branch}" } }
%span.js-ref-message.ref-message.droplab-item-ignore
%li.droplab-item-ignore
%button.btn.btn-success.js-create-target.droplab-item-ignore{ type: 'button', data: { action: 'create-mr' } }
Create merge request

5
changelogs/unreleased/21143-customize-branch-name-when-using-create-branch-in-an-issue.yml Normal file
View File

@ -0,0 +1,5 @@
---
title: Add an ability to use a custom branch name on creation from issues
merge_request: 13884
author: Vitaliy @blackst0ne Klachkov
type: added

106
spec/features/issues/create_branch_merge_request_spec.rb
View File

@ -1,106 +0,0 @@
require 'rails_helper'
feature 'Create Branch/Merge Request Dropdown on issue page', :feature, :js do
let(:user) { create(:user) }
let!(:project) { create(:project, :repository) }
let(:issue) { create(:issue, project: project, title: 'Cherry-Coloured Funk') }
context 'for team members' do
before do
project.team << [user, :developer]
sign_in(user)
end
it 'allows creating a merge request from the issue page' do
visit project_issue_path(project, issue)
perform_enqueued_jobs do
select_dropdown_option('create-mr')
expect(page).to have_content('WIP: Resolve "Cherry-Coloured Funk"')
expect(current_path).to eq(project_merge_request_path(project, MergeRequest.first))
wait_for_requests
end
visit project_issue_path(project, issue)
expect(page).to have_content("created branch 1-cherry-coloured-funk")
expect(page).to have_content("mentioned in merge request !1")
end
it 'allows creating a branch from the issue page' do
visit project_issue_path(project, issue)
select_dropdown_option('create-branch')
wait_for_requests
expect(page).to have_selector('.dropdown-toggle-text ', text: '1-cherry-coloured-funk')
expect(current_path).to eq project_tree_path(project, '1-cherry-coloured-funk')
end
context "when there is a referenced merge request" do
let!(:note) do
create(:note, :on_issue, :system, project: project, noteable: issue,
note: "mentioned in #{referenced_mr.to_reference}")
end
let(:referenced_mr) do
create(:merge_request, :simple, source_project: project, target_project: project,
description: "Fixes #{issue.to_reference}", author: user)
end
before do
referenced_mr.cache_merge_request_closes_issues!(user)
visit project_issue_path(project, issue)
end
it 'disables the create branch button' do
expect(page).to have_css('.create-mr-dropdown-wrap .unavailable:not(.hide)')
expect(page).to have_css('.create-mr-dropdown-wrap .available.hide', visible: false)
expect(page).to have_content /1 Related Merge Request/
end
end
context 'when merge requests are disabled' do
before do
project.project_feature.update(merge_requests_access_level: 0)
visit project_issue_path(project, issue)
end
it 'shows only create branch button' do
expect(page).not_to have_button('Create a merge request')
expect(page).to have_button('Create a branch')
end
end
context 'when issue is confidential' do
it 'disables the create branch button' do
issue = create(:issue, :confidential, project: project)
visit project_issue_path(project, issue)
expect(page).not_to have_css('.create-mr-dropdown-wrap')
end
end
end
context 'for visitors' do
before do
visit project_issue_path(project, issue)
end
it 'shows no buttons' do
expect(page).not_to have_selector('.create-mr-dropdown-wrap')
end
end
def select_dropdown_option(option)
find('.create-mr-dropdown-wrap .dropdown-toggle').click
find("li[data-value='#{option}']").click
find('.js-create-merge-request').click
end
end

248
spec/features/issues/user_creates_branch_and_merge_request_spec.rb Normal file
View File

@ -0,0 +1,248 @@
require 'rails_helper'
describe 'User creates branch and merge request on issue page', :js do
let(:user) { create(:user) }
let!(:project) { create(:project, :repository) }
let(:issue) { create(:issue, project: project, title: 'Cherry-Coloured Funk') }
context 'when signed out' do
before do
visit project_issue_path(project, issue)
end
it "doesn't show 'Create merge request' button" do
expect(page).not_to have_selector('.create-mr-dropdown-wrap')
end
end
context 'when signed in' do
before do
project.add_developer(user)
sign_in(user)
end
context 'when interacting with the dropdown' do
before do
visit project_issue_path(project, issue)
end
# In order to improve tests performance, all UI checks are placed in this test.
it 'shows elements' do
button_create_merge_request = find('.js-create-merge-request')
button_toggle_dropdown = find('.create-mr-dropdown-wrap .dropdown-toggle')
button_toggle_dropdown.click
dropdown = find('.create-merge-request-dropdown-menu')
page.within(dropdown) do
button_create_target = find('.js-create-target')
input_branch_name = find('.js-branch-name')
input_source = find('.js-ref')
li_create_branch = find("li[data-value='create-branch']")
li_create_merge_request = find("li[data-value='create-mr']")
# Test that all elements are presented.
expect(page).to have_content('Create merge request and branch')
expect(page).to have_content('Create branch')
expect(page).to have_content('Branch name')
expect(page).to have_content('Source (branch or tag)')
expect(page).to have_button('Create merge request')
expect(page).to have_selector('.js-branch-name:focus')
test_selection_mark(li_create_branch, li_create_merge_request, button_create_target, button_create_merge_request)
test_branch_name_checking(input_branch_name)
test_source_checking(input_source)
# The button inside dropdown should be disabled if any errors occured.
expect(page).to have_button('Create branch', disabled: true)
end
# The top level button should be disabled if any errors occured.
expect(page).to have_button('Create branch', disabled: true)
end
context 'when branch name is auto-generated' do
it 'creates a merge request' do
perform_enqueued_jobs do
select_dropdown_option('create-mr')
expect(page).to have_content('WIP: Resolve "Cherry-Coloured Funk"')
expect(current_path).to eq(project_merge_request_path(project, MergeRequest.first))
wait_for_requests
end
visit project_issue_path(project, issue)
expect(page).to have_content('created branch 1-cherry-coloured-funk')
expect(page).to have_content('mentioned in merge request !1')
end
it 'creates a branch' do
select_dropdown_option('create-branch')
wait_for_requests
expect(page).to have_selector('.dropdown-toggle-text ', text: '1-cherry-coloured-funk')
expect(current_path).to eq project_tree_path(project, '1-cherry-coloured-funk')
end
end
context 'when branch name is custom' do
let(:branch_name) { 'custom-branch-name' }
it 'creates a merge request' do
perform_enqueued_jobs do
select_dropdown_option('create-mr', branch_name)
expect(page).to have_content('WIP: Resolve "Cherry-Coloured Funk"')
expect(page).to have_content('Request to merge custom-branch-name into')
expect(current_path).to eq(project_merge_request_path(project, MergeRequest.first))
wait_for_requests
end
visit project_issue_path(project, issue)
expect(page).to have_content('created branch custom-branch-name')
expect(page).to have_content('mentioned in merge request !1')
end
it 'creates a branch' do
select_dropdown_option('create-branch', branch_name)
wait_for_requests
expect(page).to have_selector('.dropdown-toggle-text ', text: branch_name)
expect(current_path).to eq project_tree_path(project, branch_name)
end
end
end
context "when there is a referenced merge request" do
let!(:note) do
create(:note, :on_issue, :system, project: project, noteable: issue,
note: "mentioned in #{referenced_mr.to_reference}")
end
let(:referenced_mr) do
create(:merge_request, :simple, source_project: project, target_project: project,
description: "Fixes #{issue.to_reference}", author: user)
end
before do
referenced_mr.cache_merge_request_closes_issues!(user)
visit project_issue_path(project, issue)
end
it 'disables the create branch button' do
expect(page).to have_css('.create-mr-dropdown-wrap .unavailable:not(.hide)')
expect(page).to have_css('.create-mr-dropdown-wrap .available.hide', visible: false)
expect(page).to have_content /1 Related Merge Request/
end
end
context 'when merge requests are disabled' do
before do
project.project_feature.update(merge_requests_access_level: 0)
visit project_issue_path(project, issue)
end
it 'shows only create branch button' do
expect(page).not_to have_button('Create merge request')
expect(page).to have_button('Create branch')
end
end
context 'when issue is confidential' do
let(:issue) { create(:issue, :confidential, project: project) }
it 'disables the create branch button' do
visit project_issue_path(project, issue)
expect(page).not_to have_css('.create-mr-dropdown-wrap')
end
end
end
private
def select_dropdown_option(option, branch_name = nil)
find('.create-mr-dropdown-wrap .dropdown-toggle').click
find("li[data-value='#{option}']").click
if branch_name
find('.js-branch-name').set(branch_name)
# Javascript debounces AJAX calls.
# So we have to wait until AJAX requests are started.
# Details are in app/assets/javascripts/create_merge_request_dropdown.js
# this.refDebounce = _.debounce(...)
sleep 0.5
wait_for_requests
end
find('.js-create-merge-request').click
end
def test_branch_name_checking(input_branch_name)
expect(input_branch_name.value).to eq(issue.to_branch_name)
input_branch_name.set('new-branch-name')
branch_name_message = find('.js-branch-message')
expect(branch_name_message).to have_text('Checking branch name availability…')
wait_for_requests
expect(branch_name_message).to have_text('Branch name is available')
input_branch_name.set(project.default_branch)
expect(branch_name_message).to have_text('Checking branch name availability…')
wait_for_requests
expect(branch_name_message).to have_text('Branch is already taken')
end
def test_selection_mark(li_create_branch, li_create_merge_request, button_create_target, button_create_merge_request)
page.within(li_create_merge_request) do
expect(page).to have_css('i.fa.fa-check')
expect(button_create_target).to have_text('Create merge request')
expect(button_create_merge_request).to have_text('Create merge request')
end
li_create_branch.click
page.within(li_create_branch) do
expect(page).to have_css('i.fa.fa-check')
expect(button_create_target).to have_text('Create branch')
expect(button_create_merge_request).to have_text('Create branch')
end
end
def test_source_checking(input_source)
expect(input_source.value).to eq(project.default_branch)
input_source.set('mas') # Intentionally entered first 3 letters of `master` to check autocomplete feature later.
source_message = find('.js-ref-message')
expect(source_message).to have_text('Checking source availability…')
wait_for_requests
expect(source_message).to have_text('Source is not available')
# JavaScript gets refs started with `mas` (entered above) and places the first match.
# User sees `mas` in black color (the part he entered) and the `ter` in gray color (a hint).
# Since hinting is implemented via text selection and rspec/capybara doesn't have matchers for it,
# we just checking the whole source name.
expect(input_source.value).to eq(project.default_branch)
end
end

8
spec/javascripts/droplab/drop_down_spec.js
View File

@ -279,7 +279,12 @@ describe('DropDown', function () {
describe('addEvents', function () {
beforeEach(function () {
this.list = { addEventListener: () => {} };
this.dropdown = { list: this.list, clickEvent: () => {}, eventWrapper: {} };
this.dropdown = {
list: this.list,
clickEvent: () => {},
closeDropdown: () => {},
eventWrapper: {},
};
spyOn(this.list, 'addEventListener');
@ -288,6 +293,7 @@ describe('DropDown', function () {
it('should call .addEventListener', function () {
expect(this.list.addEventListener).toHaveBeenCalledWith('click', jasmine.any(Function));
expect(this.list.addEventListener).toHaveBeenCalledWith('keyup', jasmine.any(Function));
});
});

2
spec/javascripts/droplab/hook_spec.js
View File

@ -24,7 +24,7 @@ describe('Hook', function () {
});
it('should call DropDown constructor', function () {
expect(dropdownSrc.default).toHaveBeenCalledWith(this.list);
expect(dropdownSrc.default).toHaveBeenCalledWith(this.list, this.config);
});
it('should set .type', function () {

24
spec/services/merge_requests/create_from_issue_service_spec.rb
View File

@ -6,8 +6,10 @@ describe MergeRequests::CreateFromIssueService do
let(:label_ids) { create_pair(:label, project: project).map(&:id) }
let(:milestone_id) { create(:milestone, project: project).id }
let(:issue) { create(:issue, project: project, milestone_id: milestone_id) }
let(:custom_source_branch) { 'custom-source-branch' }
subject(:service) { described_class.new(project, user, issue_iid: issue.iid) }
subject(:service_with_custom_source_branch) { described_class.new(project, user, issue_iid: issue.iid, branch_name: custom_source_branch) }
before do
project.add_developer(user)
@ -17,8 +19,8 @@ describe MergeRequests::CreateFromIssueService do
it 'returns an error with invalid issue iid' do
result = described_class.new(project, user, issue_iid: -1).execute
expect(result[:status]).to eq :error
expect(result[:message]).to eq 'Invalid issue iid'
expect(result[:status]).to eq(:error)
expect(result[:message]).to eq('Invalid issue iid')
end
it 'delegates issue search to IssuesFinder' do
@ -53,6 +55,12 @@ describe MergeRequests::CreateFromIssueService do
expect(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
end
it 'creates a system note' do
expect(SystemNoteService).to receive(:new_issue_branch).with(issue, project, user, issue.to_branch_name)
@ -72,19 +80,25 @@ describe MergeRequests::CreateFromIssueService do
it 'sets the merge request author to current user' do
result = service.execute
expect(result[:merge_request].author).to eq user
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
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
expect(result[:merge_request].target_branch).to eq(project.default_branch)
end
end
end