Move merge request approval settings

This is for the upcoming feature for MR approval rules (https://gitlab.com/gitlab-org/gitlab-ee/issues/1979)

- Changelog added
- Updated gitlab.pot

app/views/projects/edit.html.haml

@@ -95,6 +95,8 @@
         = render 'projects/merge_request_settings', form: f
         = f.submit 'Save changes', class: "btn btn-success qa-save-merge-request-changes"
 
+  = render_if_exists 'projects/merge_request_approvals_settings', expanded: expanded
+
   = render_if_exists 'projects/service_desk_settings'
 
   %section.settings.no-animate{ class: ('expanded' if expanded) }

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

-- return unless project.feature_available?(:merge_request_approvers)
-- can_override_approvers = project.can_override_approvers?
-
-.form-group
-  %strong= _('Merge request approvals')
-  = link_to icon('question-circle'), help_page_path("user/project/merge_requests/merge_request_approvals"), target: '_blank'
-
-.nested-settings
-  .form-group
-    = form.label :approver_ids, class: 'label-bold' do
-      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 approvers(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
-        = icon('spinner spin', class: 'approver-list-loader')
-      .card-header
-        Approvers
-        %span.badge.badge-pill
-          - ids = []
-          - project.approvers.each do |user|
-            - ids << user.user_id
-          - project.approver_groups.each do |group|
-            - group.users.each do |user|
-              - unless ids.include?(user.id)
-                - ids << user.id
-          = ids.count
-      %ul.content-list.approver-list
-        - project.approvers.each do |approver|
-          %li.approver.settings-flex-row.js-approver{ data: { id: approver.user_id } }
-            = link_to approver.user.name, approver.user
-            .float-right
-              %button{ href: project_approver_path(project, approver), data: { confirm: "Are you sure you want to remove approver #{approver.user.name}"}, class: "btn btn-remove js-approver-remove", title: 'Remove approver' }
-                = icon("trash")
-        - project.approver_groups.each do |approver_group|
-          %li.approver-group.settings-flex-row.js-approver-group{ data: { id: approver_group.group.id } }
-            %span
-              %span.light
-                Group:
-              = link_to approver_group.group.name, approver_group.group
-              %span.badge.badge-pill
-                = approver_group.group.members.count
-            .float-right
-              %button{ href: project_approver_group_path(project, approver_group), data: { confirm: "Are you sure you want to remove group #{approver_group.group.name}" }, class: "btn btn-remove js-approver-remove", title: 'Remove group' }
-                = icon("trash")
-        - if project.approvers.empty? && project.approver_groups.empty?
-          %li There are no approvers
-
-  .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-text.text-muted
-      Set number of approvers required before open merge requests can be merged
-
-  .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
-        = 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
-
-  .form-group.self-approval
-    .form-check
-      = form.check_box :merge_requests_author_approval, class: 'form-check-input'
-      = form.label :merge_requests_author_approval, class: 'form-check-label' do
-        %strong Enable self approval of merge requests
-        = 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'
-
+- return unless @project.feature_available?(:merge_request_approvers)
+
+%section.settings.merge-requests-feature.no-animate#js-merge-request-approval-settings{ class: [('expanded' if expanded), ('hidden' if @project.project_feature.send(:merge_requests_access_level) == 0)] }
+  .settings-header
+    %h4
+      Merge request approvals
+    %button.btn.js-settings-toggle{ type: 'button' }
+      = expanded ? 'Collapse' : 'Expand'
+    %p
+      Customize your merge request approval settings.
+      = link_to icon('question-circle'), help_page_path("user/project/merge_requests/merge_request_approvals"), target: '_blank'
+  .settings-content
+    = form_for [@project.namespace.becomes(Namespace), @project], remote: true, html: { class: "merge-request-approval-settings-form" }, authenticity_token: true do |f|
+      %input{ name: 'update_section', type: 'hidden', value: 'js-merge-request-approval-settings' }
+      = render 'projects/merge_request_approvals_settings_form', form: f, project: @project
+      = f.submit 'Save changes', class: "btn btn-success"

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

+- can_override_approvers = project.can_override_approvers?
+
+.form-group
+  = form.label :approver_ids, class: 'label-bold' do
+    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 approvers(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
+      = icon('spinner spin', class: 'approver-list-loader')
+    .card-header
+      Approvers
+      %span.badge.badge-pill
+        - ids = []
+        - project.approvers.each do |user|
+          - ids << user.user_id
+        - project.approver_groups.each do |group|
+          - group.users.each do |user|
+            - unless ids.include?(user.id)
+              - ids << user.id
+        = ids.count
+    %ul.content-list.approver-list
+      - project.approvers.each do |approver|
+        %li.approver.settings-flex-row.js-approver{ data: { id: approver.user_id } }
+          = link_to approver.user.name, approver.user
+          .float-right
+            %button{ href: project_approver_path(project, approver), data: { confirm: "Are you sure you want to remove approver #{approver.user.name}"}, class: "btn btn-remove js-approver-remove", title: 'Remove approver' }
+              = icon("trash")
+      - project.approver_groups.each do |approver_group|
+        %li.approver-group.settings-flex-row.js-approver-group{ data: { id: approver_group.group.id } }
+          %span
+            %span.light
+              Group:
+            = link_to approver_group.group.name, approver_group.group
+            %span.badge.badge-pill
+              = approver_group.group.members.count
+          .float-right
+            %button{ href: project_approver_group_path(project, approver_group), data: { confirm: "Are you sure you want to remove group #{approver_group.group.name}" }, class: "btn btn-remove js-approver-remove", title: 'Remove group' }
+              = icon("trash")
+      - if project.approvers.empty? && project.approver_groups.empty?
+        %li There are no approvers
+
+.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-text.text-muted
+    Set number of approvers required before open merge requests can be merged
+
+.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
+      = 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
+
+.form-group.self-approval
+  .form-check
+    = form.check_box :merge_requests_author_approval, class: 'form-check-input'
+    = form.label :merge_requests_author_approval, class: 'form-check-label' do
+      %strong Enable self approval of merge requests
+      = 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

@@ -11,6 +11,4 @@
     .hint
       Description parsed with #{link_to "GitLab Flavored Markdown", help_page_path('user/markdown'), target: '_blank'}.
 
-= render 'projects/merge_request_approvals_settings', project: @project, form: form
-
 = render_ce 'projects/merge_request_merge_settings', form: form

ee/changelogs/unreleased/1979-move-mr-approval-settings.yml

+---
+title: Move merge request approval settings
+merge_request: 8493
+author:
+type: changed

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

@@ -11,7 +11,7 @@ describe 'EE > Projects > Settings > User manages merge requests template' do
 
   it 'saves merge request template' do
     fill_in 'project_merge_requests_template', with: "This merge request should contain the following."
-    page.within '.merge-requests-feature' do
+    page.within '#js-merge-request-settings' do
       click_button 'Save changes'
     end
 

locale/gitlab.pot

@@ -5085,9 +5085,6 @@ msgstr ""
 msgid "Merge request"
 msgstr ""
 
-msgid "Merge request approvals"
-msgstr ""
-
 msgid "Merge requests"
 msgstr ""
 

spec/features/projects/settings/user_manages_merge_requests_settings_spec.rb

@@ -10,19 +10,19 @@ describe 'Projects > Settings > User manages merge request settings' do
   end
 
   it 'shows "Merge commit" strategy' do
-    page.within '.merge-requests-feature' do
+    page.within '#js-merge-request-settings' do
       expect(page).to have_content 'Merge commit'
     end
   end
 
   it 'shows "Merge commit with semi-linear history " strategy' do
-    page.within '.merge-requests-feature' do
+    page.within '#js-merge-request-settings' do
       expect(page).to have_content 'Merge commit with semi-linear history'
     end
   end
 
   it 'shows "Fast-forward merge" strategy' do
-    page.within '.merge-requests-feature' do
+    page.within '#js-merge-request-settings' do
       expect(page).to have_content 'Fast-forward merge'
     end
   end