Merge branch 'dblessing_migrate_delayed_project_removal' into 'master'

Migrate namespaces delayed_project_removal to namespace_settings [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!53916

app/models/namespace.rb

@@ -82,6 +82,8 @@ class Namespace < ApplicationRecord
   before_destroy(prepend: true) { prepare_for_destroy }
   after_destroy :rm_dir
 
+  before_save :ensure_delayed_project_removal_assigned_to_namespace_settings, if: :delayed_project_removal_changed?
+
   scope :for_user, -> { where('type IS NULL') }
   scope :sort_by_type, -> { order(Gitlab::Database.nulls_first_order(:type)) }
   scope :include_route, -> { includes(:route) }
@@ -408,6 +410,13 @@ class Namespace < ApplicationRecord
 
   private
 
+  def ensure_delayed_project_removal_assigned_to_namespace_settings
+    return if Feature.disabled?(:migrate_delayed_project_removal, default_enabled: true)
+
+    self.namespace_settings || build_namespace_settings
+    namespace_settings.delayed_project_removal = delayed_project_removal
+  end
+
   def all_projects_with_pages
     if all_projects.pages_metadata_not_migrated.exists?
       Gitlab::BackgroundMigration::MigratePagesMetadata.new.perform_on_relation(

app/services/groups/create_service.rb

@@ -33,7 +33,7 @@ module Groups
       Group.transaction do
         if @group.save
           @group.add_owner(current_user)
-          @group.create_namespace_settings
+          @group.create_namespace_settings unless @group.namespace_settings
           Service.create_from_active_default_integrations(@group, :group_id)
           OnboardingProgress.onboard(@group)
         end

changelogs/unreleased/dblessing_migrate_delayed_project_removal.yml

+---
+title: Migrate namespaces delayed_project_removal to namespace_settings
+merge_request: 53916
+author:
+type: changed

config/feature_flags/development/migrate_delayed_project_removal.yml

+---
+name: migrate_delayed_project_removal
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/53916
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/300207
+milestone: '13.9'
+type: development
+group: group::access
+default_enabled: true

db/migrate/20210214201118_add_delayed_project_removal_to_namespace_settings.rb

+# frozen_string_literal: true
+
+class AddDelayedProjectRemovalToNamespaceSettings < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def change
+    add_column :namespace_settings, :delayed_project_removal, :boolean, default: false, null: false
+  end
+end

db/migrate/20210214205155_add_index_to_namespaces_delayed_project_removal.rb

+# frozen_string_literal: true
+
+class AddIndexToNamespacesDelayedProjectRemoval < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  INDEX_NAME = 'tmp_idx_on_namespaces_delayed_project_removal'
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :namespaces, :id, name: INDEX_NAME, where: 'delayed_project_removal = TRUE'
+  end
+
+  def down
+    remove_concurrent_index_by_name :namespaces, INDEX_NAME
+  end
+end

db/post_migrate/20210215095328_migrate_delayed_project_removal_from_namespaces_to_namespace_settings.rb

+# frozen_string_literal: true
+
+class MigrateDelayedProjectRemovalFromNamespacesToNamespaceSettings < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  class Namespace < ActiveRecord::Base
+    self.table_name = 'namespaces'
+
+    include ::EachBatch
+  end
+
+  def up
+    Namespace.select(:id).where(delayed_project_removal: true).each_batch do |batch|
+      values = batch.map { |record| "(#{record.id}, TRUE, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)" }
+
+      execute <<-EOF.strip_heredoc
+        INSERT INTO namespace_settings (namespace_id, delayed_project_removal, created_at, updated_at)
+        VALUES #{values.join(', ')}
+        ON CONFLICT (namespace_id) DO UPDATE
+          SET delayed_project_removal = TRUE
+      EOF
+    end
+  end
+
+  def down
+    # no-op
+  end
+end

db/schema_migrations/20210214201118

+8c1da1c7edba16993da93d9075ad2a3624b8c12ccf73a241e1a166014a99e254
\ No newline at end of file

db/schema_migrations/20210214205155

+7678d97de752e7a9a571d80febc74eb44c699c7b1967690d9a2391036caea5d2
\ No newline at end of file

db/schema_migrations/20210215095328

+25820a3d060826a082565f12a3ac96deafbbde750f5756d71e34d14801ec6148
\ No newline at end of file

db/structure.sql

@@ -14370,6 +14370,7 @@ CREATE TABLE namespace_settings (
     allow_mfa_for_subgroups boolean DEFAULT true NOT NULL,
     default_branch_name text,
     repository_read_only boolean DEFAULT false NOT NULL,
+    delayed_project_removal boolean DEFAULT false NOT NULL,
     CONSTRAINT check_0ba93c78c7 CHECK ((char_length(default_branch_name) <= 255))
 );
 
@@ -23875,6 +23876,8 @@ CREATE UNIQUE INDEX term_agreements_unique_index ON term_agreements USING btree
 
 CREATE INDEX tmp_idx_deduplicate_vulnerability_occurrences ON vulnerability_occurrences USING btree (project_id, report_type, location_fingerprint, primary_identifier_id, id);
 
+CREATE INDEX tmp_idx_on_namespaces_delayed_project_removal ON namespaces USING btree (id) WHERE (delayed_project_removal = true);
+
 CREATE INDEX tmp_index_on_security_findings_scan_id ON security_findings USING btree (scan_id) WHERE (uuid IS NULL);
 
 CREATE INDEX tmp_index_on_vulnerabilities_non_dismissed ON vulnerabilities USING btree (id) WHERE (state <> 2);

spec/factories/groups.rb

@@ -15,7 +15,7 @@ FactoryBot.define do
         raise "Don't set owner for groups, use `group.add_owner(user)` instead"
       end
 
-      create(:namespace_settings, namespace: group)
+      create(:namespace_settings, namespace: group) unless group.namespace_settings
     end
 
     trait :public do

spec/migrations/migrate_delayed_project_removal_from_namespaces_to_namespace_settings_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+require Rails.root.join('db', 'post_migrate', '20210215095328_migrate_delayed_project_removal_from_namespaces_to_namespace_settings.rb')
+
+RSpec.describe MigrateDelayedProjectRemovalFromNamespacesToNamespaceSettings, :migration do
+  let(:namespaces) { table(:namespaces) }
+  let(:namespace_settings) { table(:namespace_settings) }
+
+  let!(:namespace_wo_settings) { namespaces.create!(name: generate(:name), path: generate(:name), delayed_project_removal: true) }
+  let!(:namespace_wo_settings_delay_false) { namespaces.create!(name: generate(:name), path: generate(:name), delayed_project_removal: false) }
+  let!(:namespace_w_settings_delay_true) { namespaces.create!(name: generate(:name), path: generate(:name), delayed_project_removal: true) }
+  let!(:namespace_w_settings_delay_false) { namespaces.create!(name: generate(:name), path: generate(:name), delayed_project_removal: false) }
+
+  let!(:namespace_settings_delay_true) { namespace_settings.create!(namespace_id: namespace_w_settings_delay_true.id, delayed_project_removal: false, created_at: DateTime.now, updated_at: DateTime.now) }
+  let!(:namespace_settings_delay_false) { namespace_settings.create!(namespace_id: namespace_w_settings_delay_false.id, delayed_project_removal: false, created_at: DateTime.now, updated_at: DateTime.now) }
+
+  it 'migrates delayed_project_removal to namespace_settings' do
+    disable_migrations_output { migrate! }
+
+    expect(namespace_settings.count).to eq(3)
+
+    expect(namespace_settings.find_by(namespace_id: namespace_wo_settings.id).delayed_project_removal).to eq(true)
+    expect(namespace_settings.find_by(namespace_id: namespace_wo_settings_delay_false.id)).to be_nil
+
+    expect(namespace_settings_delay_true.reload.delayed_project_removal).to eq(true)
+    expect(namespace_settings_delay_false.reload.delayed_project_removal).to eq(false)
+  end
+end

spec/models/namespace_spec.rb

@@ -6,7 +6,7 @@ RSpec.describe Namespace do
   include ProjectForksHelper
   include GitHelpers
 
-  let!(:namespace) { create(:namespace) }
+  let!(:namespace) { create(:namespace, :with_namespace_settings) }
   let(:gitlab_shell) { Gitlab::Shell.new }
   let(:repository_storage) { 'default' }
 
@@ -116,6 +116,28 @@ RSpec.describe Namespace do
     it { is_expected.to include_module(Namespaces::Traversal::Recursive) }
   end
 
+  describe 'callbacks' do
+    describe 'before_save :ensure_delayed_project_removal_assigned_to_namespace_settings' do
+      it 'sets the matching value in namespace_settings' do
+        expect { namespace.update!(delayed_project_removal: true) }.to change {
+          namespace.namespace_settings.delayed_project_removal
+        }.from(false).to(true)
+      end
+
+      context 'when the feature flag is disabled' do
+        before do
+          stub_feature_flags(migrate_delayed_project_removal: false)
+        end
+
+        it 'does not set the matching value in namespace_settings' do
+          expect { namespace.update!(delayed_project_removal: true) }.not_to change {
+            namespace.namespace_settings.delayed_project_removal
+          }
+        end
+      end
+    end
+  end
+
   describe '#visibility_level_field' do
     it { expect(namespace.visibility_level_field).to eq(:visibility_level) }
   end