Merge branch 'ph/284052/removeMrCollapsedApprovalsRulesFlag' into 'master'

Removes `mr_collapsed_approval_rules` feature flag

See merge request gitlab-org/gitlab!66457

app/controllers/projects/merge_requests/creations_controller.rb

@@ -10,10 +10,6 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap
   before_action :apply_diff_view_cookie!, only: [:diffs, :diff_for_path]
   before_action :build_merge_request, except: [:create]
 
-  before_action do
-    push_frontend_feature_flag(:mr_collapsed_approval_rules, @project)
-  end
-
   def new
     define_new_vars
   end

app/controllers/projects/merge_requests_controller.rb

@@ -55,7 +55,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
   end
 
   before_action do
-    push_frontend_feature_flag(:mr_collapsed_approval_rules, @project)
     push_frontend_feature_flag(:show_relevant_approval_rule_approvers, @project, default_enabled: :yaml)
   end
 

app/views/shared/issuable/_form.html.haml

@@ -31,8 +31,6 @@
 
 = render 'shared/issuable/form/metadata', issuable: issuable, form: form, project: project, presenter: presenter
 
-= render_if_exists 'shared/issuable/approvals', issuable: issuable, presenter: presenter, form: form
-
 = render 'shared/issuable/form/merge_params', issuable: issuable, project: project
 
 = render 'shared/issuable/form/contribution', issuable: issuable, form: form

app/views/shared/issuable/form/_metadata_issuable_reviewer.html.haml

@@ -8,5 +8,4 @@
       = hidden_field_tag "#{issuable.to_ability_name}[reviewer_ids][]", 0, id: nil, data: { meta: '' }
 
     = dropdown_tag(users_dropdown_label(issuable.reviewers), options: reviewers_dropdown_options(issuable.to_ability_name, issuable.iid, issuable.target_branch))
-  - if Feature.enabled?(:mr_collapsed_approval_rules, @project)
-    = render_if_exists 'shared/issuable/approver_suggestion', issuable: issuable, presenter: presenter
+  = render_if_exists 'shared/issuable/approver_suggestion', issuable: issuable, presenter: presenter

config/feature_flags/development/mr_collapsed_approval_rules.yml

----
-name: mr_collapsed_approval_rules
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/47475
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/284052
-milestone: '13.6'
-type: development
-group: group::source code
-default_enabled: false

ee/app/assets/javascripts/approvals/components/mr_edit/app.vue

@@ -32,9 +32,6 @@ export default {
     accordionTitle() {
       return s__('ApprovalRule|Approval rules');
     },
-    isCollapseFeatureEnabled() {
-      return this.glFeatures.mrCollapsedApprovalRules;
-    },
     hasOptionalRules() {
       return this.rules.every((r) => r.approvalsRequired === 0);
     },
@@ -105,7 +102,7 @@ export default {
 </script>
 
 <template>
-  <div v-if="isCollapseFeatureEnabled" class="gl-mt-2">
+  <div class="gl-mt-2">
     <p
       v-safe-html="collapsedSummary"
       class="gl-mb-0 gl-text-gray-500"
@@ -146,12 +143,4 @@ export default {
       </gl-accordion-item>
     </gl-accordion>
   </div>
-  <app v-else>
-    <template #rules>
-      <mr-rules />
-    </template>
-    <template #footer>
-      <mr-rules-hidden-inputs />
-    </template>
-  </app>
 </template>

ee/app/views/shared/issuable/_approvals.html.haml

-- form = local_assigns.fetch(:form)
-
-- return unless issuable.is_a?(MergeRequest)
-- return unless issuable.approval_feature_available?
-
-- if !Feature.enabled?(:mr_collapsed_approval_rules, @project)
-  .form-group.row
-    .col-sm-2.col-form-label.gl-static
-      .gl-display-flex.gl-align-items-center.gl-sm-justify-content-end
-        - root_group = @project.group&.root_ancestor
-        - run_highlight_paid_features_during_active_trial_experiment(root_group) do
-          - feature_name = _('merge request approvals')
-          .gl-xs-ml-3.gl-sm-mr-3.gl-mb-2.gl-order-1.gl-sm-order-init
-            #js-paid-feature-badge{ data: paid_feature_badge_data_attrs(feature_name) }
-          #js-paid-feature-popover{ data: paid_feature_popover_data_attrs(group: root_group, feature_name: feature_name).merge(promo_image_path: image_path('illustrations/golden_tanuki.svg'), promo_image_alt_text: s_('ImageAltText|Sparkling golden tanuki logo')) }
-        = form.label :approver_ids, "Approval rules"
-    .col-sm-10
-      = render_if_exists 'shared/issuable/approver_suggestion', issuable: issuable, presenter: presenter

ee/app/views/shared/issuable/_approver_suggestion.haml

@@ -16,9 +16,4 @@
   'eligible_approvers_docs_path': help_page_path('user/project/merge_requests/approvals/rules', anchor: 'eligible-approvers'),
   'project_settings_path': presenter.api_project_approval_settings_path } }
   = sprite_icon('spinner', size: 24, css_class: 'gl-spinner')
-- if can_update_approvers && show_code_owner_tip && Feature.disabled?(:mr_collapsed_approval_rules, @project, default_enabled: :yaml)
-  .form-text.text-muted
-    = _('Tip: add a')
-    = link_to 'CODEOWNERS', help_page_path('user/project/code_owners'), target: '_blank', tabindex: -1
-    = _('to automatically add approvers based on file paths and file types.')
 = render 'projects/merge_requests/code_owner_approval_rules', merge_request: @mr_presenter

ee/spec/features/merge_request/user_sets_approvers_spec.rb

@@ -11,26 +11,6 @@ RSpec.describe 'Merge request > User sets approvers', :js do
   let!(:config_selector) { '.js-approval-rules' }
   let!(:modal_selector) { '#mr-edit-approvals-create-modal' }
 
-  context 'with feature flag off' do
-    let!(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
-
-    def visit_mr(mr_collapsed_approval_rules: false)
-      stub_feature_flags(mr_collapsed_approval_rules: mr_collapsed_approval_rules)
-      project.add_developer(user)
-      sign_in(user)
-      visit edit_project_merge_request_path(project, merge_request)
-    end
-
-    def non_collapse_approval_rules
-      expect(page).to have_button('Add approval rule')
-    end
-
-    it 'does not hide approval rules inside collapse when mr_collapsed_approval_rules is off' do
-      visit_mr(mr_collapsed_approval_rules: false)
-      non_collapse_approval_rules
-    end
-  end
-
   context 'when editing an MR with a different author' do
     let(:author) { create(:user) }
     let(:merge_request) { create(:merge_request, author: author, source_project: project) }

ee/spec/views/shared/issuable/_approvals.html.haml_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe 'shared/issuable/_approvals.html.haml' do
-  let(:user) { create(:user) }
-  let(:project) { create(:project, :repository) }
-  let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
-  let(:presenter) { merge_request.present(current_user: user) }
-  let(:approver_presenter) { double(any?: false, show_code_owner_tips?: true) }
-  let(:form) { double('form') }
-
-  before do
-    allow(view).to receive(:can?).and_return(true)
-    allow(view).to receive(:current_user).and_return(user)
-    allow(form).to receive(:label)
-    allow(form).to receive(:number_field)
-    allow(MergeRequestApproverPresenter).to receive(:new).and_return(approver_presenter)
-    assign(:project, project)
-    assign(:target_project, project)
-    assign(:mr_presenter, merge_request.present(current_user: user))
-  end
-
-  context 'has no approvers' do
-    context 'can not override approvers' do
-      before do
-        allow(view).to receive(:can?).with(user, :update_approvers, merge_request).and_return(false)
-        render 'shared/issuable/approvals', form: form, issuable: merge_request, presenter: presenter
-      end
-
-      it 'hides select approvers field' do
-        expect(rendered).not_to have_css('#merge_request_approver_ids')
-      end
-
-      it 'hides select approver groups field' do
-        expect(rendered).not_to have_css('#merge_request_approver_group_ids')
-      end
-    end
-  end
-
-  context 'has approvers' do
-    let(:user) { create(:user) }
-    let(:approver) { create(:approver, user: user, target: merge_request) }
-    let(:approver_group) { create(:approver_group, target: merge_request) }
-
-    before do
-      assign(:approver, approver)
-      assign(:approver_group, approver_group)
-      assign(:presenter, merge_request.present(current_user: user))
-    end
-
-    it 'shows approver in table' do
-      render 'shared/issuable/approvals', form: form, issuable: merge_request, project: project, presenter: presenter
-
-      expect(rendered).to have_text(approver[:name])
-      expect(rendered).to have_text(approver_group[:name])
-    end
-
-    context 'can not override approvers' do
-      it 'hides remove button' do
-        allow(view).to receive(:can?).with(user, :update_approvers, merge_request).and_return(false)
-
-        render 'shared/issuable/approvals', form: form, issuable: merge_request, presenter: presenter
-
-        expect(rendered).not_to have_css('.btn-danger')
-      end
-    end
-  end
-
-  context 'when running the highlight paid features experiment', :experiment do
-    let(:group) { create(:group) }
-    let(:project) { create(:project, :repository, namespace: group) }
-
-    before do
-      create(:gitlab_subscription, :active_trial, namespace: group)
-      group.add_maintainer(user)
-      stub_application_setting(check_namespace_plan: true)
-      stub_feature_flags(mr_collapsed_approval_rules: false)
-      stub_experiments(highlight_paid_features_during_active_trial: variant)
-      render 'shared/issuable/approvals', form: form, issuable: merge_request, project: project, presenter: presenter
-    end
-
-    context 'when user is in the control' do
-      let(:variant) { :control }
-
-      it 'does not render the paid feature badge' do
-        expect(rendered).not_to have_css('#js-paid-feature-badge')
-      end
-
-      it 'does not render the paid feature popover' do
-        expect(rendered).not_to have_css('#js-paid-feature-popover')
-      end
-    end
-
-    context 'when user is in the candidate' do
-      let(:variant) { :candidate }
-
-      it 'renders the paid feature badge' do
-        expect(rendered).to have_css('#js-paid-feature-badge')
-      end
-
-      it 'renders the paid feature popover' do
-        expect(rendered).to have_css('#js-paid-feature-popover')
-      end
-    end
-  end
-end

locale/gitlab.pot

@@ -16496,9 +16496,6 @@ msgstr ""
 msgid "Image details"
 msgstr ""
 
-msgid "ImageAltText|Sparkling golden tanuki logo"
-msgstr ""
-
 msgid "ImageDiffViewer|2-up"
 msgstr ""
 
@@ -34126,9 +34123,6 @@ msgstr ""
 msgid "Tip: Hover over a job to see the jobs it depends on to run."
 msgstr ""
 
-msgid "Tip: add a"
-msgstr ""
-
 msgid "Tip: add a %{linkStart}CODEOWNERS%{linkEnd} to automatically add approvers based on file paths and file types."
 msgstr ""
 
@@ -38955,9 +38949,6 @@ msgid_plural "merge requests"
 msgstr[0] ""
 msgstr[1] ""
 
-msgid "merge request approvals"
-msgstr ""
-
 msgid "merged %{timeAgo}"
 msgstr ""
 
@@ -39655,9 +39646,6 @@ msgstr ""
 msgid "time summary"
 msgstr ""
 
-msgid "to automatically add approvers based on file paths and file types."
-msgstr ""
-
 msgid "to help your contributors communicate effectively!"
 msgstr ""