Merge branch '16188-warn-before-applying-issue-templates' into 'master'

Warn before applying issue template

Closes #16188

See merge request gitlab-org/gitlab!16865

app/assets/javascripts/blob/template_selector.js

@@ -27,11 +27,16 @@ export default class TemplateSelector {
       search: {
         fields: ['name'],
       },
-      clicked: options => this.fetchFileTemplate(options),
+      clicked: options => this.onDropdownClicked(options),
       text: item => item.name,
     });
   }
 
+  // Subclasses can override this method to conditionally prevent fetching file templates
+  onDropdownClicked(options) {
+    this.fetchFileTemplate(options);
+  }
+
   initAutosizeUpdateEvent() {
     this.autosizeUpdateEvent = document.createEvent('Event');
     this.autosizeUpdateEvent.initEvent('autosize:update', true, false);
@@ -81,6 +86,10 @@ export default class TemplateSelector {
     }
   }
 
+  getEditorContent() {
+    return this.editor.getValue();
+  }
+
   startLoadingSpinner() {
     this.$dropdownIcon.addClass('fa-spinner fa-spin').removeClass('fa-chevron-down');
   }

app/assets/javascripts/gl_dropdown.js

@@ -717,6 +717,7 @@ GitLabDropdown = (function() {
         selectedObject = this.renderedData[groupName][selectedIndex];
       } else {
         selectedIndex = el.closest('li').index();
+        this.selectedIndex = selectedIndex;
         selectedObject = this.renderedData[selectedIndex];
       }
     }

app/assets/javascripts/pages/projects/issues/form.js

@@ -15,7 +15,9 @@ export default () => {
   new IssuableForm($('.issue-form'));
   new LabelsSelect();
   new MilestoneSelect();
-  new IssuableTemplateSelectors();
+  new IssuableTemplateSelectors({
+    warnTemplateOverride: true,
+  });
 
   initSuggestions();
 };

app/assets/javascripts/pages/projects/merge_requests/init_merge_request.js

@@ -16,5 +16,7 @@ export default () => {
   new IssuableForm($('.merge-request-form'));
   new LabelsSelect();
   new MilestoneSelect();
-  new IssuableTemplateSelectors();
+  new IssuableTemplateSelectors({
+    warnTemplateOverride: true,
+  });
 };

app/assets/javascripts/templates/issuable_template_selector.js

@@ -8,10 +8,13 @@ import { __ } from '~/locale';
 export default class IssuableTemplateSelector extends TemplateSelector {
   constructor(...args) {
     super(...args);
+
     this.projectPath = this.dropdown.data('projectPath');
     this.namespacePath = this.dropdown.data('namespacePath');
     this.issuableType = this.$dropdownContainer.data('issuableType');
     this.titleInput = $(`#${this.issuableType}_title`);
+    this.templateWarningEl = $('.js-issuable-template-warning');
+    this.warnTemplateOverride = args[0].warnTemplateOverride;
 
     const initialQuery = {
       name: this.dropdown.data('selected'),
@@ -24,14 +27,61 @@ export default class IssuableTemplateSelector extends TemplateSelector {
     });
 
     $('.no-template', this.dropdown.parent()).on('click', () => {
-      this.currentTemplate.content = '';
-      this.setInputValueToTemplateContent();
-      $('.dropdown-toggle-text', this.dropdown).text(__('Choose a template'));
+      this.reset();
+    });
+
+    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) {
     this.startLoadingSpinner();
+
     Api.issueTemplate(
       this.namespacePath,
       this.projectPath,

app/assets/javascripts/templates/issuable_template_selectors.js

@@ -4,7 +4,7 @@ import $ from 'jquery';
 import IssuableTemplateSelector from './issuable_template_selector';
 
 export default class IssuableTemplateSelectors {
-  constructor({ $dropdowns, editor } = {}) {
+  constructor({ $dropdowns, editor, warnTemplateOverride } = {}) {
     this.$dropdowns = $dropdowns || $('.js-issuable-selector');
     this.editor = editor || this.initEditor();
 
@@ -16,6 +16,7 @@ export default class IssuableTemplateSelectors {
         wrapper: $dropdown.closest('.js-issuable-selector-wrap'),
         dropdown: $dropdown,
         editor: this.editor,
+        warnTemplateOverride,
       });
     });
   }

app/assets/stylesheets/framework/common.scss

@@ -214,18 +214,26 @@ li.note {
 @mixin message($background-color, $border-color, $text-color) {
   border-left: 4px solid $border-color;
   color: $text-color;
-  padding: 10px;
-  margin-bottom: 10px;
-  background: $background-color;
-  padding-left: 20px;
+  padding: $gl-padding $gl-padding-24;
+  margin-bottom: $gl-padding-12;
+  background-color: $background-color;
 
   &.centered {
     text-align: center;
   }
+
+  .close {
+    svg {
+      width: $gl-font-size-large;
+      height: $gl-font-size-large;
+    }
+
+    color: inherit;
+  }
 }
 
 .warning_message {
-  @include message($orange-100, $orange-200, $orange-700);
+  @include message($orange-100, $orange-200, $orange-800);
 }
 
 .danger_message {

app/views/shared/form_elements/_apply_template_warning.html.haml

+.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")

app/views/shared/issuable/_form.html.haml

@@ -19,6 +19,7 @@
   = 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 } }
 
+= render 'shared/form_elements/apply_template_warning'
 = render 'shared/form_elements/description', model: issuable, form: form, project: project
 
 - if issuable.respond_to?(:confidential)

changelogs/unreleased/16188-warn-before-applying-issue-templates.yml

+---
+title: Warn before applying issue templates
+merge_request: 16865
+author:
+type: changed

locale/gitlab.pot

@@ -1692,6 +1692,12 @@ msgstr ""
 msgid "Apply suggestion"
 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"
 msgstr ""
 

spec/features/projects/issuable_templates_spec.rb

@@ -92,6 +92,9 @@ describe 'issuable templates', :js do
 
   context 'user creates a merge request using templates' do
     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) }
 
     before do
@@ -101,6 +104,12 @@ describe 'issuable templates', :js do
         template_content,
         message: 'added merge request template',
         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
       fill_in :'merge_request[title]', with: 'test merge request title'
     end
@@ -111,6 +120,27 @@ describe 'issuable templates', :js do
       assert_template
       save_changes
     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
 
   context 'user creates a merge request from a forked project using templates' do