Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Add ability to destroy a user with 0 posts

This commit is contained in:
Neil Lalonde 2013-04-11 16:04:20 -04:00
parent 8014d7fd25
commit 651cfba93f
21 changed files with 412 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
app/assets/javascripts/admin/models/admin_user.js
View File

@ -17,8 +17,11 @@ Discourse.AdminUser = Discourse.Model.extend({
deleteAllPosts: function() {
var user = this;
this.set('can_delete_all_posts', false);
Discourse.ajax(Discourse.getURL("/admin/users/") + (this.get('id')) + "/delete_all_posts", {type: 'PUT'});
Discourse.ajax(Discourse.getURL("/admin/users/") + (this.get('id')) + "/delete_all_posts", {type: 'PUT'}).then(function(result){
user.set('post_count', 0);
});
},
// Revoke the user's admin access
@ -130,6 +133,41 @@ Discourse.AdminUser = Discourse.Model.extend({
bootbox.alert(Em.String.i18n('admin.impersonate.invalid'));
}
});
},
deleteForbidden: function() {
return (this.get('post_count') > 0);
}.property('post_count'),
deleteButtonTitle: function() {
if (this.get('deleteForbidden')) {
return Em.String.i18n('admin.user.delete_forbidden');
} else {
return null;
}
}.property('deleteForbidden'),
destroy: function() {
var user = this;
bootbox.confirm(Em.String.i18n("admin.user.delete_confirm"), Em.String.i18n("no_value"), Em.String.i18n("yes_value"), function(result) {
if(result) {
Discourse.ajax(Discourse.getURL("/admin/users/") + user.get('id') + '.json', { type: 'DELETE' }).then(function(data) {
if (data.deleted) {
bootbox.alert(Em.String.i18n("admin.user.deleted"), function() {
document.location = "/admin/users/list/active";
});
} else {
bootbox.alert(Em.String.i18n("admin.user.delete_failed"));
if (data.user) {
user.mergeAttributes(data.user);
}
}
}, function(jqXHR, status, error) {
Discourse.AdminUser.find( user.get('username') ).then(function(u){ user.mergeAttributes(u); });
bootbox.alert(Em.String.i18n("admin.user.delete_failed"));
});
}
});
}
});
@ -155,7 +193,7 @@ Discourse.AdminUser.reopenClass({
find: function(username) {
return Discourse.ajax({url: Discourse.getURL("/admin/users/") + username}).then(function (result) {
return Discourse.AdminUser.create(result);
})
});
},
findAll: function(query, filter) {

8
app/assets/javascripts/admin/templates/user.js.handlebars
View File

@ -187,3 +187,11 @@
</div>
</section>
<section>
<hr/>
<button class="btn pull-right" {{action destroy target="content"}} {{bindAttr disabled="deleteForbidden"}} {{bindAttr title="deleteButtonTitle"}}>
<i class="icon icon-trash"></i>
{{i18n admin.user.delete}}
</button>
</section>
<div class="clearfix"></div>

34
app/assets/javascripts/discourse/helpers/application_helpers.js
View File

@ -130,24 +130,28 @@ Handlebars.registerHelper('avatar', function(user, options) {
user = Ember.Handlebars.get(this, user, options);
}
var username = Em.get(user, 'username');
if (!username) username = Em.get(user, options.hash.usernamePath);
if( user ) {
var username = Em.get(user, 'username');
if (!username) username = Em.get(user, options.hash.usernamePath);
var avatarTemplate = Ember.get(user, 'avatar_template');
if (!avatarTemplate) avatarTemplate = Em.get(user, 'user.avatar_template');
var avatarTemplate = Ember.get(user, 'avatar_template');
if (!avatarTemplate) avatarTemplate = Em.get(user, 'user.avatar_template');
var title;
if (!options.hash.ignoreTitle) {
title = Em.get(user, 'title') || Em.get(user, 'description');
var title;
if (!options.hash.ignoreTitle) {
title = Em.get(user, 'title') || Em.get(user, 'description');
}
return new Handlebars.SafeString(Discourse.Utilities.avatarImg({
size: options.hash.imageSize,
extraClasses: Em.get(user, 'extras') || options.hash.extraClasses,
username: username,
title: title || username,
avatarTemplate: avatarTemplate
}));
} else {
return '';
}
return new Handlebars.SafeString(Discourse.Utilities.avatarImg({
size: options.hash.imageSize,
extraClasses: Em.get(user, 'extras') || options.hash.extraClasses,
username: username,
title: title || username,
avatarTemplate: avatarTemplate
}));
});
/**

13
app/controllers/admin/users_controller.rb
View File

@ -1,3 +1,5 @@
require_dependency 'user_destroyer'
class Admin::UsersController < Admin::AdminController
def index
@ -96,5 +98,14 @@ class Admin::UsersController < Admin::AdminController
render nothing: true
end
end
def destroy
user = User.where(id: params[:id]).first
guardian.ensure_can_delete_user!(user)
if UserDestroyer.new(current_user).destroy(user)
render json: {deleted: true}
else
render json: {deleted: false, user: AdminDetailedUserSerializer.new(user, root: false).as_json}
end
end
end

16
app/models/admin_log.rb Normal file
View File

@ -0,0 +1,16 @@
# AdminLog stores information about actions that admins and moderators have taken,
# like deleting users, changing site settings, etc.
# Use the AdminLogger class to log records to this table.
class AdminLog < ActiveRecord::Base
attr_accessible :action, :admin_id, :target_user_id, :details
belongs_to :admin, class_name: 'User'
belongs_to :target_user, class_name: 'User' # can be nil
validates_presence_of :admin_id
validates_presence_of :action
def self.actions
@actions ||= Enum.new(:delete_user)
end
end

16
app/models/topic.rb
View File

@ -550,12 +550,16 @@ class Topic < ActiveRecord::Base
@posters_summary << al[last_post_user_id]
end
@posters_summary.map! do |p|
result = TopicPoster.new
result.user = p
result.description = descriptions[p.id].join(', ')
result.extras = "latest" if al[last_post_user_id] == p
result
end
if p
result = TopicPoster.new
result.user = p
result.description = descriptions[p.id].join(', ')
result.extras = "latest" if al[last_post_user_id] == p
result
else
nil
end
end.compact!
@posters_summary
end

8
app/models/user.rb
View File

@ -10,7 +10,7 @@ class User < ActiveRecord::Base
has_many :notifications
has_many :topic_users
has_many :topics
has_many :user_open_ids
has_many :user_open_ids, dependent: :destroy
has_many :user_actions
has_many :post_actions
has_many :email_logs
@ -21,8 +21,8 @@ class User < ActiveRecord::Base
has_many :views
has_many :user_visits
has_many :invites
has_one :twitter_user_info
has_one :github_user_info
has_one :twitter_user_info, dependent: :destroy
has_one :github_user_info, dependent: :destroy
belongs_to :approved_by, class_name: 'User'
validates_presence_of :username
@ -397,7 +397,9 @@ class User < ActiveRecord::Base
posts.order("post_number desc").each do |p|
if p.post_number == 1
p.topic.destroy
# TODO: But the post is not destroyed. Why?
else
# TODO: This should be using the PostDestroyer!
p.destroy
end
end

26
app/serializers/topic_view_serializer.rb
View File

@ -210,21 +210,23 @@ class TopicViewSerializer < ApplicationSerializer
@highest_number_in_posts = 0
if object.posts.present?
object.posts.each_with_index do |p, idx|
@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
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
post_json = ps.as_json
post_json = ps.as_json
if object.index_reverse
post_json[:index] = object.index_offset - idx
else
post_json[:index] = object.index_offset + idx + 1
if object.index_reverse
post_json[:index] = object.index_offset - idx
else
post_json[:index] = object.index_offset + idx + 1
end
@posts << post_json
end
@posts << post_json
end
end
@posts

16
app/views/topics/show.html.erb
View File

@ -4,13 +4,15 @@
<hr/>
<% @topic_view.posts.each do |post| %>
<div class='creator'>
#<%=post.post_number%> <%= t 'by'%>: <b><%= post.user.name %></b>, <%= post.created_at.to_formatted_s(:long_ordinal) %>
</div>
<div class='post'>
<%= post.cooked.html_safe %>
</div>
<hr/>
<% if post.user %>
<div class='creator'>
#<%=post.post_number%> <%= t 'by'%>: <b><%= post.user.name %></b>, <%= post.created_at.to_formatted_s(:long_ordinal) %>
</div>
<div class='post'>
<%= post.cooked.html_safe %>
</div>
<hr/>
<% end %>
<% end %>
<% if @topic_view.next_page %>

24
app/views/topics/show.rss.erb
View File

@ -6,17 +6,19 @@
<description><%= @topic_view.posts.first.raw %></description>
<atom:link href="<%= Discourse.base_url %><%= @topic_view.relative_url %>.rss" rel="self" type="application/rss+xml" />
<% @topic_view.recent_posts.each do |post| %>
<item>
<title><%= @topic_view.title %> at <%= post.created_at %></title>
<description><![CDATA[
<p><%= post.author_readable %> wrote:</p>
<%= post.cooked.html_safe %>
]]></description>
<link><%= Discourse.base_url %><%= post.url %></link>
<pubDate><%= post.created_at.rfc2822 %></pubDate>
<guid><%= Discourse.base_url %><%= post.url %></guid>
<source url="<%= Discourse.base_url %><%= @topic_view.relative_url %>.rss"><%= @topic_view.title %></source>
</item>
<% if post.user %>
<item>
<title><%= @topic_view.title %> at <%= post.created_at %></title>
<description><![CDATA[
<p><%= post.author_readable %> wrote:</p>
<%= post.cooked.html_safe %>
]]></description>
<link><%= Discourse.base_url %><%= post.url %></link>
<pubDate><%= post.created_at.rfc2822 %></pubDate>
<guid><%= Discourse.base_url %><%= post.url %></guid>
<source url="<%= Discourse.base_url %><%= @topic_view.relative_url %>.rss"><%= @topic_view.title %></source>
</item>
<% end %>
<% end %>
</channel>
</rss>

5
config/locales/client.en.yml
View File

@ -926,6 +926,11 @@ en:
approve: 'Approve'
approved_by: "approved by"
time_read: "Read Time"
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!"
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."
site_content:
none: "Choose a type of content to begin editing."

2
config/locales/server.en.yml
View File

@ -886,3 +886,5 @@ en:
user_content_license: |
User contributions are licensed under a [Creative Commons Attribution-NonCommercial-ShareAlike 3.0 Unported License](http://creativecommons.org/licenses/by-nc-sa/3.0/deed.en_US). Without limiting any of those representations or warranties, %{company_short_name} has the right (though not the obligation) to, in %{company_short_name}’s sole discretion (i) refuse or remove any content that, in %{company_short_name}’s reasonable opinion, violates any %{company_short_name} policy or is in any way harmful or objectionable, or (ii) terminate or deny access to and use of the Website to any individual or entity for any reason, in %{company_short_name}’s sole discretion. %{company_short_name} will have no obligation to provide a refund of any amounts previously paid.
miscellaneous: "This Agreement constitutes the entire agreement between %{company_short_name} and you concerning the subject matter hereof, and they may only be modified by a written amendment signed by an authorized executive of %{company_short_name}, or by the posting by %{company_short_name} of a revised version. Except to the extent applicable law, if any, provides otherwise, this Agreement, any access to or use of the Website will be governed by the laws of the state of California, U.S.A., excluding its conflict of law provisions, and the proper venue for any disputes arising out of or relating to any of the same will be the state and federal courts located in San Francisco County, California. Except for claims for injunctive or equitable relief or claims regarding intellectual property rights (which may be brought in any competent court without the posting of a bond), any dispute arising under this Agreement shall be finally settled in accordance with the Comprehensive Arbitration Rules of the Judicial Arbitration and Mediation Service, Inc. (“JAMS”) by three arbitrators appointed in accordance with such Rules. The arbitration shall take place in San Francisco, California, in the English language and the arbitral decision may be enforced in any court. The prevailing party in any action or proceeding to enforce this Agreement shall be entitled to costs and attorneys’ fees. If any part of this Agreement is held invalid or unenforceable, that part will be construed to reflect the parties’ original intent, and the remaining portions will remain in full force and effect. A waiver by either party of any term or condition of this Agreement or any breach thereof, in any one instance, will not waive such term or condition or any subsequent breach thereof. You may assign your rights under this Agreement to any party that consents to, and agrees to be bound by, its terms and conditions; %{company_short_name} may assign its rights under this Agreement without condition. This Agreement will be binding upon and will inure to the benefit of the parties, their successors and permitted assigns."
deleted: 'deleted'

15
db/migrate/20130411205132_create_admin_logs.rb Normal file
View File

@ -0,0 +1,15 @@
class CreateAdminLogs < ActiveRecord::Migration
def up
create_table :admin_logs do |t|
t.integer :action, null: false
t.integer :admin_id, null: false
t.integer :target_user_id
t.text :details
t.timestamps
end
end
def down
drop_table :admin_logs
end
end

16
lib/admin_logger.rb Normal file
View File

@ -0,0 +1,16 @@
# Responsible for logging the actions of admins and moderators.
class AdminLogger
def initialize(admin)
@admin = admin
raise Discourse::InvalidParameters.new('admin is nil') unless @admin and @admin.is_a?(User)
end
def log_user_deletion(deleted_user)
raise Discourse::InvalidParameters.new('user is nil') unless deleted_user and deleted_user.is_a?(User)
AdminLog.create(
action: AdminLog.actions[:delete_user],
admin_id: @admin.id,
details: [:id, :username, :name, :created_at, :trust_level, :last_seen_at, :last_emailed_at].map { |x| "#{x}: #{deleted_user.send(x)}" }.join(', ')
)
end
end

7
lib/guardian.rb
View File

@ -148,6 +148,13 @@ class Guardian
true
end
def can_delete_user?(user_to_delete)
return false unless @user.try(:admin?)
return false if user_to_delete.blank?
return false if user_to_delete.post_count > 0
true
end
# Can we see who acted on a post in a particular way?
def can_see_post_actors?(topic, post_action_type_id)
return false unless topic.present?

27
lib/user_destroyer.rb Normal file
View File

@ -0,0 +1,27 @@
require_dependency 'admin_logger'
# Responsible for destroying a User record
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?
end
# Returns false if the user failed to be deleted.
# Returns a frozen instance of the User if the delete succeeded.
def destroy(user)
raise Discourse::InvalidParameters.new('user is nil') unless user and user.is_a?(User)
raise PostsExistError if user.post_count != 0
user.destroy.tap do |u|
if u
AdminLogger.new(@admin).log_user_deletion(user)
MessageBus.publish "/file-change", ["refresh"], user_ids: [user.id]
end
end
end
end

35
spec/components/admin_logger_spec.rb Normal file
View File

@ -0,0 +1,35 @@
require 'spec_helper'
require_dependency 'admin_logger'
describe AdminLogger do
describe 'new' do
it 'raises an error when user is nil' do
expect { AdminLogger.new(nil) }.to raise_error(Discourse::InvalidParameters)
end
it 'raises an error when user is not a User' do
expect { AdminLogger.new(5) }.to raise_error(Discourse::InvalidParameters)
end
end
describe 'log_user_deletion' do
let(:admin) { Fabricate(:admin) }
let(:deleted_user) { Fabricate(:user) }
subject(:log_user_deletion) { AdminLogger.new(admin).log_user_deletion(deleted_user) }
it 'raises an error when user is nil' do
expect { AdminLogger.new(admin).log_user_deletion(nil) }.to raise_error(Discourse::InvalidParameters)
end
it 'raises an error when user is not a User' do
expect { AdminLogger.new(admin).log_user_deletion(1) }.to raise_error(Discourse::InvalidParameters)
end
it 'creates a new AdminLog record' do
expect { log_user_deletion }.to change { AdminLog.count }.by(1)
end
end
end

29
spec/components/guardian_spec.rb
View File

@ -862,5 +862,34 @@ describe Guardian do
end
context "can_delete_user?" do
it "is false without a logged in user" do
Guardian.new(nil).can_delete_user?(user).should be_false
end
it "is false without a user to look at" do
Guardian.new(admin).can_delete_user?(nil).should be_false
end
it "is false for regular users" 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
end
context "for admins" do
it "is false if user has posts" do
Fabricate(:post, user: user)
Guardian.new(admin).can_delete_user?(user).should be_false
end
it "is true if user has no posts" do
Guardian.new(admin).can_delete_user?(user).should be_true
end
end
end
end

97
spec/components/user_destroyer_spec.rb Normal file
View File

@ -0,0 +1,97 @@
require 'spec_helper'
require_dependency 'user_destroyer'
describe UserDestroyer do
describe 'new' do
it 'raises an error when user is nil' do
expect { UserDestroyer.new(nil) }.to raise_error(Discourse::InvalidParameters)
end
it 'raises an error when user is not a User' do
expect { UserDestroyer.new(5) }.to raise_error(Discourse::InvalidParameters)
end
it 'raises an error when user is a regular user' 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)
end
it 'returns an instance of UserDestroyer when user is an admin' do
UserDestroyer.new( Fabricate(:admin) ).should be_a(UserDestroyer)
end
end
describe 'destroy' do
before do
@admin = Fabricate(:admin)
@user = Fabricate(:user)
end
subject(:destroy) { UserDestroyer.new(@admin).destroy(@user) }
it 'raises an error when user is nil' do
expect { UserDestroyer.new(@admin).destroy(nil) }.to raise_error(Discourse::InvalidParameters)
end
it 'raises an error when user is not a User' do
expect { UserDestroyer.new(@admin).destroy('nothing') }.to raise_error(Discourse::InvalidParameters)
end
context 'user has posts' do
before do
Fabricate(:post, user: @user)
end
it 'should not delete the user' do
expect { destroy rescue nil }.to_not change { User.count }
end
it 'should raise an error' do
expect { destroy }.to raise_error( UserDestroyer::PostsExistError )
end
it 'should not log the action' do
AdminLogger.any_instance.expects(:log_user_deletion).never
destroy rescue nil
end
end
context 'user has no posts' do
context 'and destroy succeeds' do
it 'should delete the user' do
expect { destroy }.to change { User.count }.by(-1)
end
it 'should return the deleted user record' do
return_value = destroy
return_value.should == @user
return_value.should be_destroyed
end
it 'should log the action' do
AdminLogger.any_instance.expects(:log_user_deletion).with(@user).once
destroy
end
end
context 'and destroy fails' do
before do
@user.stubs(:destroy).returns(false)
end
it 'should return false' do
destroy.should == false
end
it 'should not log the action' do
AdminLogger.any_instance.expects(:log_user_deletion).never
destroy
end
end
end
end
end

28
spec/controllers/admin/users_controller_spec.rb
View File

@ -161,6 +161,34 @@ describe Admin::UsersController do
end
end
context '.destroy' do
before do
@delete_me = Fabricate(:user)
end
it "raises an error when the user doesn't have permission" do
Guardian.any_instance.expects(:can_delete_user?).with(@delete_me).returns(false)
xhr :delete, :destroy, id: @delete_me.id
response.should be_forbidden
end
it "returns a 403 if the user doesn't exist" do
xhr :delete, :destroy, id: 123123
response.should be_forbidden
end
it "returns an error if the user has posts" do
Fabricate(:post, user: @delete_me)
xhr :delete, :destroy, id: @delete_me.id
response.should be_forbidden
end
it "deletes the user record" do
UserDestroyer.any_instance.expects(:destroy).returns(true)
xhr :delete, :destroy, id: @delete_me.id
end
end
end
end

5
spec/models/admin_log_spec.rb Normal file
View File

@ -0,0 +1,5 @@
require 'spec_helper'
describe AdminLog do
# Nothing fancy going on in this model. See AdminLogger.
end