FEATURE: improve error message when double liking (#23698)

If a user somehow is looking at an old version of the page and attempts
to like a post they already like. Display a more reasonable error message.

Previously we would display:

> You are not permitted to view the requested resource.

New error message is:

> Oops! You already performed this action. Can you try refreshing the page?

Triggering this error condition is very tricky, you need to stop the
message bus. A possible reason for it could be bad network connectivity.
This commit is contained in:
Sam 2023-09-28 16:53:48 +10:00 committed by GitHub
parent 76e5a939d4
commit a2da2e02e7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 25 additions and 7 deletions

View File

@ -336,6 +336,8 @@ en:
reading_time: "Reading time" reading_time: "Reading time"
likes: "Likes" likes: "Likes"
action_already_performed: "Oops! You already performed this action. Can you try refreshing the page?"
too_many_replies: too_many_replies:
one: "We're sorry, but new users are temporarily limited to %{count} reply in the same topic." one: "We're sorry, but new users are temporarily limited to %{count} reply in the same topic."
other: "We're sorry, but new users are temporarily limited to %{count} replies in the same topic." other: "We're sorry, but new users are temporarily limited to %{count} replies in the same topic."

View File

@ -80,17 +80,27 @@ class PostActionCreator
@post_action_name, @post_action_name,
opts: { opts: {
is_warning: @is_warning, is_warning: @is_warning,
taken_actions: PostAction.counts_for([@post].compact, @created_by)[@post&.id], taken_actions: taken_actions,
}, },
) )
end end
def taken_actions
return @taken_actions if defined?(@taken_actions)
@taken_actions = PostAction.counts_for([@post].compact, @created_by)[@post&.id]
end
def perform def perform
result = CreateResult.new result = CreateResult.new
if !post_can_act? || (@queue_for_review && !guardian.is_staff?) if !post_can_act? || (@queue_for_review && !guardian.is_staff?)
result.forbidden = true result.forbidden = true
result.add_error(I18n.t("invalid_access"))
if taken_actions&.keys&.include?(PostActionType.types[@post_action_name])
result.add_error(I18n.t("action_already_performed"))
else
result.add_error(I18n.t("invalid_access"))
end
return result return result
end end

View File

@ -61,6 +61,12 @@ RSpec.describe PostActionCreator do
expect(result.post_action.user).to eq(user) expect(result.post_action.user).to eq(user)
expect(result.post_action.post).to eq(post) expect(result.post_action.post).to eq(post)
expect(result.post_action.post_action_type_id).to eq(like_type_id) expect(result.post_action.post_action_type_id).to eq(like_type_id)
# also test double like
result = PostActionCreator.new(user, post, like_type_id).perform
expect(result.success).not_to eq(true)
expect(result.forbidden).to eq(true)
expect(result.errors.full_messages.join).to eq(I18n.t("action_already_performed"))
end end
it "notifies subscribers" do it "notifies subscribers" do
@ -172,7 +178,7 @@ RSpec.describe PostActionCreator do
end end
it "hides the post when the flagger is a TL3 user and the poster is a TL0 user" do it "hides the post when the flagger is a TL3 user and the poster is a TL0 user" do
result = PostActionCreator.create(user, post, :spam) PostActionCreator.create(user, post, :spam)
expect(post.hidden?).to eq(true) expect(post.hidden?).to eq(true)
end end
@ -180,7 +186,7 @@ RSpec.describe PostActionCreator do
it "does not hide the post if the setting is disabled" do it "does not hide the post if the setting is disabled" do
SiteSetting.high_trust_flaggers_auto_hide_posts = false SiteSetting.high_trust_flaggers_auto_hide_posts = false
result = PostActionCreator.create(user, post, :spam) PostActionCreator.create(user, post, :spam)
expect(post.hidden?).to eq(false) expect(post.hidden?).to eq(false)
end end
@ -223,14 +229,14 @@ RSpec.describe PostActionCreator do
it "succeeds with other flag action types" do it "succeeds with other flag action types" do
freeze_time 10.seconds.from_now freeze_time 10.seconds.from_now
spam_result = PostActionCreator.create(user, post, :spam) _spam_result = PostActionCreator.create(user, post, :spam)
expect(reviewable.reload.pending?).to eq(true) expect(reviewable.reload.pending?).to eq(true)
end end
it "fails when other flag action types are open" do it "fails when other flag action types are open" do
freeze_time 10.seconds.from_now freeze_time 10.seconds.from_now
spam_result = PostActionCreator.create(user, post, :spam) _spam_result = PostActionCreator.create(user, post, :spam)
inappropriate_result = PostActionCreator.create(Fabricate(:user), post, :inappropriate) inappropriate_result = PostActionCreator.create(Fabricate(:user), post, :inappropriate)
@ -307,7 +313,7 @@ RSpec.describe PostActionCreator do
it "hides the topic even if it has replies" do it "hides the topic even if it has replies" do
Fabricate(:post, topic: post.topic) Fabricate(:post, topic: post.topic)
result = build_creator.perform _result = build_creator.perform
expect(post.topic.reload.visible?).to eq(false) expect(post.topic.reload.visible?).to eq(false)
end end