Commit 3e1a1242 authored by Timothy Andrew's avatar Timothy Andrew

Move a user's award emoji to the ghost user

... when the user is destroyed.

1. Normally, for a given awardable and award emoji name, a user is only allowed
   to create a single award emoji.

2. This validation needs to be removed for ghost users, since:

   - User A and User B have created award emoji - with the same name and against
     the same awardable
   - User A is deleted. Their award emoji is moved to the ghost user
   - User B is deleted. Their award emoji needs to be moved to the ghost user.
     However, this breaks the uniqueness validation, since the ghost user is
     only allowed to have one award emoji of a given name for a given awardable
parent 68298754
...@@ -3,13 +3,14 @@ class AwardEmoji < ActiveRecord::Base ...@@ -3,13 +3,14 @@ class AwardEmoji < ActiveRecord::Base
UPVOTE_NAME = "thumbsup".freeze UPVOTE_NAME = "thumbsup".freeze
include Participable include Participable
include GhostUser
belongs_to :awardable, polymorphic: true belongs_to :awardable, polymorphic: true
belongs_to :user belongs_to :user
validates :awardable, :user, presence: true validates :awardable, :user, presence: true
validates :name, presence: true, inclusion: { in: Gitlab::Emoji.emojis_names } 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 participant :user
......
module GhostUser
extend ActiveSupport::Concern
def ghost_user?
user && user.ghost?
end
end
...@@ -23,6 +23,7 @@ module Users::MigrateToGhostUser ...@@ -23,6 +23,7 @@ module Users::MigrateToGhostUser
migrate_merge_requests(user) migrate_merge_requests(user)
migrate_notes(user) migrate_notes(user)
migrate_abuse_reports(user) migrate_abuse_reports(user)
migrate_award_emoji(user)
end end
user.reload user.reload
...@@ -45,4 +46,8 @@ module Users::MigrateToGhostUser ...@@ -45,4 +46,8 @@ module Users::MigrateToGhostUser
def migrate_abuse_reports(user) def migrate_abuse_reports(user)
AbuseReport.where(reporter_id: user.id).update_all(reporter_id: ghost_user.id) AbuseReport.where(reporter_id: user.id).update_all(reporter_id: ghost_user.id)
end end
def migrate_award_emoji(user)
user.award_emoji.update_all(user_id: ghost_user.id)
end
end end
...@@ -25,6 +25,20 @@ describe AwardEmoji, models: true do ...@@ -25,6 +25,20 @@ describe AwardEmoji, models: true do
expect(new_award).not_to be_valid expect(new_award).not_to be_valid
end end
# Assume User A and User B both created award emoji of the same name
# on the same awardable. When User A is deleted, User A's award emoji
# is moved to the ghost user. When User B is deleted, User B's award emoji
# also needs to be moved to the ghost user - this cannot happen unless
# the uniqueness validation is disabled for ghost users.
it "allows duplicate award emoji for ghost users" do
user = create(:user, :ghost)
issue = create(:issue)
create(:award_emoji, user: user, awardable: issue)
new_award = build(:award_emoji, user: user, awardable: issue)
expect(new_award).to be_valid
end
end end
end end
end end
...@@ -166,7 +166,35 @@ describe Users::DestroyService, services: true do ...@@ -166,7 +166,35 @@ describe Users::DestroyService, services: true do
context 'abuse reports' do context 'abuse reports' do
include_examples "migrating a deleted user's associated records to the ghost user", AbuseReport, { skip_assignee_specs: true } do include_examples "migrating a deleted user's associated records to the ghost user", AbuseReport, { skip_assignee_specs: true } do
let(:created_record) { create(:abuse_report, reporter: user, user: create(:user)) } let(:created_record) { create(:abuse_report, reporter: user, user: create(:user)) }
let(:author_method) { :reporter } end
end
context 'award emoji' do
include_examples "migrating a deleted user's associated records to the ghost user", AwardEmoji, { skip_assignee_specs: true } do
let(:created_record) { create(:award_emoji, user: user) }
let(:author_alias) { :user }
context "when the awardable already has an award emoji of the same name assigned to the ghost user" do
let(:awardable) { create(:issue) }
let!(:existing_award_emoji) { create(:award_emoji, user: User.ghost, name: "thumbsup", awardable: awardable) }
let!(:award_emoji) { create(:award_emoji, user: user, name: "thumbsup", awardable: awardable) }
it "migrates the award emoji regardless" do
service.execute(user)
migrated_record = AwardEmoji.find_by_id(award_emoji.id)
expect(migrated_record.user).to eq(User.ghost)
end
it "does not leave the migrated award emoji in an invalid state" do
service.execute(user)
migrated_record = AwardEmoji.find_by_id(award_emoji.id)
expect(migrated_record).to be_valid
end
end
end end
end end
end end
......
...@@ -23,7 +23,11 @@ shared_examples "migrating a deleted user's associated records to the ghost user ...@@ -23,7 +23,11 @@ shared_examples "migrating a deleted user's associated records to the ghost user
migrated_record = record_class.find_by_id(record.id) migrated_record = record_class.find_by_id(record.id)
if migrated_record.respond_to?(:author)
expect(migrated_record.author).to eq(User.ghost) expect(migrated_record.author).to eq(User.ghost)
else
expect(migrated_record.send(author_alias)).to eq(User.ghost)
end
end end
it "blocks the user before migrating #{record_class_name}s to the 'Ghost User'" do it "blocks the user before migrating #{record_class_name}s to the 'Ghost User'" do
......
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