Merge branch 'remove_scoped_approval_rules_feature_flag' into 'master'

Remove scoped_approval_rules feature flag

Closes #198400

See merge request gitlab-org/gitlab!28864

changelogs/unreleased/remove_scoped_approval_rules_feature_flag.yml

+---
+title: Remove `scoped_approval_rules` feature flag
+merge_request: 28864
+author: Lee Tickett
+type: added

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

 <script>
 import { mapActions } from 'vuex';
 import { GlDeprecatedButton } from '@gitlab/ui';
-import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import RuleInput from './mr_edit/rule_input.vue';
 import EmptyRuleName from './empty_rule_name.vue';
 import RuleBranches from './rule_branches.vue';
@@ -13,7 +12,6 @@ export default {
     RuleBranches,
     GlDeprecatedButton,
   },
-  mixins: [glFeatureFlagsMixin()],
   props: {
     rule: {
       type: Object,
@@ -40,7 +38,7 @@ export default {
   },
   computed: {
     showProtectedBranch() {
-      return this.glFeatures.scopedApprovalRules && !this.isMrEdit && this.allowMultiRule;
+      return !this.isMrEdit && this.allowMultiRule;
     },
   },
   methods: {

ee/app/assets/javascripts/approvals/components/project_settings/project_rules.vue

@@ -93,21 +93,18 @@ export default {
 
 <template>
   <rules :rules="rules">
-    <template
-      slot="thead"
-      slot-scope="{ name, members, approvalsRequired, branches, glFeaturesScopedApprovalRules }"
-    >
+    <template slot="thead" slot-scope="{ name, members, approvalsRequired, branches }">
       <tr class="d-none d-sm-table-row">
         <th class="w-25">{{ hasNamedRule ? name : members }}</th>
         <th :class="settings.allowMultiRule ? 'w-50 d-none d-sm-table-cell' : 'w-75'">
           <span v-if="hasNamedRule">{{ members }}</span>
         </th>
-        <th v-if="glFeaturesScopedApprovalRules && settings.allowMultiRule">{{ branches }}</th>
+        <th v-if="settings.allowMultiRule">{{ branches }}</th>
         <th>{{ approvalsRequired }}</th>
         <th></th>
       </tr>
     </template>
-    <template slot="tbody" slot-scope="{ rules, glFeaturesScopedApprovalRules }">
+    <template slot="tbody" slot-scope="{ rules }">
       <template v-for="(rule, index) in rules">
         <empty-rule
           v-if="rule.ruleType === 'any_approver'"
@@ -125,7 +122,7 @@ export default {
           <td class="js-members" :class="settings.allowMultiRule ? 'd-none d-sm-table-cell' : null">
             <user-avatar-list :items="rule.approvers" :img-size="24" empty-text="" />
           </td>
-          <td v-if="glFeaturesScopedApprovalRules && settings.allowMultiRule" class="js-branches">
+          <td v-if="settings.allowMultiRule" class="js-branches">
             <rule-branches :rule="rule" />
           </td>
           <td class="js-approvals-required">

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

@@ -2,7 +2,6 @@
 import { mapState, mapActions } from 'vuex';
 import { groupBy, isNumber } from 'lodash';
 import { sprintf, __ } from '~/locale';
-import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import ApproversList from './approvers_list.vue';
 import ApproversSelect from './approvers_select.vue';
 import BranchesSelect from './branches_select.vue';
@@ -18,7 +17,6 @@ export default {
     ApproversSelect,
     BranchesSelect,
   },
-  mixins: [glFeatureFlagsMixin()],
   props: {
     initRule: {
       type: Object,
@@ -150,7 +148,7 @@ export default {
       };
     },
     showProtectedBranch() {
-      return this.glFeatures.scopedApprovalRules && !this.isMrEdit && this.settings.allowMultiRule;
+      return !this.isMrEdit && this.settings.allowMultiRule;
     },
   },
   watch: {

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

 <script>
 import { s__ } from '~/locale';
-import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 
 const HEADERS = {
   name: s__('ApprovalRule|Name'),
@@ -10,18 +9,12 @@ const HEADERS = {
 };
 
 export default {
-  mixins: [glFeatureFlagsMixin()],
   props: {
     rules: {
       type: Array,
       required: true,
     },
   },
-  computed: {
-    scopedApprovalRules() {
-      return this.glFeatures.scopedApprovalRules;
-    },
-  },
   HEADERS,
 };
 </script>
@@ -29,18 +22,10 @@ export default {
 <template>
   <table class="table m-0">
     <thead class="thead-white text-nowrap">
-      <slot
-        name="thead"
-        v-bind="$options.HEADERS"
-        :gl-features-scoped-approval-rules="scopedApprovalRules"
-      ></slot>
+      <slot name="thead" v-bind="$options.HEADERS"></slot>
     </thead>
     <tbody>
-      <slot
-        name="tbody"
-        :rules="rules"
-        :gl-features-scoped-approval-rules="scopedApprovalRules"
-      ></slot>
+      <slot name="tbody" :rules="rules"></slot>
     </tbody>
   </table>
 </template>

ee/app/controllers/ee/projects_controller.rb

@@ -166,11 +166,5 @@ module EE
     def log_unarchive_audit_event
       log_audit_event(message: 'Project unarchived')
     end
-
-    override :render_edit
-    def render_edit
-      push_frontend_feature_flag(:scoped_approval_rules, project, default_enabled: true)
-      super
-    end
   end
 end

ee/app/models/approval_state.rb

@@ -158,9 +158,7 @@ class ApprovalState
       if approval_rules_overwritten?
         user_defined_merge_request_rules
       else
-        branch = project.scoped_approval_rules_enabled? ? target_branch : nil
-
-        project.visible_user_defined_rules(branch: branch).map do |rule|
+        project.visible_user_defined_rules(branch: target_branch).map do |rule|
           ApprovalWrappedRule.wrap(merge_request, rule)
         end
       end
@@ -206,8 +204,7 @@ class ApprovalState
 
   def wrapped_rules
     strong_memoize(:wrapped_rules) do
-      merge_request_rules = merge_request.approval_rules
-      merge_request_rules = merge_request_rules.applicable_to_branch(target_branch) if project.scoped_approval_rules_enabled?
+      merge_request_rules = merge_request.approval_rules.applicable_to_branch(target_branch)
 
       merge_request_rules.map do |rule|
         ApprovalWrappedRule.wrap(merge_request, rule)

ee/app/models/ee/project.rb

@@ -333,10 +333,6 @@ module EE
         feature_available?(:github_project_service_integration)
     end
 
-    def scoped_approval_rules_enabled?
-      ::Feature.enabled?(:scoped_approval_rules, self, default_enabled: true)
-    end
-
     def service_desk_enabled
       ::EE::Gitlab::ServiceDesk.enabled?(project: self) && super
     end

ee/app/services/approval_rules/params_filtering_service.rb

@@ -131,7 +131,7 @@ module ApprovalRules
     # `master` rule should be available.
     def append_user_defined_inapplicable_rules(source_rule_ids)
       return if updating?
-      return unless project.scoped_approval_rules_enabled? && project.multiple_approval_rules_available?
+      return unless project.multiple_approval_rules_available?
 
       project
         .visible_user_defined_inapplicable_rules(params[:target_branch])

ee/spec/models/approval_state_spec.rb

@@ -1513,33 +1513,18 @@ describe ApprovalState do
             project_rule.update!(protected_branches: [another_protected_branch])
           end
 
-          context 'and scoped_approval_rules feature is enabled' do
-            it 'returns the rules that are applicable to the merge request target branch' do
-              expect(subject.user_defined_rules.map(&:approval_rule)).to eq([
-                another_project_rule
-              ])
-            end
-
-            context 'and target_branch is specified' do
-              subject { described_class.new(merge_request, target_branch: 'v1-stable') }
-
-              it 'returns the rules that are applicable to the specified target_branch' do
-                expect(subject.user_defined_rules.map(&:approval_rule)).to eq([
-                  project_rule
-                ])
-              end
-            end
+          it 'returns the rules that are applicable to the merge request target branch' do
+            expect(subject.user_defined_rules.map(&:approval_rule)).to eq([
+              another_project_rule
+            ])
           end
 
-          context 'but scoped_approval_rules feature is disabled' do
-            before do
-              stub_feature_flags(scoped_approval_rules: false)
-            end
+          context 'and target_branch is specified' do
+            subject { described_class.new(merge_request, target_branch: 'v1-stable') }
 
-            it 'returns unscoped rules' do
-              expect(subject.user_defined_rules.map(&:approval_rule)).to match_array([
-                project_rule,
-                another_project_rule
+            it 'returns the rules that are applicable to the specified target_branch' do
+              expect(subject.user_defined_rules.map(&:approval_rule)).to eq([
+                project_rule
               ])
             end
           end
@@ -1603,33 +1588,18 @@ describe ApprovalState do
             )
           end
 
-          context 'and scoped_approval_rules feature is enabled' do
-            it 'returns the rules that are applicable to the merge request target branch' do
-              expect(subject.user_defined_rules.map(&:approval_rule)).to eq([
-                another_mr_rule
-              ])
-            end
-
-            context 'and target_branch is specified' do
-              subject { described_class.new(merge_request, target_branch: 'v1-stable') }
-
-              it 'returns the rules that are applicable to the specified target_branch' do
-                expect(subject.user_defined_rules.map(&:approval_rule)).to eq([
-                  mr_rule
-                ])
-              end
-            end
+          it 'returns the rules that are applicable to the merge request target branch' do
+            expect(subject.user_defined_rules.map(&:approval_rule)).to eq([
+              another_mr_rule
+            ])
           end
 
-          context 'but scoped_approval_rules feature is disabled' do
-            before do
-              stub_feature_flags(scoped_approval_rules: false)
-            end
+          context 'and target_branch is specified' do
+            subject { described_class.new(merge_request, target_branch: 'v1-stable') }
 
-            it 'returns unscoped rules' do
-              expect(subject.user_defined_rules.map(&:approval_rule)).to match_array([
-                mr_rule,
-                another_mr_rule
+            it 'returns the rules that are applicable to the specified target_branch' do
+              expect(subject.user_defined_rules.map(&:approval_rule)).to eq([
+                mr_rule
               ])
             end
           end

ee/spec/services/approval_rules/params_filtering_service_spec.rb

@@ -121,25 +121,6 @@ describe ApprovalRules::ParamsFilteringService do
               )
             end
           end
-
-          context 'when scoped_approval_rules feature is disabled' do
-            before do
-              stub_feature_flags(scoped_approval_rules: false)
-            end
-
-            it 'does not add inapplicable user defined rules' do
-              params = service.execute
-              approval_rules_attrs = params[:approval_rules_attributes]
-
-              aggregate_failures do
-                expect(approval_rules_attrs.size).to eq(1)
-                expect(approval_rules_attrs.first).to include(
-                  name: another_source_rule.name,
-                  approval_project_rule_id: another_source_rule.id
-                )
-              end
-            end
-          end
         end
 
         context 'when multiple_approval_rules feature is not available' do