Change the way nuked users' posts are handled. Allow null in the user_id column of posts. Show these posts in the posts stream.

This commit is contained in:
Neil Lalonde 2013-09-03 17:19:29 -04:00
parent 1a6170a47c
commit 117fc8db58
16 changed files with 123 additions and 50 deletions

View File

@ -30,6 +30,7 @@ Discourse.Post = Discourse.Model.extend({
deletedViaTopic: Em.computed.and('firstPost', 'topic.deleted_at'), deletedViaTopic: Em.computed.and('firstPost', 'topic.deleted_at'),
deleted: Em.computed.or('deleted_at', 'deletedViaTopic'), deleted: Em.computed.or('deleted_at', 'deletedViaTopic'),
notDeleted: Em.computed.not('deleted'), notDeleted: Em.computed.not('deleted'),
userDeleted: Em.computed.empty('user_id'),
postDeletedBy: function() { postDeletedBy: function() {
if (this.get('firstPost')) { return this.get('topic.deleted_by'); } if (this.get('firstPost')) { return this.get('topic.deleted_by'); }

View File

@ -17,11 +17,18 @@
{{/if}} {{/if}}
<div class='topic-meta-data span2'> <div class='topic-meta-data span2'>
{{#unless userDeleted}}
<div {{bindAttr class=":contents byTopicCreator:topic-creator"}}> <div {{bindAttr class=":contents byTopicCreator:topic-creator"}}>
<a href='{{unbound usernameUrl}}'>{{avatar this imageSize="large"}}</a> <a href='{{unbound usernameUrl}}'>{{avatar this imageSize="large"}}</a>
<h3 {{bindAttr class="staff new_user"}}><a href='{{unbound usernameUrl}}'>{{breakUp username}}</a></h3> <h3 {{bindAttr class="staff new_user"}}><a href='{{unbound usernameUrl}}'>{{breakUp username}}</a></h3>
{{#if user_title}}<div class="user-title">{{user_title}}</div>{{/if}} {{#if user_title}}<div class="user-title">{{user_title}}</div>{{/if}}
</div> </div>
{{else}}
<div class="contents">
<i class="icon icon-trash deleted-user-avatar"></i>
<h3 class="deleted-username">{{i18n user.deleted}}</h3>
</div>
{{/unless}}
</div> </div>
<div class='topic-body span14'> <div class='topic-body span14'>

View File

@ -355,6 +355,10 @@
font-size: 13px; font-size: 13px;
line-height: 18px; line-height: 18px;
} }
.deleted-user-avatar {
font-size: 36px;
line-height: 36px;
}
.staff a { .staff a {
@include border-radius-all(3px); @include border-radius-all(3px);

View File

@ -45,7 +45,6 @@ class Post < ActiveRecord::Base
scope :public_posts, -> { joins(:topic).where('topics.archetype <> ?', Archetype.private_message) } scope :public_posts, -> { joins(:topic).where('topics.archetype <> ?', Archetype.private_message) }
scope :private_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 :with_topic_subtype, ->(subtype) { joins(:topic).where('topics.subtype = ?', subtype) }
scope :without_nuked_users, -> { where(nuked_user: false) }
def self.hidden_reasons def self.hidden_reasons
@hidden_reasons ||= Enum.new(:flag_threshold_reached, :flag_threshold_reached_again, :new_user_spam_threshold_reached) @hidden_reasons ||= Enum.new(:flag_threshold_reached, :flag_threshold_reached_again, :new_user_spam_threshold_reached)
@ -383,7 +382,7 @@ end
# Table name: posts # Table name: posts
# #
# id :integer not null, primary key # id :integer not null, primary key
# user_id :integer not null # user_id :integer
# topic_id :integer not null # topic_id :integer not null
# post_number :integer not null # post_number :integer not null
# raw :text not null # raw :text not null
@ -419,7 +418,6 @@ end
# notify_user_count :integer default(0), not null # notify_user_count :integer default(0), not null
# like_score :integer default(0), not null # like_score :integer default(0), not null
# deleted_by_id :integer # deleted_by_id :integer
# nuked_user :boolean default(FALSE)
# #
# Indexes # Indexes
# #

View File

@ -13,22 +13,22 @@ class User < ActiveRecord::Base
include Roleable include Roleable
has_many :posts has_many :posts
has_many :notifications has_many :notifications, dependent: :destroy
has_many :topic_users has_many :topic_users, dependent: :destroy
has_many :topics has_many :topics
has_many :user_open_ids, dependent: :destroy has_many :user_open_ids, dependent: :destroy
has_many :user_actions has_many :user_actions, dependent: :destroy
has_many :post_actions has_many :post_actions, dependent: :destroy
has_many :email_logs has_many :email_logs, dependent: :destroy
has_many :post_timings has_many :post_timings
has_many :topic_allowed_users has_many :topic_allowed_users, dependent: :destroy
has_many :topics_allowed, through: :topic_allowed_users, source: :topic has_many :topics_allowed, through: :topic_allowed_users, source: :topic
has_many :email_tokens has_many :email_tokens, dependent: :destroy
has_many :views has_many :views
has_many :user_visits has_many :user_visits, dependent: :destroy
has_many :invites has_many :invites, dependent: :destroy
has_many :topic_links has_many :topic_links, dependent: :destroy
has_many :uploads has_many :uploads, dependent: :destroy
has_one :facebook_user_info, dependent: :destroy has_one :facebook_user_info, dependent: :destroy
has_one :twitter_user_info, dependent: :destroy has_one :twitter_user_info, dependent: :destroy
@ -37,11 +37,11 @@ class User < ActiveRecord::Base
has_one :oauth2_user_info, dependent: :destroy has_one :oauth2_user_info, dependent: :destroy
belongs_to :approved_by, class_name: 'User' belongs_to :approved_by, class_name: 'User'
has_many :group_users has_many :group_users, dependent: :destroy
has_many :groups, through: :group_users has_many :groups, through: :group_users
has_many :secure_categories, through: :groups, source: :categories has_many :secure_categories, through: :groups, source: :categories
has_one :user_search_data has_one :user_search_data, dependent: :destroy
belongs_to :uploaded_avatar, class_name: 'Upload', dependent: :destroy belongs_to :uploaded_avatar, class_name: 'Upload', dependent: :destroy
@ -61,6 +61,12 @@ class User < ActiveRecord::Base
after_create :create_email_token after_create :create_email_token
before_destroy do
# These tables don't have primary keys, so destroying them with activerecord is tricky:
PostTiming.delete_all(user_id: self.id)
View.delete_all(user_id: self.id)
end
# Whether we need to be sending a system message after creation # Whether we need to be sending a system message after creation
attr_accessor :send_welcome_message attr_accessor :send_welcome_message

View File

@ -8,15 +8,15 @@ class BasicPostSerializer < ApplicationSerializer
:cooked :cooked
def name def name
object.user.name object.user.try(:name)
end end
def username def username
object.user.username object.user.try(:username)
end end
def avatar_template def avatar_template
object.user.avatar_template object.user.try(:avatar_template)
end end
def cooked def cooked

View File

@ -46,11 +46,11 @@ class PostSerializer < BasicPostSerializer
def moderator? def moderator?
object.user.moderator? object.user.try(:moderator?) || false
end end
def staff? def staff?
object.user.staff? object.user.try(:staff?) || false
end end
def yours def yours
@ -70,7 +70,7 @@ class PostSerializer < BasicPostSerializer
end end
def display_username def display_username
object.user.name object.user.try(:name)
end end
def link_counts def link_counts
@ -101,11 +101,11 @@ class PostSerializer < BasicPostSerializer
end end
def user_title def user_title
object.user.title object.user.try(:title)
end end
def trust_level def trust_level
object.user.trust_level object.user.try(:trust_level)
end end
def reply_to_user def reply_to_user

View File

@ -15,7 +15,6 @@ module PostStreamSerializerMixin
@highest_number_in_posts = 0 @highest_number_in_posts = 0
if object.posts.present? if object.posts.present?
object.posts.each_with_index do |p, idx| object.posts.each_with_index do |p, idx|
if p.user
@highest_number_in_posts = p.post_number if p.post_number > @highest_number_in_posts @highest_number_in_posts = p.post_number if p.post_number > @highest_number_in_posts
ps = PostSerializer.new(p, scope: scope, root: false) ps = PostSerializer.new(p, scope: scope, root: false)
ps.topic_slug = object.topic.slug ps.topic_slug = object.topic.slug
@ -25,7 +24,6 @@ module PostStreamSerializerMixin
@posts << ps.as_json @posts << ps.as_json
end end
end end
end
@posts @posts
end end

View File

@ -200,6 +200,7 @@ en:
change: "change" change: "change"
moderator: "{{user}} is a moderator" moderator: "{{user}} is a moderator"
admin: "{{user}} is an admin" admin: "{{user}} is an admin"
deleted: "User Was Deleted"
messages: messages:
all: "All" all: "All"

View File

@ -0,0 +1,12 @@
class AllowNullUserIdOnPosts < ActiveRecord::Migration
def up
change_column :posts, :user_id, :integer, null: true
execute "UPDATE posts SET user_id = NULL WHERE nuked_user = true"
remove_column :posts, :nuked_user
end
def down
add_column :posts, :nuked_user, :boolean, default: false
change_column :posts, :user_id, :integer, null: false
end
end

View File

@ -89,7 +89,7 @@ class TopicView
def image_url def image_url
return nil if desired_post.blank? return nil if desired_post.blank?
desired_post.user.small_avatar_url desired_post.user.try(:small_avatar_url)
end end
def filter_posts(opts = {}) def filter_posts(opts = {})
@ -256,7 +256,7 @@ class TopicView
def setup_filtered_posts def setup_filtered_posts
@filtered_posts = @topic.posts @filtered_posts = @topic.posts
@filtered_posts = @filtered_posts.with_deleted.without_nuked_users if @user.try(:staff?) @filtered_posts = @filtered_posts.with_deleted if @user.try(:staff?)
@filtered_posts = @filtered_posts.best_of if @filter == 'best_of' @filtered_posts = @filtered_posts.best_of if @filter == 'best_of'
@filtered_posts = @filtered_posts.where('posts.post_type <> ?', Post.types[:moderator_action]) if @best.present? @filtered_posts = @filtered_posts.where('posts.post_type <> ?', Post.types[:moderator_action]) if @best.present?
return unless @username_filters.present? return unless @username_filters.present?

View File

@ -34,7 +34,7 @@ class UserDestroyer
b = ScreenedEmail.block(u.email, ip_address: u.ip_address) b = ScreenedEmail.block(u.email, ip_address: u.ip_address)
b.record_match! if b b.record_match! if b
end end
Post.with_deleted.where(user_id: user.id).update_all("nuked_user = true") Post.with_deleted.where(user_id: user.id).update_all("user_id = NULL")
StaffActionLogger.new(@staff).log_user_deletion(user, opts.slice(:context)) StaffActionLogger.new(@staff).log_user_deletion(user, opts.slice(:context))
DiscourseHub.unregister_nickname(user.username) if SiteSetting.call_discourse_hub? DiscourseHub.unregister_nickname(user.username) if SiteSetting.call_discourse_hub?
MessageBus.publish "/file-change", ["refresh"], user_ids: [user.id] MessageBus.publish "/file-change", ["refresh"], user_ids: [user.id]

View File

@ -13,9 +13,12 @@ class Validators::PostValidator < ActiveModel::Validator
end end
def presence(post) def presence(post)
[:raw,:user_id,:topic_id].each do |attr_name| [:raw,:topic_id].each do |attr_name|
post.errors.add(attr_name, :blank, options) if post.send(attr_name).blank? post.errors.add(attr_name, :blank, options) if post.send(attr_name).blank?
end end
if post.new_record? and post.user_id.nil?
post.errors.add(:user_id, :blank, options)
end
end end
def stripped_length(post) def stripped_length(post)

View File

@ -216,6 +216,7 @@ describe TopicView do
# Create the posts in a different order than the sort_order # Create the posts in a different order than the sort_order
let!(:p5) { Fabricate(:post, topic: topic, user: coding_horror)} let!(:p5) { Fabricate(:post, topic: topic, user: coding_horror)}
let!(:p2) { Fabricate(:post, topic: topic, user: coding_horror)} let!(:p2) { Fabricate(:post, topic: topic, user: coding_horror)}
let!(:p6) { Fabricate(:post, topic: topic, user: Fabricate(:user), deleted_at: Time.now)}
let!(:p4) { Fabricate(:post, topic: topic, user: coding_horror, deleted_at: Time.now)} let!(:p4) { Fabricate(:post, topic: topic, user: coding_horror, deleted_at: Time.now)}
let!(:p1) { Fabricate(:post, topic: topic, user: first_poster)} let!(:p1) { Fabricate(:post, topic: topic, user: first_poster)}
let!(:p3) { Fabricate(:post, topic: topic, user: first_poster)} let!(:p3) { Fabricate(:post, topic: topic, user: first_poster)}
@ -224,10 +225,12 @@ describe TopicView do
SiteSetting.stubs(:posts_per_page).returns(3) SiteSetting.stubs(:posts_per_page).returns(3)
# Update them to the sort order we're checking for # Update them to the sort order we're checking for
[p1, p2, p3, p4, p5].each_with_index do |p, idx| [p1, p2, p3, p4, p5, p6].each_with_index do |p, idx|
p.sort_order = idx + 1 p.sort_order = idx + 1
p.save p.save
end end
p6.user_id = nil # user got nuked
p6.save!
end end
describe '#filter_posts_paged' do describe '#filter_posts_paged' do
@ -236,6 +239,7 @@ describe TopicView do
it 'returns correct posts for all pages' do it 'returns correct posts for all pages' do
topic_view.filter_posts_paged(1).should == [p1, p2] topic_view.filter_posts_paged(1).should == [p1, p2]
topic_view.filter_posts_paged(2).should == [p3, p5] topic_view.filter_posts_paged(2).should == [p3, p5]
topic_view.filter_posts_paged(3).should == []
topic_view.filter_posts_paged(100).should == [] topic_view.filter_posts_paged(100).should == []
end end
end end
@ -271,6 +275,13 @@ describe TopicView do
near_view.posts.should == [p2, p3, p4] near_view.posts.should == [p2, p3, p4]
end end
it "returns deleted posts by nuked users to an admin" do
coding_horror.admin = true
near_view = topic_view_near(p5)
near_view.desired_post.should == p5
near_view.posts.should == [p4, p5, p6]
end
context "when 'posts per page' exceeds the number of posts" do context "when 'posts per page' exceeds the number of posts" do
before { SiteSetting.stubs(:posts_per_page).returns(100) } before { SiteSetting.stubs(:posts_per_page).returns(100) }
@ -278,6 +289,12 @@ describe TopicView do
near_view = topic_view_near(p5) near_view = topic_view_near(p5)
near_view.posts.should == [p1, p2, p3, p5] near_view.posts.should == [p1, p2, p3, p5]
end end
it 'returns deleted posts to admins' do
coding_horror.admin = true
near_view = topic_view_near(p5)
near_view.posts.should == [p1, p2, p3, p4, p5, p6]
end
end end
end end
end end

View File

@ -121,25 +121,26 @@ describe UserDestroyer do
it "deletes the posts" do it "deletes the posts" do
destroy destroy
post.reload.deleted_at.should_not be_nil post.reload.deleted_at.should_not be_nil
post.nuked_user.should be_true post.user_id.should be_nil
end end
end end
end end
context 'user has deleted posts' do
let!(:deleted_post) { Fabricate(:post, user: @user, deleted_at: 1.hour.ago) }
it "should mark the user's deleted posts as belonging to a nuked user" do
expect { UserDestroyer.new(@admin).destroy(@user) }.to change { User.count }.by(-1)
deleted_post.reload.user_id.should be_nil
end
end
context 'user has no posts' do context 'user has no posts' do
context 'and destroy succeeds' do context 'and destroy succeeds' do
let(:destroy_opts) { {} } let(:destroy_opts) { {} }
subject(:destroy) { UserDestroyer.new(@admin).destroy(@user) } subject(:destroy) { UserDestroyer.new(@admin).destroy(@user) }
include_examples "successfully destroy a user" include_examples "successfully destroy a user"
include_examples "email block list" include_examples "email block list"
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 end
context 'and destroy fails' do context 'and destroy fails' do

View File

@ -0,0 +1,25 @@
require 'spec_helper'
describe PostSerializer do
context "a post by a nuked user" do
let!(:post) { Fabricate(:post, user: Fabricate(:user), deleted_at: Time.zone.now) }
before do
post.user_id = nil
post.save!
end
subject { PostSerializer.new(post, scope: Guardian.new(Fabricate(:admin)), root: false).as_json }
it "serializes correctly" do
[:name, :username, :display_username, :avatar_template].each do |attr|
subject[attr].should be_nil
end
[:moderator?, :staff?, :yours, :user_title, :trust_level].each do |attr|
subject[attr].should be_false
end
end
end
end