Merge branch '263252-mr-template-api' into 'master'

Resolve "Select MR template in the static site editor"

See merge request gitlab-org/gitlab!46488

app/assets/javascripts/api.js

@@ -34,6 +34,7 @@ const Api = {
   mergeRequestsPath: '/api/:version/merge_requests',
   groupLabelsPath: '/groups/:namespace_path/-/labels',
   issuableTemplatePath: '/:namespace_path/:project_path/templates/:type/:key',
+  issuableTemplatesPath: '/:namespace_path/:project_path/templates/:type',
   projectTemplatePath: '/api/:version/projects/:id/templates/:type/:key',
   projectTemplatesPath: '/api/:version/projects/:id/templates/:type',
   userCountsPath: '/api/:version/user_counts',
@@ -460,17 +461,38 @@ const Api = {
   },
 
   issueTemplate(namespacePath, projectPath, key, type, callback) {
-    const url = Api.buildUrl(Api.issuableTemplatePath)
-      .replace(':key', encodeURIComponent(key))
-      .replace(':type', type)
-      .replace(':project_path', projectPath)
-      .replace(':namespace_path', namespacePath);
+    const url = this.buildIssueTemplateUrl(
+      Api.issuableTemplatePath,
+      type,
+      projectPath,
+      namespacePath,
+    ).replace(':key', encodeURIComponent(key));
     return axios
       .get(url)
       .then(({ data }) => callback(null, data))
       .catch(callback);
   },
 
+  issueTemplates(namespacePath, projectPath, type, callback) {
+    const url = this.buildIssueTemplateUrl(
+      Api.issuableTemplatesPath,
+      type,
+      projectPath,
+      namespacePath,
+    );
+    return axios
+      .get(url)
+      .then(({ data }) => callback(null, data))
+      .catch(callback);
+  },
+
+  buildIssueTemplateUrl(path, type, projectPath, namespacePath) {
+    return Api.buildUrl(path)
+      .replace(':type', type)
+      .replace(':project_path', projectPath)
+      .replace(':namespace_path', namespacePath);
+  },
+
   users(query, options) {
     const url = Api.buildUrl(this.usersPath);
     return axios.get(url, {

app/assets/javascripts/static_site_editor/components/edit_meta_modal.vue

 <script>
 import { GlModal } from '@gitlab/ui';
 import { __, s__, sprintf } from '~/locale';
+import Api from '~/api';
 import LocalStorageSync from '~/vue_shared/components/local_storage_sync.vue';
 
 import EditMetaControls from './edit_meta_controls.vue';
 
-import { MR_META_LOCAL_STORAGE_KEY } from '../constants';
+import { ISSUABLE_TYPE, MR_META_LOCAL_STORAGE_KEY } from '../constants';
 
 export default {
   components: {
@@ -18,6 +19,14 @@ export default {
       type: String,
       required: true,
     },
+    namespace: {
+      type: String,
+      required: true,
+    },
+    project: {
+      type: String,
+      required: true,
+    },
   },
   data() {
     return {
@@ -49,10 +58,20 @@ export default {
       };
     },
   },
+  mounted() {
+    this.initTemplates();
+  },
   methods: {
     hide() {
       this.$refs.modal.hide();
     },
+    initTemplates() {
+      const { namespace, project } = this;
+      Api.issueTemplates(namespace, project, ISSUABLE_TYPE, (err, templates) => {
+        if (err) return; // Error handled by global AJAX error handler
+        this.mergeRequestTemplates = templates;
+      });
+    },
     show() {
       this.$refs.modal.show();
     },

app/assets/javascripts/static_site_editor/constants.js

@@ -2,6 +2,7 @@ import { s__, __ } from '~/locale';
 
 export const BRANCH_SUFFIX_COUNT = 8;
 export const DEFAULT_TARGET_BRANCH = 'master';
+export const ISSUABLE_TYPE = 'merge_request';
 
 export const SUBMIT_CHANGES_BRANCH_ERROR = s__('StaticSiteEditor|Branch could not be created.');
 export const SUBMIT_CHANGES_COMMIT_ERROR = s__(

app/assets/javascripts/static_site_editor/pages/home.vue

@@ -64,6 +64,9 @@ export default {
     isContentLoaded() {
       return Boolean(this.sourceContent);
     },
+    projectSplit() {
+      return this.appData.project.split('/'); // TODO: refactor so `namespace` and `project` remain distinct
+    },
   },
   mounted() {
     Tracking.event(document.body.dataset.page, TRACKING_ACTION_INITIALIZE_EDITOR);
@@ -148,6 +151,8 @@ export default {
       <edit-meta-modal
         ref="editMetaModal"
         :source-path="appData.sourcePath"
+        :namespace="projectSplit[0]"
+        :project="projectSplit[1]"
         @primary="onSubmit"
         @hide="onHideModal"
       />

app/controllers/projects/templates_controller.rb

@@ -7,6 +7,14 @@ class Projects::TemplatesController < Projects::ApplicationController
 
   feature_category :templates
 
+  def index
+    templates = @template_type.template_subsets(project)
+
+    respond_to do |format|
+      format.json { render json: templates.to_json }
+    end
+  end
+
   def show
     template = @template_type.find(params[:key], project)
 

changelogs/unreleased/263252-mr-template-api.yml

+---
+title: Add merge request description templates to Static Site Editor
+merge_request: 46488
+author:
+type: added

config/routes/project.rb

@@ -403,6 +403,11 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
       #
       # Templates
       #
+      get '/templates/:template_type' => 'templates#index', # rubocop:todo Cop/PutProjectRoutesUnderScope
+          as: :templates,
+          defaults: { format: 'json' },
+          constraints: { template_type: %r{issue|merge_request}, format: 'json' }
+
       get '/templates/:template_type/:key' => 'templates#show', # rubocop:todo Cop/PutProjectRoutesUnderScope
           as: :template,
           defaults: { format: 'json' },

lib/gitlab/template/base_template.rb

@@ -105,6 +105,20 @@ module Gitlab
             files.map { |t| { name: t.name } }
           end
         end
+
+        def template_subsets(project = nil)
+          return [] if project && !project.repository.exists?
+
+          if categories.any?
+            categories.keys.map do |category|
+              files = self.by_category(category, project)
+              [category, files.map { |t| { key: t.key, name: t.name, content: t.content } }]
+            end.to_h
+          else
+            files = self.all(project)
+            files.map { |t| { key: t.key, name: t.name, content: t.content } }
+          end
+        end
       end
     end
   end

spec/controllers/projects/templates_controller_spec.rb

@@ -5,35 +5,84 @@ require 'spec_helper'
 RSpec.describe Projects::TemplatesController do
   let(:project) { create(:project, :repository, :private) }
   let(:user) { create(:user) }
-  let(:file_path_1) { '.gitlab/issue_templates/issue_template.md' }
-  let(:file_path_2) { '.gitlab/merge_request_templates/merge_request_template.md' }
-  let!(:file_1) { project.repository.create_file(user, file_path_1, 'issue content', message: 'message', branch_name: 'master') }
-  let!(:file_2) { project.repository.create_file(user, file_path_2, 'merge request content', message: 'message', branch_name: 'master') }
+  let(:issue_template_path_1) { '.gitlab/issue_templates/issue_template_1.md' }
+  let(:issue_template_path_2) { '.gitlab/issue_templates/issue_template_2.md' }
+  let(:merge_request_template_path_1) { '.gitlab/merge_request_templates/merge_request_template_1.md' }
+  let(:merge_request_template_path_2) { '.gitlab/merge_request_templates/merge_request_template_2.md' }
+  let!(:issue_template_file_1) { project.repository.create_file(user, issue_template_path_1, 'issue content 1', message: 'message 1', branch_name: 'master') }
+  let!(:issue_template_file_2) { project.repository.create_file(user, issue_template_path_2, 'issue content 2', message: 'message 2', branch_name: 'master') }
+  let!(:merge_request_template_file_1) { project.repository.create_file(user, merge_request_template_path_1, 'merge request content 1', message: 'message 1', branch_name: 'master') }
+  let!(:merge_request_template_file_2) { project.repository.create_file(user, merge_request_template_path_2, 'merge request content 2', message: 'message 2', branch_name: 'master') }
+  let(:expected_issue_template_1) { { 'key' => 'issue_template_1', 'name' => 'issue_template_1', 'content' => 'issue content 1' } }
+  let(:expected_issue_template_2) { { 'key' => 'issue_template_2', 'name' => 'issue_template_2', 'content' => 'issue content 2' } }
+  let(:expected_merge_request_template_1) { { 'key' => 'merge_request_template_1', 'name' => 'merge_request_template_1', 'content' => 'merge request content 1' } }
+  let(:expected_merge_request_template_2) { { 'key' => 'merge_request_template_2', 'name' => 'merge_request_template_2', 'content' => 'merge request content 2' } }
+
+  describe '#index' do
+    before do
+      project.add_developer(user)
+      sign_in(user)
+    end
+
+    shared_examples 'templates request' do
+      it 'returns the templates' do
+        get(:index, params: { namespace_id: project.namespace, template_type: template_type, project_id: project }, format: :json)
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response).to match(expected_templates)
+      end
+
+      it 'fails for user with no access' do
+        other_user = create(:user)
+        sign_in(other_user)
+
+        get(:index, params: { namespace_id: project.namespace, template_type: template_type, project_id: project }, format: :json)
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+
+    context 'when querying for issue templates' do
+      it_behaves_like 'templates request' do
+        let(:template_type) { 'issue' }
+        let(:expected_templates) { [expected_issue_template_1, expected_issue_template_2] }
+      end
+    end
+
+    context 'when querying for merge_request templates' do
+      it_behaves_like 'templates request' do
+        let(:template_type) { 'merge_request' }
+        let(:expected_templates) { [expected_merge_request_template_1, expected_merge_request_template_2] }
+      end
+    end
+  end
 
   describe '#show' do
     shared_examples 'renders issue templates as json' do
+      let(:expected_issue_template) { expected_issue_template_2 }
+
       it do
-        get(:show, params: { namespace_id: project.namespace, template_type: 'issue', key: 'issue_template', project_id: project }, format: :json)
+        get(:show, params: { namespace_id: project.namespace, template_type: 'issue', key: 'issue_template_2', project_id: project }, format: :json)
 
         expect(response).to have_gitlab_http_status(:ok)
-        expect(json_response['name']).to eq('issue_template')
-        expect(json_response['content']).to eq('issue content')
+        expect(json_response).to match(expected_issue_template)
       end
     end
 
     shared_examples 'renders merge request templates as json' do
+      let(:expected_merge_request_template) { expected_merge_request_template_2 }
+
       it do
-        get(:show, params: { namespace_id: project.namespace, template_type: 'merge_request', key: 'merge_request_template', project_id: project }, format: :json)
+        get(:show, params: { namespace_id: project.namespace, template_type: 'merge_request', key: 'merge_request_template_2', project_id: project }, format: :json)
 
         expect(response).to have_gitlab_http_status(:ok)
-        expect(json_response['name']).to eq('merge_request_template')
-        expect(json_response['content']).to eq('merge request content')
+        expect(json_response).to match(expected_merge_request_template)
       end
     end
 
     shared_examples 'renders 404 when requesting an issue template' do
       it do
-        get(:show, params: { namespace_id: project.namespace, template_type: 'issue', key: 'issue_template', project_id: project }, format: :json)
+        get(:show, params: { namespace_id: project.namespace, template_type: 'issue', key: 'issue_template_1', project_id: project }, format: :json)
 
         expect(response).to have_gitlab_http_status(:not_found)
       end
@@ -41,21 +90,23 @@ RSpec.describe Projects::TemplatesController do
 
     shared_examples 'renders 404 when requesting a merge request template' do
       it do
-        get(:show, params: { namespace_id: project.namespace, template_type: 'merge_request', key: 'merge_request_template', project_id: project }, format: :json)
+        get(:show, params: { namespace_id: project.namespace, template_type: 'merge_request', key: 'merge_request_template_1', project_id: project }, format: :json)
 
         expect(response).to have_gitlab_http_status(:not_found)
       end
     end
 
-    shared_examples 'renders 404 when params are invalid' do
+    shared_examples 'raises error when template type is invalid' do
       it 'does not route when the template type is invalid' do
         expect do
-          get(:show, params: { namespace_id: project.namespace, template_type: 'invalid_type', key: 'issue_template', project_id: project }, format: :json)
+          get(:show, params: { namespace_id: project.namespace, template_type: 'invalid_type', key: 'issue_template_1', project_id: project }, format: :json)
         end.to raise_error(ActionController::UrlGenerationError)
       end
+    end
 
+    shared_examples 'renders 404 when params are invalid' do
       it 'renders 404 when the format type is invalid' do
-        get(:show, params: { namespace_id: project.namespace, template_type: 'issue', key: 'issue_template', project_id: project }, format: :html)
+        get(:show, params: { namespace_id: project.namespace, template_type: 'issue', key: 'issue_template_1', project_id: project }, format: :html)
 
         expect(response).to have_gitlab_http_status(:not_found)
       end
@@ -74,7 +125,6 @@ RSpec.describe Projects::TemplatesController do
 
       include_examples 'renders 404 when requesting an issue template'
       include_examples 'renders 404 when requesting a merge request template'
-      include_examples 'renders 404 when params are invalid'
     end
 
     context 'when user is a member of the project' do
@@ -85,8 +135,12 @@ RSpec.describe Projects::TemplatesController do
 
       include_examples 'renders issue templates as json'
       include_examples 'renders merge request templates as json'
+
+      context 'when params are invalid' do
+        include_examples 'raises error when template type is invalid'
         include_examples 'renders 404 when params are invalid'
       end
+    end
 
     context 'when user is a guest of the project' do
       before do
@@ -96,7 +150,6 @@ RSpec.describe Projects::TemplatesController do
 
       include_examples 'renders issue templates as json'
       include_examples 'renders 404 when requesting a merge request template'
-      include_examples 'renders 404 when params are invalid'
     end
   end
 
@@ -111,8 +164,8 @@ RSpec.describe Projects::TemplatesController do
         get(:names, params: { namespace_id: project.namespace, template_type: template_type, project_id: project }, format: :json)
 
         expect(response).to have_gitlab_http_status(:ok)
-        expect(json_response.size).to eq(1)
-        expect(json_response[0]['name']).to eq(expected_template_name)
+        expect(json_response.size).to eq(2)
+        expect(json_response).to match(expected_template_names)
       end
 
       it 'fails for user with no access' do
@@ -128,14 +181,14 @@ RSpec.describe Projects::TemplatesController do
     context 'when querying for issue templates' do
       it_behaves_like 'template names request' do
         let(:template_type) { 'issue' }
-        let(:expected_template_name) { 'issue_template' }
+        let(:expected_template_names) { [{ 'name' => 'issue_template_1' }, { 'name' => 'issue_template_2' }] }
       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' }
+        let(:expected_template_names) { [{ 'name' => 'merge_request_template_1' }, { 'name' => 'merge_request_template_2' }] }
       end
     end
   end

spec/frontend/api_spec.js

@@ -553,7 +553,6 @@ describe('Api', () => {
   });
 
   describe('issueTemplate', () => {
-    it('fetches an issue template', done => {
     const namespace = 'some namespace';
     const project = 'some project';
     const templateKey = ' template #%?.key ';
@@ -561,6 +560,8 @@ describe('Api', () => {
     const expectedUrl = `${dummyUrlRoot}/${namespace}/${project}/templates/${templateType}/${encodeURIComponent(
       templateKey,
     )}`;
+
+    it('fetches an issue template', done => {
       mock.onGet(expectedUrl).reply(httpStatus.OK, 'test');
 
       Api.issueTemplate(namespace, project, templateKey, templateType, (error, response) => {
@@ -568,6 +569,49 @@ describe('Api', () => {
         done();
       });
     });
+
+    describe('when an error occurs while fetching an issue template', () => {
+      it('rejects the Promise', () => {
+        mock.onGet(expectedUrl).replyOnce(httpStatus.INTERNAL_SERVER_ERROR);
+
+        Api.issueTemplate(namespace, project, templateKey, templateType, () => {
+          expect(mock.history.get).toHaveLength(1);
+        });
+      });
+    });
+  });
+
+  describe('issueTemplates', () => {
+    const namespace = 'some namespace';
+    const project = 'some project';
+    const templateType = 'template type';
+    const expectedUrl = `${dummyUrlRoot}/${namespace}/${project}/templates/${templateType}`;
+
+    it('fetches all templates by type', done => {
+      const expectedData = [
+        { key: 'Template1', name: 'Template 1', content: 'This is template 1!' },
+      ];
+      mock.onGet(expectedUrl).reply(httpStatus.OK, expectedData);
+
+      Api.issueTemplates(namespace, project, templateType, (error, response) => {
+        expect(response.length).toBe(1);
+        const { key, name, content } = response[0];
+        expect(key).toBe('Template1');
+        expect(name).toBe('Template 1');
+        expect(content).toBe('This is template 1!');
+        done();
+      });
+    });
+
+    describe('when an error occurs while fetching issue templates', () => {
+      it('rejects the Promise', () => {
+        mock.onGet(expectedUrl).replyOnce(httpStatus.INTERNAL_SERVER_ERROR);
+
+        Api.issueTemplates(namespace, project, templateType, () => {
+          expect(mock.history.get).toHaveLength(1);
+        });
+      });
+    });
   });
 
   describe('projectTemplates', () => {

spec/frontend/static_site_editor/components/edit_meta_modal_spec.js

 import { shallowMount } from '@vue/test-utils';
 import { GlModal } from '@gitlab/ui';
 import { useLocalStorageSpy } from 'helpers/local_storage_helper';
+import MockAdapter from 'axios-mock-adapter';
+import axios from '~/lib/utils/axios_utils';
 import LocalStorageSync from '~/vue_shared/components/local_storage_sync.vue';
 import EditMetaModal from '~/static_site_editor/components/edit_meta_modal.vue';
 import EditMetaControls from '~/static_site_editor/components/edit_meta_controls.vue';
 import { MR_META_LOCAL_STORAGE_KEY } from '~/static_site_editor/constants';
-import { sourcePath, mergeRequestMeta, mergeRequestTemplates } from '../mock_data';
+import {
+  sourcePath,
+  mergeRequestMeta,
+  mergeRequestTemplates,
+  project as namespaceProject,
+} from '../mock_data';
 
 describe('~/static_site_editor/components/edit_meta_modal.vue', () => {
   useLocalStorageSpy();
 
   let wrapper;
-  let resetCachedEditable;
-  let mockEditMetaControlsInstance;
+  let mockAxios;
   const { title, description } = mergeRequestMeta;
+  const [namespace, project] = namespaceProject.split('/');
 
   const buildWrapper = (propsData = {}, data = {}) => {
     wrapper = shallowMount(EditMetaModal, {
       propsData: {
         sourcePath,
+        namespace,
+        project,
         ...propsData,
       },
       data: () => data,
     });
   };
 
-  const buildMocks = () => {
-    resetCachedEditable = jest.fn();
-    mockEditMetaControlsInstance = { resetCachedEditable };
-    wrapper.vm.$refs.editMetaControls = mockEditMetaControlsInstance;
+  const buildMockAxios = () => {
+    mockAxios = new MockAdapter(axios);
+    const templatesMergeRequestsPath = `templates/merge_request`;
+    mockAxios
+      .onGet(`${namespace}/${project}/${templatesMergeRequestsPath}`)
+      .reply(200, mergeRequestTemplates);
+  };
+
+  const buildMockRefs = () => {
+    wrapper.vm.$refs.editMetaControls = { resetCachedEditable: jest.fn() };
   };
 
   const findGlModal = () => wrapper.find(GlModal);
@@ -37,16 +52,17 @@ describe('~/static_site_editor/components/edit_meta_modal.vue', () => {
 
   beforeEach(() => {
     localStorage.setItem(MR_META_LOCAL_STORAGE_KEY);
-  });
 
-  beforeEach(() => {
+    buildMockAxios();
     buildWrapper();
-    buildMocks();
+    buildMockRefs();
 
     return wrapper.vm.$nextTick();
   });
 
   afterEach(() => {
+    mockAxios.restore();
+
     wrapper.destroy();
     wrapper = null;
   });

spec/frontend/static_site_editor/pages/home_spec.js

@@ -12,7 +12,7 @@ import { SUCCESS_ROUTE } from '~/static_site_editor/router/constants';
 import { TRACKING_ACTION_INITIALIZE_EDITOR } from '~/static_site_editor/constants';
 
 import {
-  projectId as project,
+  project,
   returnUrl,
   sourceContentYAML as content,
   sourceContentTitle as title,