Add templates#names endpoint64177-only-fetch-templates-when-editing-issues-pd

Load templates asynchronously

Fix style issues

Style: Add catch

Fix databinding for templates

Cleanup

Add specs

Add missing issuableTemplateNamesPath to fix specs

Fix karma specs: replaced calls of openForm with new method

Disabled unused vars because this import is needed

Cleanup: This should no longer be needed

Fix specs: remove field from expected result

Applied suggestion and spy on updateAndShowForm method

Applied code review suggestions for frontend code refactoring

Move template names request to service

Change name of template names url to description_templates/names

Remove coding comment

10 files changed, 144 insertions, 18 deletions

diff --git a/app/assets/javascripts/issue_show/components/app.vue b/app/assets/javascripts/issue_show/components/app.vue
index 88975c2cc73..8cedf808a70 100644
--- a/app/assets/javascripts/issue_show/components/app.vue
+++ b/app/assets/javascripts/issue_show/components/app.vue
@@ -102,10 +102,9 @@ export default {
       required: false,
       default: '',
     },
-    issuableTemplates: {
-      type: Array,
-      required: false,
-      default: () => [],
+    issuableTemplateNamesPath: {
+      type: String,
+      required: true,
     },
     markdownPreviewPath: {
       type: String,
@@ -156,9 +155,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 +236,7 @@ export default {
       }
       return undefined;
     },
+
     updateStoreState() {
       return this.service
         .getData()
@@ -245,7 +249,7 @@ export default {
         });
     },
 
-    openForm() {
+    updateAndShowForm(templates = []) {
       if (!this.showForm) {
         this.showForm = true;
         this.store.setFormState({
@@ -254,9 +258,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;
     },
diff --git a/app/assets/javascripts/issue_show/services/index.js b/app/assets/javascripts/issue_show/services/index.js
index 3c8334bee50..020b31bdcf9 100644
--- a/app/assets/javascripts/issue_show/services/index.js
+++ b/app/assets/javascripts/issue_show/services/index.js
@@ -17,4 +17,9 @@ export default class Service {
   updateIssuable(data) {
     return axios.put(this.endpoint, data);
   }
+
+  // eslint-disable-next-line class-methods-use-this
+  loadTemplates(templateNamesEndpoint) {
+    return axios.get(templateNamesEndpoint);
+  }
 }
diff --git a/app/assets/javascripts/issue_show/stores/index.js b/app/assets/javascripts/issue_show/stores/index.js
index 3c17e73ccec..d32747b5053 100644
--- a/app/assets/javascripts/issue_show/stores/index.js
+++ b/app/assets/javascripts/issue_show/stores/index.js
@@ -9,6 +9,7 @@ export default class Store {
       lockedWarningVisible: false,
       updateLoading: false,
       lock_version: 0,
+      issuableTemplates: [],
     };
   }
 
diff --git a/app/controllers/projects/templates_controller.rb b/app/controllers/projects/templates_controller.rb
index f987033a26c..95739f96d39 100644
--- a/app/controllers/projects/templates_controller.rb
+++ b/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:
diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb
index b88b25eb845..223e55fef41 100644
--- a/app/helpers/issuables_helper.rb
+++ b/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: project_template_names_path(parent, template_type: issuable.class.name.underscore),
       initialTitleHtml: markdown_field(issuable, :title),
       initialTitleText: issuable.title,
       initialDescriptionHtml: markdown_field(issuable, :description),
diff --git a/config/routes/project.rb b/config/routes/project.rb
index 29e462f904d..8e88de9cb46 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -189,6 +189,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
diff --git a/spec/controllers/projects/templates_controller_spec.rb b/spec/controllers/projects/templates_controller_spec.rb
index d5ef2b0e114..07b8a36fefc 100644
--- a/spec/controllers/projects/templates_controller_spec.rb
+++ b/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
diff --git a/spec/features/issues/gfm_autocomplete_spec.rb b/spec/features/issues/gfm_autocomplete_spec.rb
index cc834df367b..0ff3809a915 100644
--- a/spec/features/issues/gfm_autocomplete_spec.rb
+++ b/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
diff --git a/spec/helpers/issuables_helper_spec.rb b/spec/helpers/issuables_helper_spec.rb
index 3c8179460ac..583b8f90db9 100644
--- a/spec/helpers/issuables_helper_spec.rb
+++ b/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,
diff --git a/spec/javascripts/issue_show/components/app_spec.js b/spec/javascripts/issue_show/components/app_spec.js
index 2770743937e..c3d1440c34e 100644
--- a/spec/javascripts/issue_show/components/app_spec.js
+++ b/spec/javascripts/issue_show/components/app_spec.js
@@ -1,3 +1,5 @@
+/* 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();