Commit f9715416 authored by Fatih Acet's avatar Fatih Acet Committed by Kushal Pandya

Warn before applying issue template

If there are a previously applied template and its content
is changed, we now warn users before applying another
issue template to prevent a possible data loss
parent 842e1d4f
...@@ -27,11 +27,16 @@ export default class TemplateSelector { ...@@ -27,11 +27,16 @@ export default class TemplateSelector {
search: { search: {
fields: ['name'], fields: ['name'],
}, },
clicked: options => this.fetchFileTemplate(options), clicked: options => this.onDropdownClicked(options),
text: item => item.name, text: item => item.name,
}); });
} }
// Subclasses can override this method to conditionally prevent fetching file templates
onDropdownClicked(options) {
this.fetchFileTemplate(options);
}
initAutosizeUpdateEvent() { initAutosizeUpdateEvent() {
this.autosizeUpdateEvent = document.createEvent('Event'); this.autosizeUpdateEvent = document.createEvent('Event');
this.autosizeUpdateEvent.initEvent('autosize:update', true, false); this.autosizeUpdateEvent.initEvent('autosize:update', true, false);
...@@ -81,6 +86,10 @@ export default class TemplateSelector { ...@@ -81,6 +86,10 @@ export default class TemplateSelector {
} }
} }
getEditorContent() {
return this.editor.getValue();
}
startLoadingSpinner() { startLoadingSpinner() {
this.$dropdownIcon.addClass('fa-spinner fa-spin').removeClass('fa-chevron-down'); this.$dropdownIcon.addClass('fa-spinner fa-spin').removeClass('fa-chevron-down');
} }
......
...@@ -717,6 +717,7 @@ GitLabDropdown = (function() { ...@@ -717,6 +717,7 @@ GitLabDropdown = (function() {
selectedObject = this.renderedData[groupName][selectedIndex]; selectedObject = this.renderedData[groupName][selectedIndex];
} else { } else {
selectedIndex = el.closest('li').index(); selectedIndex = el.closest('li').index();
this.selectedIndex = selectedIndex;
selectedObject = this.renderedData[selectedIndex]; selectedObject = this.renderedData[selectedIndex];
} }
} }
......
...@@ -15,7 +15,9 @@ export default () => { ...@@ -15,7 +15,9 @@ export default () => {
new IssuableForm($('.issue-form')); new IssuableForm($('.issue-form'));
new LabelsSelect(); new LabelsSelect();
new MilestoneSelect(); new MilestoneSelect();
new IssuableTemplateSelectors(); new IssuableTemplateSelectors({
warnTemplateOverride: true,
});
initSuggestions(); initSuggestions();
}; };
...@@ -16,5 +16,7 @@ export default () => { ...@@ -16,5 +16,7 @@ export default () => {
new IssuableForm($('.merge-request-form')); new IssuableForm($('.merge-request-form'));
new LabelsSelect(); new LabelsSelect();
new MilestoneSelect(); new MilestoneSelect();
new IssuableTemplateSelectors(); new IssuableTemplateSelectors({
warnTemplateOverride: true,
});
}; };
...@@ -8,10 +8,13 @@ import { __ } from '~/locale'; ...@@ -8,10 +8,13 @@ import { __ } from '~/locale';
export default class IssuableTemplateSelector extends TemplateSelector { export default class IssuableTemplateSelector extends TemplateSelector {
constructor(...args) { constructor(...args) {
super(...args); super(...args);
this.projectPath = this.dropdown.data('projectPath'); this.projectPath = this.dropdown.data('projectPath');
this.namespacePath = this.dropdown.data('namespacePath'); this.namespacePath = this.dropdown.data('namespacePath');
this.issuableType = this.$dropdownContainer.data('issuableType'); this.issuableType = this.$dropdownContainer.data('issuableType');
this.titleInput = $(`#${this.issuableType}_title`); this.titleInput = $(`#${this.issuableType}_title`);
this.templateWarningEl = $('.js-issuable-template-warning');
this.warnTemplateOverride = args[0].warnTemplateOverride;
const initialQuery = { const initialQuery = {
name: this.dropdown.data('selected'), name: this.dropdown.data('selected'),
...@@ -24,14 +27,61 @@ export default class IssuableTemplateSelector extends TemplateSelector { ...@@ -24,14 +27,61 @@ export default class IssuableTemplateSelector extends TemplateSelector {
}); });
$('.no-template', this.dropdown.parent()).on('click', () => { $('.no-template', this.dropdown.parent()).on('click', () => {
this.currentTemplate.content = ''; this.reset();
this.setInputValueToTemplateContent(); });
$('.dropdown-toggle-text', this.dropdown).text(__('Choose a template'));
this.templateWarningEl.find('.js-close-btn').on('click', () => {
if (this.previousSelectedIndex) {
this.dropdown.data('glDropdown').selectRowAtIndex(this.previousSelectedIndex);
} else {
this.reset();
}
this.templateWarningEl.addClass('hidden');
});
this.templateWarningEl.find('.js-override-template').on('click', () => {
this.requestFile(this.overridingTemplate);
this.setSelectedIndex();
this.templateWarningEl.addClass('hidden');
this.overridingTemplate = null;
}); });
} }
reset() {
if (this.currentTemplate) {
this.currentTemplate.content = '';
}
this.setInputValueToTemplateContent();
$('.dropdown-toggle-text', this.dropdown).text(__('Choose a template'));
this.previousSelectedIndex = null;
}
setSelectedIndex() {
this.previousSelectedIndex = this.dropdown.data('glDropdown').selectedIndex;
}
onDropdownClicked(query) {
const content = this.getEditorContent();
const isContentUnchanged =
content === '' || (this.currentTemplate && content === this.currentTemplate.content);
if (!this.warnTemplateOverride || isContentUnchanged) {
super.onDropdownClicked(query);
this.setSelectedIndex();
return;
}
this.overridingTemplate = query.selectedObj;
this.templateWarningEl.removeClass('hidden');
}
requestFile(query) { requestFile(query) {
this.startLoadingSpinner(); this.startLoadingSpinner();
Api.issueTemplate( Api.issueTemplate(
this.namespacePath, this.namespacePath,
this.projectPath, this.projectPath,
......
...@@ -4,7 +4,7 @@ import $ from 'jquery'; ...@@ -4,7 +4,7 @@ import $ from 'jquery';
import IssuableTemplateSelector from './issuable_template_selector'; import IssuableTemplateSelector from './issuable_template_selector';
export default class IssuableTemplateSelectors { export default class IssuableTemplateSelectors {
constructor({ $dropdowns, editor } = {}) { constructor({ $dropdowns, editor, warnTemplateOverride } = {}) {
this.$dropdowns = $dropdowns || $('.js-issuable-selector'); this.$dropdowns = $dropdowns || $('.js-issuable-selector');
this.editor = editor || this.initEditor(); this.editor = editor || this.initEditor();
...@@ -16,6 +16,7 @@ export default class IssuableTemplateSelectors { ...@@ -16,6 +16,7 @@ export default class IssuableTemplateSelectors {
wrapper: $dropdown.closest('.js-issuable-selector-wrap'), wrapper: $dropdown.closest('.js-issuable-selector-wrap'),
dropdown: $dropdown, dropdown: $dropdown,
editor: this.editor, editor: this.editor,
warnTemplateOverride,
}); });
}); });
} }
......
...@@ -214,18 +214,26 @@ li.note { ...@@ -214,18 +214,26 @@ li.note {
@mixin message($background-color, $border-color, $text-color) { @mixin message($background-color, $border-color, $text-color) {
border-left: 4px solid $border-color; border-left: 4px solid $border-color;
color: $text-color; color: $text-color;
padding: 10px; padding: $gl-padding $gl-padding-24;
margin-bottom: 10px; margin-bottom: $gl-padding-12;
background: $background-color; background-color: $background-color;
padding-left: 20px;
&.centered { &.centered {
text-align: center; text-align: center;
} }
.close {
svg {
width: $gl-font-size-large;
height: $gl-font-size-large;
}
color: inherit;
}
} }
.warning_message { .warning_message {
@include message($orange-100, $orange-200, $orange-700); @include message($orange-100, $orange-200, $orange-800);
} }
.danger_message { .danger_message {
......
.form-group.row.js-template-warning.mb-0.hidden.js-issuable-template-warning
.offset-sm-2.col-sm-10
.warning_message.mb-0{ role: 'alert' }
%btn.js-close-btn.close{ type: "button", "aria-hidden": true, "aria-label": _("Close") }
= sprite_icon("close")
%p
= _("Applying a template will replace the existing issue description. Any changes you have made will be lost.")
%button.js-override-template.btn.btn-warning.mr-2{ type: 'button' }
= _("Apply template")
%button.js-cancel-btn.btn.btn-inverted{ type: 'button' }
= _("Cancel")
...@@ -19,6 +19,7 @@ ...@@ -19,6 +19,7 @@
= render 'shared/issuable/form/title', issuable: issuable, form: form, has_wip_commits: commits && commits.detect(&:work_in_progress?) = render 'shared/issuable/form/title', issuable: issuable, form: form, has_wip_commits: commits && commits.detect(&:work_in_progress?)
#js-suggestions{ data: { project_path: @project.full_path } } #js-suggestions{ data: { project_path: @project.full_path } }
= render 'shared/form_elements/apply_template_warning'
= render 'shared/form_elements/description', model: issuable, form: form, project: project = render 'shared/form_elements/description', model: issuable, form: form, project: project
- if issuable.respond_to?(:confidential) - if issuable.respond_to?(:confidential)
......
---
title: Warn before applying issue templates
merge_request: 16865
author:
type: changed
...@@ -1692,6 +1692,12 @@ msgstr "" ...@@ -1692,6 +1692,12 @@ msgstr ""
msgid "Apply suggestion" msgid "Apply suggestion"
msgstr "" msgstr ""
msgid "Apply template"
msgstr ""
msgid "Applying a template will replace the existing issue description. Any changes you have made will be lost."
msgstr ""
msgid "Applying command" msgid "Applying command"
msgstr "" msgstr ""
......
...@@ -92,6 +92,9 @@ describe 'issuable templates', :js do ...@@ -92,6 +92,9 @@ describe 'issuable templates', :js do
context 'user creates a merge request using templates' do context 'user creates a merge request using templates' do
let(:template_content) { 'this is a test "feature-proposal" template' } let(:template_content) { 'this is a test "feature-proposal" template' }
let(:bug_template_content) { 'this is merge request bug template' }
let(:template_override_warning) { 'Applying a template will replace the existing issue description.' }
let(:updated_description) { 'updated merge request description' }
let(:merge_request) { create(:merge_request, :with_diffs, source_project: project) } let(:merge_request) { create(:merge_request, :with_diffs, source_project: project) }
before do before do
...@@ -101,6 +104,12 @@ describe 'issuable templates', :js do ...@@ -101,6 +104,12 @@ describe 'issuable templates', :js do
template_content, template_content,
message: 'added merge request template', message: 'added merge request template',
branch_name: 'master') branch_name: 'master')
project.repository.create_file(
user,
'.gitlab/merge_request_templates/bug.md',
bug_template_content,
message: 'added merge request bug template',
branch_name: 'master')
visit edit_project_merge_request_path project, merge_request visit edit_project_merge_request_path project, merge_request
fill_in :'merge_request[title]', with: 'test merge request title' fill_in :'merge_request[title]', with: 'test merge request title'
end end
...@@ -111,6 +120,27 @@ describe 'issuable templates', :js do ...@@ -111,6 +120,27 @@ describe 'issuable templates', :js do
assert_template assert_template
save_changes save_changes
end end
context 'changes template' do
before do
select_template 'bug'
wait_for_requests
fill_in :'merge_request[description]', with: updated_description
select_template 'feature-proposal'
expect(page).to have_content template_override_warning
end
it 'user selects "bug" template, then updates description, then selects "feature-proposal" template, then cancels template change' do
page.find('.js-template-warning .js-cancel-btn').click
expect(find('textarea')['value']).to eq(updated_description)
end
it 'user selects "bug" template, then updates description, then selects "feature-proposal" template, then applies template change' do
page.find('.js-template-warning .js-override-template').click
wait_for_requests
assert_template
end
end
end end
context 'user creates a merge request from a forked project using templates' do context 'user creates a merge request from a forked project using templates' do
......
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