diff --git a/app/assets/javascripts/discourse/models/post.js b/app/assets/javascripts/discourse/models/post.js index 1305a33201c..ab1e088d2ab 100644 --- a/app/assets/javascripts/discourse/models/post.js +++ b/app/assets/javascripts/discourse/models/post.js @@ -30,6 +30,7 @@ Discourse.Post = Discourse.Model.extend({ deletedViaTopic: Em.computed.and('firstPost', 'topic.deleted_at'), deleted: Em.computed.or('deleted_at', 'deletedViaTopic'), notDeleted: Em.computed.not('deleted'), + userDeleted: Em.computed.empty('user_id'), postDeletedBy: function() { if (this.get('firstPost')) { return this.get('topic.deleted_by'); } diff --git a/app/assets/javascripts/discourse/templates/post.js.handlebars b/app/assets/javascripts/discourse/templates/post.js.handlebars index 23924a53517..7817cd39a4b 100644 --- a/app/assets/javascripts/discourse/templates/post.js.handlebars +++ b/app/assets/javascripts/discourse/templates/post.js.handlebars @@ -17,11 +17,18 @@ {{/if}}
-
- {{avatar this imageSize="large"}} -

{{breakUp username}}

- {{#if user_title}}
{{user_title}}
{{/if}} -
+ {{#unless userDeleted}} +
+ {{avatar this imageSize="large"}} +

{{breakUp username}}

+ {{#if user_title}}
{{user_title}}
{{/if}} +
+ {{else}} +
+ +

{{i18n user.deleted}}

+
+ {{/unless}}
diff --git a/app/assets/stylesheets/application/topic-post.css.scss b/app/assets/stylesheets/application/topic-post.css.scss index 806e10ae36f..901f23ae056 100644 --- a/app/assets/stylesheets/application/topic-post.css.scss +++ b/app/assets/stylesheets/application/topic-post.css.scss @@ -355,6 +355,10 @@ font-size: 13px; line-height: 18px; } + .deleted-user-avatar { + font-size: 36px; + line-height: 36px; + } .staff a { @include border-radius-all(3px); diff --git a/app/models/post.rb b/app/models/post.rb index 7c76e80c533..c30d4df925b 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -45,7 +45,6 @@ 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) @@ -383,7 +382,7 @@ end # Table name: posts # # id :integer not null, primary key -# user_id :integer not null +# user_id :integer # topic_id :integer not null # post_number :integer not null # raw :text not null @@ -419,7 +418,6 @@ end # notify_user_count :integer default(0), not null # like_score :integer default(0), not null # deleted_by_id :integer -# nuked_user :boolean default(FALSE) # # Indexes # diff --git a/app/models/user.rb b/app/models/user.rb index fb2870798a9..f4b8d9a327c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -13,22 +13,22 @@ class User < ActiveRecord::Base include Roleable has_many :posts - has_many :notifications - has_many :topic_users + has_many :notifications, dependent: :destroy + has_many :topic_users, dependent: :destroy has_many :topics has_many :user_open_ids, dependent: :destroy - has_many :user_actions - has_many :post_actions - has_many :email_logs + has_many :user_actions, dependent: :destroy + has_many :post_actions, dependent: :destroy + has_many :email_logs, dependent: :destroy 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 :email_tokens + has_many :email_tokens, dependent: :destroy has_many :views - has_many :user_visits - has_many :invites - has_many :topic_links - has_many :uploads + has_many :user_visits, dependent: :destroy + has_many :invites, dependent: :destroy + has_many :topic_links, dependent: :destroy + has_many :uploads, dependent: :destroy has_one :facebook_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 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 :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 @@ -61,6 +61,12 @@ class User < ActiveRecord::Base 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 attr_accessor :send_welcome_message diff --git a/app/serializers/basic_post_serializer.rb b/app/serializers/basic_post_serializer.rb index 1fedd6f6f1d..f200193aa40 100644 --- a/app/serializers/basic_post_serializer.rb +++ b/app/serializers/basic_post_serializer.rb @@ -8,15 +8,15 @@ class BasicPostSerializer < ApplicationSerializer :cooked def name - object.user.name + object.user.try(:name) end def username - object.user.username + object.user.try(:username) end def avatar_template - object.user.avatar_template + object.user.try(:avatar_template) end def cooked diff --git a/app/serializers/post_serializer.rb b/app/serializers/post_serializer.rb index 09f4d9bec89..39031cc7a4d 100644 --- a/app/serializers/post_serializer.rb +++ b/app/serializers/post_serializer.rb @@ -46,11 +46,11 @@ class PostSerializer < BasicPostSerializer def moderator? - object.user.moderator? + object.user.try(:moderator?) || false end def staff? - object.user.staff? + object.user.try(:staff?) || false end def yours @@ -70,7 +70,7 @@ class PostSerializer < BasicPostSerializer end def display_username - object.user.name + object.user.try(:name) end def link_counts @@ -101,11 +101,11 @@ class PostSerializer < BasicPostSerializer end def user_title - object.user.title + object.user.try(:title) end def trust_level - object.user.trust_level + object.user.try(:trust_level) end def reply_to_user diff --git a/app/serializers/post_stream_serializer_mixin.rb b/app/serializers/post_stream_serializer_mixin.rb index 64a717adf05..fad70bbebae 100644 --- a/app/serializers/post_stream_serializer_mixin.rb +++ b/app/serializers/post_stream_serializer_mixin.rb @@ -15,15 +15,13 @@ module PostStreamSerializerMixin @highest_number_in_posts = 0 if object.posts.present? 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 - ps = PostSerializer.new(p, scope: scope, root: false) - ps.topic_slug = object.topic.slug - ps.topic_view = object - p.topic = object.topic + @highest_number_in_posts = p.post_number if p.post_number > @highest_number_in_posts + ps = PostSerializer.new(p, scope: scope, root: false) + ps.topic_slug = object.topic.slug + ps.topic_view = object + p.topic = object.topic - @posts << ps.as_json - end + @posts << ps.as_json end end @posts diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index d3b74cd092c..dc26b2cd696 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -200,6 +200,7 @@ en: change: "change" moderator: "{{user}} is a moderator" admin: "{{user}} is an admin" + deleted: "User Was Deleted" messages: all: "All" diff --git a/db/migrate/20130903154323_allow_null_user_id_on_posts.rb b/db/migrate/20130903154323_allow_null_user_id_on_posts.rb new file mode 100644 index 00000000000..8a9141268d5 --- /dev/null +++ b/db/migrate/20130903154323_allow_null_user_id_on_posts.rb @@ -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 diff --git a/lib/topic_view.rb b/lib/topic_view.rb index da30b59d49a..bdece17c8c2 100644 --- a/lib/topic_view.rb +++ b/lib/topic_view.rb @@ -89,7 +89,7 @@ class TopicView def image_url return nil if desired_post.blank? - desired_post.user.small_avatar_url + desired_post.user.try(:small_avatar_url) end def filter_posts(opts = {}) @@ -256,7 +256,7 @@ class TopicView def setup_filtered_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.where('posts.post_type <> ?', Post.types[:moderator_action]) if @best.present? return unless @username_filters.present? diff --git a/lib/user_destroyer.rb b/lib/user_destroyer.rb index 4cd10232c8a..6e21d67e363 100644 --- a/lib/user_destroyer.rb +++ b/lib/user_destroyer.rb @@ -34,7 +34,7 @@ class UserDestroyer b = ScreenedEmail.block(u.email, ip_address: u.ip_address) b.record_match! if b 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)) DiscourseHub.unregister_nickname(user.username) if SiteSetting.call_discourse_hub? MessageBus.publish "/file-change", ["refresh"], user_ids: [user.id] diff --git a/lib/validators/post_validator.rb b/lib/validators/post_validator.rb index f8e14a6df07..e9ae7704c61 100644 --- a/lib/validators/post_validator.rb +++ b/lib/validators/post_validator.rb @@ -13,9 +13,12 @@ class Validators::PostValidator < ActiveModel::Validator end 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? end + if post.new_record? and post.user_id.nil? + post.errors.add(:user_id, :blank, options) + end end def stripped_length(post) diff --git a/spec/components/topic_view_spec.rb b/spec/components/topic_view_spec.rb index 6ee0831c5ae..3b21443d012 100644 --- a/spec/components/topic_view_spec.rb +++ b/spec/components/topic_view_spec.rb @@ -216,6 +216,7 @@ describe TopicView do # Create the posts in a different order than the sort_order let!(:p5) { 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!(:p1) { 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) # 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.save end + p6.user_id = nil # user got nuked + p6.save! end describe '#filter_posts_paged' do @@ -236,6 +239,7 @@ describe TopicView do it 'returns correct posts for all pages' do topic_view.filter_posts_paged(1).should == [p1, p2] topic_view.filter_posts_paged(2).should == [p3, p5] + topic_view.filter_posts_paged(3).should == [] topic_view.filter_posts_paged(100).should == [] end end @@ -271,6 +275,13 @@ describe TopicView do near_view.posts.should == [p2, p3, p4] 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 before { SiteSetting.stubs(:posts_per_page).returns(100) } @@ -278,6 +289,12 @@ describe TopicView do near_view = topic_view_near(p5) near_view.posts.should == [p1, p2, p3, p5] 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 diff --git a/spec/components/user_destroyer_spec.rb b/spec/components/user_destroyer_spec.rb index 01bb4634b5d..907dc7d2eb5 100644 --- a/spec/components/user_destroyer_spec.rb +++ b/spec/components/user_destroyer_spec.rb @@ -121,25 +121,26 @@ describe UserDestroyer do it "deletes the posts" do destroy post.reload.deleted_at.should_not be_nil - post.nuked_user.should be_true + post.user_id.should be_nil 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 'and destroy succeeds' do - let(:destroy_opts) { {} } subject(:destroy) { UserDestroyer.new(@admin).destroy(@user) } include_examples "successfully destroy a user" 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 context 'and destroy fails' do diff --git a/spec/serializers/post_serializer_spec.rb b/spec/serializers/post_serializer_spec.rb new file mode 100644 index 00000000000..d60261cdcfb --- /dev/null +++ b/spec/serializers/post_serializer_spec.rb @@ -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