Merge branch 'add_codeowners_supersede_for_web_ide' into 'master'

Add allowed to push superseding codeowners for web ide

See merge request gitlab-org/gitlab!50608

changelogs/unreleased/add_codeowners_supersede_for_web_ide.yml

+---
+title: Fix codeowners superseding web ide and single file edit
+merge_request: 50608
+author:
+type: fixed

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

@@ -33,6 +33,7 @@ module EE
 
       def check_against_codeowners(project, branch, paths)
         return if paths.empty?
+        return if ::Feature.enabled?(:push_rules_supersede_code_owners, project, default_enabled: true)
 
         ::Gitlab::CodeOwners::Validator.new(project, branch, paths).execute
       end

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

@@ -8,39 +8,51 @@ RSpec.describe Projects::BlobController do
 
   let(:project) { create(:project, :public, :repository) }
 
-  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. " \
-      "Please submit these changes via a merge request.  The following " \
-      "pattern(s) from `CODEOWNERS` were matched: - docs/ "
+  shared_examples_for "handling the codeowners interaction" do
+    it "redirects to blob" do
+      default_params[:file_path] = "docs/EXAMPLE_FILE"
+
+      subject
+
+      expect(flash[:alert]).to eq(nil)
+      expect(response).to be_redirect
     end
 
-    before do
-      allow(::ProtectedBranch).to receive(:branch_requires_code_owner_approval?)
-        .and_return(true)
-
-      expect_next_instance_of(Repository) do |repo|
-        allow(repo).to receive(:code_owners_blob)
-          .with(ref: "master")
-          .and_return(
-            fake_blob(
-              path: "CODEOWNERS",
-              data: "*.rb @#{user.username}\ndocs/ @#{user.username}"
-            )
-          )
+    context 'when push_rules_supersede_code_owners is false' 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. " \
+          "Please submit these changes via a merge request.  The following " \
+          "pattern(s) from `CODEOWNERS` were matched: - docs/ "
       end
 
-      stub_licensed_features(code_owner_approval_required: true)
-    end
+      before do
+        allow(::ProtectedBranch).to receive(:branch_requires_code_owner_approval?)
+          .and_return(true)
+
+        expect_next_instance_of(Repository) do |repo|
+          allow(repo).to receive(:code_owners_blob)
+            .with(ref: "master")
+            .and_return(
+              fake_blob(
+                path: "CODEOWNERS",
+                data: "*.rb @#{user.username}\ndocs/ @#{user.username}"
+              )
+            )
+        end
 
-    it "renders to the edit page with an error msg" do
-      default_params[:file_path] = "docs/EXAMPLE_FILE"
+        stub_licensed_features(code_owner_approval_required: true)
+        stub_feature_flags(push_rules_supersede_code_owners: false)
+      end
 
-      subject
+      it "renders to the edit page with an error msg" do
+        default_params[:file_path] = "docs/EXAMPLE_FILE"
+
+        subject
 
-      expect(flash[:alert]).to eq(error_msg)
-      expect(response).to render_template(expected_view)
+        expect(flash[:alert]).to eq(error_msg)
+        expect(response).to render_template(expected_view)
+      end
     end
   end
 
@@ -70,7 +82,7 @@ RSpec.describe Projects::BlobController do
       expect(response).to be_redirect
     end
 
-    it_behaves_like "renders to the expected_view with a code owners error msg" do
+    it_behaves_like "handling the codeowners interaction" do
       subject { post :create, params: default_params }
 
       let(:expected_view) { :new }
@@ -96,7 +108,7 @@ RSpec.describe Projects::BlobController do
       sign_in(user)
     end
 
-    it_behaves_like "renders to the expected_view with a code owners error msg" do
+    it_behaves_like "handling the codeowners interaction" do
       subject { put :update, params: default_params }
 
       let(:expected_view) { :edit }

ee/spec/features/ide/user_commits_changes_spec.rb

@@ -24,11 +24,25 @@ RSpec.describe 'EE IDE user commits changes', :js do
     ide_visit(project)
   end
 
-  it 'shows error message' do
+  it 'does not show an error message' do
     ide_create_new_file('test.rb', content: '# A ruby file')
 
     ide_commit
 
-    expect(page).to have_content('CODEOWNERS rule violation')
+    expect(page).not_to have_content('CODEOWNERS rule violation')
+  end
+
+  context 'when the push_rules_supersede_codeowners is false' do
+    before do
+      stub_feature_flags(push_rules_supersede_code_owners: false)
+    end
+
+    it 'shows error message' do
+      ide_create_new_file('test.rb', content: '# A ruby file')
+
+      ide_commit
+
+      expect(page).to have_content('CODEOWNERS rule violation')
+    end
   end
 end

ee/spec/requests/api/commits_spec.rb

@@ -11,31 +11,42 @@ RSpec.describe API::Commits do
     project.add_maintainer(user)
   end
 
-  shared_examples_for "returns a 400 from a codeowners violation" do
-    let(:error_msg) { "CodeOwners error msg" }
-
-    before do
-      allow(ProtectedBranch)
-        .to receive(:branch_requires_code_owner_approval?)
-        .with(project, branch).and_return(code_owner_approval_required)
-    end
-
-    it "creates a new validator with expected parameters" do
+  shared_examples_for "handling the codeowners interaction" do
+    it "does not create a new validator" do
       expect(Gitlab::CodeOwners::Validator)
-        .to receive(:new).with(project, branch, Array(paths)).and_call_original
+        .not_to receive(:new)
 
       subject
     end
 
-    specify do
-      expect_next_instance_of(Gitlab::CodeOwners::Validator) do |validator|
-        expect(validator).to receive(:execute).and_return(error_msg)
+    context 'when push_rules_supersede_code_owners is false' do
+      let(:error_msg) { "CodeOwners error msg" }
+
+      before do
+        stub_feature_flags(push_rules_supersede_code_owners: false)
+
+        allow(ProtectedBranch)
+          .to receive(:branch_requires_code_owner_approval?)
+          .with(project, branch).and_return(code_owner_approval_required)
       end
 
-      subject
+      it "creates a new validator with expected parameters" do
+        expect(Gitlab::CodeOwners::Validator)
+          .to receive(:new).with(project, branch, Array(paths)).and_call_original
 
-      expect(response).to have_gitlab_http_status(:bad_request)
-      expect(json_response["message"]).to include(error_msg)
+        subject
+      end
+
+      specify do
+        expect_next_instance_of(Gitlab::CodeOwners::Validator) do |validator|
+          expect(validator).to receive(:execute).and_return(error_msg)
+        end
+
+        subject
+
+        expect(response).to have_gitlab_http_status(:bad_request)
+        expect(json_response["message"]).to include(error_msg)
+      end
     end
   end
 
@@ -80,7 +91,7 @@ RSpec.describe API::Commits do
           let(:branch) { valid_c_params[:branch] }
           let(:paths)  { valid_c_params[:actions].first[:file_path] }
 
-          it_behaves_like "returns a 400 from a codeowners violation"
+          it_behaves_like "handling the codeowners interaction"
         end
       end
     end
@@ -115,7 +126,7 @@ RSpec.describe API::Commits do
         let(:branch) { valid_d_params[:branch] }
         let(:paths)  { valid_d_params[:actions].first[:file_path] }
 
-        it_behaves_like "returns a 400 from a codeowners violation"
+        it_behaves_like "handling the codeowners interaction"
       end
     end
 
@@ -154,7 +165,7 @@ RSpec.describe API::Commits do
           [action[:file_path], action[:previous_path]]
         end
 
-        it_behaves_like "returns a 400 from a codeowners violation"
+        it_behaves_like "handling the codeowners interaction"
       end
     end
   end
@@ -185,7 +196,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 }
 
-        it_behaves_like "returns a 400 from a codeowners violation"
+        it_behaves_like "handling the codeowners interaction"
       end
     end
   end
@@ -219,7 +230,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 }
 
-        it_behaves_like "returns a 400 from a codeowners violation"
+        it_behaves_like "handling the codeowners interaction"
       end
     end
   end

ee/spec/services/ee/commits/create_service_spec.rb

@@ -5,13 +5,15 @@ require 'spec_helper'
 RSpec.describe Commits::CreateService do
   let(:project) { create(:project) }
   let(:user) { create(:user) }
+  let(:branch_name) { 'master' }
+  let(:extra_params) { {} }
 
   before do
     project.add_maintainer(user)
   end
 
   subject(:service) do
-    described_class.new(project, user, start_branch: 'master', branch_name: 'master')
+    described_class.new(project, user, start_branch: branch_name, branch_name: branch_name, **extra_params)
   end
 
   describe '#execute' do
@@ -29,5 +31,30 @@ RSpec.describe Commits::CreateService do
       expect(result[:status]).to be(:error)
       expect(result[:message]).to eq('Your changes could not be committed, because this repository has exceeded its size limit of 1 Byte by 1 Byte')
     end
+
+    context 'when validating codeowners' do
+      let(:extra_params) { { file_path: 'path', actions: [{ file_path: 'a', previous_path: 'b' }] } }
+
+      context 'when the paths are empty' do
+        let(:extra_params) { {} }
+
+        it 'does not validate' do
+          expect(::Gitlab::CodeOwners::Validator).not_to receive(:new)
+          result
+        end
+      end
+
+      it 'does not validate when the push_rules_supersede_code_owners flag is true' do
+        expect(::Gitlab::CodeOwners::Validator).not_to receive(:new)
+        result
+      end
+
+      it 'validates the code owners file when the push_rules_supersede_code_owners flag is false' do
+        stub_feature_flags(push_rules_supersede_code_owners: false)
+        expect(::Gitlab::CodeOwners::Validator).to receive(:new).with(project, branch_name, %w[path a b]).and_call_original
+
+        result
+      end
+    end
   end
 end