Merge branch 'dblessing_ignore_namespace_delayed_project_removal' into 'master'

Move usage of delayed_project_removal to namespace settings

See merge request gitlab-org/gitlab!56397

app/models/namespace.rb

@@ -15,6 +15,8 @@ class Namespace < ApplicationRecord
   include Namespaces::Traversal::Recursive
   include Namespaces::Traversal::Linear
 
+  ignore_column :delayed_project_removal, remove_with: '14.1', remove_after: '2021-05-22'
+
   # Prevent users from creating unreasonably deep level of nesting.
   # The number 20 was taken based on maximum nesting level of
   # Android repo (15) + some extra backup.
@@ -84,8 +86,6 @@ 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,13 +408,6 @@ 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/models/namespace_setting.rb

@@ -8,7 +8,7 @@ class NamespaceSetting < ApplicationRecord
 
   before_validation :normalize_default_branch_name
 
-  NAMESPACE_SETTINGS_PARAMS = [:default_branch_name].freeze
+  NAMESPACE_SETTINGS_PARAMS = [:default_branch_name, :delayed_project_removal].freeze
 
   self.primary_key = :namespace_id
 

app/services/groups/base_service.rb

@@ -10,6 +10,18 @@ module Groups
 
     private
 
+    def handle_namespace_settings
+      settings_params = params.slice(*::NamespaceSetting::NAMESPACE_SETTINGS_PARAMS)
+
+      return if settings_params.empty?
+
+      ::NamespaceSetting::NAMESPACE_SETTINGS_PARAMS.each do |nsp|
+        params.delete(nsp)
+      end
+
+      ::NamespaceSettings::UpdateService.new(current_user, group, settings_params).execute
+    end
+
     def remove_unallowed_params
       # overridden in EE
     end

app/services/groups/create_service.rb

@@ -11,7 +11,10 @@ module Groups
       remove_unallowed_params
       set_visibility_level
 
-      @group = Group.new(params)
+      @group = Group.new(params.except(*::NamespaceSetting::NAMESPACE_SETTINGS_PARAMS))
+
+      @group.build_namespace_settings
+      handle_namespace_settings
 
       after_build_hook(@group, params)
 
@@ -33,7 +36,6 @@ module Groups
       Group.transaction do
         if @group.save
           @group.add_owner(current_user)
-          @group.create_namespace_settings unless @group.namespace_settings
           Service.create_from_active_default_integrations(@group, :group_id)
           OnboardingProgress.onboard(@group)
         end

app/services/groups/update_service.rb

@@ -46,18 +46,6 @@ module Groups
 
     private
 
-    def handle_namespace_settings
-      settings_params = params.slice(*::NamespaceSetting::NAMESPACE_SETTINGS_PARAMS)
-
-      return if settings_params.empty?
-
-      ::NamespaceSetting::NAMESPACE_SETTINGS_PARAMS.each do |nsp|
-        params.delete(nsp)
-      end
-
-      ::NamespaceSettings::UpdateService.new(current_user, group, settings_params).execute
-    end
-
     def valid_path_change_with_npm_packages?
       return true unless group.packages_feature_enabled?
       return true if params[:path].blank?

changelogs/unreleased/dblessing_ignore_namespace_delayed_project_removal.yml

+---
+title: Move usage of delayed_project_removal to namespace settings
+merge_request: 56397
+author:
+type: changed

ee/app/models/ee/project.rb

@@ -857,7 +857,7 @@ module EE
 
     # Return the group's setting for delayed deletion, false for user namespace projects
     def group_deletion_mode_configured?
-      group && group.delayed_project_removal?
+      group && group.namespace_settings.delayed_project_removal?
     end
   end
 end

ee/app/views/groups/settings/_delayed_project_removal.html.haml

@@ -2,7 +2,7 @@
 
 .form-group.gl-mb-3
   .form-check
-    = f.check_box :delayed_project_removal, checked: group.delayed_project_removal?, class: 'form-check-input'
+    = f.check_box :delayed_project_removal, checked: group.namespace_settings.delayed_project_removal?, class: 'form-check-input'
     = f.label :delayed_project_removal, class: 'form-check-label' do
       %span.gl-display-block= s_('GroupSettings|Enable delayed project removal')
       %span.gl-text-gray-400= s_('GroupSettings|Projects will be permanently deleted after a %{waiting_period}-day delay. This delay can be %{customization_link} in instance settings').html_safe % { waiting_period: ::Gitlab::CurrentSettings.deletion_adjourned_period, customization_link: link_to('customized by an admin', general_admin_application_settings_path) }

ee/spec/controllers/ee/groups_controller_spec.rb

@@ -535,7 +535,7 @@ RSpec.describe GroupsController do
           subject
 
           expect(response).to have_gitlab_http_status(:found)
-          expect(group.reload.delayed_project_removal).to eq(params[:delayed_project_removal])
+          expect(group.reload.namespace_settings.delayed_project_removal).to eq(params[:delayed_project_removal])
         end
       end
 
@@ -546,7 +546,7 @@ RSpec.describe GroupsController do
           subject
 
           expect(response).to have_gitlab_http_status(:found)
-          expect(group.reload.delayed_project_removal).not_to eq(params[:delayed_project_removal])
+          expect(group.reload.namespace_settings.delayed_project_removal).not_to eq(params[:delayed_project_removal])
         end
       end
     end

ee/spec/controllers/projects_controller_spec.rb

@@ -764,7 +764,7 @@ RSpec.describe ProjectsController do
 
       context 'when feature is enabled for group' do
         before do
-          allow(group).to receive(:delayed_project_removal?).and_return(true)
+          allow(group.namespace_settings).to receive(:delayed_project_removal?).and_return(true)
         end
 
         it_behaves_like 'marks project for deletion'
@@ -796,7 +796,7 @@ RSpec.describe ProjectsController do
 
       context 'when feature is disabled for group' do
         before do
-          allow(group).to receive(:delayed_project_removal).and_return(false)
+          allow(group.namespace_settings).to receive(:delayed_project_removal?).and_return(false)
         end
 
         it_behaves_like 'deletes project right away'

ee/spec/models/project_spec.rb

@@ -2709,7 +2709,7 @@ RSpec.describe Project do
       before do
         stub_licensed_features(adjourned_deletion_for_projects_and_groups: licensed?)
         stub_application_setting(deletion_adjourned_period: adjourned_period)
-        allow(group).to receive(:delayed_project_removal?).and_return(feature_enabled_on_group?)
+        allow(group.namespace_settings).to receive(:delayed_project_removal?).and_return(feature_enabled_on_group?)
       end
 
       it { is_expected.to be result }

ee/spec/requests/api/projects_spec.rb

@@ -1064,7 +1064,11 @@ RSpec.describe API::Projects do
       end
 
       context 'delayed project removal is enabled for group' do
-        let(:group) { create(:group, delayed_project_removal: true) }
+        let(:group) { create(:group) }
+
+        before do
+          group.namespace_settings.update(delayed_project_removal: true)
+        end
 
         it_behaves_like 'marks project for deletion'
 

ee/spec/services/groups/create_service_spec.rb

@@ -73,7 +73,7 @@ RSpec.describe Groups::CreateService, '#execute' do
 
         expect(group.shared_runners_minutes_limit).to eq(1000)
         expect(group.extra_shared_runners_minutes_limit).to eq(100)
-        expect(group.delayed_project_removal).to be true
+        expect(group.namespace_settings.delayed_project_removal).to be true
       end
     end
 
@@ -83,7 +83,7 @@ RSpec.describe Groups::CreateService, '#execute' do
 
         expect(group.shared_runners_minutes_limit).to be_nil
         expect(group.extra_shared_runners_minutes_limit).to be_nil
-        expect(group.delayed_project_removal).to be false
+        expect(group.namespace_settings.delayed_project_removal).to be false
       end
     end
   end

spec/models/namespace_spec.rb

@@ -184,28 +184,6 @@ RSpec.describe Namespace do
     end
   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