Merge branch...

Merge branch '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!19342

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

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

db/migrate/20191029060358_add_mark_for_deletion_to_namespaces.rb

+# frozen_string_literal: true
+
+class AddMarkForDeletionToNamespaces < ActiveRecord::Migration[5.2]
+  DOWNTIME = false
+
+  def change
+    add_column :namespaces, :marked_for_deletion_at, :date
+    add_column :namespaces, :marked_for_deletion_by_user_id, :integer
+  end
+end

db/migrate/20191029061556_add_mark_for_deletion_indices_to_namespaces.rb

+# frozen_string_literal: true
+
+class AddMarkForDeletionIndicesToNamespaces < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_foreign_key :namespaces, :users, column: :marked_for_deletion_by_user_id, on_delete: :nullify
+    add_concurrent_index :namespaces, :marked_for_deletion_by_user_id, where: 'marked_for_deletion_by_user_id IS NOT NULL'
+    add_concurrent_index :namespaces, :marked_for_deletion_at, where: 'marked_for_deletion_at IS NOT NULL'
+  end
+
+  def down
+    remove_foreign_key_if_exists :namespaces, column: :marked_for_deletion_by_user_id
+    remove_concurrent_index :namespaces, :marked_for_deletion_by_user_id
+    remove_concurrent_index :namespaces, :marked_for_deletion_at
+  end
+end

db/schema.rb

@@ -2492,11 +2492,15 @@ ActiveRecord::Schema.define(version: 2019_11_05_094625) do
     t.boolean "emails_disabled"
     t.integer "max_pages_size"
     t.integer "max_artifacts_size"
+    t.date "marked_for_deletion_at"
+    t.integer "marked_for_deletion_by_user_id"
     t.index ["created_at"], name: "index_namespaces_on_created_at"
     t.index ["custom_project_templates_group_id", "type"], name: "index_namespaces_on_custom_project_templates_group_id_and_type", where: "(custom_project_templates_group_id IS NOT NULL)"
     t.index ["file_template_project_id"], name: "index_namespaces_on_file_template_project_id"
     t.index ["ldap_sync_last_successful_update_at"], name: "index_namespaces_on_ldap_sync_last_successful_update_at"
     t.index ["ldap_sync_last_update_at"], name: "index_namespaces_on_ldap_sync_last_update_at"
+    t.index ["marked_for_deletion_at"], name: "index_namespaces_on_marked_for_deletion_at", where: "(marked_for_deletion_at IS NOT NULL)"
+    t.index ["marked_for_deletion_by_user_id"], name: "index_namespaces_on_marked_for_deletion_by_user_id", where: "(marked_for_deletion_by_user_id IS NOT NULL)"
     t.index ["name", "parent_id"], name: "index_namespaces_on_name_and_parent_id", unique: true
     t.index ["name"], name: "index_namespaces_on_name_trigram", opclass: :gin_trgm_ops, using: :gin
     t.index ["owner_id"], name: "index_namespaces_on_owner_id"
@@ -4363,6 +4367,7 @@ ActiveRecord::Schema.define(version: 2019_11_05_094625) do
   add_foreign_key "namespaces", "namespaces", column: "custom_project_templates_group_id", name: "fk_e7a0b20a6b", on_delete: :nullify
   add_foreign_key "namespaces", "plans", name: "fk_fdd12e5b80", on_delete: :nullify
   add_foreign_key "namespaces", "projects", column: "file_template_project_id", name: "fk_319256d87a", on_delete: :nullify
+  add_foreign_key "namespaces", "users", column: "marked_for_deletion_by_user_id", name: "fk_9ff61b4c22", on_delete: :nullify
   add_foreign_key "note_diff_files", "notes", column: "diff_note_id", on_delete: :cascade
   add_foreign_key "notes", "projects", name: "fk_99e097b079", on_delete: :cascade
   add_foreign_key "notes", "reviews", name: "fk_2e82291620", on_delete: :nullify

ee/app/models/ee/group.rb

@@ -43,6 +43,7 @@ module EE
       has_many :cycle_analytics_stages, class_name: 'Analytics::CycleAnalytics::GroupStage'
 
       belongs_to :file_template_project, class_name: "Project"
+      belongs_to :deleting_user, foreign_key: 'marked_for_deletion_by_user_id', class_name: 'User'
 
       # Use +checked_file_template_project+ instead, which implements important
       # visibility checks
@@ -53,6 +54,8 @@ module EE
 
       validate :custom_project_templates_group_allowed, if: :custom_project_templates_group_id_changed?
 
+      scope :aimed_for_deletion, -> (date) { where('marked_for_deletion_at <= ?', date) }
+
       scope :where_group_links_with_provider, ->(provider) do
         joins(:ldap_group_links).where(ldap_group_links: { provider: provider })
       end

ee/spec/models/group_spec.rb

@@ -17,6 +17,7 @@ describe Group do
     # the presence check works, but since this is a private method that
     # method can't be called with a public_send.
     it { is_expected.to belong_to(:file_template_project).class_name('Project').without_validating_presence }
+    it { is_expected.to belong_to(:deleting_user).class_name('User').with_foreign_key(:marked_for_deletion_by_user_id) }
     it { is_expected.to have_many(:dependency_proxy_blobs) }
     it { is_expected.to have_many(:cycle_analytics_stages) }
     it { is_expected.to have_many(:ip_restrictions) }
@@ -47,6 +48,22 @@ 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, marked_for_deletion_at: nil)
+        group_marked_for_deletion_after_specified_date = create(:group, marked_for_deletion_at: date + 2.days)
+        group_marked_for_deletion_before_specified_date = create(:group, marked_for_deletion_at: date - 2.days)
+        group_marked_for_deletion_on_specified_date = create(:group, marked_for_deletion_at: date)
+
+        expect(subject).to include(group_marked_for_deletion_before_specified_date, group_marked_for_deletion_on_specified_date)
+        expect(subject).not_to include(group_marked_for_deletion_after_specified_date, group_not_marked_for_deletion)
+      end
+    end
   end
 
   describe 'validations' do

lib/gitlab/import_export/import_export.yml

@@ -153,6 +153,8 @@ excluded_attributes:
   namespaces:
     - :runners_token
     - :runners_token_encrypted
+    - :marked_for_deletion_at
+    - :marked_for_deletion_by_user_id
   project_import_state:
     - :last_error
     - :jid