Merge branch '64177-only-fetch-templates-when-editing-issues-pd' into 'master'

Only fetch issue template names when editing an issue

See merge request gitlab-org/gitlab!16707

app/assets/javascripts/issue_show/components/app.vue

@@ -102,10 +102,10 @@ export default {
       required: false,
       default: '',
     },
-    issuableTemplates: {
-      type: Array,
+    issuableTemplateNamesPath: {
+      type: String,
       required: false,
-      default: () => [],
+      default: '',
     },
     markdownPreviewPath: {
       type: String,
@@ -156,9 +156,13 @@ export default {
       store,
       state: store.state,
       showForm: false,
+      templatesRequested: false,
     };
   },
   computed: {
+    issuableTemplates() {
+      return this.store.formState.issuableTemplates;
+    },
     formState() {
       return this.store.formState;
     },
@@ -233,6 +237,7 @@ export default {
       }
       return undefined;
     },
+
     updateStoreState() {
       return this.service
         .getData()
@@ -245,7 +250,7 @@ export default {
         });
     },
 
-    openForm() {
+    updateAndShowForm(templates = []) {
       if (!this.showForm) {
         this.showForm = true;
         this.store.setFormState({
@@ -254,9 +259,32 @@ export default {
           lock_version: this.state.lock_version,
           lockedWarningVisible: false,
           updateLoading: false,
+          issuableTemplates: templates,
+        });
+      }
+    },
+
+    requestTemplatesAndShowForm() {
+      return this.service
+        .loadTemplates(this.issuableTemplateNamesPath)
+        .then(res => {
+          this.updateAndShowForm(res.data);
+        })
+        .catch(() => {
+          createFlash(this.defaultErrorMessage);
+          this.updateAndShowForm();
         });
+    },
+
+    openForm() {
+      if (!this.templatesRequested) {
+        this.templatesRequested = true;
+        this.requestTemplatesAndShowForm();
+      } else {
+        this.updateAndShowForm(this.issuableTemplates);
       }
     },
+
     closeForm() {
       this.showForm = false;
     },

app/assets/javascripts/issue_show/services/index.js

@@ -17,4 +17,13 @@ export default class Service {
   updateIssuable(data) {
     return axios.put(this.endpoint, data);
   }
+
+  // eslint-disable-next-line class-methods-use-this
+  loadTemplates(templateNamesEndpoint) {
+    if (!templateNamesEndpoint) {
+      return Promise.resolve([]);
+    }
+
+    return axios.get(templateNamesEndpoint);
+  }
 }

app/assets/javascripts/issue_show/stores/index.js

@@ -9,6 +9,7 @@ export default class Store {
       lockedWarningVisible: false,
       updateLoading: false,
       lock_version: 0,
+      issuableTemplates: [],
     };
   }
 

app/controllers/projects/templates_controller.rb

@@ -13,6 +13,14 @@ class Projects::TemplatesController < Projects::ApplicationController
     end
   end
 
+  def names
+    templates = @template_type.dropdown_names(project)
+
+    respond_to do |format|
+      format.json { render json: templates }
+    end
+  end
+
   private
 
   # User must have:

app/helpers/issuables_helper.rb

@@ -272,7 +272,7 @@ module IssuablesHelper
       markdownPreviewPath: preview_markdown_path(parent),
       markdownDocsPath: help_page_path('user/markdown'),
       lockVersion: issuable.lock_version,
-      issuableTemplates: issuable_templates(issuable),
+      issuableTemplateNamesPath: template_names_path(parent, issuable),
       initialTitleHtml: markdown_field(issuable, :title),
       initialTitleText: issuable.title,
       initialDescriptionHtml: markdown_field(issuable, :description),
@@ -429,6 +429,12 @@ module IssuablesHelper
     end
   end
 
+  def template_names_path(parent, issuable)
+    return '' unless parent.is_a?(Project)
+
+    project_template_names_path(parent, template_type: issuable.class.name.underscore)
+  end
+
   def issuable_sidebar_options(issuable)
     {
       endpoint: "#{issuable[:issuable_json_path]}?serializer=sidebar_extras",

config/routes/project.rb

@@ -195,6 +195,12 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
           defaults: { format: 'json' },
           constraints: { key: %r{[^/]+}, template_type: %r{issue|merge_request}, format: 'json' }
 
+      get '/description_templates/names/:template_type',
+          to: 'templates#names',
+          as: :template_names,
+          defaults: { format: 'json' },
+          constraints: { template_type: %r{issue|merge_request}, format: 'json' }
+
       resources :commit, only: [:show], constraints: { id: /\h{7,40}/ } do
         member do
           get :branches

ee/spec/helpers/ee/issuables_helper_spec.rb

@@ -27,7 +27,7 @@ describe IssuablesHelper do
         issuableRef: "&#{epic.iid}",
         markdownPreviewPath: "/groups/#{@group.full_path}/preview_markdown",
         markdownDocsPath: '/help/user/markdown',
-        issuableTemplates: nil,
+        issuableTemplateNamesPath: '',
         lockVersion: epic.lock_version,
         fullPath: @group.full_path,
         groupPath: @group.path,

spec/controllers/projects/templates_controller_spec.rb

@@ -99,4 +99,44 @@ describe Projects::TemplatesController do
       include_examples 'renders 404 when params are invalid'
     end
   end
+
+  describe '#names' do
+    before do
+      project.add_developer(user)
+      sign_in(user)
+    end
+
+    shared_examples 'template names request' do
+      it 'returns the template names' do
+        get(:names, params: { namespace_id: project.namespace, template_type: template_type, project_id: project }, format: :json)
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response.size).to eq(1)
+        expect(json_response[0]['name']).to eq(expected_template_name)
+      end
+
+      it 'fails for user with no access' do
+        other_user = create(:user)
+        sign_in(other_user)
+
+        get(:names, params: { namespace_id: project.namespace, template_type: template_type, project_id: project }, format: :json)
+
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+
+    context 'when querying for issue templates' do
+      it_behaves_like 'template names request' do
+        let(:template_type) { 'issue' }
+        let(:expected_template_name) { 'issue_template' }
+      end
+    end
+
+    context 'when querying for merge_request templates' do
+      it_behaves_like 'template names request' do
+        let(:template_type) { 'merge_request' }
+        let(:expected_template_name) { 'merge_request_template' }
+      end
+    end
+  end
 end

spec/features/issues/gfm_autocomplete_spec.rb

@@ -27,6 +27,8 @@ describe 'GFM autocomplete', :js do
   it 'updates issue description with GFM reference' do
     find('.js-issuable-edit').click
 
+    wait_for_requests
+
     simulate_input('#issue-description', "@#{user.name[0...3]}")
 
     wait_for_requests

spec/helpers/issuables_helper_spec.rb

@@ -190,7 +190,6 @@ describe IssuablesHelper do
         issuableRef: "##{issue.iid}",
         markdownPreviewPath: "/#{@project.full_path}/preview_markdown",
         markdownDocsPath: '/help/user/markdown',
-        issuableTemplates: [],
         lockVersion: issue.lock_version,
         projectPath: @project.path,
         projectNamespace: @project.namespace.path,

spec/javascripts/issue_show/components/app_spec.js

+/* eslint-disable no-unused-vars */
+import GLDropdown from '~/gl_dropdown';
 import Vue from 'vue';
 import MockAdapter from 'axios-mock-adapter';
 import axios from '~/lib/utils/axios_utils';
@@ -52,6 +54,7 @@ describe('Issuable output', () => {
         markdownDocsPath: '/',
         projectNamespace: '/',
         projectPath: '/',
+        issuableTemplateNamesPath: '/issuable-templates-path',
       },
     }).$mount();
 
@@ -129,11 +132,11 @@ describe('Issuable output', () => {
   });
 
   it('does not update formState if form is already open', done => {
-    vm.openForm();
+    vm.updateAndShowForm();
 
     vm.state.titleText = 'testing 123';
 
-    vm.openForm();
+    vm.updateAndShowForm();
 
     Vue.nextTick(() => {
       expect(vm.store.formState.title).not.toBe('testing 123');
@@ -284,7 +287,7 @@ describe('Issuable output', () => {
         });
       });
 
-      it('shows error mesage from backend if exists', done => {
+      it('shows error message from backend if exists', done => {
         const msg = 'Custom error message from backend';
         spyOn(vm.service, 'updateIssuable').and.callFake(
           // eslint-disable-next-line prefer-promise-reject-errors
@@ -405,20 +408,20 @@ describe('Issuable output', () => {
     });
   });
 
-  describe('open form', () => {
+  describe('updateAndShowForm', () => {
     it('shows locked warning if form is open & data is different', done => {
       vm.$nextTick()
         .then(() => {
-          vm.openForm();
+          vm.updateAndShowForm();
 
           vm.poll.makeRequest();
+
+          return new Promise(resolve => {
+            vm.$watch('formState.lockedWarningVisible', value => {
+              if (value) resolve();
+            });
+          });
         })
-        // Wait for the request
-        .then(vm.$nextTick)
-        // Wait for the successCallback to update the store state
-        .then(vm.$nextTick)
-        // Wait for the new state to flow to the Vue components
-        .then(vm.$nextTick)
         .then(() => {
           expect(vm.formState.lockedWarningVisible).toEqual(true);
           expect(vm.formState.lock_version).toEqual(1);
@@ -429,6 +432,41 @@ describe('Issuable output', () => {
     });
   });
 
+  describe('requestTemplatesAndShowForm', () => {
+    beforeEach(() => {
+      spyOn(vm, 'updateAndShowForm');
+    });
+
+    it('shows the form if template names request is successful', done => {
+      const mockData = [{ name: 'Bug' }];
+      mock.onGet('/issuable-templates-path').reply(() => Promise.resolve([200, mockData]));
+
+      vm.requestTemplatesAndShowForm()
+        .then(() => {
+          expect(vm.updateAndShowForm).toHaveBeenCalledWith(mockData);
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+
+    it('shows the form if template names request failed', done => {
+      mock
+        .onGet('/issuable-templates-path')
+        .reply(() => Promise.reject(new Error('something went wrong')));
+
+      vm.requestTemplatesAndShowForm()
+        .then(() => {
+          expect(document.querySelector('.flash-container .flash-text').textContent).toContain(
+            'Error updating issue',
+          );
+
+          expect(vm.updateAndShowForm).toHaveBeenCalledWith();
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+  });
+
   describe('show inline edit button', () => {
     it('should not render by default', () => {
       expect(vm.$el.querySelector('.title-container .note-action-button')).toBeDefined();