UX: improve user delete error message & return correct post count. (#13282)

Post count was incorrect on admin page causing confusion when admins attempted to delete users.
This commit is contained in:
Vinoth Kannan 2021-06-11 10:37:34 +05:30 committed by GitHub
parent 4681c670c0
commit cd6ab7bdd7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 9 additions and 2 deletions

View File

@ -420,7 +420,7 @@ class Admin::UsersController < Admin::AdminController
rescue UserDestroyer::PostsExistError rescue UserDestroyer::PostsExistError
render json: { render json: {
deleted: false, deleted: false,
message: "User #{user.username} has #{user.post_count} posts, so they can't be deleted." message: I18n.t("user.cannot_delete_has_posts", username: user.username, post_count: user.posts.joins(:topic).count),
}, status: 403 }, status: 403
end end
end end

View File

@ -15,7 +15,7 @@ class UserDestroyer
# Returns a frozen instance of the User if the delete succeeded. # Returns a frozen instance of the User if the delete succeeded.
def destroy(user, opts = {}) def destroy(user, opts = {})
raise Discourse::InvalidParameters.new('user is nil') unless user && user.is_a?(User) raise Discourse::InvalidParameters.new('user is nil') unless user && user.is_a?(User)
raise PostsExistError if !opts[:delete_posts] && user.posts.count != 0 raise PostsExistError if !opts[:delete_posts] && user.posts.joins(:topic).count != 0
@guardian.ensure_can_delete_user!(user) @guardian.ensure_can_delete_user!(user)
# default to using a transaction # default to using a transaction

View File

@ -2594,6 +2594,7 @@ en:
email_in_spam_header: "User's first email was flagged as spam" email_in_spam_header: "User's first email was flagged as spam"
already_silenced: "User was already silenced by %{staff} %{time_ago}." already_silenced: "User was already silenced by %{staff} %{time_ago}."
already_suspended: "User was already suspended by %{staff} %{time_ago}." already_suspended: "User was already suspended by %{staff} %{time_ago}."
cannot_delete_has_posts: "User %{username} has %{post_count} posts(s), either public posts or personal messages, so they can't be deleted."
reviewables_reminder: reviewables_reminder:
submitted: submitted:

View File

@ -626,10 +626,16 @@ RSpec.describe Admin::UsersController do
let!(:post) { Fabricate(:post, topic: topic, user: delete_me) } let!(:post) { Fabricate(:post, topic: topic, user: delete_me) }
it "returns an api response that the user can't be deleted because it has posts" do it "returns an api response that the user can't be deleted because it has posts" do
post_count = delete_me.posts.joins(:topic).count
delete_me_topic = Fabricate(:topic)
Fabricate(:post, topic: delete_me_topic, user: delete_me)
PostDestroyer.new(admin, delete_me_topic.first_post, context: "Deleted by admin").destroy
delete "/admin/users/#{delete_me.id}.json" delete "/admin/users/#{delete_me.id}.json"
expect(response.status).to eq(403) expect(response.status).to eq(403)
json = response.parsed_body json = response.parsed_body
expect(json['deleted']).to eq(false) expect(json['deleted']).to eq(false)
expect(json['message']).to eq(I18n.t("user.cannot_delete_has_posts", username: delete_me.username, post_count: post_count))
end end
it "doesn't return an error if delete_posts == true" do it "doesn't return an error if delete_posts == true" do