diff options
| author | Timothy Andrew <mail@timothyandrew.net> | 2017-04-06 22:33:07 +0530 |
|---|---|---|
| committer | Timothy Andrew <mail@timothyandrew.net> | 2017-04-06 22:39:40 +0530 |
| commit | 1c42505b026d922df50c59d5f9e85073b5f5345f (patch) | |
| tree | b94259c6aa9c4a6e2e24d00d91c4d548a7bd6953 /app/services | |
| parent | e152f3f3daf09b25e5a5952a0e62580b3ef96c50 (diff) | |
| download | gitlab-ce-1c42505b026d922df50c59d5f9e85073b5f5345f.tar.gz | |
Implement review comments from @DouweM for !10467.28695-move-all-associated-records-to-ghost-user
1. Have `MigrateToGhostUser` be a service rather than a mixed-in module, to keep
things explicit. Specs testing the behavior of this class are moved into a
separate service spec file.
2. Add a `user.reported_abuse_reports` association to make the
`migrate_abuse_reports` method more consistent with the other `migrate_`
methods.
Diffstat (limited to 'app/services')
| -rw-r--r-- | app/services/concerns/users/migrate_to_ghost_user.rb | 53 | ||||
| -rw-r--r-- | app/services/users/destroy_service.rb | 4 | ||||
| -rw-r--r-- | app/services/users/migrate_to_ghost_user_service.rb | 59 |
3 files changed, 60 insertions, 56 deletions
diff --git a/app/services/concerns/users/migrate_to_ghost_user.rb b/app/services/concerns/users/migrate_to_ghost_user.rb deleted file mode 100644 index 76335e45e54..00000000000 --- a/app/services/concerns/users/migrate_to_ghost_user.rb +++ /dev/null @@ -1,53 +0,0 @@ -# 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::MigrateToGhostUser - extend ActiveSupport::Concern - - attr_reader :ghost_user - - def move_associated_records_to_ghost_user(user) - # 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(user) - migrate_merge_requests(user) - migrate_notes(user) - migrate_abuse_reports(user) - migrate_award_emoji(user) - end - - user.reload - end - - private - - def migrate_issues(user) - user.issues.update_all(author_id: ghost_user.id) - end - - def migrate_merge_requests(user) - user.merge_requests.update_all(author_id: ghost_user.id) - end - - def migrate_notes(user) - user.notes.update_all(author_id: ghost_user.id) - end - - def migrate_abuse_reports(user) - AbuseReport.where(reporter_id: user.id).update_all(reporter_id: ghost_user.id) - end - - def migrate_award_emoji(user) - user.award_emoji.update_all(user_id: ghost_user.id) - end -end diff --git a/app/services/users/destroy_service.rb b/app/services/users/destroy_service.rb index e6608e316dc..ba58b174cc0 100644 --- a/app/services/users/destroy_service.rb +++ b/app/services/users/destroy_service.rb @@ -1,7 +1,5 @@ module Users class DestroyService - include MigrateToGhostUser - attr_accessor :current_user def initialize(current_user) @@ -28,7 +26,7 @@ module Users ::Projects::DestroyService.new(project, current_user, skip_repo: true).execute end - move_associated_records_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 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 |