Merge branch '227853-remove-use_legacy_codeowner_validations-flag' into 'master'

Remove :use_legacy_codeowner_validations feature flag

See merge request gitlab-org/gitlab!37371

app/controllers/projects/blob_controller.rb

@@ -257,5 +257,3 @@ class Projects::BlobController < Projects::ApplicationController
     params.permit(:full, :since, :to, :bottom, :unfold, :offset, :indent)
   end
 end
-
-Projects::BlobController.prepend_if_ee('EE::Projects::BlobController')

ee/app/controllers/ee/projects/blob_controller.rb

-# frozen_string_literal: true
-
-module EE
-  module Projects
-    module BlobController
-      extend ActiveSupport::Concern
-
-      prepended do
-        before_action :validate_codeowner_rules, only: [:create, :update]
-      end
-
-      private
-
-      # rubocop:disable Gitlab/ModuleWithInstanceVariables
-      def validate_codeowner_rules
-        return unless ::Feature.enabled?(:use_legacy_codeowner_validations)
-        return if @file_path.blank?
-
-        codeowners_error = codeowners_check_error(project, branch_name, @file_path)
-
-        if codeowners_error.present?
-          flash.now[:alert] = codeowners_error.tr("\n", " ")
-          view = params[:action] == 'update' ? :edit : :new
-
-          render view
-        end
-      end
-      # rubocop:enable Gitlab/ModuleWithInstanceVariables
-
-      def codeowners_check_error(project, branch_name, paths)
-        ::Gitlab::CodeOwners::Validator.new(project, branch_name, paths).execute
-      end
-    end
-  end
-end

ee/app/services/ee/commits/create_service.rb

@@ -24,8 +24,6 @@ module EE
       end
 
       def validate_against_codeowner_rules!
-        return if ::Feature.enabled?(:use_legacy_codeowner_validations)
-
         codeowners_error = check_against_codeowners(project, params[:branch_name], extracted_paths)
 
         if codeowners_error.present?

ee/spec/controllers/ee/projects/blob_controller_spec.rb

@@ -8,28 +8,7 @@ RSpec.describe Projects::BlobController do
 
   let(:project) { create(:project, :public, :repository) }
 
-  shared_examples "file matches a codeowners rule" do
-    # Expected behavior for both these contexts should be equivalent, however
-    #   the feature flag is used in code to control which code path is followed.
-    #
-    context ":use_legacy_codeowner_validations is true" do
-      before do
-        stub_feature_flags(use_legacy_codeowner_validations: true)
-      end
-
-      it_behaves_like "renders to the expected_view with an error msg"
-    end
-
-    context ":use_legacy_codeowner_validations is false" do
-      before do
-        stub_feature_flags(use_legacy_codeowner_validations: false)
-      end
-
-      it_behaves_like "renders to the expected_view with an error msg"
-    end
-  end
-
-  shared_examples "renders to the expected_view with an error msg" do
+  shared_examples "renders to the expected_view with a code owners error msg" do
     let(:error_msg) do
       "Pushes to protected branches that contain changes to files that match " \
       "patterns defined in `CODEOWNERS` are disabled for this project. " \
@@ -91,7 +70,7 @@ RSpec.describe Projects::BlobController do
       expect(response).to be_redirect
     end
 
-    it_behaves_like "file matches a codeowners rule" do
+    it_behaves_like "renders to the expected_view with a code owners error msg" do
       subject { post :create, params: default_params }
 
       let(:expected_view) { :new }
@@ -117,7 +96,7 @@ RSpec.describe Projects::BlobController do
       sign_in(user)
     end
 
-    it_behaves_like "file matches a codeowners rule" do
+    it_behaves_like "renders to the expected_view with a code owners error msg" do
       subject { put :update, params: default_params }
 
       let(:expected_view) { :edit }

ee/spec/requests/api/commits_spec.rb

@@ -39,14 +39,6 @@ RSpec.describe API::Commits do
     end
   end
 
-  shared_examples_for "does not create a CodeOwners::Validator object" do
-    specify do
-      expect(Gitlab::CodeOwners::Validator).not_to receive(:new)
-
-      subject
-    end
-  end
-
   describe "POST /projects/:id/repository/commits" do
     let!(:url) { "/projects/#{project_id}/repository/commits" }
 
@@ -88,21 +80,7 @@ RSpec.describe API::Commits do
           let(:branch) { valid_c_params[:branch] }
           let(:paths)  { valid_c_params[:actions].first[:file_path] }
 
-          context ":use_legacy_codeowner_validations is true" do
-            before do
-              stub_feature_flags(use_legacy_codeowner_validations: true)
-            end
-
-            it_behaves_like "does not create a CodeOwners::Validator object"
-          end
-
-          context ":use_legacy_codeowner_validations is false" do
-            before do
-              stub_feature_flags(use_legacy_codeowner_validations: false)
-            end
-
-            it_behaves_like "returns a 400 from a codeowners violation"
-          end
+          it_behaves_like "returns a 400 from a codeowners violation"
         end
       end
     end
@@ -137,21 +115,7 @@ RSpec.describe API::Commits do
         let(:branch) { valid_d_params[:branch] }
         let(:paths)  { valid_d_params[:actions].first[:file_path] }
 
-        context ":use_legacy_codeowner_validations is true" do
-          before do
-            stub_feature_flags(use_legacy_codeowner_validations: true)
-          end
-
-          it_behaves_like "does not create a CodeOwners::Validator object"
-        end
-
-        context ":use_legacy_codeowner_validations is false" do
-          before do
-            stub_feature_flags(use_legacy_codeowner_validations: false)
-          end
-
-          it_behaves_like "returns a 400 from a codeowners violation"
-        end
+        it_behaves_like "returns a 400 from a codeowners violation"
       end
     end
 
@@ -190,21 +154,7 @@ RSpec.describe API::Commits do
           [action[:file_path], action[:previous_path]]
         end
 
-        context ":use_legacy_codeowner_validations is true" do
-          before do
-            stub_feature_flags(use_legacy_codeowner_validations: true)
-          end
-
-          it_behaves_like "does not create a CodeOwners::Validator object"
-        end
-
-        context ":use_legacy_codeowner_validations is false" do
-          before do
-            stub_feature_flags(use_legacy_codeowner_validations: false)
-          end
-
-          it_behaves_like "returns a 400 from a codeowners violation"
-        end
+        it_behaves_like "returns a 400 from a codeowners violation"
       end
     end
   end
@@ -235,21 +185,7 @@ RSpec.describe API::Commits do
         let(:code_owner_approval_required) { true }
         let(:paths) { commit.raw_deltas.flat_map { |diff| [diff.new_path, diff.old_path] }.uniq }
 
-        context ":use_legacy_codeowner_validations is true" do
-          before do
-            stub_feature_flags(use_legacy_codeowner_validations: true)
-          end
-
-          it_behaves_like "does not create a CodeOwners::Validator object"
-        end
-
-        context ":use_legacy_codeowner_validations is false" do
-          before do
-            stub_feature_flags(use_legacy_codeowner_validations: false)
-          end
-
-          it_behaves_like "returns a 400 from a codeowners violation"
-        end
+        it_behaves_like "returns a 400 from a codeowners violation"
       end
     end
   end
@@ -283,20 +219,7 @@ RSpec.describe API::Commits do
         let(:code_owner_approval_required) { true }
         let(:paths) { commit.raw_deltas.flat_map { |diff| [diff.new_path, diff.old_path] }.uniq }
 
-        context ":use_legacy_codeowner_validations is true" do
-          before do
-            stub_feature_flags(use_legacy_codeowner_validations: true)
-          end
-
-          it_behaves_like "does not create a CodeOwners::Validator object"
-        end
-
-        context ":use_legacy_codeowner_validations is false" do
-          before do
-            stub_feature_flags(use_legacy_codeowner_validations: false)
-          end
-          it_behaves_like "returns a 400 from a codeowners violation"
-        end
+        it_behaves_like "returns a 400 from a codeowners violation"
       end
     end
   end