Merge branch 'revert-5b0099a7' into 'master'

Revert "Merge branch '10395-COAR-phase-2' into 'master'"

See merge request gitlab-org/gitlab!17165

app/models/protected_branch.rb

@@ -40,11 +40,6 @@ class ProtectedBranch < ApplicationRecord
   def self.protected_refs(project)
     project.protected_branches.select(:name)
   end
-
-  def self.branch_requires_code_owner_approval?(project, branch_name)
-    # NOOP
-    #
-  end
 end
 
 ProtectedBranch.prepend_if_ee('EE::ProtectedBranch')

db/post_migrate/20190827102026_migrate_code_owner_approval_status_to_protected_branches_in_batches.rb

-# frozen_string_literal: true
-
-class MigrateCodeOwnerApprovalStatusToProtectedBranchesInBatches < ActiveRecord::Migration[5.2]
-  include Gitlab::Database::MigrationHelpers
-
-  disable_ddl_transaction!
-
-  DOWNTIME = false
-  BATCH_SIZE = 200
-
-  class Project < ActiveRecord::Base
-    include EachBatch
-
-    self.table_name = 'projects'
-    self.inheritance_column = :_type_disabled
-
-    has_many :protected_branches
-  end
-
-  class ProtectedBranch < ActiveRecord::Base
-    include EachBatch
-
-    self.table_name = 'protected_branches'
-    self.inheritance_column = :_type_disabled
-
-    belongs_to :project
-  end
-
-  def up
-    add_concurrent_index :projects, :id, name: "temp_active_projects_with_prot_branches", where: 'archived = false and pending_delete = false and merge_requests_require_code_owner_approval = true'
-
-    ProtectedBranch
-      .joins(:project)
-      .where(projects: { archived: false, pending_delete: false, merge_requests_require_code_owner_approval: true })
-      .each_batch(of: BATCH_SIZE) do |batch|
-        batch.update_all(code_owner_approval_required: true)
-      end
-
-    remove_concurrent_index_by_name(:projects, "temp_active_projects_with_prot_branches")
-  end
-
-  def down
-    # noop
-    #
-  end
-end

ee/app/models/approval_wrapped_rule.rb

@@ -92,15 +92,16 @@ class ApprovalWrappedRule
 
   def code_owner_approvals_required
     strong_memoize(:code_owner_approvals_required) do
-      next 0 unless branch_requires_code_owner_approval?
+      next 0 unless merge_requests_require_code_owner_approval?
 
       approvers.any? ? REQUIRED_APPROVALS_PER_CODE_OWNER_RULE : 0
     end
   end
 
-  def branch_requires_code_owner_approval?
+  def merge_requests_require_code_owner_approval?
     return false unless project.code_owner_approval_required_available?
 
-    ProtectedBranch.branch_requires_code_owner_approval?(project, merge_request.target_branch)
+    project.merge_requests_require_code_owner_approval? ||
+      project.branch_requires_code_owner_approval?(merge_request.target_branch)
   end
 end

ee/app/models/concerns/ee/protected_branch.rb

@@ -8,14 +8,6 @@ module EE
       protected_ref_access_levels :unprotect
     end
 
-    class_methods do
-      def branch_requires_code_owner_approval?(project, branch_name)
-        return false unless project.code_owner_approval_required_available?
-
-        project.protected_branches.requiring_code_owner_approval.matching(branch_name).any?
-      end
-    end
-
     def code_owner_approval_required
       super && project.code_owner_approval_required_available?
     end

ee/app/models/ee/project.rb

@@ -102,7 +102,7 @@ module EE
       scope :verification_failed_wikis, -> { joins(:repository_state).merge(ProjectRepositoryState.verification_failed_wikis) }
       scope :for_plan_name, -> (name) { joins(namespace: :plan).where(plans: { name: name }) }
       scope :requiring_code_owner_approval,
-            -> { joins(:protected_branches).where(protected_branches: { code_owner_approval_required: true }) }
+            -> { where(merge_requests_require_code_owner_approval: true) }
 
       scope :with_security_reports_stored, -> { where('EXISTS (?)', ::Vulnerabilities::Occurrence.scoped_project.select(1)) }
       scope :with_security_reports, -> { where('EXISTS (?)', ::Ci::JobArtifact.security_reports.scoped_project.select(1)) }
@@ -377,14 +377,13 @@ module EE
     end
 
     def merge_requests_require_code_owner_approval?
-      code_owner_approval_required_available? &&
-        protected_branches.requiring_code_owner_approval.any?
+      super && code_owner_approval_required_available?
     end
 
     def branch_requires_code_owner_approval?(branch_name)
       return false unless code_owner_approval_required_available?
 
-      ::ProtectedBranch.branch_requires_code_owner_approval?(self, branch_name)
+      protected_branches.requiring_code_owner_approval.matching(branch_name).any?
     end
 
     def require_password_to_approve

ee/changelogs/unreleased/10395-COAR-phase-2.yml

----
-title: Move Require code owner approval setting from Project to ProtectedBranches
-merge_request: 16187
-author:
-type: changed

ee/spec/factories/projects.rb

@@ -55,5 +55,9 @@ FactoryBot.modify do
     trait :random_last_repository_updated_at do
       last_repository_updated_at { rand(1.year).seconds.ago }
     end
+
+    trait :requiring_code_owner_approval do
+      merge_requests_require_code_owner_approval true
+    end
   end
 end

ee/spec/features/merge_request/user_sees_approval_widget_spec.rb

@@ -107,11 +107,7 @@ describe 'Merge request > User sees approval widget', :js do
         context 'when code owner approval is required' do
           before do
             stub_licensed_features(code_owner_approval_required: true, multiple_approval_rules: true)
-
             project.update!(merge_requests_require_code_owner_approval: true)
-
-            allow(ProtectedBranch)
-              .to receive(:branch_requires_code_owner_approval?).and_return(true)
           end
 
           it 'shows the code owner rule as required' do

ee/spec/features/projects/merge_requests/user_edits_merge_request_spec.rb

@@ -16,6 +16,7 @@ describe 'Projects > Merge Requests > User edits a merge request' do
 
     let(:project) do
       create(:project, :custom_repo,
+             merge_requests_require_code_owner_approval: true,
              files: { 'docs/CODEOWNERS' => "*.rb @ruby-owner\n*.js @js-owner" })
     end
 
@@ -35,10 +36,6 @@ describe 'Projects > Merge Requests > User edits a merge request' do
                                      message: 'Add a ruby file',
                                      branch_name: 'feature')
 
-      create(:protected_branch,
-        code_owner_approval_required: true,
-        project: project)
-
       # To make sure the rules are created for the merge request, the services
       # that do that aren't triggered from factories
       MergeRequests::SyncCodeOwnerApprovalRules.new(merge_request).execute

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

@@ -35,12 +35,6 @@ describe 'EE > Projects > Settings > User manages approval rule settings' do
   context 'when `code_owner_approval_required` is available' do
     let(:licensed_features) { { code_owner_approval_required: true } }
 
-    before do
-      create(:protected_branch,
-        code_owner_approval_required: true,
-        project: project)
-    end
-
     it_behaves_like 'dirty submit form', [{ form: '#js-merge-request-approval-settings', input: '#project_merge_requests_author_approval' }]
 
     it 'allows the user to enforce code owner approval' do

ee/spec/lib/gitlab/usage_data_spec.rb

@@ -195,15 +195,9 @@ describe Gitlab::UsageData do
 
   describe 'code owner approval required' do
     before do
-      create(:protected_branch, code_owner_approval_required: true)
-
-      create(:protected_branch,
-        code_owner_approval_required: true,
-        project: create(:project, :archived))
-
-      create(:protected_branch,
-        code_owner_approval_required: true,
-        project: create(:project, pending_delete: true))
+      create(:project, :archived, :requiring_code_owner_approval)
+      create(:project, :requiring_code_owner_approval, pending_delete: true)
+      create(:project, :requiring_code_owner_approval)
     end
 
     it 'counts the projects actively requiring code owner approval' do

ee/spec/models/approval_wrapped_rule_spec.rb

@@ -196,12 +196,23 @@ describe ApprovalWrappedRule do
               .to receive(:code_owner_approval_required_available?).and_return(true)
           end
 
+          context "when no protected_branches require code owner approval" do
+            it 'returns the correct number of approvals' do
+              allow(subject.project)
+                .to receive(:merge_requests_require_code_owner_approval?).and_return(feature_enabled)
+              allow(subject.project)
+                .to receive(:branch_requires_code_owner_approval?).with(branch.name).and_return(false)
+
+              expect(subject.approvals_required).to eq(expected_required_approvals)
+            end
+          end
+
           context "when the project doesn't require code owner approval on all MRs" do
             it 'returns the expected number of approvals for protected_branches that do require approval' do
               allow(subject.project)
                 .to receive(:merge_requests_require_code_owner_approval?).and_return(false)
-              allow(ProtectedBranch)
-                .to receive(:branch_requires_code_owner_approval?).with(subject.project, branch.name).and_return(feature_enabled)
+              allow(subject.project)
+                .to receive(:branch_requires_code_owner_approval?).with(branch.name).and_return(feature_enabled)
 
               expect(subject.approvals_required).to eq(expected_required_approvals)
             end

ee/spec/models/project_spec.rb

@@ -45,15 +45,11 @@ describe Project do
 
   context 'scopes' do
     describe '.requiring_code_owner_approval' do
-      let!(:project) { create(:project) }
-      let!(:expected_project) { protected_branch_needing_approval.project }
-      let!(:protected_branch_needing_approval) { create(:protected_branch, code_owner_approval_required: true) }
-
       it 'only includes the right projects' do
-        scoped_query_result = described_class.requiring_code_owner_approval
+        create(:project)
+        expected_project = create(:project, merge_requests_require_code_owner_approval: true)
 
-        expect(described_class.count).to eq(2)
-        expect(scoped_query_result).to contain_exactly(expected_project)
+        expect(described_class.requiring_code_owner_approval).to contain_exactly(expected_project)
       end
     end
 
@@ -910,17 +906,13 @@ describe Project do
       true  | true  | true
       false | true  | false
       true  | false | false
+      true  | nil   | false
     end
 
     with_them do
       before do
         stub_licensed_features(code_owner_approval_required: feature_available)
-
-        if feature_enabled
-          create(:protected_branch,
-            project: project,
-            code_owner_approval_required: true)
-        end
+        project.merge_requests_require_code_owner_approval = feature_enabled
       end
 
       it 'requires code owner approval when needed' do

spec/migrations/migrate_code_owner_approval_status_to_protected_branches_in_batches_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-require Rails.root.join('db', 'post_migrate', '20190827102026_migrate_code_owner_approval_status_to_protected_branches_in_batches.rb')
-
-describe MigrateCodeOwnerApprovalStatusToProtectedBranchesInBatches, :migration do
-  let(:namespaces)         { table(:namespaces) }
-  let(:projects)           { table(:projects) }
-  let(:protected_branches) { table(:protected_branches) }
-
-  let(:namespace) do
-    namespaces.create!(
-      path: 'gitlab-instance-administrators',
-      name: 'GitLab Instance Administrators'
-    )
-  end
-
-  let(:project) do
-    projects.create!(
-      namespace_id: namespace.id,
-      name: 'GitLab Instance Administration'
-    )
-  end
-
-  let!(:protected_branch_1) do
-    protected_branches.create!(
-      name: "branch name",
-      project_id: project.id
-    )
-  end
-
-  describe '#up' do
-    context "when there's no projects needing approval" do
-      it "doesn't change any protected branch records" do
-        expect { migrate! }
-          .not_to change { ProtectedBranch.where(code_owner_approval_required: true).count }
-      end
-    end
-
-    context "when there's a project needing approval" do
-      let!(:project_needing_approval) do
-        projects.create!(
-          namespace_id: namespace.id,
-          name: 'GitLab Instance Administration',
-          merge_requests_require_code_owner_approval: true
-        )
-      end
-
-      let!(:protected_branch_2) do
-        protected_branches.create!(
-          name: "branch name",
-          project_id: project_needing_approval.id
-        )
-      end
-
-      it "changes N protected branch records" do
-        expect { migrate! }
-          .to change { ProtectedBranch.where(code_owner_approval_required: true).count }
-          .by(1)
-      end
-    end
-  end
-end