diff --git a/app/assets/javascripts/admin/controllers/admin_flags_controller.js b/app/assets/javascripts/admin/controllers/admin_flags_controller.js index 8218f1734..f65c89cdb 100644 --- a/app/assets/javascripts/admin/controllers/admin_flags_controller.js +++ b/app/assets/javascripts/admin/controllers/admin_flags_controller.js @@ -56,6 +56,16 @@ Discourse.AdminFlagsController = Ember.ArrayController.extend({ }); }, + /** + Deletes a user and all posts and topics created by that user. + + @method deleteSpammer + @param {Discourse.FlaggedPost} item The post to delete + **/ + deleteSpammer: function(item) { + item.get('user').deleteAsSpammer(function() { window.location.reload(); }); + }, + /** Are we viewing the 'old' view? diff --git a/app/assets/javascripts/admin/models/admin_user.js b/app/assets/javascripts/admin/models/admin_user.js index 8f20c1a28..0ec3f8ca5 100644 --- a/app/assets/javascripts/admin/models/admin_user.js +++ b/app/assets/javascripts/admin/models/admin_user.js @@ -264,6 +264,36 @@ Discourse.AdminUser = Discourse.User.extend({ bootbox.dialog(message, buttons, {"classes": "delete-user-modal"}); }, + deleteAsSpammer: function(successCallback) { + var user = this; + var message = I18n.t('flagging.delete_confirm', {posts: user.get('post_count'), topics: user.get('topic_count'), email: user.get('email')}); + var buttons = [{ + "label": I18n.t("composer.cancel"), + "class": "cancel", + "link": true + }, { + "label": I18n.t("flagging.yes_delete_spammer"), + "class": "btn btn-danger", + "callback": function() { + Discourse.ajax("/admin/users/" + user.get('id') + '.json', { + type: 'DELETE', + data: {delete_posts: true, block_email: true} + }).then(function(data) { + if (data.deleted) { + bootbox.alert(I18n.t("admin.user.deleted"), function() { + if (successCallback) successCallback(); + }); + } else { + bootbox.alert(I18n.t("admin.user.delete_failed")); + } + }, function(jqXHR, status, error) { + bootbox.alert(I18n.t("admin.user.delete_failed")); + }); + } + }]; + bootbox.dialog(message, buttons, {"classes": "flagging-delete-spammer"}); + }, + loadDetails: function() { var model = this; if (model.get('loadedDetails')) { return Ember.RSVP.resolve(model); } diff --git a/app/assets/javascripts/admin/models/flagged_post.js b/app/assets/javascripts/admin/models/flagged_post.js index 4acc260d4..05d26e1ff 100644 --- a/app/assets/javascripts/admin/models/flagged_post.js +++ b/app/assets/javascripts/admin/models/flagged_post.js @@ -57,6 +57,14 @@ Discourse.FlaggedPost = Discourse.Post.extend({ return !this.get('topic_visible'); }.property('topic_hidden'), + flaggedForSpam: function() { + return !_.every(this.get('post_actions'), function(action) { return action.name_key !== 'spam'; }); + }.property('post_actions.@each.name_key'), + + canDeleteAsSpammer: function() { + return (Discourse.User.current('staff') && this.get('flaggedForSpam') && this.get('user.can_delete_all_posts') && this.get('user.can_be_deleted')); + }.property('flaggedForSpam'), + deletePost: function() { if (this.get('post_number') === '1') { return Discourse.ajax('/t/' + this.topic_id, { type: 'DELETE', cache: false }); diff --git a/app/assets/javascripts/admin/templates/flags.js.handlebars b/app/assets/javascripts/admin/templates/flags.js.handlebars index 20f3b91f2..2baad82fa 100644 --- a/app/assets/javascripts/admin/templates/flags.js.handlebars +++ b/app/assets/javascripts/admin/templates/flags.js.handlebars @@ -22,17 +22,17 @@ - {{#each flag in content}} - + {{#each flaggedPost in content}} + - {{#if flag.user}}{{#linkTo 'adminUser' flag.user}}{{avatar flag.user imageSize="small"}}{{/linkTo}}{{/if}} + {{#if flaggedPost.user}}{{#linkTo 'adminUser' flaggedPost.user}}{{avatar flaggedPost.user imageSize="small"}}{{/linkTo}}{{/if}} - {{#if flag.topicHidden}} {{/if}}

{{flag.title}}


{{{flag.excerpt}}} + {{#if flaggedPost.topicHidden}} {{/if}}

{{flaggedPost.title}}


{{{flaggedPost.excerpt}}} - {{#each flag.flaggers}} + {{#each flaggedPost.flaggers}} - {{#each flag.messages}} + {{#each flaggedPost.messages}} diff --git a/app/assets/javascripts/discourse/controllers/flag_controller.js b/app/assets/javascripts/discourse/controllers/flag_controller.js index af353a229..cf0b16502 100644 --- a/app/assets/javascripts/discourse/controllers/flag_controller.js +++ b/app/assets/javascripts/discourse/controllers/flag_controller.js @@ -63,8 +63,33 @@ Discourse.FlagController = Discourse.ObjectController.extend(Discourse.ModalFunc }, function(errors) { flagController.displayErrors(errors); }); + }, + + canDeleteSpammer: function() { + if (Discourse.User.current('staff') && this.get('selected.name_key') === 'spam') { + return this.get('userDetails.can_be_deleted') && this.get('userDetails.can_delete_all_posts'); + } else { + return false; + } + }.property('selected.name_key', 'userDetails.can_be_deleted', 'userDetails.can_delete_all_posts'), + + deleteSpammer: function() { + this.send('closeModal'); + this.get('userDetails').deleteAsSpammer(function() { window.location.reload(); }); + }, + + usernameChanged: function() { + this.set('userDetails', null); + this.fetchUserDetails(); + }.observes('username'), + + fetchUserDetails: function() { + if( Discourse.User.current('staff') && this.get('username') ) { + var flagController = this; + Discourse.AdminUser.find(this.get('username')).then(function(user){ + flagController.set('userDetails', user); + }); + } } }); - - diff --git a/app/assets/javascripts/discourse/templates/modal/flag.js.handlebars b/app/assets/javascripts/discourse/templates/modal/flag.js.handlebars index 190a7c64b..0d7b8a9a8 100644 --- a/app/assets/javascripts/discourse/templates/modal/flag.js.handlebars +++ b/app/assets/javascripts/discourse/templates/modal/flag.js.handlebars @@ -26,5 +26,9 @@ {{#if canTakeAction}} {{/if}} + + {{#if canDeleteSpammer}} + + {{/if}} diff --git a/app/assets/stylesheets/application/modal.css.scss b/app/assets/stylesheets/application/modal.css.scss index 45e2da204..62ac58ade 100644 --- a/app/assets/stylesheets/application/modal.css.scss +++ b/app/assets/stylesheets/application/modal.css.scss @@ -316,6 +316,13 @@ } } +.flagging-delete-spammer { + .modal-footer .cancel { + text-decoration: underline; + margin-left: 10px; + } +} + .permission-list{ list-style:none; margin: 0 0 30px; diff --git a/app/controllers/admin/flags_controller.rb b/app/controllers/admin/flags_controller.rb index 0dae74424..4e2bf04b7 100644 --- a/app/controllers/admin/flags_controller.rb +++ b/app/controllers/admin/flags_controller.rb @@ -7,7 +7,7 @@ class Admin::FlagsController < Admin::AdminController if posts.blank? render json: {users: [], posts: []} else - render json: MultiJson.dump({users: serialize_data(users, BasicUserSerializer), posts: posts}) + render json: MultiJson.dump({users: serialize_data(users, AdminDetailedUserSerializer), posts: posts}) end end diff --git a/app/models/post_action.rb b/app/models/post_action.rb index 979941f38..c898d09c4 100644 --- a/app/models/post_action.rb +++ b/app/models/post_action.rb @@ -334,6 +334,7 @@ class PostAction < ActiveRecord::Base post = post_lookup[pa.post_id] post.post_actions ||= [] action = pa.attributes + action[:name_key] = PostActionType.types.key(pa.post_action_type_id) if (pa.related_post && pa.related_post.topic) action.merge!(topic_id: pa.related_post.topic_id, slug: pa.related_post.topic.slug, @@ -345,7 +346,7 @@ class PostAction < ActiveRecord::Base # TODO add serializer so we can skip this posts.map!(&:marshal_dump) - [posts, User.select([:id, :username, :email]).where(id: users.to_a).to_a] + [posts, User.where(id: users.to_a).to_a] end protected diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb index 4908a26db..a3caa70ce 100644 --- a/app/models/site_setting.rb +++ b/app/models/site_setting.rb @@ -235,6 +235,9 @@ class SiteSetting < ActiveRecord::Base client_setting(:relative_date_duration, 14) + setting(:delete_user_max_age, 7) + setting(:delete_all_posts_max, 10) + def self.generate_api_key! self.api_key = SecureRandom.hex(32) end diff --git a/app/serializers/admin_detailed_user_serializer.rb b/app/serializers/admin_detailed_user_serializer.rb index feeaaa052..fad8927db 100644 --- a/app/serializers/admin_detailed_user_serializer.rb +++ b/app/serializers/admin_detailed_user_serializer.rb @@ -8,10 +8,12 @@ class AdminDetailedUserSerializer < AdminUserSerializer :can_impersonate, :like_count, :post_count, + :topic_count, :flags_given_count, :flags_received_count, :private_topics_count, - :can_delete_all_posts + :can_delete_all_posts, + :can_be_deleted has_one :approved_by, serializer: BasicUserSerializer, embed: :objects @@ -35,8 +37,16 @@ class AdminDetailedUserSerializer < AdminUserSerializer scope.can_delete_all_posts?(object) end + def can_be_deleted + scope.can_delete_user?(object) + end + def moderator object.moderator end + def topic_count + object.topics.count + end + end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 2369bfe1b..3a1ce1510 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -925,6 +925,9 @@ en: action: 'Flag Post' take_action: "Take Action" notify_action: 'Notify' + delete_spammer: "Delete Spammer" + delete_confirm: "You are about to delete %{posts} posts and %{topics} topics from this user, remove their account, and add their email address %{email} to a permanent block list. Are you sure this user is really a spammer?" + yes_delete_spammer: "Yes, Delete Spammer" cant: "Sorry, you can't flag this post at this time." custom_placeholder_notify_user: "Why does this post require you to speak to this user directly and privately? Be specific, be constructive, and always be kind." custom_placeholder_notify_moderators: "Why does this post require moderator attention? Let us know specifically what you are concerned about, and provide relevant links where possible." @@ -1072,6 +1075,7 @@ en: disagree_unhide_title: "Remove any flags from this post and make the post visible again" disagree: "Disagree" disagree_title: "Disagree with flag, remove any flags from this post" + delete_spammer_title: "Delete the user and all its posts and topics." flagged_by: "Flagged by" error: "Something went wrong" @@ -1234,8 +1238,8 @@ en: delete: "Delete User" delete_forbidden: "This user can't be deleted because there are posts. Delete all this user's posts first." delete_confirm: "Are you SURE you want to permanently delete this user from the site? This action is permanent!" - delete_and_block: "Yes, and block signups with the same email address" - delete_dont_block: "Yes, and allow signups with the same email address" + delete_and_block: "Yes, and block signups with the same email address" + delete_dont_block: "Yes, and allow signups with the same email address" deleted: "The user was deleted." delete_failed: "There was an error deleting that user. Make sure all posts are deleted before trying to delete the user." send_activation_email: "Send Activation Email" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 6ce5772d5..33739ba48 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -659,6 +659,8 @@ en: minimum_topics_similar: "How many topics need to exist in the database before similar topics are presented." relative_date_duration: "Number of days after posting where post dates will be shown as relative instead of absolute. Examples: relative date: 7d, absolute date: 20 Feb" + delete_user_max_age: "The maximum age of a user, in days, which can be deleted by an admin." + delete_all_posts_max: "The maximum number of posts that can be deleted at once with the Delete All Posts button. If a user has more than this many posts, the posts cannot all be deleted at once and the user can't be deleted." notification_types: diff --git a/lib/guardian.rb b/lib/guardian.rb index 2030a572c..a4d59b694 100644 --- a/lib/guardian.rb +++ b/lib/guardian.rb @@ -151,7 +151,7 @@ class Guardian end def can_delete_user?(user) - can_administer?(user) + user && is_staff? && !user.admin? && user.created_at > SiteSetting.delete_user_max_age.to_i.days.ago end # Can we see who acted on a post in a particular way? @@ -203,7 +203,7 @@ class Guardian end def can_delete_all_posts?(user) - is_staff? && user.created_at >= 7.days.ago + is_staff? && user && !user.admin? && user.created_at >= 7.days.ago && user.post_count <= SiteSetting.delete_all_posts_max.to_i end def can_remove_allowed_users?(topic) diff --git a/lib/user_destroyer.rb b/lib/user_destroyer.rb index 85780d224..8a158e6ea 100644 --- a/lib/user_destroyer.rb +++ b/lib/user_destroyer.rb @@ -3,10 +3,10 @@ class UserDestroyer class PostsExistError < RuntimeError; end - def initialize(admin) - @admin = admin - raise Discourse::InvalidParameters.new('admin is nil') unless @admin and @admin.is_a?(User) - raise Discourse::InvalidAccess unless @admin.admin? + def initialize(staff) + @staff = staff + raise Discourse::InvalidParameters.new('staff user is nil') unless @staff and @staff.is_a?(User) + raise Discourse::InvalidAccess unless @staff.staff? end # Returns false if the user failed to be deleted. @@ -17,7 +17,7 @@ class UserDestroyer User.transaction do if opts[:delete_posts] user.posts.each do |post| - PostDestroyer.new(@admin, post).destroy + PostDestroyer.new(@staff, post).destroy end raise PostsExistError if user.reload.post_count != 0 end @@ -28,7 +28,7 @@ class UserDestroyer b.record_match! if b end Post.with_deleted.where(user_id: user.id).update_all("nuked_user = true") - StaffActionLogger.new(@admin).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? MessageBus.publish "/file-change", ["refresh"], user_ids: [user.id] end diff --git a/spec/components/guardian_spec.rb b/spec/components/guardian_spec.rb index f69e9d0ae..d0b8f2ab8 100644 --- a/spec/components/guardian_spec.rb +++ b/spec/components/guardian_spec.rb @@ -990,7 +990,7 @@ describe Guardian do end - context "can_delete_user?" do + describe "can_delete_user?" do it "is false without a logged in user" do Guardian.new(nil).can_delete_user?(user).should be_false end @@ -1003,19 +1003,83 @@ describe Guardian do Guardian.new(user).can_delete_user?(coding_horror).should be_false end - it "is false for moderators" do - Guardian.new(moderator).can_delete_user?(coding_horror).should be_false + shared_examples "can_delete_user examples" do + let(:deletable_user) { Fabricate.build(:user, created_at: 5.minutes.ago) } + + it "is true if user is not an admin and is not too old" do + Guardian.new(actor).can_delete_user?(deletable_user).should be_true + end + + it "is false if user is an admin" do + Guardian.new(actor).can_delete_user?(another_admin).should be_false + end + + it "is false if user is too old" do + SiteSetting.stubs(:delete_user_max_age).returns(7) + Guardian.new(actor).can_delete_user?(Fabricate(:user, created_at: 8.days.ago)).should be_false + end + end + + context "for moderators" do + let(:actor) { moderator } + include_examples "can_delete_user examples" end context "for admins" do - it "is true if user has posts" do # UserDestroyer is responsible for checking for posts - user.stubs(:post_count).returns(1) - Guardian.new(admin).can_delete_user?(user).should be_true + let(:actor) { admin } + include_examples "can_delete_user examples" + end + end + + describe "can_delete_all_posts?" do + it "is false without a logged in user" do + Guardian.new(nil).can_delete_all_posts?(user).should be_false + end + + it "is false without a user to look at" do + Guardian.new(admin).can_delete_all_posts?(nil).should be_false + end + + it "is false for regular users" do + Guardian.new(user).can_delete_all_posts?(coding_horror).should be_false + end + + shared_examples "can_delete_all_posts examples" do + it "is true if user is newer than 7 days old" do + Guardian.new(actor).can_delete_all_posts?(Fabricate.build(:user, created_at: 6.days.ago)).should be_true end - it "is true if user has no posts" do - Guardian.new(admin).can_delete_user?(user).should be_true + it "is false if user is older than 7 days old" do + Guardian.new(actor).can_delete_all_posts?(Fabricate.build(:user, created_at: 8.days.ago)).should be_false end + + it "is false if user is an admin" do + Guardian.new(actor).can_delete_all_posts?(admin).should be_false + end + + it "is true if number of posts is small" do + u = Fabricate.build(:user, created_at: 1.day.ago) + u.stubs(:post_count).returns(1) + SiteSetting.stubs(:delete_all_posts_max).returns(10) + Guardian.new(actor).can_delete_all_posts?(u).should be_true + end + + it "is false if number of posts is not small" do + u = Fabricate.build(:user, created_at: 1.day.ago) + u.stubs(:post_count).returns(11) + SiteSetting.stubs(:delete_all_posts_max).returns(10) + Guardian.new(actor).can_delete_all_posts?(u).should be_false + end + end + + context "for moderators" do + let(:actor) { moderator } + include_examples "can_delete_all_posts examples" + end + + context "for admins" do + let(:actor) { admin } + include_examples "can_delete_all_posts examples" end end diff --git a/spec/components/user_destroyer_spec.rb b/spec/components/user_destroyer_spec.rb index ffcacdc22..4b23a2847 100644 --- a/spec/components/user_destroyer_spec.rb +++ b/spec/components/user_destroyer_spec.rb @@ -20,8 +20,8 @@ describe UserDestroyer do expect { UserDestroyer.new( Fabricate(:user) ) }.to raise_error(Discourse::InvalidAccess) end - it 'raises an error when user is a moderator' do - expect { UserDestroyer.new( Fabricate(:moderator) ) }.to raise_error(Discourse::InvalidAccess) + it 'returns an instance of UserDestroyer when user is a moderator' do + UserDestroyer.new( Fabricate(:moderator) ).should be_a(UserDestroyer) end it 'returns an instance of UserDestroyer when user is an admin' do diff --git a/test/javascripts/controllers/flag_controller_test.js b/test/javascripts/controllers/flag_controller_test.js new file mode 100644 index 000000000..7b226bcf4 --- /dev/null +++ b/test/javascripts/controllers/flag_controller_test.js @@ -0,0 +1,55 @@ +var buildPost = function(args) { + return Discourse.Post.create(_.merge({ + id: 1, + can_delete: true, + version: 1 + }, args || {})); +}; + +var buildAdminUser = function(args) { + return Discourse.AdminUser.create(_.merge({ + id: 11, + username: 'urist' + }, args || {})); +}; + +module("Discourse.FlagController canDeleteSpammer"); + +test("canDeleteSpammer not staff", function(){ + var flagController = controllerFor('flag', buildPost()); + this.stub(Discourse.User, 'current').returns(false); // Discourse.User.current('staff') returns false + flagController.set('selected', Discourse.PostActionType.create({name_key: 'spam'})); + equal(flagController.get('canDeleteSpammer'), false, 'false if current user is not staff'); +}); + +var canDeleteSpammer = function(test, postActionType, expected, testName) { + test.flagController.set('selected', Discourse.PostActionType.create({name_key: postActionType})); + equal(test.flagController.get('canDeleteSpammer'), expected, testName); +}; + +test("canDeleteSpammer spam not selected", function(){ + this.stub(Discourse.User, 'current').returns(true); // Discourse.User.current('staff') returns true + this.flagController = controllerFor('flag', buildPost()); + this.flagController.set('userDetails', buildAdminUser({can_delete_all_posts: true, can_be_deleted: true})); + canDeleteSpammer(this, 'off_topic', false, 'false if current user is staff, but selected is off_topic'); + canDeleteSpammer(this, 'inappropriate', false, 'false if current user is staff, but selected is inappropriate'); + canDeleteSpammer(this, 'notify_user', false, 'false if current user is staff, but selected is notify_user'); + canDeleteSpammer(this, 'notify_moderators', false, 'false if current user is staff, but selected is notify_moderators'); +}); + +test("canDeleteSpammer spam selected", function(){ + this.stub(Discourse.User, 'current').returns(true); + this.flagController = controllerFor('flag', buildPost()); + + this.flagController.set('userDetails', buildAdminUser({can_delete_all_posts: true, can_be_deleted: true})); + canDeleteSpammer(this, 'spam', true, 'true if current user is staff, selected is spam, posts and user can be deleted'); + + this.flagController.set('userDetails', buildAdminUser({can_delete_all_posts: false, can_be_deleted: true})); + canDeleteSpammer(this, 'spam', false, 'false if current user is staff, selected is spam, posts cannot be deleted'); + + this.flagController.set('userDetails', buildAdminUser({can_delete_all_posts: true, can_be_deleted: false})); + canDeleteSpammer(this, 'spam', false, 'false if current user is staff, selected is spam, user cannot be deleted'); + + this.flagController.set('userDetails', buildAdminUser({can_delete_all_posts: false, can_be_deleted: false})); + canDeleteSpammer(this, 'spam', false, 'false if current user is staff, selected is spam, user cannot be deleted'); +});
{{#linkTo 'adminUser' this.user}}{{avatar this.user imageSize="small"}} {{/linkTo}} @@ -50,7 +50,7 @@
@@ -64,14 +64,19 @@
{{#if adminActiveFlagsView}} - {{#if flag.postHidden}} - - + {{#if flaggedPost.postHidden}} + + {{else}} - - + + {{/if}} - + + {{#if flaggedPost.canDeleteAsSpammer}} + + {{/if}} + + {{/if}}