summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPhil Hughes <me@iamphill.com>2017-05-04 08:09:22 +0000
committerPhil Hughes <me@iamphill.com>2017-05-04 08:09:22 +0000
commitf5b93d6940c9add833b2205251243193af26a077 (patch)
tree7b10e0e1938f871a2a228fe38adf3641ef6f5ce5
parent8983ade27df200f7f9376d61de17f329d9d27a33 (diff)
parentb64a37c4ed5561d423ee607f9821b75fd0337168 (diff)
downloadgitlab-ce-f5b93d6940c9add833b2205251243193af26a077.tar.gz
Merge branch '28558-create-new-branch-from-issue-page' into 'master'
Allow to create new branch and empty WIP merge request from issue page See merge request !10018
-rw-r--r--app/assets/javascripts/create_merge_request_dropdown.js193
-rw-r--r--app/assets/javascripts/issue.js74
-rw-r--r--app/assets/stylesheets/pages/issues.scss83
-rw-r--r--app/controllers/projects/branches_controller.rb34
-rw-r--r--app/controllers/projects/issues_controller.rb21
-rw-r--r--app/models/issue.rb8
-rw-r--r--app/serializers/merge_request_create_entity.rb7
-rw-r--r--app/serializers/merge_request_create_serializer.rb3
-rw-r--r--app/services/merge_requests/create_from_issue_service.rb54
-rw-r--r--app/views/projects/issues/_new_branch.html.haml36
-rw-r--r--app/views/projects/issues/show.html.haml7
-rw-r--r--changelogs/unreleased/28558-create-new-branch-from-issue-page.yml4
-rw-r--r--config/routes/project.rb1
-rw-r--r--spec/controllers/projects/branches_controller_spec.rb38
-rw-r--r--spec/controllers/projects/issues_controller_spec.rb24
-rw-r--r--spec/features/issues/create_branch_merge_request_spec.rb91
-rw-r--r--spec/features/issues/new_branch_button_spec.rb62
-rw-r--r--spec/fixtures/api/schemas/branch.json12
-rw-r--r--spec/fixtures/api/schemas/merge_request.json12
-rw-r--r--spec/javascripts/issue_spec.js4
-rw-r--r--spec/models/issue_spec.rb21
-rw-r--r--spec/services/merge_requests/create_from_issue_service_spec.rb74
22 files changed, 730 insertions, 133 deletions
diff --git a/app/assets/javascripts/create_merge_request_dropdown.js b/app/assets/javascripts/create_merge_request_dropdown.js
new file mode 100644
index 00000000000..ff2f2c81971
--- /dev/null
+++ b/app/assets/javascripts/create_merge_request_dropdown.js
@@ -0,0 +1,193 @@
+/* eslint-disable no-new */
+/* global Flash */
+import DropLab from './droplab/drop_lab';
+import ISetter from './droplab/plugins/input_setter';
+
+// Todo: Remove this when fixing issue in input_setter plugin
+const InputSetter = Object.assign({}, ISetter);
+
+const CREATE_MERGE_REQUEST = 'create-mr';
+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.unavailableButton = this.wrapperEl.querySelector('.unavailable');
+ this.unavailableButtonArrow = this.unavailableButton.querySelector('.fa');
+ this.unavailableButtonText = this.unavailableButton.querySelector('.text');
+
+ this.createBranchPath = this.wrapperEl.dataset.createBranchPath;
+ this.canCreatePath = this.wrapperEl.dataset.canCreatePath;
+ this.createMrPath = this.wrapperEl.dataset.createMrPath;
+ this.droplabInitialized = false;
+ this.isCreatingMergeRequest = false;
+ this.mergeRequestCreated = false;
+ this.isCreatingBranch = false;
+ this.branchCreated = false;
+
+ 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';
+ }
+ }
+
+ checkAbilityToCreateBranch() {
+ return $.ajax({
+ type: 'GET',
+ dataType: 'json',
+ url: this.canCreatePath,
+ beforeSend: () => this.setUnavailableButtonState(),
+ })
+ .done((data) => {
+ this.setUnavailableButtonState(false);
+
+ if (data.can_create_branch) {
+ this.available();
+ this.enable();
+
+ if (!this.droplabInitialized) {
+ this.droplabInitialized = true;
+ this.initDroplab();
+ this.bindEvents();
+ }
+ } else if (data.has_related_branch) {
+ this.hide();
+ }
+ }).fail(() => {
+ this.unavailable();
+ this.disable();
+ new Flash('Failed to check if a new branch can be created.');
+ });
+ }
+
+ 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();
+ }
+
+ createMergeRequest() {
+ return $.ajax({
+ method: 'POST',
+ dataType: 'json',
+ url: this.createMrPath,
+ beforeSend: () => (this.isCreatingMergeRequest = true),
+ })
+ .done((data) => {
+ this.mergeRequestCreated = true;
+ window.location.href = data.url;
+ })
+ .fail(() => new Flash('Failed to create Merge Request. Please try again.'));
+ }
+
+ 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.'));
+ }
+}
diff --git a/app/assets/javascripts/issue.js b/app/assets/javascripts/issue.js
index 011043e992f..694c6177a07 100644
--- a/app/assets/javascripts/issue.js
+++ b/app/assets/javascripts/issue.js
@@ -1,5 +1,6 @@
/* eslint-disable func-names, space-before-function-paren, no-var, prefer-rest-params, wrap-iife, one-var, no-underscore-dangle, one-var-declaration-per-line, object-shorthand, no-unused-vars, no-new, comma-dangle, consistent-return, quotes, dot-notation, quote-props, prefer-arrow-callback, max-len */
-/* global Flash */
+ /* global Flash */
+import CreateMergeRequestDropdown from './create_merge_request_dropdown';
require('./flash');
require('~/lib/utils/text_utility');
@@ -18,48 +19,49 @@ class Issue {
document.querySelector('#task_status_short').innerText = result.task_status_short;
}
});
- Issue.initIssueBtnEventListeners();
+ this.initIssueBtnEventListeners();
}
Issue.$btnNewBranch = $('#new-branch');
+ Issue.createMrDropdownWrap = document.querySelector('.create-mr-dropdown-wrap');
Issue.initMergeRequests();
Issue.initRelatedBranches();
- Issue.initCanCreateBranch();
+
+ if (Issue.createMrDropdownWrap) {
+ this.createMergeRequestDropdown = new CreateMergeRequestDropdown(Issue.createMrDropdownWrap);
+ }
}
- static initIssueBtnEventListeners() {
+ initIssueBtnEventListeners() {
const issueFailMessage = 'Unable to update this issue at this time.';
-
const closeButtons = $('a.btn-close');
const isClosedBadge = $('div.status-box-closed');
const isOpenBadge = $('div.status-box-open');
const projectIssuesCounter = $('.issue_counter');
const reopenButtons = $('a.btn-reopen');
- return closeButtons.add(reopenButtons).on('click', function(e) {
- var $this, shouldSubmit, url;
+ return closeButtons.add(reopenButtons).on('click', (e) => {
+ var $button, shouldSubmit, url;
e.preventDefault();
e.stopImmediatePropagation();
- $this = $(this);
- shouldSubmit = $this.hasClass('btn-comment');
+ $button = $(e.currentTarget);
+ shouldSubmit = $button.hasClass('btn-comment');
if (shouldSubmit) {
- Issue.submitNoteForm($this.closest('form'));
+ Issue.submitNoteForm($button.closest('form'));
}
- $this.prop('disabled', true);
- Issue.setNewBranchButtonState(true, null);
- url = $this.attr('href');
+ $button.prop('disabled', true);
+ url = $button.attr('href');
return $.ajax({
type: 'PUT',
url: url
- }).fail(function(jqXHR, textStatus, errorThrown) {
- new Flash(issueFailMessage);
- Issue.initCanCreateBranch();
- }).done(function(data, textStatus, jqXHR) {
+ })
+ .fail(() => new Flash(issueFailMessage))
+ .done((data) => {
if ('id' in data) {
$(document).trigger('issuable:change');
- const isClosed = $this.hasClass('btn-close');
+ const isClosed = $button.hasClass('btn-close');
closeButtons.toggleClass('hidden', isClosed);
reopenButtons.toggleClass('hidden', !isClosed);
isClosedBadge.toggleClass('hidden', !isClosed);
@@ -68,12 +70,21 @@ class Issue {
let numProjectIssues = Number(projectIssuesCounter.text().replace(/[^\d]/, ''));
numProjectIssues = isClosed ? numProjectIssues - 1 : numProjectIssues + 1;
projectIssuesCounter.text(gl.text.addDelimiter(numProjectIssues));
+
+ if (this.createMergeRequestDropdown) {
+ if (isClosed) {
+ this.createMergeRequestDropdown.unavailable();
+ this.createMergeRequestDropdown.disable();
+ } else {
+ // We should check in case a branch was created in another tab
+ this.createMergeRequestDropdown.checkAbilityToCreateBranch();
+ }
+ }
} else {
new Flash(issueFailMessage);
}
- $this.prop('disabled', false);
- Issue.initCanCreateBranch();
+ $button.prop('disabled', false);
});
});
}
@@ -109,29 +120,6 @@ class Issue {
}
});
}
-
- static initCanCreateBranch() {
- // If the user doesn't have the required permissions the container isn't
- // rendered at all.
- if (Issue.$btnNewBranch.length === 0) {
- return;
- }
- return $.getJSON(Issue.$btnNewBranch.data('path')).fail(function() {
- Issue.setNewBranchButtonState(false, false);
- new Flash('Failed to check if a new branch can be created.');
- }).done(function(data) {
- Issue.setNewBranchButtonState(false, data.can_create_branch);
- });
- }
-
- static setNewBranchButtonState(isPending, canCreate) {
- if (Issue.$btnNewBranch.length === 0) {
- return;
- }
-
- Issue.$btnNewBranch.find('.available').toggle(!isPending && canCreate);
- Issue.$btnNewBranch.find('.unavailable').toggle(!isPending && !canCreate);
- }
}
export default Issue;
diff --git a/app/assets/stylesheets/pages/issues.scss b/app/assets/stylesheets/pages/issues.scss
index 2aa52986e0a..b18bbc329c3 100644
--- a/app/assets/stylesheets/pages/issues.scss
+++ b/app/assets/stylesheets/pages/issues.scss
@@ -161,3 +161,86 @@ ul.related-merge-requests > li {
.recaptcha {
margin-bottom: 30px;
}
+
+.new-branch-col {
+ padding-top: 10px;
+}
+
+.create-mr-dropdown-wrap {
+ .btn-group:not(.hide) {
+ display: flex;
+ }
+
+ .js-create-merge-request {
+ flex-grow: 1;
+ flex-shrink: 0;
+ }
+
+ .dropdown-menu {
+ width: 300px;
+ opacity: 1;
+ visibility: visible;
+ transform: translateY(0);
+ display: none;
+ }
+
+ .dropdown-toggle {
+ .fa-caret-down {
+ pointer-events: none;
+ margin-left: 0;
+ color: inherit;
+ margin-left: 0;
+ }
+ }
+
+ li:not(.divider) {
+ padding: 6px;
+ cursor: pointer;
+
+ &:hover,
+ &:focus {
+ background-color: $dropdown-hover-color;
+ color: $white-light;
+ }
+
+ &.droplab-item-selected {
+ .icon-container {
+ i {
+ visibility: visible;
+ }
+ }
+ }
+
+ .icon-container {
+ float: left;
+ padding-left: 6px;
+
+ i {
+ visibility: hidden;
+ }
+ }
+
+ .description {
+ padding-left: 30px;
+ font-size: 13px;
+
+ strong {
+ display: block;
+ font-weight: 600;
+ }
+ }
+ }
+}
+
+@media (min-width: $screen-sm-min) {
+ .new-branch-col {
+ padding-top: 0;
+ text-align: right;
+ }
+
+ .create-mr-dropdown-wrap {
+ .btn-group:not(.hide) {
+ display: inline-block;
+ }
+ }
+}
diff --git a/app/controllers/projects/branches_controller.rb b/app/controllers/projects/branches_controller.rb
index 840405f38cb..f0f031303d8 100644
--- a/app/controllers/projects/branches_controller.rb
+++ b/app/controllers/projects/branches_controller.rb
@@ -46,20 +46,28 @@ class Projects::BranchesController < Projects::ApplicationController
SystemNoteService.new_issue_branch(issue, @project, current_user, branch_name) if issue
end
- if result[:status] == :success
- @branch = result[:branch]
-
- if redirect_to_autodeploy
- redirect_to(
- url_to_autodeploy_setup(project, branch_name),
- notice: view_context.autodeploy_flash_notice(branch_name))
- else
- redirect_to namespace_project_tree_path(@project.namespace, @project,
- @branch.name)
+ respond_to do |format|
+ format.html do
+ if result[:status] == :success
+ if redirect_to_autodeploy
+ redirect_to url_to_autodeploy_setup(project, branch_name),
+ notice: view_context.autodeploy_flash_notice(branch_name)
+ else
+ redirect_to namespace_project_tree_path(@project.namespace, @project, branch_name)
+ end
+ else
+ @error = result[:message]
+ render action: 'new'
+ end
+ end
+
+ format.json do
+ if result[:status] == :success
+ render json: { name: branch_name, url: namespace_project_tree_url(@project.namespace, @project, branch_name) }
+ else
+ render json: result[:messsage], status: :unprocessable_entity
+ end
end
- else
- @error = result[:message]
- render action: 'new'
end
end
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index cbf67137261..af9157bfbb5 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -11,7 +11,7 @@ class Projects::IssuesController < Projects::ApplicationController
before_action :redirect_to_external_issue_tracker, only: [:index, :new]
before_action :module_enabled
before_action :issue, only: [:edit, :update, :show, :referenced_merge_requests,
- :related_branches, :can_create_branch, :rendered_title]
+ :related_branches, :can_create_branch, :rendered_title, :create_merge_request]
# Allow read any issue
before_action :authorize_read_issue!, only: [:show, :rendered_title]
@@ -22,6 +22,9 @@ class Projects::IssuesController < Projects::ApplicationController
# Allow modify issue
before_action :authorize_update_issue!, only: [:edit, :update]
+ # Allow create a new branch and empty WIP merge request from current issue
+ before_action :authorize_create_merge_request!, only: [:create_merge_request]
+
respond_to :html
def index
@@ -191,7 +194,7 @@ class Projects::IssuesController < Projects::ApplicationController
respond_to do |format|
format.json do
- render json: { can_create_branch: can_create }
+ render json: { can_create_branch: can_create, has_related_branch: @issue.has_related_branch? }
end
end
end
@@ -201,6 +204,16 @@ class Projects::IssuesController < Projects::ApplicationController
render json: { title: view_context.markdown_field(@issue, :title) }
end
+ def create_merge_request
+ result = MergeRequests::CreateFromIssueService.new(project, current_user, issue_iid: issue.iid).execute
+
+ if result[:status] == :success
+ render json: MergeRequestCreateSerializer.new.represent(result[:merge_request])
+ else
+ render json: result[:messsage], status: :unprocessable_entity
+ end
+ end
+
protected
def issue
@@ -224,6 +237,10 @@ class Projects::IssuesController < Projects::ApplicationController
return render_404 unless can?(current_user, :admin_issue, @project)
end
+ def authorize_create_merge_request!
+ return render_404 unless can?(current_user, :push_code, @project) && @issue.can_be_worked_on?(current_user)
+ end
+
def module_enabled
return render_404 unless @project.feature_available?(:issues, current_user) && @project.default_issues_tracker?
end
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 305fc01f041..78bde6820da 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -143,6 +143,14 @@ class Issue < ActiveRecord::Base
branches_with_iid - branches_with_merge_request
end
+ # Returns boolean if a related branch exists for the current issue
+ # ignores merge requests branchs
+ def has_related_branch?
+ project.repository.branch_names.any? do |branch|
+ /\A#{iid}-(?!\d+-stable)/i =~ branch
+ end
+ end
+
# To allow polymorphism with MergeRequest.
def source_project
project
diff --git a/app/serializers/merge_request_create_entity.rb b/app/serializers/merge_request_create_entity.rb
new file mode 100644
index 00000000000..11234313293
--- /dev/null
+++ b/app/serializers/merge_request_create_entity.rb
@@ -0,0 +1,7 @@
+class MergeRequestCreateEntity < Grape::Entity
+ expose :iid
+
+ expose :url do |merge_request|
+ Gitlab::UrlBuilder.build(merge_request)
+ end
+end
diff --git a/app/serializers/merge_request_create_serializer.rb b/app/serializers/merge_request_create_serializer.rb
new file mode 100644
index 00000000000..08daf473319
--- /dev/null
+++ b/app/serializers/merge_request_create_serializer.rb
@@ -0,0 +1,3 @@
+class MergeRequestCreateSerializer < BaseSerializer
+ entity MergeRequestCreateEntity
+end
diff --git a/app/services/merge_requests/create_from_issue_service.rb b/app/services/merge_requests/create_from_issue_service.rb
new file mode 100644
index 00000000000..738cedbaed7
--- /dev/null
+++ b/app/services/merge_requests/create_from_issue_service.rb
@@ -0,0 +1,54 @@
+module MergeRequests
+ class CreateFromIssueService < MergeRequests::CreateService
+ def execute
+ return error('Invalid issue iid') unless issue_iid.present? && issue.present?
+
+ result = CreateBranchService.new(project, current_user).execute(branch_name, ref)
+ return result if result[:status] == :error
+
+ SystemNoteService.new_issue_branch(issue, project, current_user, branch_name)
+
+ new_merge_request = create(merge_request)
+
+ if new_merge_request.valid?
+ success(new_merge_request)
+ else
+ error(new_merge_request.errors)
+ end
+ end
+
+ 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)
+ end
+
+ def branch_name
+ @branch_name ||= issue.to_branch_name
+ end
+
+ def ref
+ project.default_branch || 'master'
+ end
+
+ def merge_request
+ MergeRequests::BuildService.new(project, current_user, merge_request_params).execute
+ end
+
+ def merge_request_params
+ {
+ source_project_id: project.id,
+ source_branch: branch_name,
+ target_project_id: project.id
+ }
+ end
+
+ def success(merge_request)
+ super().merge(merge_request: merge_request)
+ end
+ end
+end
diff --git a/app/views/projects/issues/_new_branch.html.haml b/app/views/projects/issues/_new_branch.html.haml
index 13e2150f997..6bc6bf76e18 100644
--- a/app/views/projects/issues/_new_branch.html.haml
+++ b/app/views/projects/issues/_new_branch.html.haml
@@ -1,9 +1,29 @@
- if can?(current_user, :push_code, @project)
- .pull-right
- #new-branch.new-branch{ 'data-path' => can_create_branch_namespace_project_issue_path(@project.namespace, @project, @issue) }
- = link_to namespace_project_branches_path(@project.namespace, @project, branch_name: @issue.to_branch_name, issue_iid: @issue.iid),
- method: :post, class: 'btn btn-new btn-inverted btn-grouped has-tooltip available hide', title: @issue.to_branch_name do
- New branch
- = link_to '#', class: 'unavailable btn btn-grouped hide', disabled: 'disabled' do
- = icon('exclamation-triangle')
- New branch unavailable
+ .create-mr-dropdown-wrap{ data: { can_create_path: can_create_branch_namespace_project_issue_path(@project.namespace, @project, @issue), create_mr_path: create_merge_request_namespace_project_issue_path(@project.namespace, @project, @issue), create_branch_path: namespace_project_branches_path(@project.namespace, @project, branch_name: @issue.to_branch_name, issue_iid: @issue.iid) } }
+ .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: 'Create a merge request', data: { action: 'create-mr' } }
+ %button.btn.btn-inverted.dropdown-toggle.btn-inverted.btn-success.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 } }
+ %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 branch named after this issue and a merge request. The source branch is '#{@project.default_branch}' by default.
+ %li.divider.droplab-item-ignore
+ %li{ 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. The source branch is '#{@project.default_branch}' by default.
diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml
index 2a871966aa8..1418ad73553 100644
--- a/app/views/projects/issues/show.html.haml
+++ b/app/views/projects/issues/show.html.haml
@@ -70,8 +70,11 @@
// This element is filled in using JavaScript.
.content-block.content-block-small
- = render 'new_branch' unless @issue.confidential?
- = render 'award_emoji/awards_block', awardable: @issue, inline: true
+ .row
+ .col-sm-6
+ = render 'award_emoji/awards_block', awardable: @issue, inline: true
+ .col-sm-6.new-branch-col
+ = render 'new_branch' unless @issue.confidential?
%section.issuable-discussion
= render 'projects/issues/discussion'
diff --git a/changelogs/unreleased/28558-create-new-branch-from-issue-page.yml b/changelogs/unreleased/28558-create-new-branch-from-issue-page.yml
new file mode 100644
index 00000000000..e43b043d6c5
--- /dev/null
+++ b/changelogs/unreleased/28558-create-new-branch-from-issue-page.yml
@@ -0,0 +1,4 @@
+---
+title: Allow to create new branch and empty WIP merge request from issue page
+merge_request:
+author:
diff --git a/config/routes/project.rb b/config/routes/project.rb
index a15e365cc2f..956afe4faa3 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -234,6 +234,7 @@ constraints(ProjectUrlConstrainer.new) do
get :related_branches
get :can_create_branch
get :rendered_title
+ post :create_merge_request
end
collection do
post :bulk_update
diff --git a/spec/controllers/projects/branches_controller_spec.rb b/spec/controllers/projects/branches_controller_spec.rb
index d20e7368086..8f915d9d210 100644
--- a/spec/controllers/projects/branches_controller_spec.rb
+++ b/spec/controllers/projects/branches_controller_spec.rb
@@ -14,7 +14,7 @@ describe Projects::BranchesController do
controller.instance_variable_set(:@project, project)
end
- describe "POST create" do
+ describe "POST create with HTML format" do
render_views
context "on creation of a new branch" do
@@ -152,6 +152,42 @@ describe Projects::BranchesController do
end
end
+ describe 'POST create with JSON format' do
+ before do
+ sign_in(user)
+ end
+
+ context 'with valid params' do
+ it 'returns a successful 200 response' do
+ create_branch name: 'my-branch', ref: 'master'
+
+ expect(response).to have_http_status(200)
+ end
+
+ it 'returns the created branch' do
+ create_branch name: 'my-branch', ref: 'master'
+
+ expect(response).to match_response_schema('branch')
+ end
+ end
+
+ context 'with invalid params' do
+ it 'returns an unprocessable entity 422 response' do
+ create_branch name: "<script>alert('merge');</script>", ref: "<script>alert('ref');</script>"
+
+ expect(response).to have_http_status(422)
+ end
+ end
+
+ def create_branch(name:, ref:)
+ post :create, namespace_id: project.namespace.to_param,
+ project_id: project.to_param,
+ branch_name: name,
+ ref: ref,
+ format: :json
+ end
+ end
+
describe "POST destroy with HTML format" do
render_views
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index 79034b8d24d..5f1f892821a 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -756,4 +756,28 @@ describe Projects::IssuesController do
expect(response).to have_http_status(200)
end
end
+
+ describe 'POST create_merge_request' do
+ before do
+ project.add_developer(user)
+ sign_in(user)
+ end
+
+ it 'creates a new merge request' do
+ expect { create_merge_request }.to change(project.merge_requests, :count).by(1)
+ end
+
+ it 'render merge request as json' do
+ create_merge_request
+
+ expect(response).to match_response_schema('merge_request')
+ end
+
+ def create_merge_request
+ post :create_merge_request, namespace_id: project.namespace.to_param,
+ project_id: project.to_param,
+ id: issue.to_param,
+ format: :json
+ end
+ end
end
diff --git a/spec/features/issues/create_branch_merge_request_spec.rb b/spec/features/issues/create_branch_merge_request_spec.rb
new file mode 100644
index 00000000000..44c19275ae5
--- /dev/null
+++ b/spec/features/issues/create_branch_merge_request_spec.rb
@@ -0,0 +1,91 @@
+require 'rails_helper'
+
+feature 'Create Branch/Merge Request Dropdown on issue page', feature: true, js: true do
+ let(:user) { create(:user) }
+ let!(:project) { create(:project) }
+ let(:issue) { create(:issue, project: project, title: 'Cherry-Coloured Funk') }
+
+ context 'for team members' do
+ before do
+ project.team << [user, :developer]
+ login_as(user)
+ end
+
+ it 'allows creating a merge request from the issue page' do
+ visit namespace_project_issue_path(project.namespace, project, issue)
+
+ select_dropdown_option('create-mr')
+
+ wait_for_ajax
+
+ expect(page).to have_content("created branch 1-cherry-coloured-funk")
+ expect(page).to have_content("mentioned in merge request !1")
+
+ visit namespace_project_merge_request_path(project.namespace, project, MergeRequest.first)
+
+ expect(page).to have_content('WIP: Resolve "Cherry-Coloured Funk"')
+ expect(current_path).to eq(namespace_project_merge_request_path(project.namespace, project, MergeRequest.first))
+ end
+
+ it 'allows creating a branch from the issue page' do
+ visit namespace_project_issue_path(project.namespace, project, issue)
+
+ select_dropdown_option('create-branch')
+
+ wait_for_ajax
+
+ expect(page).to have_selector('.dropdown-toggle-text ', text: '1-cherry-coloured-funk')
+ expect(current_path).to eq namespace_project_tree_path(project.namespace, 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 namespace_project_issue_path(project.namespace, 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 issue is confidential' do
+ it 'disables the create branch button' do
+ issue = create(:issue, :confidential, project: project)
+
+ visit namespace_project_issue_path(project.namespace, project, issue)
+
+ expect(page).not_to have_css('.create-mr-dropdown-wrap')
+ end
+ end
+ end
+
+ context 'for visitors' do
+ before do
+ visit namespace_project_issue_path(project.namespace, 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
diff --git a/spec/features/issues/new_branch_button_spec.rb b/spec/features/issues/new_branch_button_spec.rb
deleted file mode 100644
index c0ab42c6822..00000000000
--- a/spec/features/issues/new_branch_button_spec.rb
+++ /dev/null
@@ -1,62 +0,0 @@
-require 'rails_helper'
-
-feature 'Start new branch from an issue', feature: true, js: 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 'shows the new branch button' do
- visit namespace_project_issue_path(project.namespace, project, issue)
-
- expect(page).to have_css('#new-branch .available')
- 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 namespace_project_issue_path(project.namespace, project, issue)
- end
-
- it "hides the new branch button" do
- expect(page).to have_css('#new-branch .unavailable')
- expect(page).not_to have_css('#new-branch .available')
- expect(page).to have_content /1 Related Merge Request/
- end
- end
-
- context 'when issue is confidential' do
- it 'hides the new branch button' do
- issue = create(:issue, :confidential, project: project)
-
- visit namespace_project_issue_path(project.namespace, project, issue)
-
- expect(page).not_to have_css('#new-branch')
- end
- end
- end
-
- context 'for visitors' do
- it 'shows no buttons' do
- visit namespace_project_issue_path(project.namespace, project, issue)
-
- expect(page).not_to have_css('#new-branch')
- end
- end
-end
diff --git a/spec/fixtures/api/schemas/branch.json b/spec/fixtures/api/schemas/branch.json
new file mode 100644
index 00000000000..0bb74577010
--- /dev/null
+++ b/spec/fixtures/api/schemas/branch.json
@@ -0,0 +1,12 @@
+{
+ "type": "object",
+ "required" : [
+ "name",
+ "url"
+ ],
+ "properties" : {
+ "name": { "type": "string" },
+ "url": { "type": "uri" }
+ },
+ "additionalProperties": false
+}
diff --git a/spec/fixtures/api/schemas/merge_request.json b/spec/fixtures/api/schemas/merge_request.json
new file mode 100644
index 00000000000..36962660cd9
--- /dev/null
+++ b/spec/fixtures/api/schemas/merge_request.json
@@ -0,0 +1,12 @@
+{
+ "type": "object",
+ "required" : [
+ "iid",
+ "url"
+ ],
+ "properties" : {
+ "iid": { "type": "integer" },
+ "url": { "type": "uri" }
+ },
+ "additionalProperties": false
+}
diff --git a/spec/javascripts/issue_spec.js b/spec/javascripts/issue_spec.js
index 9a2570ef7e9..0fd573eae3f 100644
--- a/spec/javascripts/issue_spec.js
+++ b/spec/javascripts/issue_spec.js
@@ -108,8 +108,8 @@ describe('Issue', function() {
expect(this.$triggeredButton).toHaveProp('disabled', true);
expectNewBranchButtonState(true, false);
return this.issueStateDeferred;
- } else if (req.url === Issue.$btnNewBranch.data('path')) {
- expect(req.type).toBe('get');
+ } else if (req.url === Issue.createMrDropdownWrap.dataset.canCreatePath) {
+ expect(req.type).toBe('GET');
expectNewBranchButtonState(true, false);
return this.canCreateBranchDeferred;
}
diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index 11befd4edfe..8748b98a4e3 100644
--- a/spec/models/issue_spec.rb
+++ b/spec/models/issue_spec.rb
@@ -291,6 +291,27 @@ describe Issue, models: true do
end
end
+ describe '#has_related_branch?' do
+ let(:issue) { create(:issue, title: "Blue Bell Knoll") }
+ subject { issue.has_related_branch? }
+
+ context 'branch found' do
+ before do
+ allow(issue.project.repository).to receive(:branch_names).and_return(["iceblink-luck", issue.to_branch_name])
+ end
+
+ it { is_expected.to eq true }
+ end
+
+ context 'branch not found' do
+ before do
+ allow(issue.project.repository).to receive(:branch_names).and_return(["lazy-calm"])
+ end
+
+ it { is_expected.to eq false }
+ end
+ end
+
it_behaves_like 'an editable mentionable' do
subject { create(:issue, project: create(:project, :repository)) }
diff --git a/spec/services/merge_requests/create_from_issue_service_spec.rb b/spec/services/merge_requests/create_from_issue_service_spec.rb
new file mode 100644
index 00000000000..1588d30c394
--- /dev/null
+++ b/spec/services/merge_requests/create_from_issue_service_spec.rb
@@ -0,0 +1,74 @@
+require 'spec_helper'
+
+describe MergeRequests::CreateFromIssueService, services: true do
+ let(:project) { create(:project, :repository) }
+ let(:user) { create(:user) }
+ let(:issue) { create(:issue, project: project) }
+
+ subject(:service) { described_class.new(project, user, issue_iid: issue.iid) }
+
+ before do
+ project.add_developer(user)
+ end
+
+ describe '#execute' 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'
+ end
+
+ it 'delegates issue search to IssuesFinder' do
+ expect_any_instance_of(IssuesFinder).to receive(:execute).once.and_call_original
+
+ described_class.new(project, user, issue_iid: -1).execute
+ end
+
+ it 'delegates the branch creation to CreateBranchService' do
+ expect_any_instance_of(CreateBranchService).to receive(:execute).once.and_call_original
+
+ service.execute
+ end
+
+ it 'creates a branch based on issue title' do
+ service.execute
+
+ expect(project.repository.branch_exists?(issue.to_branch_name)).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)
+
+ service.execute
+ end
+
+ it 'creates a merge request' do
+ expect { service.execute }.to change(project.merge_requests, :count).by(1)
+ end
+
+ it 'sets the merge request title to: "WIP: Resolves "$issue-title"' do
+ result = service.execute
+
+ expect(result[:merge_request].title).to eq("WIP: Resolve \"#{issue.title}\"")
+ end
+
+ it 'sets the merge request author to current user' do
+ result = service.execute
+
+ 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
+ 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
+ end
+ end
+end