diff options
author | Douwe Maan <douwe@gitlab.com> | 2017-04-06 21:54:57 +0300 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2017-04-06 21:54:57 +0300 |
commit | bef1aca8837f2306ecbd8ff70deb2204bf1da539 (patch) | |
tree | 8b8e1e2edaca871756f98bfa991aeff0a69bb106 /app | |
parent | 9205caa3361fb38233588c8a18d27288b1e076ee (diff) | |
parent | 1c42505b026d922df50c59d5f9e85073b5f5345f (diff) |
Merge branch '28695-move-all-associated-records-to-ghost-user' into 'master'
Resolve "Deleting a user shouldn't delete associated records"
Closes #28695 and #30514
See merge request !10467
Diffstat (limited to 'app')
-rw-r--r-- | app/models/award_emoji.rb | 3 | ||||
-rw-r--r-- | app/models/concerns/ghost_user.rb | 7 | ||||
-rw-r--r-- | app/models/user.rb | 3 | ||||
-rw-r--r-- | app/services/users/destroy_service.rb | 19 | ||||
-rw-r--r-- | app/services/users/migrate_to_ghost_user_service.rb | 59 |
5 files changed, 71 insertions, 20 deletions
diff --git a/app/models/award_emoji.rb b/app/models/award_emoji.rb index 6937ad3bdd9..6ada6fae4eb 100644 --- a/app/models/award_emoji.rb +++ b/app/models/award_emoji.rb @@ -3,13 +3,14 @@ class AwardEmoji < ActiveRecord::Base UPVOTE_NAME = "thumbsup".freeze include Participable + include GhostUser belongs_to :awardable, polymorphic: true belongs_to :user validates :awardable, :user, presence: true validates :name, presence: true, inclusion: { in: Gitlab::Emoji.emojis_names } - validates :name, uniqueness: { scope: [:user, :awardable_type, :awardable_id] } + validates :name, uniqueness: { scope: [:user, :awardable_type, :awardable_id] }, unless: :ghost_user? participant :user diff --git a/app/models/concerns/ghost_user.rb b/app/models/concerns/ghost_user.rb new file mode 100644 index 00000000000..da696127a80 --- /dev/null +++ b/app/models/concerns/ghost_user.rb @@ -0,0 +1,7 @@ +module GhostUser + extend ActiveSupport::Concern + + def ghost_user? + user && user.ghost? + end +end diff --git a/app/models/user.rb b/app/models/user.rb index c358b1b8d2b..87eeee204f8 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -89,7 +89,8 @@ class User < ActiveRecord::Base has_many :subscriptions, dependent: :destroy has_many :recent_events, -> { order "id DESC" }, foreign_key: :author_id, class_name: "Event" has_many :oauth_applications, class_name: 'Doorkeeper::Application', as: :owner, dependent: :destroy - has_one :abuse_report, dependent: :destroy + has_one :abuse_report, dependent: :destroy, foreign_key: :user_id + has_many :reported_abuse_reports, dependent: :destroy, foreign_key: :reporter_id, class_name: "AbuseReport" has_many :spam_logs, dependent: :destroy has_many :builds, dependent: :nullify, class_name: 'Ci::Build' has_many :pipelines, dependent: :nullify, class_name: 'Ci::Pipeline' diff --git a/app/services/users/destroy_service.rb b/app/services/users/destroy_service.rb index a3b32a71a64..ba58b174cc0 100644 --- a/app/services/users/destroy_service.rb +++ b/app/services/users/destroy_service.rb @@ -26,7 +26,7 @@ module Users ::Projects::DestroyService.new(project, current_user, skip_repo: true).execute end - move_issues_to_ghost_user(user) + MigrateToGhostUserService.new(user).execute # Destroy the namespace after destroying the user since certain methods may depend on the namespace existing namespace = user.namespace @@ -35,22 +35,5 @@ module Users user_data end - - private - - def move_issues_to_ghost_user(user) - # Block the user before moving issues to prevent a data race. - # If the user creates an issue after `move_issues_to_ghost_user` - # runs and before the user is destroyed, the destroy will fail with - # an exception. We block the user so that issues can't be created - # after `move_issues_to_ghost_user` runs and before the destroy happens. - user.block - - ghost_user = User.ghost - - user.issues.update_all(author_id: ghost_user.id) - - user.reload - end end end diff --git a/app/services/users/migrate_to_ghost_user_service.rb b/app/services/users/migrate_to_ghost_user_service.rb new file mode 100644 index 00000000000..1e1ed1791ec --- /dev/null +++ b/app/services/users/migrate_to_ghost_user_service.rb @@ -0,0 +1,59 @@ +# When a user is destroyed, some of their associated records are +# moved to a "Ghost User", to prevent these associated records from +# being destroyed. +# +# For example, all the issues/MRs a user has created are _not_ destroyed +# when the user is destroyed. +module Users + class MigrateToGhostUserService + extend ActiveSupport::Concern + + attr_reader :ghost_user, :user + + def initialize(user) + @user = user + end + + def execute + # Block the user before moving records to prevent a data race. + # For example, if the user creates an issue after `migrate_issues` + # runs and before the user is destroyed, the destroy will fail with + # an exception. + user.block + + user.transaction do + @ghost_user = User.ghost + + migrate_issues + migrate_merge_requests + migrate_notes + migrate_abuse_reports + migrate_award_emoji + end + + user.reload + end + + private + + def migrate_issues + user.issues.update_all(author_id: ghost_user.id) + end + + def migrate_merge_requests + user.merge_requests.update_all(author_id: ghost_user.id) + end + + def migrate_notes + user.notes.update_all(author_id: ghost_user.id) + end + + def migrate_abuse_reports + user.reported_abuse_reports.update_all(reporter_id: ghost_user.id) + end + + def migrate_award_emoji + user.award_emoji.update_all(user_id: ghost_user.id) + end + end +end |