Add templates#names endpoint

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

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;
     },

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);
+  }
 }

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: project_template_names_path(parent, template_type: issuable.class.name.underscore),
       initialTitleHtml: markdown_field(issuable, :title),
       initialTitleText: issuable.title,
       initialDescriptionHtml: markdown_field(issuable, :description),

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

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();