From 8814f9ed05bca8698284ba794f8e4a33037d1f49 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Wed, 10 Jul 2013 16:52:34 -0400 Subject: [PATCH] Fix a case when a staff user views a topic with a deleted post by a nuked user; might be a temporary solution until we decide what to do with nuked records --- app/models/admin_log.rb | 2 +- app/models/post.rb | 1 + db/migrate/20130710201248_add_nuked_user_to_posts.rb | 5 +++++ lib/admin_logger.rb | 1 + lib/topic_view.rb | 2 +- lib/user_destroyer.rb | 1 + spec/components/admin_logger_spec.rb | 1 + spec/components/user_destroyer_spec.rb | 6 ++++++ 8 files changed, 17 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20130710201248_add_nuked_user_to_posts.rb diff --git a/app/models/admin_log.rb b/app/models/admin_log.rb index 405bac7f7..e98323ddb 100644 --- a/app/models/admin_log.rb +++ b/app/models/admin_log.rb @@ -3,7 +3,7 @@ # Use the AdminLogger class to log records to this table. class AdminLog < ActiveRecord::Base belongs_to :admin, class_name: 'User' - belongs_to :target_user, class_name: 'User' # can be nil + belongs_to :target_user, class_name: 'User' # can be nil, or return nil if user record was nuked validates_presence_of :admin_id validates_presence_of :action diff --git a/app/models/post.rb b/app/models/post.rb index ece65e770..c2d4901f1 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -45,6 +45,7 @@ class Post < ActiveRecord::Base scope :public_posts, -> { joins(:topic).where('topics.archetype <> ?', Archetype.private_message) } scope :private_posts, -> { joins(:topic).where('topics.archetype = ?', Archetype.private_message) } scope :with_topic_subtype, ->(subtype) { joins(:topic).where('topics.subtype = ?', subtype) } + scope :without_nuked_users, -> { where(nuked_user: false) } def self.hidden_reasons @hidden_reasons ||= Enum.new(:flag_threshold_reached, :flag_threshold_reached_again, :new_user_spam_threshold_reached) diff --git a/db/migrate/20130710201248_add_nuked_user_to_posts.rb b/db/migrate/20130710201248_add_nuked_user_to_posts.rb new file mode 100644 index 000000000..6c72dd268 --- /dev/null +++ b/db/migrate/20130710201248_add_nuked_user_to_posts.rb @@ -0,0 +1,5 @@ +class AddNukedUserToPosts < ActiveRecord::Migration + def change + add_column :posts, :nuked_user, :boolean, default: false + end +end diff --git a/lib/admin_logger.rb b/lib/admin_logger.rb index 4adbf90d5..c74dfee38 100644 --- a/lib/admin_logger.rb +++ b/lib/admin_logger.rb @@ -10,6 +10,7 @@ class AdminLogger AdminLog.create( action: AdminLog.actions[:delete_user], admin_id: @admin.id, + target_user_id: deleted_user.id, details: [:id, :username, :name, :created_at, :trust_level, :last_seen_at, :last_emailed_at].map { |x| "#{x}: #{deleted_user.send(x)}" }.join(', ') ) end diff --git a/lib/topic_view.rb b/lib/topic_view.rb index dad6767c1..506b7e8fe 100644 --- a/lib/topic_view.rb +++ b/lib/topic_view.rb @@ -27,7 +27,7 @@ class TopicView @limit = options[:limit] || SiteSetting.posts_per_page; @filtered_posts = @topic.posts - @filtered_posts = @filtered_posts.with_deleted if user.try(:staff?) + @filtered_posts = @filtered_posts.with_deleted.without_nuked_users if user.try(:staff?) @filtered_posts = @filtered_posts.best_of if options[:filter] == 'best_of' @filtered_posts = @filtered_posts.where('posts.post_type <> ?', Post.types[:moderator_action]) if options[:best].present? diff --git a/lib/user_destroyer.rb b/lib/user_destroyer.rb index 72bbb19bf..bd6b4aa93 100644 --- a/lib/user_destroyer.rb +++ b/lib/user_destroyer.rb @@ -19,6 +19,7 @@ class UserDestroyer User.transaction do user.destroy.tap do |u| if u + Post.with_deleted.where(user_id: user.id).update_all("nuked_user = true") AdminLogger.new(@admin).log_user_deletion(user) DiscourseHub.unregister_nickname(user.username) if SiteSetting.call_discourse_hub? MessageBus.publish "/file-change", ["refresh"], user_ids: [user.id] diff --git a/spec/components/admin_logger_spec.rb b/spec/components/admin_logger_spec.rb index f98517e6b..c88f487da 100644 --- a/spec/components/admin_logger_spec.rb +++ b/spec/components/admin_logger_spec.rb @@ -29,6 +29,7 @@ describe AdminLogger do it 'creates a new AdminLog record' do expect { log_user_deletion }.to change { AdminLog.count }.by(1) + AdminLog.last.target_user_id.should == deleted_user.id end end diff --git a/spec/components/user_destroyer_spec.rb b/spec/components/user_destroyer_spec.rb index 0276e8e9f..357e10c79 100644 --- a/spec/components/user_destroyer_spec.rb +++ b/spec/components/user_destroyer_spec.rb @@ -97,6 +97,12 @@ describe UserDestroyer do DiscourseHub.expects(:unregister_nickname).never destroy end + + it "should mark the user's deleted posts as belonging to a nuked user" do + post = Fabricate(:post, user: @user, deleted_at: 1.hour.ago) + expect { destroy }.to change { User.count }.by(-1) + post.reload.nuked_user.should be_true + end end context 'and destroy fails' do