Commit 2de24b79 authored by Gosia Ksionek's avatar Gosia Ksionek Committed by Imre Farkas

Resolve "Preventing accidental project deletion" - db changes

parent 79038226
---
title: Add migrations and changes for soft-delete for projects
merge_request: 18791
author:
type: added
# frozen_string_literal: true
class AddMarkForDeletionToProjects < ActiveRecord::Migration[5.2]
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
def change
add_column :projects, :marked_for_deletion_at, :date
add_column :projects, :marked_for_deletion_by_user_id, :integer
end
end
# frozen_string_literal: true
class AddMarkForDeletionIndexesToProjects < ActiveRecord::Migration[5.2]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
add_concurrent_foreign_key :projects, :users, column: :marked_for_deletion_by_user_id, on_delete: :nullify
add_concurrent_index :projects, :marked_for_deletion_by_user_id, where: 'marked_for_deletion_by_user_id IS NOT NULL'
end
def down
remove_foreign_key_if_exists :projects, column: :marked_for_deletion_by_user_id
remove_concurrent_index :projects, :marked_for_deletion_by_user_id
end
end
# frozen_string_literal: true
class AddProjectDeletionAdjournedPeriodToApplicationSettings < ActiveRecord::Migration[5.2]
DOWNTIME = false
DEFAULT_NUMBER_OF_DAYS_BEFORE_REMOVAL = 7
def change
add_column :application_settings, :deletion_adjourned_period, :integer, default: DEFAULT_NUMBER_OF_DAYS_BEFORE_REMOVAL, null: false
end
end
...@@ -343,6 +343,7 @@ ActiveRecord::Schema.define(version: 2019_10_26_124116) do ...@@ -343,6 +343,7 @@ ActiveRecord::Schema.define(version: 2019_10_26_124116) do
t.string "custom_http_clone_url_root", limit: 511 t.string "custom_http_clone_url_root", limit: 511
t.boolean "pendo_enabled", default: false, null: false t.boolean "pendo_enabled", default: false, null: false
t.string "pendo_url", limit: 255 t.string "pendo_url", limit: 255
t.integer "deletion_adjourned_period", default: 7, null: false
t.index ["custom_project_templates_group_id"], name: "index_application_settings_on_custom_project_templates_group_id" t.index ["custom_project_templates_group_id"], name: "index_application_settings_on_custom_project_templates_group_id"
t.index ["file_template_project_id"], name: "index_application_settings_on_file_template_project_id" t.index ["file_template_project_id"], name: "index_application_settings_on_file_template_project_id"
t.index ["instance_administration_project_id"], name: "index_applicationsettings_on_instance_administration_project_id" t.index ["instance_administration_project_id"], name: "index_applicationsettings_on_instance_administration_project_id"
...@@ -3073,6 +3074,8 @@ ActiveRecord::Schema.define(version: 2019_10_26_124116) do ...@@ -3073,6 +3074,8 @@ ActiveRecord::Schema.define(version: 2019_10_26_124116) do
t.integer "max_artifacts_size" t.integer "max_artifacts_size"
t.string "pull_mirror_branch_prefix", limit: 50 t.string "pull_mirror_branch_prefix", limit: 50
t.boolean "remove_source_branch_after_merge" t.boolean "remove_source_branch_after_merge"
t.date "marked_for_deletion_at"
t.integer "marked_for_deletion_by_user_id"
t.index "lower((name)::text)", name: "index_projects_on_lower_name" t.index "lower((name)::text)", name: "index_projects_on_lower_name"
t.index ["archived", "pending_delete", "merge_requests_require_code_owner_approval"], name: "projects_requiring_code_owner_approval", where: "((pending_delete = false) AND (archived = false) AND (merge_requests_require_code_owner_approval = true))" t.index ["archived", "pending_delete", "merge_requests_require_code_owner_approval"], name: "projects_requiring_code_owner_approval", where: "((pending_delete = false) AND (archived = false) AND (merge_requests_require_code_owner_approval = true))"
t.index ["created_at"], name: "index_projects_on_created_at" t.index ["created_at"], name: "index_projects_on_created_at"
...@@ -3085,6 +3088,7 @@ ActiveRecord::Schema.define(version: 2019_10_26_124116) do ...@@ -3085,6 +3088,7 @@ ActiveRecord::Schema.define(version: 2019_10_26_124116) do
t.index ["last_repository_check_at"], name: "index_projects_on_last_repository_check_at", where: "(last_repository_check_at IS NOT NULL)" t.index ["last_repository_check_at"], name: "index_projects_on_last_repository_check_at", where: "(last_repository_check_at IS NOT NULL)"
t.index ["last_repository_check_failed"], name: "index_projects_on_last_repository_check_failed" t.index ["last_repository_check_failed"], name: "index_projects_on_last_repository_check_failed"
t.index ["last_repository_updated_at"], name: "index_projects_on_last_repository_updated_at" t.index ["last_repository_updated_at"], name: "index_projects_on_last_repository_updated_at"
t.index ["marked_for_deletion_by_user_id"], name: "index_projects_on_marked_for_deletion_by_user_id", where: "(marked_for_deletion_by_user_id IS NOT NULL)"
t.index ["mirror_last_successful_update_at"], name: "index_projects_on_mirror_last_successful_update_at" t.index ["mirror_last_successful_update_at"], name: "index_projects_on_mirror_last_successful_update_at"
t.index ["mirror_user_id"], name: "index_projects_on_mirror_user_id" t.index ["mirror_user_id"], name: "index_projects_on_mirror_user_id"
t.index ["name"], name: "index_projects_on_name_trigram", opclass: :gin_trgm_ops, using: :gin t.index ["name"], name: "index_projects_on_name_trigram", opclass: :gin_trgm_ops, using: :gin
...@@ -4356,6 +4360,7 @@ ActiveRecord::Schema.define(version: 2019_10_26_124116) do ...@@ -4356,6 +4360,7 @@ ActiveRecord::Schema.define(version: 2019_10_26_124116) do
add_foreign_key "project_statistics", "projects", on_delete: :cascade add_foreign_key "project_statistics", "projects", on_delete: :cascade
add_foreign_key "project_tracing_settings", "projects", on_delete: :cascade add_foreign_key "project_tracing_settings", "projects", on_delete: :cascade
add_foreign_key "projects", "pool_repositories", name: "fk_6e5c14658a", on_delete: :nullify add_foreign_key "projects", "pool_repositories", name: "fk_6e5c14658a", on_delete: :nullify
add_foreign_key "projects", "users", column: "marked_for_deletion_by_user_id", name: "fk_25d8780d11", on_delete: :nullify
add_foreign_key "prometheus_alert_events", "projects", on_delete: :cascade add_foreign_key "prometheus_alert_events", "projects", on_delete: :cascade
add_foreign_key "prometheus_alert_events", "prometheus_alerts", on_delete: :cascade add_foreign_key "prometheus_alert_events", "prometheus_alerts", on_delete: :cascade
add_foreign_key "prometheus_alerts", "environments", on_delete: :cascade add_foreign_key "prometheus_alerts", "environments", on_delete: :cascade
......
...@@ -35,6 +35,7 @@ module EE ...@@ -35,6 +35,7 @@ module EE
if: ->(project) { project.mirror? && project.import_url_updated? } if: ->(project) { project.mirror? && project.import_url_updated? }
belongs_to :mirror_user, foreign_key: 'mirror_user_id', class_name: 'User' belongs_to :mirror_user, foreign_key: 'mirror_user_id', class_name: 'User'
belongs_to :deleting_user, foreign_key: 'marked_for_deletion_by_user_id', class_name: 'User'
has_one :repository_state, class_name: 'ProjectRepositoryState', inverse_of: :project has_one :repository_state, class_name: 'ProjectRepositoryState', inverse_of: :project
has_one :project_registry, class_name: 'Geo::ProjectRegistry', inverse_of: :project has_one :project_registry, class_name: 'Geo::ProjectRegistry', inverse_of: :project
...@@ -132,6 +133,7 @@ module EE ...@@ -132,6 +133,7 @@ module EE
scope :with_slack_service, -> { joins(:slack_service) } scope :with_slack_service, -> { joins(:slack_service) }
scope :with_slack_slash_commands_service, -> { joins(:slack_slash_commands_service) } scope :with_slack_slash_commands_service, -> { joins(:slack_slash_commands_service) }
scope :with_prometheus_service, -> { joins(:prometheus_service) } scope :with_prometheus_service, -> { joins(:prometheus_service) }
scope :aimed_for_deletion, -> (date) { where('marked_for_deletion_at <= ?', date).without_deleted }
delegate :shared_runners_minutes, :shared_runners_seconds, :shared_runners_seconds_last_reset, delegate :shared_runners_minutes, :shared_runners_seconds, :shared_runners_seconds_last_reset,
to: :statistics, allow_nil: true to: :statistics, allow_nil: true
...@@ -655,6 +657,17 @@ module EE ...@@ -655,6 +657,17 @@ module EE
.exists? .exists?
end end
def adjourned_deletion?
feature_available?(:marking_project_for_deletion) &&
::Gitlab::CurrentSettings.deletion_adjourned_period > 0
end
def marked_for_deletion?
return false unless feature_available?(:marking_project_for_deletion)
marked_for_deletion_at.present?
end
private private
def set_override_pull_mirror_available def set_override_pull_mirror_available
......
...@@ -75,6 +75,7 @@ class License < ApplicationRecord ...@@ -75,6 +75,7 @@ class License < ApplicationRecord
issues_analytics issues_analytics
jira_dev_panel_integration jira_dev_panel_integration
ldap_group_sync_filter ldap_group_sync_filter
marking_project_for_deletion
merge_pipelines merge_pipelines
merge_request_performance_metrics merge_request_performance_metrics
merge_trains merge_trains
......
...@@ -22,6 +22,8 @@ describe Project do ...@@ -22,6 +22,8 @@ describe Project do
it { is_expected.to delegate_method(:shared_runners_minutes_limit_enabled?).to(:shared_runners_limit_namespace) } it { is_expected.to delegate_method(:shared_runners_minutes_limit_enabled?).to(:shared_runners_limit_namespace) }
it { is_expected.to delegate_method(:shared_runners_minutes_used?).to(:shared_runners_limit_namespace) } it { is_expected.to delegate_method(:shared_runners_minutes_used?).to(:shared_runners_limit_namespace) }
it { is_expected.to belong_to(:deleting_user) }
it { is_expected.to have_one(:import_state).class_name('ProjectImportState') } it { is_expected.to have_one(:import_state).class_name('ProjectImportState') }
it { is_expected.to have_one(:repository_state).class_name('ProjectRepositoryState').inverse_of(:project) } it { is_expected.to have_one(:repository_state).class_name('ProjectRepositoryState').inverse_of(:project) }
it { is_expected.to have_one(:alerting_setting).class_name('Alerting::ProjectAlertingSetting') } it { is_expected.to have_one(:alerting_setting).class_name('Alerting::ProjectAlertingSetting') }
...@@ -2213,4 +2215,52 @@ describe Project do ...@@ -2213,4 +2215,52 @@ describe Project do
end end
end end
end end
describe '#adjourned_deletion?' do
context 'when marking for deletion feature is available' do
let(:project) { create(:project) }
before do
stub_licensed_features(marking_project_for_deletion: true)
end
context 'when number of days is set to more than 0' do
it 'returns true' do
stub_application_setting(deletion_adjourned_period: 1)
expect(project.adjourned_deletion?).to eq(true)
end
end
context 'when number of days is set to 0' do
it 'returns false' do
stub_application_setting(deletion_adjourned_period: 0)
expect(project.adjourned_deletion?).to eq(false)
end
end
end
context 'when marking for deletion feature is not available' do
let(:project) { create(:project) }
before do
stub_licensed_features(marking_project_for_deletion: false)
end
context 'when number of days is set to more than 0' do
it 'returns false' do
stub_application_setting(deletion_adjourned_period: 1)
expect(project.adjourned_deletion?).to eq(false)
end
end
context 'when number of days is set to 0' do
it 'returns false' do
stub_application_setting(deletion_adjourned_period: 0)
expect(project.adjourned_deletion?).to eq(false)
end
end
end
end
end end
...@@ -148,6 +148,8 @@ excluded_attributes: ...@@ -148,6 +148,8 @@ excluded_attributes:
- :emails_disabled - :emails_disabled
- :max_pages_size - :max_pages_size
- :max_artifacts_size - :max_artifacts_size
- :marked_for_deletion_at
- :marked_for_deletion_by_user_id
namespaces: namespaces:
- :runners_token - :runners_token
- :runners_token_encrypted - :runners_token_encrypted
......
...@@ -423,6 +423,7 @@ project: ...@@ -423,6 +423,7 @@ project:
- alerts_service - alerts_service
- grafana_integration - grafana_integration
- remove_source_branch_after_merge - remove_source_branch_after_merge
- deleting_user
award_emoji: award_emoji:
- awardable - awardable
- user - user
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment