Merge branch...

Merge branch 'new-33257-prevent-accidental-deletions-via-soft-delete-for-groups-db-changes' into 'master'

Resolve "Prevent accidental deletions via soft delete for groups" - db changes (MR: 1/n)

See merge request gitlab-org/gitlab!20276

changelogs/unreleased/new-33257-prevent-accidental-deletions-via-soft-delete-for-groups-db-chan.yml

+---
+title: Add migrations for 'soft-delete for groups' feature
+merge_request: 20276
+author:
+type: added

db/migrate/20191118053631_add_group_deletion_schedules.rb

+# frozen_string_literal: true
+
+class AddGroupDeletionSchedules < ActiveRecord::Migration[5.2]
+  DOWNTIME = false
+
+  def up
+    create_table :group_deletion_schedules, id: false do |t|
+      t.references :group,
+        foreign_key: { on_delete: :cascade, to_table: :namespaces },
+        default: nil,
+        index: false,
+        primary_key: true
+
+      t.references :user,
+        index: true,
+        foreign_key: { on_delete: :nullify },
+        null: false
+
+      t.date :marked_for_deletion_on,
+        index: true,
+        null: false
+    end
+  end
+
+  def down
+    drop_table :group_deletion_schedules
+  end
+end

db/schema.rb

@@ -1892,6 +1892,13 @@ ActiveRecord::Schema.define(version: 2019_11_19_023952) do
     t.index ["key", "value"], name: "index_group_custom_attributes_on_key_and_value"
   end
 
+  create_table "group_deletion_schedules", primary_key: "group_id", id: :bigint, default: nil, force: :cascade do |t|
+    t.bigint "user_id", null: false
+    t.date "marked_for_deletion_on", null: false
+    t.index ["marked_for_deletion_on"], name: "index_group_deletion_schedules_on_marked_for_deletion_on"
+    t.index ["user_id"], name: "index_group_deletion_schedules_on_user_id"
+  end
+
   create_table "group_group_links", force: :cascade do |t|
     t.datetime_with_timezone "created_at", null: false
     t.datetime_with_timezone "updated_at", null: false
@@ -4413,6 +4420,8 @@ ActiveRecord::Schema.define(version: 2019_11_19_023952) do
   add_foreign_key "gpg_signatures", "projects", on_delete: :cascade
   add_foreign_key "grafana_integrations", "projects", on_delete: :cascade
   add_foreign_key "group_custom_attributes", "namespaces", column: "group_id", on_delete: :cascade
+  add_foreign_key "group_deletion_schedules", "namespaces", column: "group_id", on_delete: :cascade
+  add_foreign_key "group_deletion_schedules", "users", on_delete: :nullify
   add_foreign_key "group_group_links", "namespaces", column: "shared_group_id", on_delete: :cascade
   add_foreign_key "group_group_links", "namespaces", column: "shared_with_group_id", on_delete: :cascade
   add_foreign_key "identities", "saml_providers", name: "fk_aade90f0fc", on_delete: :cascade

ee/app/models/ee/group.rb

@@ -42,6 +42,9 @@ module EE
       has_many :managed_users, class_name: 'User', foreign_key: 'managing_group_id', inverse_of: :managing_group
       has_many :cycle_analytics_stages, class_name: 'Analytics::CycleAnalytics::GroupStage'
 
+      has_one :deletion_schedule, class_name: 'GroupDeletionSchedule'
+      delegate :deleting_user, :marked_for_deletion_on, to: :deletion_schedule, allow_nil: true
+
       belongs_to :file_template_project, class_name: "Project"
 
       # Use +checked_file_template_project+ instead, which implements important
@@ -53,6 +56,8 @@ module EE
 
       validate :custom_project_templates_group_allowed, if: :custom_project_templates_group_id_changed?
 
+      scope :aimed_for_deletion, -> (date) { joins(:deletion_schedule).where('group_deletion_schedules.marked_for_deletion_on <= ?', date) }
+
       scope :where_group_links_with_provider, ->(provider) do
         joins(:ldap_group_links).where(ldap_group_links: { provider: provider })
       end

ee/app/models/group_deletion_schedule.rb

+# frozen_string_literal: true
+
+class GroupDeletionSchedule < ApplicationRecord
+  belongs_to :group
+  belongs_to :deleting_user, foreign_key: 'user_id', class_name: 'User'
+end

ee/spec/factories/group_deletion_schedules.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :group_deletion_schedule do
+    association :group, factory: :group
+    association :deleting_user, factory: :user
+    marked_for_deletion_on { nil }
+  end
+end

ee/spec/factories/groups.rb

@@ -37,4 +37,19 @@ FactoryBot.define do
       end
     end
   end
+
+  factory :group_with_deletion_schedule, parent: :group do
+    transient do
+      deleting_user { create(:user) }
+      marked_for_deletion_on { nil }
+    end
+
+    after(:create) do |group, evaluator|
+      create(:group_deletion_schedule,
+        group: group,
+        deleting_user: evaluator.deleting_user,
+        marked_for_deletion_on: evaluator.marked_for_deletion_on
+      )
+    end
+  end
 end

ee/spec/models/group_deletion_schedule_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe GroupDeletionSchedule do
+  describe 'Associations' do
+    it { is_expected.to belong_to :group }
+    it { is_expected.to belong_to(:deleting_user).class_name('User').with_foreign_key('user_id') }
+  end
+end

ee/spec/models/group_spec.rb

@@ -21,6 +21,7 @@ describe Group do
     it { is_expected.to have_many(:cycle_analytics_stages) }
     it { is_expected.to have_many(:ip_restrictions) }
     it { is_expected.to have_one(:dependency_proxy_setting) }
+    it { is_expected.to have_one(:deletion_schedule) }
   end
 
   describe 'scopes' do
@@ -47,6 +48,30 @@ describe Group do
         expect(group.checked_file_template_project).to be_present
       end
     end
+
+    describe '.aimed_for_deletion' do
+      let!(:date) { 10.days.ago }
+
+      subject(:relation) { described_class.aimed_for_deletion(date) }
+
+      it 'only includes groups that are marked for deletion on or before the specified date' do
+        group_not_marked_for_deletion = create(:group)
+
+        group_marked_for_deletion_after_specified_date = create(:group_with_deletion_schedule,
+                                                                marked_for_deletion_on: date + 2.days)
+
+        group_marked_for_deletion_before_specified_date = create(:group_with_deletion_schedule,
+                                                                 marked_for_deletion_on: date - 2.days)
+
+        group_marked_for_deletion_on_specified_date = create(:group_with_deletion_schedule,
+                                                             marked_for_deletion_on: date)
+
+        expect(relation).to include(group_marked_for_deletion_before_specified_date,
+                                    group_marked_for_deletion_on_specified_date)
+        expect(relation).not_to include(group_marked_for_deletion_after_specified_date,
+                                        group_not_marked_for_deletion)
+      end
+    end
   end
 
   describe 'validations' do