FIX: Properly display error when post action fails to create.

This commit is contained in:
Guo Xiang Tan 2018-06-20 21:19:37 +08:00
parent fa43969fe2
commit 0365806b93
5 changed files with 38 additions and 10 deletions

View File

@ -3,6 +3,7 @@ import ActionSummary from "discourse/models/action-summary";
import { MAX_MESSAGE_LENGTH } from "discourse/models/post-action-type"; import { MAX_MESSAGE_LENGTH } from "discourse/models/post-action-type";
import computed from "ember-addons/ember-computed-decorators"; import computed from "ember-addons/ember-computed-decorators";
import optionalService from "discourse/lib/optional-service"; import optionalService from "discourse/lib/optional-service";
import { popupAjaxError } from "discourse/lib/ajax-error";
export default Ember.Controller.extend(ModalFunctionality, { export default Ember.Controller.extend(ModalFunctionality, {
adminTools: optionalService(), adminTools: optionalService(),
@ -163,13 +164,9 @@ export default Ember.Controller.extend(ModalFunctionality, {
id: this.get("model.id") id: this.get("model.id")
}); });
}) })
.catch(errors => { .catch(error => {
this.send("closeModal"); this.send("closeModal");
if (errors && errors.responseText) { popupAjaxError(error);
bootbox.alert($.parseJSON(errors.responseText).errors);
} else {
bootbox.alert(I18n.t("generic_error"));
}
}); });
}, },

View File

@ -26,7 +26,11 @@ class PostActionsController < ApplicationController
args[:take_action] = true if guardian.is_staff? && params[:take_action] == 'true' args[:take_action] = true if guardian.is_staff? && params[:take_action] == 'true'
args[:flag_topic] = true if params[:flag_topic] == 'true' args[:flag_topic] = true if params[:flag_topic] == 'true'
post_action = PostAction.act(current_user, @post, @post_action_type_id, args) begin
post_action = PostAction.act(current_user, @post, @post_action_type_id, args)
rescue PostAction::FailedToCreatePost => e
return render_json_error(e.message)
end
if post_action.blank? || post_action.errors.present? if post_action.blank? || post_action.errors.present?
render_json_error(post_action) render_json_error(post_action)

View File

@ -3,6 +3,7 @@ require_dependency 'system_message'
class PostAction < ActiveRecord::Base class PostAction < ActiveRecord::Base
class AlreadyActed < StandardError; end class AlreadyActed < StandardError; end
class FailedToCreatePost < StandardError; end
include RateLimiter::OnCreateRecord include RateLimiter::OnCreateRecord
include Trashable include Trashable
@ -281,7 +282,12 @@ class PostAction < ActiveRecord::Base
def self.act(user, post, post_action_type_id, opts = {}) def self.act(user, post, post_action_type_id, opts = {})
limit_action!(user, post, post_action_type_id) limit_action!(user, post, post_action_type_id)
related_post_id = create_message_for_post_action(user, post, post_action_type_id, opts) begin
related_post_id = create_message_for_post_action(user, post, post_action_type_id, opts)
rescue ActiveRecord::RecordNotSaved => e
raise FailedToCreatePost.new(e.message)
end
staff_took_action = opts[:take_action] || false staff_took_action = opts[:take_action] || false
targets_topic = targets_topic =

View File

@ -205,7 +205,7 @@ class PostCreator
create create
if !self.errors.full_messages.empty? if !self.errors.full_messages.empty?
raise ActiveRecord::RecordNotSaved.new("Failed to create post: #{self.errors.full_messages}") raise ActiveRecord::RecordNotSaved.new(self.errors.full_messages.to_sentence)
end end
@post @post

View File

@ -87,7 +87,28 @@ RSpec.describe PostActionsController do
post_action_type_id: PostActionType.types[:bookmark] post_action_type_id: PostActionType.types[:bookmark]
} }
expect(response).to be_forbidden expect(response.status).to eq(403)
end
it 'fails when the user tries to notify user that has disabled PM' do
sign_in(Fabricate(:user))
user2 = Fabricate(:user)
post = Fabricate(:post, user: user2)
user2.user_option.update!(allow_private_messages: false)
post "/post_actions.json", params: {
id: post.id,
post_action_type_id: PostActionType.types[:notify_user],
message: 'testing',
flag_topic: false
}
expect(response.status).to eq(422)
expect(JSON.parse(response.body)["errors"].first).to eq(I18n.t(
:not_accepting_pms, username: user2.username
))
end end
describe 'as a moderator' do describe 'as a moderator' do