diff --git a/db/migrate/20190910212256_add_any_approver_rule_unique_indexes.rb b/db/migrate/20190910212256_add_any_approver_rule_unique_indexes.rb new file mode 100644 index 0000000000000000000000000000000000000000..e8f5b853d1dc794e0e2e1339791bf0e78c0a40de --- /dev/null +++ b/db/migrate/20190910212256_add_any_approver_rule_unique_indexes.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddAnyApproverRuleUniqueIndexes < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + PROJECT_RULE_UNIQUE_INDEX = 'any_approver_project_rule_type_unique_index' + MERGE_REQUEST_RULE_UNIQUE_INDEX = 'any_approver_merge_request_rule_type_unique_index' + + disable_ddl_transaction! + + def up + add_concurrent_index(:approval_project_rules, [:project_id], + where: "rule_type = 3", + name: PROJECT_RULE_UNIQUE_INDEX, unique: true) + + add_concurrent_index(:approval_merge_request_rules, [:merge_request_id, :rule_type], + where: "rule_type = 4", + name: MERGE_REQUEST_RULE_UNIQUE_INDEX, unique: true) + end + + def down + remove_concurrent_index_by_name(:approval_project_rules, PROJECT_RULE_UNIQUE_INDEX) + remove_concurrent_index_by_name(:approval_merge_request_rules, MERGE_REQUEST_RULE_UNIQUE_INDEX) + end +end diff --git a/db/post_migrate/20190905091812_schedule_project_any_approval_rule_migration.rb b/db/post_migrate/20190905091812_schedule_project_any_approval_rule_migration.rb new file mode 100644 index 0000000000000000000000000000000000000000..ef1cb452c2617fc26af8349c92af7caa03a700c2 --- /dev/null +++ b/db/post_migrate/20190905091812_schedule_project_any_approval_rule_migration.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class ScheduleProjectAnyApprovalRuleMigration < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + BATCH_SIZE = 5_000 + MIGRATION = 'PopulateAnyApprovalRuleForProjects' + DELAY_INTERVAL = 8.minutes.to_i + + disable_ddl_transaction! + + class Project < ActiveRecord::Base + include EachBatch + + self.table_name = 'projects' + + scope :with_approvals_before_merge, -> { where('approvals_before_merge <> 0') } + end + + def up + add_concurrent_index :projects, :id, + name: 'tmp_projects_with_approvals_before_merge', + where: 'approvals_before_merge <> 0' + + say "Scheduling `#{MIGRATION}` jobs" + + # We currently have ~43k project records with non-zero approvals_before_merge on GitLab.com. + # This means it'll schedule ~9 jobs (5k projects each) with a 8 minutes gap, + # so this should take ~1 hour for all background migrations to complete. + # + # The approximate expected number of affected rows is: 18k + + queue_background_migration_jobs_by_range_at_intervals( + ScheduleProjectAnyApprovalRuleMigration::Project.with_approvals_before_merge, + MIGRATION, DELAY_INTERVAL, batch_size: BATCH_SIZE) + + remove_concurrent_index_by_name(:projects, 'tmp_projects_with_approvals_before_merge') + end + + def down + # no-op + end +end diff --git a/db/post_migrate/20190905091831_schedule_merge_request_any_approval_rule_migration.rb b/db/post_migrate/20190905091831_schedule_merge_request_any_approval_rule_migration.rb new file mode 100644 index 0000000000000000000000000000000000000000..4a8398a9eead0b2d5dd54f60951d56d06ffd5342 --- /dev/null +++ b/db/post_migrate/20190905091831_schedule_merge_request_any_approval_rule_migration.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class ScheduleMergeRequestAnyApprovalRuleMigration < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + BATCH_SIZE = 5_000 + MIGRATION = 'PopulateAnyApprovalRuleForMergeRequests' + DELAY_INTERVAL = 8.minutes.to_i + + disable_ddl_transaction! + + class MergeRequest < ActiveRecord::Base + include EachBatch + + self.table_name = 'merge_requests' + + scope :with_approvals_before_merge, -> { where('approvals_before_merge <> 0') } + end + + def up + add_concurrent_index :merge_requests, :id, + name: 'tmp_merge_requests_with_approvals_before_merge', + where: 'approvals_before_merge <> 0' + + say "Scheduling `#{MIGRATION}` jobs" + + # We currently have ~440_000 merge request records with non-zero approvals_before_merge on GitLab.com. + # This means it'll schedule ~88 jobs (5k merge requests each) with a 8 minutes gap, + # so this should take ~12 hours for all background migrations to complete. + # + # The approximate expected number of affected rows is: 190k + + queue_background_migration_jobs_by_range_at_intervals( + ScheduleMergeRequestAnyApprovalRuleMigration::MergeRequest.with_approvals_before_merge, + MIGRATION, DELAY_INTERVAL, batch_size: BATCH_SIZE) + + remove_concurrent_index_by_name(:merge_requests, 'tmp_merge_requests_with_approvals_before_merge') + end + + def down + # no-op + end +end diff --git a/db/schema.rb b/db/schema.rb index 979698e649ecebc9510ba8254315259d5c19a8f7..98a1e80b87d8bbd87250d3325b93274d3e57e27a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -310,8 +310,9 @@ t.integer "report_type", limit: 2 t.index ["merge_request_id", "code_owner", "name"], name: "approval_rule_name_index_for_code_owners", unique: true, where: "(code_owner = true)" t.index ["merge_request_id", "code_owner"], name: "index_approval_merge_request_rules_1" - t.index ["merge_request_id", "rule_type", "name"], name: "index_approval_rule_name_for_code_owners_rule_type", unique: true, where: "(rule_type = 2)" - t.index ["merge_request_id", "rule_type"], name: "index_approval_rules_code_owners_rule_type", where: "(rule_type = 2)" + t.index ["merge_request_id", "name"], name: "index_approval_rule_name_for_code_owners_rule_type", unique: true, where: "(rule_type = 2)" + t.index ["merge_request_id", "rule_type"], name: "any_approver_merge_request_rule_type_unique_index", unique: true, where: "(rule_type = 4)" + t.index ["merge_request_id"], name: "index_approval_rules_code_owners_rule_type", where: "(rule_type = 2)" end create_table "approval_merge_request_rules_approved_approvers", force: :cascade do |t| @@ -342,6 +343,7 @@ t.integer "approvals_required", limit: 2, default: 0, null: false t.string "name", null: false t.integer "rule_type", limit: 2, default: 0, null: false + t.index ["project_id"], name: "any_approver_project_rule_type_unique_index", unique: true, where: "(rule_type = 3)" t.index ["project_id"], name: "index_approval_project_rules_on_project_id" t.index ["rule_type"], name: "index_approval_project_rules_on_rule_type" end diff --git a/ee/app/models/approval_merge_request_rule.rb b/ee/app/models/approval_merge_request_rule.rb index 9d2a333bc3d56f140ca6196095e07f65d2853bfb..3c5c196d15532ae9e137d0fec4eaa96470240d53 100644 --- a/ee/app/models/approval_merge_request_rule.rb +++ b/ee/app/models/approval_merge_request_rule.rb @@ -43,7 +43,8 @@ class ApprovalMergeRequestRule < ApplicationRecord enum rule_type: { regular: 1, code_owner: 2, - report_approver: 3 + report_approver: 3, + any_approver: 4 } enum report_type: { diff --git a/ee/app/models/approval_project_rule.rb b/ee/app/models/approval_project_rule.rb index 248d2b01306afe5c765f9ee42a5d7cf27cf49180..4fc3a185734d2568fe6138489f7d088d5669c4d8 100644 --- a/ee/app/models/approval_project_rule.rb +++ b/ee/app/models/approval_project_rule.rb @@ -8,7 +8,8 @@ class ApprovalProjectRule < ApplicationRecord enum rule_type: { regular: 0, code_owner: 1, # currently unused - report_approver: 2 + report_approver: 2, + any_approver: 3 } alias_method :code_owner, :code_owner? diff --git a/ee/app/models/concerns/approval_rule_like.rb b/ee/app/models/concerns/approval_rule_like.rb index ac11e7e1dd4e63a42dde3936e106769ef72e2a1a..839c4820b2fa46b435f0ed69b0fd427e4fbc4ddf 100644 --- a/ee/app/models/concerns/approval_rule_like.rb +++ b/ee/app/models/concerns/approval_rule_like.rb @@ -11,6 +11,7 @@ module ApprovalRuleLike DEFAULT_NAME_FOR_SECURITY_REPORT => :security }.freeze APPROVALS_REQUIRED_MAX = 100 + ALL_MEMBERS = 'All Members' included do has_and_belongs_to_many :users diff --git a/ee/app/models/concerns/deprecated_approvals_before_merge.rb b/ee/app/models/concerns/deprecated_approvals_before_merge.rb new file mode 100644 index 0000000000000000000000000000000000000000..0208f1ac5341e2d978febd3c05d5cdd8c772f9ff --- /dev/null +++ b/ee/app/models/concerns/deprecated_approvals_before_merge.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +# This module handles backward compatibility for approvals_before_merge column +module DeprecatedApprovalsBeforeMerge + extend ActiveSupport::Concern + + include AfterCommitQueue + + included do + after_save do + next unless saved_changes['approvals_before_merge'] + + run_after_commit do + update_any_approver_rule + end + end + end + + private + + def any_approver_rule + strong_memoize(:any_approver_rule) do + approval_rules.any_approver.safe_find_or_create_by(name: ApprovalRuleLike::ALL_MEMBERS) + end + end + + def update_any_approver_rule + return if any_approver_rule.approvals_required == approvals_before_merge.to_i + + any_approver_rule.update_column(:approvals_required, approvals_before_merge.to_i) + end +end diff --git a/ee/app/models/ee/merge_request.rb b/ee/app/models/ee/merge_request.rb index f187036d19189c97829e2631762127a17d0424f0..d192d591b821f130be581f163a61784c02fc506d 100644 --- a/ee/app/models/ee/merge_request.rb +++ b/ee/app/models/ee/merge_request.rb @@ -12,6 +12,7 @@ module MergeRequest prepended do include Elastic::ApplicationVersionedSearch + include DeprecatedApprovalsBeforeMerge has_many :reviews, inverse_of: :merge_request has_many :approvals, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb index 3306ea6a39ee9761ce8068209490dad2325532aa..6088f37adcec7da59594f52c4457686dd64578b4 100644 --- a/ee/app/models/ee/project.rb +++ b/ee/app/models/ee/project.rb @@ -20,6 +20,7 @@ module Project include EachBatch include InsightsFeature include Vulnerable + include DeprecatedApprovalsBeforeMerge self.ignored_columns += %i[ mirror_last_update_at diff --git a/ee/lib/gitlab/background_migration/populate_any_approval_rule_for_merge_requests.rb b/ee/lib/gitlab/background_migration/populate_any_approval_rule_for_merge_requests.rb new file mode 100644 index 0000000000000000000000000000000000000000..9152c2e55d3243d2f8c90c65a985d0dfe4fbe4b1 --- /dev/null +++ b/ee/lib/gitlab/background_migration/populate_any_approval_rule_for_merge_requests.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + # This background migration creates any approver rule records according + # to the given merge request IDs range. A _single_ INSERT is issued for the given range. + class PopulateAnyApprovalRuleForMergeRequests + def perform(from_id, to_id) + select_sql = + MergeRequest + .where(merge_request_approval_rules_not_exists_clause) + .where(id: from_id..to_id) + .where('approvals_before_merge <> 0') + .select("id, approvals_before_merge, created_at, updated_at, 4, '#{ApprovalRuleLike::ALL_MEMBERS}'") + .to_sql + + execute("INSERT INTO approval_merge_request_rules (merge_request_id, approvals_required, created_at, updated_at, rule_type, name) #{select_sql}") + end + + private + + def merge_request_approval_rules_not_exists_clause + <<~SQL + NOT EXISTS (SELECT 1 FROM approval_merge_request_rules + WHERE approval_merge_request_rules.merge_request_id = merge_requests.id) + SQL + end + + def execute(sql) + @connection ||= ActiveRecord::Base.connection + @connection.execute(sql) + end + end + end +end diff --git a/ee/lib/gitlab/background_migration/populate_any_approval_rule_for_projects.rb b/ee/lib/gitlab/background_migration/populate_any_approval_rule_for_projects.rb new file mode 100644 index 0000000000000000000000000000000000000000..7178b1e488c648cc7422965ef3360356529c5560 --- /dev/null +++ b/ee/lib/gitlab/background_migration/populate_any_approval_rule_for_projects.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + # This background migration creates any approver rule records according + # to the given project IDs range. A _single_ INSERT is issued for the given range. + class PopulateAnyApprovalRuleForProjects + def perform(from_id, to_id) + select_sql = + Project + .where(project_approval_rules_not_exists_clause) + .where(id: from_id..to_id) + .where('approvals_before_merge <> 0') + .select("id, approvals_before_merge, created_at, updated_at, 4, '#{ApprovalRuleLike::ALL_MEMBERS}'") + .to_sql + + execute("INSERT INTO approval_project_rules (project_id, approvals_required, created_at, updated_at, rule_type, name) #{select_sql}") + end + + private + + def project_approval_rules_not_exists_clause + <<~SQL + NOT EXISTS (SELECT 1 FROM approval_project_rules + WHERE approval_project_rules.project_id = projects.id) + SQL + end + + def execute(sql) + @connection ||= ActiveRecord::Base.connection + @connection.execute(sql) + end + end + end +end diff --git a/ee/spec/lib/gitlab/background_migration/migrate_approver_to_approval_rules_spec.rb b/ee/spec/lib/gitlab/background_migration/migrate_approver_to_approval_rules_spec.rb index f677f98cf7d45e03572edc9824bedee6449209b2..8a2235570ba21d425a01b842f68fb39cd9b44f85 100644 --- a/ee/spec/lib/gitlab/background_migration/migrate_approver_to_approval_rules_spec.rb +++ b/ee/spec/lib/gitlab/background_migration/migrate_approver_to_approval_rules_spec.rb @@ -117,7 +117,15 @@ def create_member_in(member, *populate_in) end context 'merge request' do - let(:target) { create(:merge_request, approvals_before_merge: 2) } + let(:target) do + merge_request = build(:merge_request, approvals_before_merge: 2) + + allow(merge_request).to receive(:update_any_approver_rule) + + merge_request.save! + merge_request + end + let(:target_type) { 'MergeRequest' } let(:approval_rule) { create(:approval_merge_request_rule, merge_request: target) } @@ -243,7 +251,15 @@ def create_member_in(member, *populate_in) end context 'project' do - let(:target) { create(:project, approvals_before_merge: 2) } + let(:target) do + project = build(:project, approvals_before_merge: 2) + + allow(project).to receive(:update_any_approver_rule) + + project.save! + project + end + let(:target_type) { 'Project' } let(:approval_rule) { create(:approval_project_rule, project: target) } diff --git a/ee/spec/lib/gitlab/background_migration/populate_any_approval_rule_for_merge_requests_spec.rb b/ee/spec/lib/gitlab/background_migration/populate_any_approval_rule_for_merge_requests_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..8b9fc1b7435fbabc849f2f6b74fb074c374a9b29 --- /dev/null +++ b/ee/spec/lib/gitlab/background_migration/populate_any_approval_rule_for_merge_requests_spec.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::BackgroundMigration::PopulateAnyApprovalRuleForMergeRequests, :migration, schema: 2019_09_05_091831 do + let(:namespaces) { table(:namespaces) } + let(:namespace) { namespaces.create(name: 'gitlab', path: 'gitlab-org') } + let(:projects) { table(:projects) } + let(:project) { projects.create(namespace_id: namespace.id, name: 'foo') } + let(:merge_requests) { table(:merge_requests) } + let(:approval_merge_request_rules) { table(:approval_merge_request_rules) } + + def create_merge_request(id, params = {}) + params.merge!(id: id, + target_project_id: project.id, + target_branch: 'master', + source_project_id: project.id, + source_branch: 'mr name', + title: "mr name#{id}") + + merge_requests.create(params) + end + + before do + create_merge_request(2, approvals_before_merge: 2) + + # Test filtering rows with empty approvals_before_merge column + create_merge_request(3, approvals_before_merge: nil) + + # Test filtering already migrated rows + create_merge_request(4, approvals_before_merge: 3) + approval_merge_request_rules.create(id: 3, + merge_request_id: 4, approvals_required: 3, rule_type: 4, name: ApprovalRuleLike::ALL_MEMBERS) + + # Test filtering MRs with existing rules + create_merge_request(5, approvals_before_merge: 3) + approval_merge_request_rules.create(id: 4, + merge_request_id: 5, approvals_required: 3, rule_type: 1, name: 'Regular rules') + + create_merge_request(6, approvals_before_merge: 5) + + # Test filtering rows with zero approvals_before_merge column + create_merge_request(7, approvals_before_merge: 0) + end + + describe '#perform' do + it 'creates approval_merge_request_rules rows according to merge_requests' do + expect { subject.perform(1, 7) }.to change(ApprovalMergeRequestRule, :count).by(2) + + created_rows = [ + { 'merge_request_id' => 2, 'approvals_required' => 2 }, + { 'merge_request_id' => 6, 'approvals_required' => 5 } + ] + existing_rows = [ + { 'merge_request_id' => 4, 'approvals_required' => 3 } + ] + + rows = approval_merge_request_rules.where(rule_type: 4).order(:id).map do |row| + row.attributes.slice('merge_request_id', 'approvals_required') + end + + expect(rows).to match_array(created_rows + existing_rows) + end + end +end diff --git a/ee/spec/lib/gitlab/background_migration/populate_any_approval_rule_for_projects_spec.rb b/ee/spec/lib/gitlab/background_migration/populate_any_approval_rule_for_projects_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..42c5bd4db893286bcfb6c9c96ea449149772172f --- /dev/null +++ b/ee/spec/lib/gitlab/background_migration/populate_any_approval_rule_for_projects_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::BackgroundMigration::PopulateAnyApprovalRuleForProjects, :migration, schema: 2019_09_05_091812 do + let(:namespaces) { table(:namespaces) } + let(:namespace) { namespaces.create(name: 'gitlab', path: 'gitlab-org') } + let(:projects) { table(:projects) } + let(:approval_project_rules) { table(:approval_project_rules) } + + def create_project(id, params = {}) + params.merge!(id: id, namespace_id: namespace.id) + + projects.create(params) + end + + before do + create_project(2, approvals_before_merge: 2) + + # Test filtering rows with empty approvals_before_merge column + create_project(3, approvals_before_merge: 0) + + # Test filtering already migrated rows + create_project(4, approvals_before_merge: 3) + approval_project_rules.create(id: 3, + project_id: 4, approvals_required: 3, rule_type: 4, name: ApprovalRuleLike::ALL_MEMBERS) + + # Test filtering MRs with existing rules + create_project(5, approvals_before_merge: 3) + approval_project_rules.create(id: 4, + project_id: 5, approvals_required: 3, rule_type: 1, name: 'Regular rules') + + create_project(6, approvals_before_merge: 5) + end + + describe '#perform' do + it 'creates approval_project_rules rows according to projects' do + expect { subject.perform(1, 6) }.to change(ApprovalProjectRule, :count).by(2) + + created_rows = [ + { 'project_id' => 2, 'approvals_required' => 2 }, + { 'project_id' => 6, 'approvals_required' => 5 } + ] + existing_rows = [ + { 'project_id' => 4, 'approvals_required' => 3 } + ] + + rows = approval_project_rules.where(rule_type: 4).order(:id).map do |row| + row.attributes.slice('project_id', 'approvals_required') + end + + expect(rows).to match_array(created_rows + existing_rows) + end + end +end diff --git a/ee/spec/migrations/schedule_merge_request_any_approval_rule_migration_spec.rb b/ee/spec/migrations/schedule_merge_request_any_approval_rule_migration_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..7f0a4eece277e6a6f107966c9a556180edf2bba1 --- /dev/null +++ b/ee/spec/migrations/schedule_merge_request_any_approval_rule_migration_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20190905091831_schedule_merge_request_any_approval_rule_migration.rb') + +describe ScheduleMergeRequestAnyApprovalRuleMigration, :migration, :sidekiq do + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:namespace) { namespaces.create(name: 'gitlab', path: 'gitlab-org') } + let(:project) { projects.create(namespace_id: namespace.id, name: 'foo') } + let(:merge_requests) { table(:merge_requests) } + + def create_merge_request(id, options = {}) + default_options = { + id: id, + target_project_id: project.id, + target_branch: 'master', + source_project_id: project.id, + source_branch: 'mr name', + title: "mr name#{id}", + approvals_before_merge: 2 + } + + merge_requests.create!(default_options.merge(options)) + end + + it 'correctly schedules background migrations' do + create_merge_request(1, approvals_before_merge: nil) + create_merge_request(2) + create_merge_request(3, approvals_before_merge: 0) + create_merge_request(4) + create_merge_request(5, approvals_before_merge: 0) + create_merge_request(6) + + stub_const("#{described_class.name}::BATCH_SIZE", 2) + + Sidekiq::Testing.fake! do + Timecop.freeze do + migrate! + + expect(described_class::MIGRATION) + .to be_scheduled_delayed_migration(8.minutes, 2, 4) + + expect(described_class::MIGRATION) + .to be_scheduled_delayed_migration(16.minutes, 6, 6) + + expect(BackgroundMigrationWorker.jobs.size).to eq(2) + end + end + end +end diff --git a/ee/spec/migrations/schedule_project_any_approval_rule_migration_spec.rb b/ee/spec/migrations/schedule_project_any_approval_rule_migration_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..d745701be7b2506b9618ef592e2e7b2c8230ac08 --- /dev/null +++ b/ee/spec/migrations/schedule_project_any_approval_rule_migration_spec.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20190905091812_schedule_project_any_approval_rule_migration.rb') + +describe ScheduleProjectAnyApprovalRuleMigration, :migration, :sidekiq do + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:namespace) { namespaces.create(name: 'gitlab', path: 'gitlab-org') } + + def create_project(id, options = {}) + default_options = { + id: id, + namespace_id: namespace.id, + name: 'foo', + approvals_before_merge: 2 + } + + projects.create(default_options.merge(options)) + end + + it 'correctly schedules background migrations' do + create_project(1, approvals_before_merge: 0) + create_project(2) + create_project(3, approvals_before_merge: 0) + create_project(4) + create_project(5, approvals_before_merge: 0) + create_project(6) + + stub_const("#{described_class.name}::BATCH_SIZE", 2) + + Sidekiq::Testing.fake! do + Timecop.freeze do + migrate! + + expect(described_class::MIGRATION) + .to be_scheduled_delayed_migration(8.minutes, 2, 4) + + expect(described_class::MIGRATION) + .to be_scheduled_delayed_migration(16.minutes, 6, 6) + + expect(BackgroundMigrationWorker.jobs.size).to eq(2) + end + end + end +end diff --git a/ee/spec/models/approval_merge_request_rule_spec.rb b/ee/spec/models/approval_merge_request_rule_spec.rb index f7897d0bdb9b459453059728b4913092834e8627..814af9d9b681706211187a57d51688d426222b76 100644 --- a/ee/spec/models/approval_merge_request_rule_spec.rb +++ b/ee/spec/models/approval_merge_request_rule_spec.rb @@ -88,6 +88,20 @@ expect(rule).not_to be_valid end end + + context 'any_approver rules' do + let(:rule) { build(:approval_merge_request_rule, merge_request: merge_request, rule_type: :any_approver) } + + it 'is valid' do + expect(rule).to be_valid + end + + it 'creating more than one any_approver rule raises an error' do + create(:approval_merge_request_rule, merge_request: merge_request, rule_type: :any_approver) + + expect { rule.save }.to raise_error(ActiveRecord::RecordNotUnique) + end + end end context 'scopes' do diff --git a/ee/spec/models/approval_project_rule_spec.rb b/ee/spec/models/approval_project_rule_spec.rb index fe3c429921c8d7a856c80cfe20c9e3f7f0886741..f132f3b39cc26d029dd95e8059b01e542e619ba5 100644 --- a/ee/spec/models/approval_project_rule_spec.rb +++ b/ee/spec/models/approval_project_rule_spec.rb @@ -123,4 +123,15 @@ end end end + + context 'any_approver rules' do + let(:project) { create(:project) } + let(:rule) { build(:approval_project_rule, project: project, rule_type: :any_approver) } + + it 'creating more than one any_approver rule raises an error' do + create(:approval_project_rule, project: project, rule_type: :any_approver) + + expect { rule.save }.to raise_error(ActiveRecord::RecordNotUnique) + end + end end diff --git a/ee/spec/models/concerns/deprecated_approvals_before_merge_spec.rb b/ee/spec/models/concerns/deprecated_approvals_before_merge_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..b483c5a57f751b752c3b462c4f763555d3ad44a2 --- /dev/null +++ b/ee/spec/models/concerns/deprecated_approvals_before_merge_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe DeprecatedApprovalsBeforeMerge do + shared_examples 'with approvals before merge deprecated' do + context 'updating approvals_before_merge' do + it 'creates any_approver rule' do + subject.update(approvals_before_merge: 3) + + expect_approvals_before_merge_to_be_updated(3) + + subject.update(approvals_before_merge: 5) + + expect_approvals_before_merge_to_be_updated(5) + end + end + end + + context 'merge request' do + subject { create(:merge_request, approvals_before_merge: 2) } + + it_behaves_like 'with approvals before merge deprecated' + end + + context 'project' do + subject { create(:project, approvals_before_merge: 2) } + + it_behaves_like 'with approvals before merge deprecated' + end + + def expect_approvals_before_merge_to_be_updated(value) + expect(subject.approval_rules.any_approver.size).to eq(1) + + rule = subject.approval_rules.any_approver.first + expect(rule.approvals_required).to eq(value) + end +end