Merge branch 'proj-settings-ok-mr-settings-only' into 'master'

Improve project merge request settings

See merge request gitlab-org/gitlab-ee!10366

app/assets/stylesheets/pages/settings.scss

@@ -232,7 +232,7 @@
   }
 }
 
-.settings-flex-row {
+.content-list > .settings-flex-row {
   display: flex;
   align-items: center;
 

app/views/projects/_merge_request_merge_method_settings.html.haml

@@ -2,34 +2,29 @@
 
 .form-group
   = label_tag :merge_method_merge, class: 'label-bold' do
-    Merge method
+    = _('Merge method')
   .form-check
     = form.radio_button :merge_method, :merge, class: "js-merge-method-radio form-check-input"
     = label_tag :project_merge_method_merge, class: 'form-check-label' do
-      %strong Merge commit
-      %br
-      %span.descr
-        A merge commit is created for every merge, and merging is allowed as long as there are no conflicts.
+      .mb-3
+        = _('Merge commit')
+        .text-secondary
+          = _('A merge commit is created for every merge, and merging is allowed as long as there are no conflicts.')
 
-  .form-check
-    = form.radio_button :merge_method, :rebase_merge, class: "js-merge-method-radio form-check-input"
-    = label_tag :project_merge_method_rebase_merge, class: 'form-check-label' do
-      %strong Merge commit with semi-linear history
-      %br
-      %span.descr
-        A merge commit is created for every merge, but merging is only allowed if fast-forward merge is possible.
-        This way you could make sure that if this merge request would build, after merging to target branch it would also build.
-      %br
-      %span.descr
-        When fast-forward merge is not possible, the user is given the option to rebase.
+.form-check
+  = form.radio_button :merge_method, :rebase_merge, class: "js-merge-method-radio form-check-input"
+  = label_tag :project_merge_method_rebase_merge, class: 'form-check-label' do
+    .mb-3
+      = _('Merge commit with semi-linear history')
+      .text-secondary
+        = _('A merge commit is created for every merge, but merging is only allowed if fast-forward merge is possible. This way you could make sure that if this merge request would build, after merging to target branch it would also build.')
+      .text-secondary
+        = _('When fast-forward merge is not possible, the user is given the option to rebase.')
 
-  .form-check
-    = form.radio_button :merge_method, :ff, class: "js-merge-method-radio qa-radio-button-merge-ff form-check-input"
-    = label_tag :project_merge_method_ff, class: 'form-check-label' do
-      %strong Fast-forward merge
-      %br
-      %span.descr
-        No merge commits are created and all merges are fast-forwarded, which means that merging is only allowed if the branch could be fast-forwarded.
-      %br
-      %span.descr
-        When fast-forward merge is not possible, the user is given the option to rebase.
+.form-check
+  = form.radio_button :merge_method, :ff, class: "js-merge-method-radio qa-radio-button-merge-ff form-check-input"
+  = label_tag :project_merge_method_ff, class: 'form-check-label' do
+    .mb-3
+      = _('Fast-forward merge')
+      .text-secondary
+        = _('No merge commits are created and all merges are fast-forwarded, which means that merging is only allowed if the branch could be fast-forwarded. When fast-forward merge is not possible, the user is given the option to rebase.')

app/views/projects/_merge_request_merge_settings.html.haml

@@ -4,21 +4,21 @@
   .form-check.builds-feature{ class: ("hidden" if @project && @project.project_feature.send(:builds_access_level) == 0) }
     = form.check_box :only_allow_merge_if_pipeline_succeeds, class: 'form-check-input'
     = form.label :only_allow_merge_if_pipeline_succeeds, class: 'form-check-label' do
-      %strong Only allow merge requests to be merged if the pipeline succeeds
-      %br
-      %span.descr
-        Pipelines need to be configured to enable this feature.
-        = link_to icon('question-circle'), help_page_path('user/project/merge_requests/merge_when_pipeline_succeeds', anchor: 'only-allow-merge-requests-to-be-merged-if-the-pipeline-succeeds'), target: '_blank'
+      .mb-3
+        = _('Only allow merge requests to be merged if the pipeline succeeds')
+        .text-secondary
+          = _('Pipelines need to be configured to enable this feature.')
+          = link_to icon('question-circle'), help_page_path('user/project/merge_requests/merge_when_pipeline_succeeds', anchor: 'only-allow-merge-requests-to-be-merged-if-the-pipeline-succeeds'), target: '_blank'
   = render_if_exists 'projects/merge_pipelines_settings', form: form
   .form-check
     = form.check_box :only_allow_merge_if_all_discussions_are_resolved, class: 'form-check-input'
     = form.label :only_allow_merge_if_all_discussions_are_resolved, class: 'form-check-label' do
-      %strong Only allow merge requests to be merged if all discussions are resolved
+      %p= _('Only allow merge requests to be merged if all discussions are resolved')
   .form-check
     = form.check_box :resolve_outdated_diff_discussions, class: 'form-check-input'
     = form.label :resolve_outdated_diff_discussions, class: 'form-check-label' do
-      %strong Automatically resolve merge request diff discussions when they become outdated
+      %p= _('Automatically resolve merge request diff discussions when they become outdated')
   .form-check
     = form.check_box :printing_merge_request_link_enabled, class: 'form-check-input'
     = form.label :printing_merge_request_link_enabled, class: 'form-check-label' do
-      %strong Show link to create/view merge request when pushing from the command line
+      %p= _('Show link to create/view merge request when pushing from the command line')

changelogs/unreleased/ce-proj-settings-ok-mr-settings-only.yml

+---
+title: Improve project merge request settings
+merge_request: 26495
+author:
+type: other

ee/app/assets/javascripts/approvers_select.js

@@ -186,7 +186,7 @@ export default class ApproversSelect {
 
   static saveApproversComplete($input, $approverSelect, $loadWrapper) {
     $input.val('');
-    $approverSelect.select2('val', '');
+    $approverSelect.select2('val', '').trigger('change');
     $loadWrapper.addClass('hidden');
   }
 

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

@@ -3,9 +3,8 @@
 .form-check.builds-feature
   = form.check_box :merge_pipelines_enabled, class: 'form-check-input'
   = form.label :merge_pipelines_enabled, class: 'form-check-label' do
-    %strong
+    %p.mb-0
       = _('Merge pipelines will try to validate the post-merge result prior to merging')
-    %br
-    %span.descr
+    .text-secondary.mb-3
       = _('Pipelines need to be configured to enable this feature.')
       = link_to icon('question-circle'), help_page_path('user/project/merge_requests/merge_when_pipeline_succeeds', anchor: 'only-allow-merge-requests-to-be-merged-if-the-pipeline-succeeds'), target: '_blank'

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

@@ -10,27 +10,27 @@
     .form-check
       = form.check_box(:merge_requests_require_code_owner_approval, class: 'form-check-input')
       = form.label :merge_requests_require_code_owner_approval, class: 'form-check-label' do
-        %strong= _('Require approval from code owners')
+        %span= _('Require approval from code owners')
         = link_to icon('question-circle'), help_page_path('user/project/merge_requests/merge_request_approvals', anchor: 'editing-approvals-premium'), target: '_blank'
 
 .form-group
   .form-check
     = form.check_box(:disable_overriding_approvers_per_merge_request, { checked: can_override_approvers, class: 'form-check-input' }, false, true)
     = form.label :disable_overriding_approvers_per_merge_request, class: 'form-check-label' do
-      %strong= _('Can override approvers and approvals required per merge request')
+      %span= _('Can override approvers and approvals required per merge request')
       = link_to icon('question-circle'), help_page_path('user/project/merge_requests/merge_request_approvals', anchor: 'overriding-the-merge-request-approvals-default-settings'), target: '_blank'
 
 .form-group.reset-approvals-on-push
   .form-check
     = form.check_box :reset_approvals_on_push, class: 'form-check-input'
     = form.label :reset_approvals_on_push, class: 'form-check-label' do
-      %strong= _('Remove all approvals in a merge request when new commits are pushed to its source branch')
+      %span= _('Remove all approvals in a merge request when new commits are pushed to its source branch')
 
 .form-group.self-approval
   .form-check
     = form.check_box :merge_requests_author_approval, { class: 'form-check-input', checked: !project.merge_requests_author_approval? }, false, true
     = form.label :merge_requests_author_approval, class: 'form-check-label' do
-      %strong= _('Prevent approval of merge requests by merge request author')
+      %span= _('Prevent approval of merge requests by merge request author')
       = link_to icon('question-circle'), help_page_path('user/project/merge_requests/merge_request_approvals',
         anchor: 'allowing-merge-request-authors-to-approve-their-own-merge-requests'), target: '_blank'
 
@@ -38,6 +38,6 @@
   .form-check
     = form.check_box :merge_requests_disable_committers_approval, class: 'form-check-input'
     = form.label :merge_requests_disable_committers_approval, class: 'form-check-label' do
-      %strong= _('Prevent approval of merge requests by merge request committers')
+      %span= _('Prevent approval of merge requests by merge request committers')
       = link_to icon('question-circle'), help_page_path('user/project/merge_requests/merge_request_approvals',
         anchor: 'allowing-merge-request-authors-to-approve-their-own-merge-requests'), target: '_blank'

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

@@ -8,7 +8,9 @@
       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'}.
+    .text-secondary
+      - link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: help_page_path('user/markdown') }
+      = _('Description parsed with %{link_start}GitLab Flavored Markdown%{link_end}').html_safe % { link_start: link_start, link_end: '</a>'.html_safe }
+
 
 = render_ce 'projects/merge_request_merge_settings', form: form

ee/app/views/shared/merge_request_approvals_settings/_multiple_rules_form.html.haml

 .form-group
   = form.label :approver_ids, class: 'label-bold' do
-    = _("Approvers")
+    = _("Add approvers")
   #js-mr-approvals-settings{ data: { 'project_id': @project.id,
     'project_path': api_v4_projects_path(id: @project.id),
     'settings_path': api_v4_projects_approval_settings_path(id: @project.id),

ee/app/views/shared/merge_request_approvals_settings/_single_rule_form.html.haml

 .form-group
   = form.label :approver_ids, class: 'label-bold' do
-    = _("Approvers")
+    = _("Add approvers")
   = hidden_field_tag "project[approver_ids]"
   = hidden_field_tag "project[approver_group_ids]"
-  .input-group.input-btn-group
-    = hidden_field_tag :approver_user_and_group_ids, '', { class: 'js-select-user-and-group input-large', tabindex: 1, 'data-name': 'project', :style => "max-width: unset;" }
-    %button.btn.btn-success.js-add-approvers{ type: 'button', title: _('Add approver(s)') }
-      = _("Add")
-  .form-text.text-muted
-    = _("Add users or groups who are allowed to approve every merge request")
+  .row
+    .col-md-9
+      .input-group.input-btn-group
+        = hidden_field_tag :approver_user_and_group_ids, '', { class: 'js-select-user-and-group input-large', tabindex: 1, 'data-name': 'project', :style => "max-width: unset;" }
+        %button.btn.btn-success.js-add-approvers.js-dirty-submit{ type: 'button', title: _('Add approver(s)') }
+          = _("Add")
+      .form-text.text-muted
+        = _("Add users or groups who are allowed to approve every merge request")
 
   .card.prepend-top-10.js-current-approvers
     .load-wrapper.hidden
@@ -50,6 +52,6 @@
 .form-group
   = form.label :approvals_before_merge, class: 'label-bold' do
     = _("Approvals required")
-  = form.number_field :approvals_before_merge, class: "form-control", min: 0
+  = form.number_field :approvals_before_merge, class: "form-control w-auto", min: 0
   .form-text.text-muted
     = _("Set number of approvers required before open merge requests can be merged")

ee/app/views/shared/promotions/_promote_mr_features.html.haml

 - if show_promotions? && show_callout?('promote_mr_features_dismissed') && !@project.feature_available?(:merge_request_approvers)
   .user-callout.promotion-callout.append-bottom-20.js-mr-approval-callout#promote_mr_features{ data: { uid: 'promote_mr_features_dismissed' } }
     .bordered-box.content-block
-      %button.btn.btn-default.close.js-close-callout{ type: 'button', 'aria-label' => 'Dismiss Merge Request promotion' }
+      %button.btn.btn-default.close.js-close-callout{ type: 'button', 'aria-label' => _('Dismiss Merge Request promotion') }
         = icon('times', class: 'dismiss-icon', 'aria-hidden' => 'true')
       .user-callout-copy
         %h4
           - if Gitlab::CurrentSettings.should_check_namespace_plan?
-            Upgrade your plan to improve Merge Requests.
+            = _('Upgrade your plan to improve Merge Requests.')
           - else
-            Improve Merge Requests and customer support with GitLab Enterprise Edition.
+            = _('Improve Merge Requests and customer support with GitLab Enterprise Edition.')
         %ul
           - unless @project.feature_available?(:merge_request_approvers)
             %li
-              = link_to 'Merge Request Approvals', help_page_path('user/project/merge_requests/merge_request_approvals.html'), target: '_blank'
+              = link_to _('Merge Request Approvals'), help_page_path('user/project/merge_requests/merge_request_approvals.html'), target: '_blank'
               %p
-                Merge request approvals allow you to set the number of necessary approvals and predefine a list of approvers that will need to approve every merge request in a project.
+                = _('Merge request approvals allow you to set the number of necessary approvals and predefine a list of approvers that will need to approve every merge request in a project.')
 
         = render 'shared/promotions/promotion_link_project'

ee/app/views/shared/promotions/_promotion_link_project.html.haml

@@ -2,17 +2,17 @@
 - if Gitlab::CurrentSettings.should_check_namespace_plan?
   - namespace = @project&.namespace || @group
   - if can?(current_user, :admin_namespace, namespace)
-    = link_to 'Upgrade your plan', upgrade_plan_url, class: 'btn btn-primary'
+    = link_to _('Upgrade your plan'), upgrade_plan_url, class: 'btn btn-primary'
   - elsif namespace.is_a?(Group)
-    %p Contact an owner of group #{ namespace.name } to upgrade the plan.
+    %p= _('Contact an owner of group %{namespace_name} to upgrade the plan.') % { namespace_name: namespace.name }
   - else
     - owner = namespace.owner
-    %p Contact owner #{ link_to(owner.name, user_path(owner)) } to upgrade the plan.
+    - link_start = "<a href='#{user_path(owner)}' rel='noowner noreferrer' target='_blank'>".html_safe
+    %p= _('Contact owner %{link_start}%{owner_name}%{link_end} to upgrade the plan.').html_safe % { owner_name: owner.name, link_start: link_start, link_end: '</a>'.html_safe }
 - elsif current_user&.admin?
   - if License.current&.expired?
-    = link_to (!short_form ? 'Buy GitLab Enterprise Edition' : 'Buy EE'), ::EE::SUBSCRIPTIONS_PLANS_URL, class: 'btn btn-primary'
+    = link_to (!short_form ? _('Buy GitLab Enterprise Edition') : _('Buy EE')), ::EE::SUBSCRIPTIONS_PLANS_URL, class: 'btn btn-primary'
   - else
-    = link_to (!short_form ? 'Start GitLab Ultimate trial' : 'Start GitLab Ultimate trial'), new_trial_url, class: 'btn btn-primary'
+    = link_to _('Start GitLab Ultimate trial'), new_trial_url, class: 'btn btn-primary'
 - else
-  %p
-    Contact your Administrator to upgrade your license.
+  %p= _('Contact your Administrator to upgrade your license.')

ee/spec/javascripts/approvers_select_spec.js

@@ -12,7 +12,10 @@ describe('ApproversSelect', () => {
       };
 
       $approverSelect = {
-        select2: jasmine.createSpy(),
+        select2: jasmine.createSpy().and.callFake(function() {
+          return this;
+        }),
+        trigger: jasmine.createSpy(),
       };
 
       $loadWrapper = {
@@ -26,8 +29,9 @@ describe('ApproversSelect', () => {
       expect($input.val).toHaveBeenCalledWith('');
     });
 
-    it('should empty the select2 value', () => {
+    it('should empty the select2 value and trigger a change event', () => {
       expect($approverSelect.select2).toHaveBeenCalledWith('val', '');
+      expect($approverSelect.trigger).toHaveBeenCalledWith('change');
     });
 
     it('should hide loadWrapper', () => {

locale/gitlab.pot

@@ -446,6 +446,12 @@ msgstr ""
 msgid "A member of GitLab's abuse team will review your report as soon as possible."
 msgstr ""
 
+msgid "A merge commit is created for every merge, and merging is allowed as long as there are no conflicts."
+msgstr ""
+
+msgid "A merge commit is created for every merge, but merging is only allowed if fast-forward merge is possible. This way you could make sure that if this merge request would build, after merging to target branch it would also build."
+msgstr ""
+
 msgid "A new branch will be created in your fork and a new merge request will be started."
 msgstr ""
 
@@ -1434,6 +1440,9 @@ msgstr ""
 msgid "Automatically marked as default internal user"
 msgstr ""
 
+msgid "Automatically resolve merge request diff discussions when they become outdated"
+msgstr ""
+
 msgid "Automatically resolved"
 msgstr ""
 
@@ -1821,6 +1830,12 @@ msgstr ""
 msgid "Business metrics (Custom)"
 msgstr ""
 
+msgid "Buy EE"
+msgstr ""
+
+msgid "Buy GitLab Enterprise Edition"
+msgstr ""
+
 msgid "By %{user_name}"
 msgstr ""
 
@@ -2945,9 +2960,18 @@ msgstr ""
 msgid "Connecting..."
 msgstr ""
 
+msgid "Contact an owner of group %{namespace_name} to upgrade the plan."
+msgstr ""
+
+msgid "Contact owner %{link_start}%{owner_name}%{link_end} to upgrade the plan."
+msgstr ""
+
 msgid "Contact sales to upgrade"
 msgstr ""
 
+msgid "Contact your Administrator to upgrade your license."
+msgstr ""
+
 msgid "Container Registry"
 msgstr ""
 
@@ -3577,6 +3601,9 @@ msgstr ""
 msgid "Description"
 msgstr ""
 
+msgid "Description parsed with %{link_start}GitLab Flavored Markdown%{link_end}"
+msgstr ""
+
 msgid "Description templates allow you to define context-specific templates for issue and merge request description fields for your project."
 msgstr ""
 
@@ -4450,6 +4477,9 @@ msgstr ""
 msgid "Failure"
 msgstr ""
 
+msgid "Fast-forward merge"
+msgstr ""
+
 msgid "Fast-forward merge without a merge commit"
 msgstr ""
 
@@ -5882,6 +5912,9 @@ msgstr ""
 msgid "Improve Issue boards with GitLab Enterprise Edition."
 msgstr ""
 
+msgid "Improve Merge Requests and customer support with GitLab Enterprise Edition."
+msgstr ""
+
 msgid "Improve issues management with Issue weight and GitLab Enterprise Edition."
 msgstr ""
 
@@ -6740,15 +6773,24 @@ msgstr ""
 msgid "Merge Request"
 msgstr ""
 
+msgid "Merge Request Approvals"
+msgstr ""
+
 msgid "Merge Requests"
 msgstr ""
 
 msgid "Merge Requests created"
 msgstr ""
 
+msgid "Merge commit"
+msgstr ""
+
 msgid "Merge commit message"
 msgstr ""
 
+msgid "Merge commit with semi-linear history"
+msgstr ""
+
 msgid "Merge events"
 msgstr ""
 
@@ -6758,6 +6800,9 @@ msgstr ""
 msgid "Merge in progress"
 msgstr ""
 
+msgid "Merge method"
+msgstr ""
+
 msgid "Merge pipelines will try to validate the post-merge result prior to merging"
 msgstr ""
 
@@ -6767,6 +6812,9 @@ msgstr ""
 msgid "Merge request approvals"
 msgstr ""
 
+msgid "Merge request approvals allow you to set the number of necessary approvals and predefine a list of approvers that will need to approve every merge request in a project."
+msgstr ""
+
 msgid "Merge requests"
 msgstr ""
 
@@ -7321,6 +7369,9 @@ msgstr ""
 msgid "No matching results"
 msgstr ""
 
+msgid "No merge commits are created and all merges are fast-forwarded, which means that merging is only allowed if the branch could be fast-forwarded. When fast-forward merge is not possible, the user is given the option to rebase."
+msgstr ""
+
 msgid "No merge requests for the selected time period."
 msgstr ""
 
@@ -7542,6 +7593,12 @@ msgstr ""
 msgid "Only admins"
 msgstr ""
 
+msgid "Only allow merge requests to be merged if all discussions are resolved"
+msgstr ""
+
+msgid "Only allow merge requests to be merged if the pipeline succeeds"
+msgstr ""
+
 msgid "Only mirror protected branches"
 msgstr ""
 
@@ -9839,6 +9896,9 @@ msgstr ""
 msgid "Show latest version"
 msgstr ""
 
+msgid "Show link to create/view merge request when pushing from the command line"
+msgstr ""
+
 msgid "Show parent pages"
 msgstr ""
 
@@ -10240,6 +10300,9 @@ msgstr ""
 msgid "Stars"
 msgstr ""
 
+msgid "Start GitLab Ultimate trial"
+msgstr ""
+
 msgid "Start Web Terminal"
 msgstr ""
 
@@ -11707,6 +11770,9 @@ msgstr ""
 msgid "Upgrade plan to unlock Canary Deployments feature"
 msgstr ""
 
+msgid "Upgrade your plan"
+msgstr ""
+
 msgid "Upgrade your plan to activate Advanced Global Search."
 msgstr ""
 
@@ -11722,6 +11788,9 @@ msgstr ""
 msgid "Upgrade your plan to improve Issue boards."
 msgstr ""
 
+msgid "Upgrade your plan to improve Merge Requests."
+msgstr ""
+
 msgid "Upload <code>GoogleCodeProjectHosting.json</code> here:"
 msgstr ""
 
@@ -12160,6 +12229,9 @@ msgstr ""
 msgid "When enabled, users cannot use GitLab until the terms have been accepted."
 msgstr ""
 
+msgid "When fast-forward merge is not possible, the user is given the option to rebase."
+msgstr ""
+
 msgid "When leaving the URL blank, classification labels can still be specified without disabling cross project features or performing external authorization checks."
 msgstr ""
 

spec/features/projects/settings/user_manages_merge_requests_settings_spec.rb

@@ -93,11 +93,13 @@ describe 'Projects > Settings > User manages merge request settings' do
     it 'when unchecked sets :printing_merge_request_link_enabled to false' do
       uncheck('project_printing_merge_request_link_enabled')
       within('.merge-request-settings-form') do
+        find('.qa-save-merge-request-changes')
         click_on('Save changes')
       end
 
-      # Wait for save to complete and page to reload
+      find('.flash-notice')
       checkbox = find_field('project_printing_merge_request_link_enabled')
+
       expect(checkbox).not_to be_checked
 
       project.reload