Merge branch 'tc-namespace-license-checks--issue-mr-template' into 'master'

Namespace license checks Issue & MR template

Closes #2580

See merge request !2321

app/models/license.rb

@@ -10,6 +10,7 @@ class License < ActiveRecord::Base
   FAST_FORWARD_MERGE_FEATURE = 'GitLab_FastForwardMerge'.freeze
   FILE_LOCK_FEATURE = 'GitLab_FileLocks'.freeze
   GEO_FEATURE = 'GitLab_Geo'.freeze
+  ISSUABLE_DEFAULT_TEMPLATES_FEATURE = 'GitLab_IssuableDefaultTemplates'.freeze
   ISSUE_BOARDS_FOCUS_MODE_FEATURE = 'IssueBoardsFocusMode'.freeze
   ISSUE_WEIGHTS_FEATURE = 'GitLab_IssueWeights'.freeze
   MERGE_REQUEST_APPROVERS_FEATURE = 'GitLab_MergeRequestApprovers'.freeze
@@ -34,6 +35,7 @@ class License < ActiveRecord::Base
     export_issues: EXPORT_ISSUES_FEATURE,
     fast_forward_merge: FAST_FORWARD_MERGE_FEATURE,
     file_lock: FILE_LOCK_FEATURE,
+    issuable_default_templates: ISSUABLE_DEFAULT_TEMPLATES_FEATURE,
     issue_board_focus_mode: ISSUE_BOARDS_FOCUS_MODE_FEATURE,
     issue_weights: ISSUE_WEIGHTS_FEATURE,
     merge_request_approvers: MERGE_REQUEST_APPROVERS_FEATURE,
@@ -52,6 +54,7 @@ class License < ActiveRecord::Base
     { ELASTIC_SEARCH_FEATURE => 1 },
     { EXPORT_ISSUES_FEATURE => 1 },
     { FAST_FORWARD_MERGE_FEATURE => 1 },
+    { ISSUABLE_DEFAULT_TEMPLATES_FEATURE => 1 },
     { ISSUE_BOARDS_FOCUS_MODE_FEATURE => 1 },
     { ISSUE_WEIGHTS_FEATURE => 1 },
     { MERGE_REQUEST_APPROVERS_FEATURE => 1 },
@@ -90,6 +93,7 @@ class License < ActiveRecord::Base
     { FAST_FORWARD_MERGE_FEATURE => 1 },
     { FILE_LOCK_FEATURE => 1 },
     { GEO_FEATURE => 1 },
+    { ISSUABLE_DEFAULT_TEMPLATES_FEATURE => 1 },
     { ISSUE_BOARDS_FOCUS_MODE_FEATURE => 1 },
     { ISSUE_WEIGHTS_FEATURE => 1 },
     { MERGE_REQUEST_APPROVERS_FEATURE => 1 },

app/services/ee/issues/build_service.rb

+module EE
+  module Issues
+    module BuildService
+      def issue_params_from_template
+        return {} unless project.feature_available?(:issuable_default_templates)
+
+        { description: project.issues_template }
+      end
+
+      # Issue params can be built from 3 types of passed params,
+      # They take precedence over eachother like this
+      # passed params > discussion params > template params
+      # The template params are filled in here, and might be overwritten by super
+      def issue_params
+        @issue_params ||= issue_params_from_template.merge(super)
+      end
+    end
+  end
+end

app/services/ee/merge_requests/build_service.rb

+module EE
+  module MergeRequests
+    module BuildService
+      def assign_title_and_description
+        super
+
+        assign_description_from_template
+      end
+
+      # Set MR description based on project template
+      def assign_description_from_template
+        return unless target_project.feature_available?(:issuable_default_templates) &&
+            target_project.merge_requests_template.present?
+
+        merge_request.description = target_project.merge_requests_template
+
+        append_closes_description
+      end
+    end
+  end
+end

app/services/issues/build_service.rb

 module Issues
   class BuildService < Issues::BaseService
     include ResolveDiscussions
+    prepend ::EE::Issues::BuildService
 
     def execute
       filter_resolve_discussion_params
       @issue = project.issues.new(issue_params)
     end
 
-    def issue_params_from_template
-      { description: project.issues_template }
-    end
-
     def issue_params_with_info_from_discussions
       return {} unless merge_request_to_resolve_discussions_of
 
@@ -58,13 +55,8 @@ module Issues
       [discussion_info, quote].join("\n\n")
     end
 
-    # Issue params can be built from 3 types of passed params,
-    # They take precedence over eachother like this
-    # passed params > discussion params > template params
     def issue_params
-      @issue_params ||= issue_params_from_template
-                          .merge(issue_params_with_info_from_discussions)
-                          .merge(whitelisted_issue_params)
+      @issue_params ||= issue_params_with_info_from_discussions.merge(whitelisted_issue_params)
     end
 
     def whitelisted_issue_params

app/services/merge_requests/build_service.rb

 module MergeRequests
   class BuildService < MergeRequests::BaseService
+    prepend EE::MergeRequests::BuildService
+
     def execute
       self.merge_request = MergeRequest.new(params)
       merge_request.compare_commits = []
@@ -105,42 +107,60 @@ module MergeRequests
     #   more than one commit in the MR
     #
     def assign_title_and_description
-      if match = source_branch.match(/\A(\d+)-/)
-        iid = match[1]
-      end
+      assign_title_and_description_from_single_commit
+
+      assign_title_from_issue
 
+      merge_request.title ||= source_branch.titleize.humanize
+
+      merge_request.title = wip_title if compare_commits.empty?
+
+      append_closes_description
+    end
+
+    def assign_title_and_description_from_single_commit
       commits = compare_commits
-      if commits && commits.count == 1
-        commit = commits.first
-        merge_request.title = commit.title
-        merge_request.description ||= commit.description.try(:strip)
-      elsif iid && issue = target_project.get_issue(iid, current_user)
-        case issue
-        when Issue
-          merge_request.title = "Resolve \"#{issue.title}\""
-        when ExternalIssue
-          merge_request.title = "Resolve #{issue.title}"
-        end
-      else
-        merge_request.title = source_branch.titleize.humanize
-      end
 
-      # Set MR description based on project template
-      if merge_request.target_project.merge_requests_template.present?
-        merge_request.description = merge_request.target_project.merge_requests_template
+      return unless commits && commits.count == 1
+
+      commit = commits.first
+      merge_request.title ||= commit.title
+      merge_request.description ||= commit.description.try(:strip)
+    end
+
+    def assign_title_from_issue
+      return unless issue
+
+      case issue
+      when Issue
+        merge_request.title ||= "Resolve \"#{issue.title}\""
+      when ExternalIssue
+        merge_request.title ||= "Resolve #{issue.title}"
       end
+    end
 
-      if iid
-        closes_issue = "Closes ##{iid}"
+    def append_closes_description
+      return unless issue_iid
 
-        if description.present?
-          merge_request.description += closes_issue.prepend("\n\n")
-        else
-          merge_request.description = closes_issue
-        end
+      closes_issue = "Closes ##{issue_iid}"
+
+      if description.present?
+        merge_request.description += closes_issue.prepend("\n\n")
+      else
+        merge_request.description = closes_issue
       end
+    end
+
+    def issue_iid
+      return @issue_iid if defined?(@issue_iid)
+
+      @issue_iid = source_branch[/\A(\d+)-/, 1]
+    end
+
+    def issue
+      return @issue if defined?(@issue)
 
-      merge_request.title = wip_title if commits.empty?
+      @issue = target_project.get_issue(issue_iid, current_user)
     end
   end
 end

app/views/projects/_issues_settings.html.haml

-%fieldset.features.append-bottom-0.issues-feature
-  %hr
-  %h5.prepend-top-0
-    Issues
-  .form-group
-    = f.label :issues_template, class: 'label-light' do
-      Default description template for issues
-      = link_to icon('question-circle'), help_page_path('user/project/description_templates', anchor: 'setting-a-default-template-for-issues-and-merge-requests'), target: '_blank'
-    = f.text_area :issues_template, class: "form-control", rows: 3
-    .hint
-      Description parsed with #{link_to "GitLab Flavored Markdown", help_page_path('user/markdown'), target: '_blank'}.

app/views/projects/edit.html.haml

@@ -128,7 +128,7 @@
                 %span.descr Enable Container Registry for this project
                 = link_to icon('question-circle'), help_page_path('user/project/container_registry'), target: '_blank'
 
-        = render 'issues_settings', f: f
+        = render 'projects/ee/issues_settings', form: f
 
         = render 'merge_request_settings', form: f
 

app/views/projects/ee/_issues_settings.html.haml

+- if @project.feature_available?(:issuable_default_templates)
+  %fieldset.features.append-bottom-0.issues-feature
+    %hr
+    %h5.prepend-top-0
+      Issues
+    .form-group
+      = form.label :issues_template, class: 'label-light' do
+        Default description template for issues
+        = link_to icon('question-circle'), help_page_path('user/project/description_templates', anchor: 'setting-a-default-template-for-issues-and-merge-requests'), target: '_blank'
+      = form.text_area :issues_template, class: "form-control", rows: 3
+      .hint
+        Description parsed with #{link_to "GitLab Flavored Markdown", help_page_path('user/markdown'), target: '_blank'}.

app/views/projects/ee/_merge_request_settings.html.haml

@@ -39,13 +39,14 @@
             %span.descr
               When fast-forward merge is not possible, the user is given the option to rebase.
 
-.form-group
-  = form.label :merge_requests_template, class: 'label-light' do
-    Default description template for merge requests
-    = link_to icon('question-circle'), help_page_path('user/project/description_templates', anchor: 'setting-a-default-template-for-issues-and-merge-requests'), target: '_blank'
-  = form.text_area :merge_requests_template, class: "form-control", rows: 3
-  .hint
-    Description parsed with #{link_to "GitLab Flavored Markdown", help_page_path('user/markdown'), target: '_blank'}.
+- if @project.feature_available?(:issuable_default_templates)
+  .form-group
+    = form.label :merge_requests_template, class: 'label-light' do
+      Default description template for merge requests
+      = link_to icon('question-circle'), help_page_path('user/project/description_templates', anchor: 'setting-a-default-template-for-issues-and-merge-requests'), target: '_blank'
+    = form.text_area :merge_requests_template, class: "form-control", rows: 3
+    .hint
+      Description parsed with #{link_to "GitLab Flavored Markdown", help_page_path('user/markdown'), target: '_blank'}.
 
 = render 'projects/ee/merge_request_approvals_settings', project: project, form: form
 

changelogs/unreleased-ee/tc-namespace-license-checks--issue-mr-template.yml

+---
+title: Namespace license checks Issue & MR template
+merge_request: 2321
+author:

spec/features/projects/settings/ee/issues_settings_spec.rb

+require 'spec_helper'
+
+describe 'Project settings > [EE] Merge Requests', feature: true, js: true do
+  include GitlabRoutingHelper
+
+  let(:user) { create(:user) }
+  let(:project) { create(:empty_project, approvals_before_merge: 1) }
+
+  before do
+    gitlab_sign_in(user)
+    project.team << [user, :master]
+  end
+
+  context 'issuable default templates feature not available' do
+    before do
+      stub_licensed_features(issuable_default_templates: false)
+    end
+
+    scenario 'input to configure issue template is not shown' do
+      visit edit_project_path(project)
+
+      expect(page).not_to have_selector('#project_issues_template')
+    end
+  end
+
+  context 'issuable default templates feature is available' do
+    before do
+      stub_licensed_features(issuable_default_templates: true)
+    end
+
+    scenario 'input to configure issue template is not shown' do
+      visit edit_project_path(project)
+
+      expect(page).to have_selector('#project_issues_template')
+    end
+  end
+end

spec/features/projects/settings/ee/merge_requests_settings_spec.rb

@@ -83,4 +83,28 @@ describe 'Project settings > [EE] Merge Requests', feature: true, js: true do
       expect(find('.js-current-approvers')).not_to have_content(group.name)
     end
   end
+
+  context 'issuable default templates feature not available' do
+    before do
+      stub_licensed_features(issuable_default_templates: false)
+    end
+
+    scenario 'input to configure merge request template is not shown' do
+      visit edit_project_path(project)
+
+      expect(page).not_to have_selector('#project_merge_requests_template')
+    end
+  end
+
+  context 'issuable default templates feature is available' do
+    before do
+      stub_licensed_features(issuable_default_templates: true)
+    end
+
+    scenario 'input to configure merge request template is not shown' do
+      visit edit_project_path(project)
+
+      expect(page).to have_selector('#project_merge_requests_template')
+    end
+  end
 end

spec/services/ee/issues/build_service_spec.rb

+require 'spec_helper.rb'
+
+describe Issues::BuildService, services: true do # rubocop:disable RSpec/FilePath
+  let(:project) { create(:project, :repository) }
+  let(:user) { create(:user) }
+
+  before do
+    project.team << [user, :developer]
+  end
+
+  context 'with an issue template' do
+    describe '#execute' do
+      it 'fills in the template in the description' do
+        project = build(:project, issues_template: 'Work hard, play hard!')
+        service = described_class.new(project, user)
+
+        issue = service.execute
+
+        expect(issue.description).to eq('Work hard, play hard!')
+      end
+    end
+  end
+
+  context 'for a single discussion' do
+    describe '#execute' do
+      let(:merge_request) { create(:merge_request, title: "Hello world", source_project: project) }
+      let(:discussion) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, note: "Almost done").to_discussion }
+      let(:service) { described_class.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid, discussion_to_resolve: discussion.id) }
+
+      context 'with an issue template' do
+        let(:project) { create(:project, :repository, issues_template: 'Work hard, play hard!') }
+
+        it 'picks the discussion description over the issue template' do
+          issue = service.execute
+
+          expect(issue.description).to include('Almost done')
+        end
+      end
+    end
+  end
+end

spec/services/ee/merge_requests/build_service_spec.rb

+require 'spec_helper'
+
+describe MergeRequests::BuildService, services: true do # rubocop:disable RSpec/FilePath
+  let(:source_project) { project }
+  let(:target_project) { project }
+  let(:user) { create(:user) }
+  let(:description) { nil }
+  let(:source_branch) { 'feature-branch' }
+  let(:target_branch) { 'master' }
+  let(:merge_request) { service.execute }
+  let(:compare) { double(:compare, commits: commits) }
+  let(:commit_1) { double(:commit_1, safe_message: "Initial commit\n\nCreate the app") }
+  let(:commit_2) { double(:commit_2, safe_message: 'This is a bad commit message!') }
+  let(:commits) { nil }
+
+  let(:service) do
+    MergeRequests::BuildService.new(project, user,
+                                    description: description,
+                                    source_branch: source_branch,
+                                    target_branch: target_branch,
+                                    source_project: source_project,
+                                    target_project: target_project)
+  end
+
+  before do
+    allow(service).to receive(:branches_valid?) { true }
+  end
+
+  context 'project default template configured' do
+    let(:template) { "I am the template, you fill me in" }
+    let(:project) { create(:empty_project, merge_requests_template: template) }
+
+    context 'issuable default templates feature not available' do
+      before do
+        stub_licensed_features(issuable_default_templates: false)
+      end
+
+      it 'does not set the MR description from template' do
+        expect(merge_request.description).not_to eq(template)
+      end
+    end
+
+    context 'issuable default templates feature available' do
+      before do
+        stub_licensed_features(issuable_default_templates: true)
+      end
+
+      it 'sets the MR description from template' do
+        expect(merge_request.description).to eq(template)
+      end
+    end
+  end
+end

spec/services/issues/build_service_spec.rb

@@ -8,19 +8,6 @@ describe Issues::BuildService, services: true do
     project.team << [user, :developer]
   end
 
-  context 'with an issue template' do
-    describe '#execute' do
-      it 'fills in the template in the description' do
-        project = build(:project, issues_template: 'Work hard, play hard!')
-        service = described_class.new(project, user)
-
-        issue = service.execute
-
-        expect(issue.description).to eq('Work hard, play hard!')
-      end
-    end
-  end
-
   context 'for a single discussion' do
     describe '#execute' do
       let(:merge_request) { create(:merge_request, title: "Hello world", source_project: project) }
@@ -38,16 +25,6 @@ describe Issues::BuildService, services: true do
 
         expect(issue.description).to include('Almost done')
       end
-
-      context 'with an issue template' do
-        let(:project) { create(:project, :repository, issues_template: 'Work hard, play hard!') }
-
-        it 'picks the discussion description over the issue template' do
-          issue = service.execute
-
-          expect(issue.description).to include('Almost done')
-        end
-      end
     end
   end