Commit 4aac8cec authored by Phil Hughes's avatar Phil Hughes Committed by Kushal Pandya

Allow users to toggle draft status on merge request

This allows users to toggle the draft status of a merge request
without the need for the user to visit the merge request
edit form.

Closes https://gitlab.com/gitlab-org/gitlab/-/issues/227421
parent 24b3ceda
......@@ -3,11 +3,12 @@
import $ from 'jquery';
import axios from './lib/utils/axios_utils';
import { __ } from '~/locale';
import eventHub from '~/vue_merge_request_widget/event_hub';
import { deprecatedCreateFlash as createFlash } from '~/flash';
import TaskList from './task_list';
import MergeRequestTabs from './merge_request_tabs';
import IssuablesHelper from './helpers/issuables_helper';
import { addDelimiter } from './lib/utils/text_utility';
import { getParameterValues, setUrlParams } from './lib/utils/url_utility';
function MergeRequest(opts) {
// Initialize MergeRequest behavior
......@@ -23,7 +24,6 @@ function MergeRequest(opts) {
this.initTabs();
this.initMRBtnListeners();
this.initCommitMessageListeners();
this.closeReopenReportToggle = IssuablesHelper.initCloseReopenReport();
if ($('.description.js-task-list-container').length) {
this.taskList = new TaskList({
......@@ -66,13 +66,38 @@ MergeRequest.prototype.showAllCommits = function() {
MergeRequest.prototype.initMRBtnListeners = function() {
const _this = this;
const draftToggles = document.querySelectorAll('.js-draft-toggle-button');
$('.report-abuse-link').on('click', e => {
// this is needed because of the implementation of
// the dropdown toggle and Report Abuse needing to be
// linked to another page.
e.stopPropagation();
});
if (draftToggles.length) {
draftToggles.forEach(draftToggle => {
draftToggle.addEventListener('click', e => {
e.preventDefault();
e.stopImmediatePropagation();
const url = draftToggle.href;
const wipEvent = getParameterValues('merge_request[wip_event]', url)[0];
const mobileDropdown = draftToggle.closest('.dropdown.show');
if (mobileDropdown) {
$(mobileDropdown.firstElementChild).dropdown('toggle');
}
draftToggle.setAttribute('disabled', 'disabled');
axios
.put(draftToggle.href, null, { params: { format: 'json' } })
.then(({ data }) => {
draftToggle.removeAttribute('disabled');
eventHub.$emit('MRWidgetUpdateRequested');
MergeRequest.toggleDraftStatus(data.title, wipEvent === 'unwip');
})
.catch(() => {
draftToggle.removeAttribute('disabled');
createFlash(__('Something went wrong. Please try again.'));
});
});
});
}
return $('.btn-close, .btn-reopen').on('click', function(e) {
const $this = $(this);
......@@ -89,8 +114,6 @@ MergeRequest.prototype.initMRBtnListeners = function() {
return;
}
if (this.closeReopenReportToggle) this.closeReopenReportToggle.setDisable();
if (shouldSubmit) {
if ($this.hasClass('btn-comment-and-close') || $this.hasClass('btn-comment-and-reopen')) {
e.preventDefault();
......@@ -151,14 +174,35 @@ MergeRequest.hideCloseButton = function() {
const closeDropdownItem = el.querySelector('li.close-item');
if (closeDropdownItem) {
closeDropdownItem.classList.add('hidden');
// Selects the next dropdown item
el.querySelector('li.report-item').click();
} else {
// No dropdown just hide the Close button
el.querySelector('.btn-close').classList.add('hidden');
}
// Dropdown for mobile screen
el.querySelector('li.js-close-item').classList.add('hidden');
};
MergeRequest.toggleDraftStatus = function(title, isReady) {
if (isReady) {
createFlash(__('The merge request can now be merged.'), 'notice');
}
const titleEl = document.querySelector('.merge-request .detail-page-description .title');
if (titleEl) {
titleEl.textContent = title;
}
const draftToggles = document.querySelectorAll('.js-draft-toggle-button');
if (draftToggles.length) {
draftToggles.forEach(el => {
const draftToggle = el;
const url = setUrlParams(
{ 'merge_request[wip_event]': isReady ? 'wip' : 'unwip' },
draftToggle.href,
);
draftToggle.setAttribute('href', url);
draftToggle.textContent = isReady ? __('Mark as draft') : __('Mark as ready');
});
}
};
export default MergeRequest;
......@@ -3,6 +3,7 @@ import $ from 'jquery';
import { GlButton } from '@gitlab/ui';
import { __ } from '~/locale';
import { deprecatedCreateFlash as createFlash } from '~/flash';
import MergeRequest from '~/merge_request';
import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import mergeRequestQueryVariablesMixin from '../../mixins/merge_request_query_variables';
import getStateQuery from '../../queries/get_state.query.graphql';
......@@ -128,8 +129,7 @@ export default {
.then(res => res.data)
.then(data => {
eventHub.$emit('UpdateWidgetData', data);
createFlash(__('The merge request can now be merged.'), 'notice');
$('.merge-request .detail-page-description .title').text(this.mr.title);
MergeRequest.toggleDraftStatus(this.mr.title, true);
})
.catch(() => {
this.isMakingRequest = false;
......
......@@ -35,6 +35,7 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont
:source_branch,
:source_project_id,
:state_event,
:wip_event,
:squash,
:target_branch,
:target_project_id,
......
......@@ -342,6 +342,12 @@ module IssuablesHelper
issuable.closed? ^ should_inverse ? reopen_issuable_path(issuable) : close_issuable_path(issuable)
end
def toggle_draft_issuable_path(issuable)
wip_event = issuable.work_in_progress? ? 'unwip' : 'wip'
issuable_path(issuable, { merge_request: { wip_event: wip_event } })
end
def issuable_path(issuable, *options)
polymorphic_path(issuable, *options)
end
......
......@@ -37,7 +37,7 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
end
def remove_wip_path
if work_in_progress? && can?(current_user, :update_merge_request, merge_request.project)
if can?(current_user, :update_merge_request, merge_request.project)
remove_wip_project_merge_request_path(project, merge_request)
end
end
......
# frozen_string_literal: true
class MergeRequestBasicEntity < Grape::Entity
expose :title
expose :public_merge_status, as: :merge_status
expose :merge_error
expose :state
......
......@@ -32,16 +32,19 @@
%ul
- if can_update_merge_request
%li= link_to 'Edit', edit_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)
- unless current_user == @merge_request.author
%li= link_to 'Report abuse', new_abuse_report_path(user_id: @merge_request.author.id, ref_url: merge_request_url(@merge_request))
- if can_update_merge_request
- unless @merge_request.closed?
%li
= link_to @merge_request.work_in_progress? ? _('Mark as ready') : _('Mark as draft'), toggle_draft_issuable_path(@merge_request), method: :put, class: "js-draft-toggle-button"
%li{ class: [merge_request_button_visibility(@merge_request, true), 'js-close-item'] }
= link_to 'Close', merge_request_path(@merge_request, merge_request: { state_event: :close }), method: :put, title: 'Close merge request'
- if can_reopen_merge_request
%li{ class: merge_request_button_visibility(@merge_request, false) }
= link_to 'Reopen', merge_request_path(@merge_request, merge_request: { state_event: :reopen }), method: :put, class: 'reopen-mr-link', title: 'Reopen merge request'
- unless current_user == @merge_request.author
%li= link_to 'Report abuse', new_abuse_report_path(user_id: @merge_request.author.id, ref_url: merge_request_url(@merge_request))
- if can_update_merge_request
= link_to 'Edit', edit_project_merge_request_path(@project, @merge_request), class: "d-none d-sm-none d-md-block btn btn-grouped js-issuable-edit qa-edit-button"
= link_to 'Edit', edit_project_merge_request_path(@project, @merge_request), class: "d-none d-sm-none d-md-block btn gl-button btn-grouped js-issuable-edit qa-edit-button"
= render 'shared/issuable/close_reopen_button', issuable: @merge_request, can_update: can_update_merge_request, can_reopen: can_reopen_merge_request
......@@ -5,7 +5,7 @@
- if defined? warn_before_close
- add_blocked_class = warn_before_close
- if is_current_user
- if is_current_user && !issuable.is_a?(MergeRequest)
- if can_update
%button{ class: "d-none d-sm-none d-md-block btn btn-grouped btn-close js-btn-issue-action #{issuable_button_visibility(issuable, true)} #{(add_blocked_class ? 'btn-issue-blocked' : '')}",
data: { remote: 'true', endpoint: close_issuable_path(issuable), qa_selector: 'close_issue_button' } }
......@@ -16,7 +16,10 @@
= _("Reopen %{display_issuable_type}") % { display_issuable_type: display_issuable_type }
- else
- if can_update && !are_close_and_open_buttons_hidden
= render 'shared/issuable/close_reopen_report_toggle', issuable: issuable, warn_before_close: add_blocked_class
- if issuable.is_a?(MergeRequest)
= render 'shared/issuable/close_reopen_draft_report_toggle', issuable: issuable
- else
= render 'shared/issuable/close_reopen_report_toggle', issuable: issuable, warn_before_close: add_blocked_class
- else
= link_to _('Report abuse'), new_abuse_report_path(user_id: issuable.author.id, ref_url: issuable_url(issuable)),
class: 'd-none d-sm-none d-md-block btn btn-grouped btn-close-color', title: _('Report abuse')
- display_issuable_type = issuable_display_type(issuable)
- button_action_class = issuable.closed? ? 'btn-default' : 'btn-warning btn-warning-secondary'
- button_class = "btn gl-button #{!issuable.closed? && 'js-draft-toggle-button'}"
- toggle_class = "btn gl-button dropdown-toggle"
.float-left.btn-group.gl-ml-3.issuable-close-dropdown.d-none.d-md-inline-flex.js-issuable-close-dropdown
= link_to issuable.closed? ? reopen_issuable_path(issuable) : toggle_draft_issuable_path(issuable), method: :put, class: "#{button_class} #{button_action_class}" do
- if issuable.closed?
= _('Reopen')
= display_issuable_type
- else
= issuable.work_in_progress? ? _('Mark as ready') : _('Mark as draft')
- if !issuable.closed? || !issuable_author_is_current_user(issuable)
= button_tag type: 'button', class: "#{toggle_class} #{button_action_class}", data: { 'toggle' => 'dropdown' } do
%span.sr-only= _('Toggle dropdown')
= sprite_icon "angle-down", size: 12
%ul.js-issuable-close-menu.dropdown-menu.dropdown-menu-right
- if issuable.open?
%li
= link_to close_issuable_path(issuable), method: :put do
.description
%strong.title
= _('Close')
= display_issuable_type
- unless issuable_author_is_current_user(issuable)
- unless issuable.closed?
%li.divider.droplab-item-ignore
%li.report-item
%a.report-abuse-link{ href: new_abuse_report_path(user_id: issuable.author.id, ref_url: issuable_url(issuable)) }
.description
%strong.title= _('Report abuse')
%p.text
= _('Report %{display_issuable_type} that are abusive, inappropriate or spam.') % { display_issuable_type: display_issuable_type.pluralize }
---
title: Adds button to update merge request draft status on merge request show page
merge_request: 43098
author:
type: added
......@@ -15441,6 +15441,12 @@ msgstr ""
msgid "Mark as done"
msgstr ""
msgid "Mark as draft"
msgstr ""
msgid "Mark as ready"
msgstr ""
msgid "Mark as resolved"
msgstr ""
......
......@@ -23,7 +23,15 @@ RSpec.describe 'Issuables Close/Reopen/Report toggle' do
expect(container).to have_content("Close #{human_model_name}")
expect(container).to have_content('Report abuse')
expect(container).to have_content("Report #{human_model_name.pluralize} that are abusive, inappropriate or spam.")
expect(container).to have_selector('.close-item.droplab-item-selected')
if issuable.is_a?(MergeRequest)
page.within('.js-issuable-close-dropdown') do
expect(page).to have_link('Close merge request')
end
else
expect(container).to have_selector('.close-item.droplab-item-selected')
end
expect(container).to have_selector('.report-item')
expect(container).not_to have_selector('.report-item.droplab-item-selected')
expect(container).not_to have_selector('.reopen-item')
......@@ -123,7 +131,7 @@ RSpec.describe 'Issuables Close/Reopen/Report toggle' do
it 'shows only the `Edit` button' do
expect(page).to have_link('Edit')
expect(page).not_to have_link('Report abuse')
expect(page).to have_link('Report abuse')
expect(page).not_to have_button('Close merge request')
expect(page).not_to have_button('Reopen merge request')
end
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe 'Merge request > User marks merge request as draft', :js do
let(:user) { create(:user) }
let(:project) { create(:project, :public, :repository) }
let(:merge_request) { create(:merge_request, source_project: project) }
before do
project.add_maintainer(user)
sign_in(user)
visit project_merge_request_path(project, merge_request)
end
it 'toggles draft status' do
click_link 'Mark as draft'
expect(page).to have_content("Draft: #{merge_request.title}")
page.within('.detail-page-header-actions') do
click_link 'Mark as ready'
end
expect(page).to have_content(merge_request.title)
end
end
......@@ -15,7 +15,11 @@ RSpec.describe 'User reopens a merge requests', :js do
end
it 'reopens a merge request' do
click_button('Reopen merge request', match: :first)
find('.js-issuable-close-dropdown .dropdown-toggle').click
click_link('Reopen merge request', match: :first)
wait_for_requests
page.within('.status-box') do
expect(page).to have_content('Open')
......
......@@ -35,7 +35,9 @@ RSpec.describe 'Merge request > User resolves Work in Progress', :js do
expect(page.find('.ci-widget-content')).to have_content("Pipeline ##{pipeline.id}")
expect(page).to have_content "This merge request is still a work in progress."
click_button('Mark as ready')
page.within('.mr-state-widget') do
click_button('Mark as ready')
end
wait_for_requests
......
{
"type": "object",
"properties" : {
"title": { "type": "string" },
"state": { "type": "string" },
"merge_status": { "type": "string" },
"source_branch_exists": { "type": "boolean" },
......
......@@ -3,8 +3,6 @@ import MockAdapter from 'axios-mock-adapter';
import { TEST_HOST } from 'spec/test_constants';
import axios from '~/lib/utils/axios_utils';
import MergeRequest from '~/merge_request';
import CloseReopenReportToggle from '~/close_reopen_report_toggle';
import IssuablesHelper from '~/helpers/issuables_helper';
describe('MergeRequest', () => {
const test = {};
......@@ -112,66 +110,7 @@ describe('MergeRequest', () => {
});
});
describe('class constructor', () => {
beforeEach(() => {
jest.spyOn($, 'ajax').mockImplementation();
});
it('calls .initCloseReopenReport', () => {
jest.spyOn(IssuablesHelper, 'initCloseReopenReport').mockImplementation(() => {});
new MergeRequest(); // eslint-disable-line no-new
expect(IssuablesHelper.initCloseReopenReport).toHaveBeenCalled();
});
it('calls .initDroplab', () => {
const container = {
querySelector: jest.fn().mockName('container.querySelector'),
};
const dropdownTrigger = {};
const dropdownList = {};
const button = {};
jest.spyOn(CloseReopenReportToggle.prototype, 'initDroplab').mockImplementation(() => {});
jest.spyOn(document, 'querySelector').mockReturnValue(container);
container.querySelector
.mockReturnValueOnce(dropdownTrigger)
.mockReturnValueOnce(dropdownList)
.mockReturnValueOnce(button);
new MergeRequest(); // eslint-disable-line no-new
expect(document.querySelector).toHaveBeenCalledWith('.js-issuable-close-dropdown');
expect(container.querySelector).toHaveBeenCalledWith('.js-issuable-close-toggle');
expect(container.querySelector).toHaveBeenCalledWith('.js-issuable-close-menu');
expect(container.querySelector).toHaveBeenCalledWith('.js-issuable-close-button');
expect(CloseReopenReportToggle.prototype.initDroplab).toHaveBeenCalled();
});
});
describe('hideCloseButton', () => {
describe('merge request of another user', () => {
beforeEach(() => {
loadFixtures('merge_requests/merge_request_with_task_list.html');
test.el = document.querySelector('.js-issuable-actions');
new MergeRequest(); // eslint-disable-line no-new
MergeRequest.hideCloseButton();
});
it('hides the dropdown close item and selects the next item', () => {
const closeItem = test.el.querySelector('li.close-item');
const smallCloseItem = test.el.querySelector('.js-close-item');
const reportItem = test.el.querySelector('li.report-item');
expect(closeItem).toHaveClass('hidden');
expect(smallCloseItem).toHaveClass('hidden');
expect(reportItem).toHaveClass('droplab-item-selected');
expect(reportItem).not.toHaveClass('hidden');
});
});
describe('merge request of current_user', () => {
beforeEach(() => {
loadFixtures('merge_requests/merge_request_of_current_user.html');
......@@ -180,10 +119,8 @@ describe('MergeRequest', () => {
});
it('hides the close button', () => {
const closeButton = test.el.querySelector('.btn-close');
const smallCloseItem = test.el.querySelector('.js-close-item');
expect(closeButton).toHaveClass('hidden');
expect(smallCloseItem).toHaveClass('hidden');
});
});
......
......@@ -492,19 +492,6 @@ describe('ReadyToMerge', () => {
});
});
it('hides close button', done => {
jest.spyOn(vm.service, 'poll').mockReturnValue(returnPromise('merged'));
jest.spyOn(vm, 'initiateRemoveSourceBranchPolling').mockImplementation(() => {});
vm.handleMergePolling(() => {}, () => {});
setImmediate(() => {
expect(document.querySelector('.btn-close').classList.contains('hidden')).toBeTruthy();
done();
});
});
it('updates merge request count badge', done => {
jest.spyOn(vm.service, 'poll').mockReturnValue(returnPromise('merged'));
jest.spyOn(vm, 'initiateRemoveSourceBranchPolling').mockImplementation(() => {});
......
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