UX: Update post actions to "Approve Post" and "Reject Post"

This should be more clear.
This commit is contained in:
Robin Ward 2019-04-23 12:18:39 -04:00
parent c99faa6b25
commit 6f56fba016
10 changed files with 44 additions and 28 deletions

View File

@ -37,9 +37,9 @@ class QueuedPostsController < ApplicationController
state = update_params[:state] state = update_params[:state]
begin begin
if state == 'approved' if state == 'approved'
reviewable.perform(current_user, :approve) reviewable.perform(current_user, :approve_post)
elsif state == 'rejected' elsif state == 'rejected'
reviewable.perform(current_user, :reject) reviewable.perform(current_user, :reject_post)
if update_params[:delete_user] == 'true' && guardian.can_delete_user?(reviewable.created_by) if update_params[:delete_user] == 'true' && guardian.can_delete_user?(reviewable.created_by)
UserDestroyer.new(current_user).destroy(reviewable.created_by, user_deletion_opts) UserDestroyer.new(current_user).destroy(reviewable.created_by, user_deletion_opts)
end end

View File

@ -16,7 +16,7 @@ module Jobs
if reviewable.is_a?(ReviewableFlaggedPost) if reviewable.is_a?(ReviewableFlaggedPost)
reviewable.perform(Discourse.system_user, :ignore) reviewable.perform(Discourse.system_user, :ignore)
elsif reviewable.is_a?(ReviewableQueuedPost) elsif reviewable.is_a?(ReviewableQueuedPost)
reviewable.perform(Discourse.system_user, :reject) reviewable.perform(Discourse.system_user, :reject_post)
elsif reviewable.is_a?(ReviewableUser) elsif reviewable.is_a?(ReviewableUser)
reviewable.perform(Discourse.system_user, :reject_user_delete) reviewable.perform(Discourse.system_user, :reject_user_delete)
end end

View File

@ -10,8 +10,20 @@ class ReviewableQueuedPost < Reviewable
def build_actions(actions, guardian, args) def build_actions(actions, guardian, args)
if guardian.is_staff? if guardian.is_staff?
actions.add(:approve) unless approved?
actions.add(:reject) unless rejected? unless approved?
actions.add(:approve_post) do |a|
a.icon = 'check'
a.label = "reviewables.actions.approve_post.title"
end
end
unless rejected?
actions.add(:reject_post) do |a|
a.icon = 'times'
a.label = "reviewables.actions.reject_post.title"
end
end
if pending? && guardian.can_delete_user?(created_by) if pending? && guardian.can_delete_user?(created_by)
actions.add(:delete_user) do |action| actions.add(:delete_user) do |action|
@ -46,7 +58,7 @@ class ReviewableQueuedPost < Reviewable
result result
end end
def perform_approve(performed_by, args) def perform_approve_post(performed_by, args)
created_post = nil created_post = nil
creator = PostCreator.new(created_by, create_options.merge( creator = PostCreator.new(created_by, create_options.merge(
@ -83,7 +95,7 @@ class ReviewableQueuedPost < Reviewable
create_result(:success, :approved) { |result| result.created_post = created_post } create_result(:success, :approved) { |result| result.created_post = created_post }
end end
def perform_reject(performed_by, args) def perform_reject_post(performed_by, args)
# Backwards compatibility, new code should listen for `reviewable_transitioned_to` # Backwards compatibility, new code should listen for `reviewable_transitioned_to`
DiscourseEvent.trigger(:rejected_post, self) DiscourseEvent.trigger(:rejected_post, self)

View File

@ -4477,6 +4477,10 @@ en:
title: "Ignore" title: "Ignore"
approve: approve:
title: "Approve" title: "Approve"
approve_post:
title: "Approve Post"
reject_post:
title: "Reject Post"
approve_user: approve_user:
title: "Approve User" title: "Approve User"
reject_user: reject_user:

View File

@ -294,7 +294,7 @@ describe NewPostManager do
expect(Reviewable.list_for(Discourse.system_user).count).to eq(1) expect(Reviewable.list_for(Discourse.system_user).count).to eq(1)
expect(@counter).to be(0) expect(@counter).to be(0)
reviewable.perform(Discourse.system_user, :approve) reviewable.perform(Discourse.system_user, :approve_post)
manager = NewPostManager.new( manager = NewPostManager.new(
topic.user, topic.user,

View File

@ -35,16 +35,16 @@ RSpec.describe ReviewableQueuedPost, type: :model do
context "actions" do context "actions" do
context "approve" do context "approve_post" do
it 'triggers an extensibility event' do it 'triggers an extensibility event' do
event = DiscourseEvent.track(:approved_post) { reviewable.perform(moderator, :approve) } event = DiscourseEvent.track(:approved_post) { reviewable.perform(moderator, :approve_post) }
expect(event).to be_present expect(event).to be_present
expect(event[:params].first).to eq(reviewable) expect(event[:params].first).to eq(reviewable)
end end
it "creates a post" do it "creates a post" do
topic_count, post_count = Topic.count, Post.count topic_count, post_count = Topic.count, Post.count
result = reviewable.perform(moderator, :approve) result = reviewable.perform(moderator, :approve_post)
expect(result.success?).to eq(true) expect(result.success?).to eq(true)
expect(result.created_post).to be_present expect(result.created_post).to be_present
expect(result.created_post).to be_valid expect(result.created_post).to be_valid
@ -64,12 +64,12 @@ RSpec.describe ReviewableQueuedPost, type: :model do
expect(notifications).to be_present expect(notifications).to be_present
# We can't approve twice # We can't approve twice
expect(-> { reviewable.perform(moderator, :approve) }).to raise_error(Reviewable::InvalidAction) expect(-> { reviewable.perform(moderator, :approve_post) }).to raise_error(Reviewable::InvalidAction)
end end
it "skips validations" do it "skips validations" do
reviewable.payload['raw'] = 'x' reviewable.payload['raw'] = 'x'
result = reviewable.perform(moderator, :approve) result = reviewable.perform(moderator, :approve_post)
expect(result.created_post).to be_present expect(result.created_post).to be_present
end end
@ -82,28 +82,28 @@ RSpec.describe ReviewableQueuedPost, type: :model do
SiteSetting.num_users_to_silence_new_user = 1 SiteSetting.num_users_to_silence_new_user = 1
expect(Guardian.new(newuser).can_create_post?(topic)).to eq(false) expect(Guardian.new(newuser).can_create_post?(topic)).to eq(false)
result = reviewable.perform(moderator, :approve) result = reviewable.perform(moderator, :approve_post)
expect(result.success?).to eq(true) expect(result.success?).to eq(true)
end end
end end
context "reject" do context "reject_post" do
it 'triggers an extensibility event' do it 'triggers an extensibility event' do
event = DiscourseEvent.track(:rejected_post) { reviewable.perform(moderator, :reject) } event = DiscourseEvent.track(:rejected_post) { reviewable.perform(moderator, :reject_post) }
expect(event).to be_present expect(event).to be_present
expect(event[:params].first).to eq(reviewable) expect(event[:params].first).to eq(reviewable)
end end
it "doesn't create a post" do it "doesn't create a post" do
post_count = Post.count post_count = Post.count
result = reviewable.perform(moderator, :reject) result = reviewable.perform(moderator, :reject_post)
expect(result.success?).to eq(true) expect(result.success?).to eq(true)
expect(result.created_post).to be_nil expect(result.created_post).to be_nil
expect(Post.count).to eq(post_count) expect(Post.count).to eq(post_count)
# We can't reject twice # We can't reject twice
expect(-> { reviewable.perform(moderator, :reject) }).to raise_error(Reviewable::InvalidAction) expect(-> { reviewable.perform(moderator, :reject_post) }).to raise_error(Reviewable::InvalidAction)
end end
end end
@ -147,7 +147,7 @@ RSpec.describe ReviewableQueuedPost, type: :model do
it "creates the post and topic when approved" do it "creates the post and topic when approved" do
topic_count, post_count = Topic.count, Post.count topic_count, post_count = Topic.count, Post.count
result = reviewable.perform(moderator, :approve) result = reviewable.perform(moderator, :approve_post)
expect(result.success?).to eq(true) expect(result.success?).to eq(true)
expect(result.created_post).to be_present expect(result.created_post).to be_present
@ -163,7 +163,7 @@ RSpec.describe ReviewableQueuedPost, type: :model do
it "creates the post and topic when rejected" do it "creates the post and topic when rejected" do
topic_count, post_count = Topic.count, Post.count topic_count, post_count = Topic.count, Post.count
result = reviewable.perform(moderator, :reject) result = reviewable.perform(moderator, :reject_post)
expect(result.success?).to eq(true) expect(result.success?).to eq(true)
expect(result.created_post).to be_blank expect(result.created_post).to be_blank

View File

@ -204,25 +204,25 @@ RSpec.describe Reviewable, type: :model do
it "triggers a notification on pending -> approve" do it "triggers a notification on pending -> approve" do
reviewable = Fabricate(:reviewable_queued_post) reviewable = Fabricate(:reviewable_queued_post)
Jobs.expects(:enqueue).with(:notify_reviewable, has_key(:reviewable_id)) Jobs.expects(:enqueue).with(:notify_reviewable, has_key(:reviewable_id))
reviewable.perform(moderator, :approve) reviewable.perform(moderator, :approve_post)
end end
it "triggers a notification on pending -> reject" do it "triggers a notification on pending -> reject" do
reviewable = Fabricate(:reviewable_queued_post) reviewable = Fabricate(:reviewable_queued_post)
Jobs.expects(:enqueue).with(:notify_reviewable, has_key(:reviewable_id)) Jobs.expects(:enqueue).with(:notify_reviewable, has_key(:reviewable_id))
reviewable.perform(moderator, :reject) reviewable.perform(moderator, :reject_post)
end end
it "doesn't trigger a notification on approve -> reject" do it "doesn't trigger a notification on approve -> reject" do
reviewable = Fabricate(:reviewable_queued_post, status: Reviewable.statuses[:approved]) reviewable = Fabricate(:reviewable_queued_post, status: Reviewable.statuses[:approved])
Jobs.expects(:enqueue).with(:notify_reviewable, has_key(:reviewable_id)).never Jobs.expects(:enqueue).with(:notify_reviewable, has_key(:reviewable_id)).never
reviewable.perform(moderator, :reject) reviewable.perform(moderator, :reject_post)
end end
it "doesn't trigger a notification on reject -> approve" do it "doesn't trigger a notification on reject -> approve" do
reviewable = Fabricate(:reviewable_queued_post, status: Reviewable.statuses[:approved]) reviewable = Fabricate(:reviewable_queued_post, status: Reviewable.statuses[:approved])
Jobs.expects(:enqueue).with(:notify_reviewable, has_key(:reviewable_id)).never Jobs.expects(:enqueue).with(:notify_reviewable, has_key(:reviewable_id)).never
reviewable.perform(moderator, :reject) reviewable.perform(moderator, :reject_post)
end end
end end

View File

@ -491,14 +491,14 @@ describe WebHook do
payload = JSON.parse(job_args["payload"]) payload = JSON.parse(job_args["payload"])
expect(payload["id"]).to eq(reviewable.id) expect(payload["id"]).to eq(reviewable.id)
reviewable.perform(Discourse.system_user, :approve) reviewable.perform(Discourse.system_user, :approve_post)
job_args = Jobs::EmitWebHookEvent.jobs.last["args"].first job_args = Jobs::EmitWebHookEvent.jobs.last["args"].first
expect(job_args["event_name"]).to eq("approved_post") expect(job_args["event_name"]).to eq("approved_post")
payload = JSON.parse(job_args["payload"]) payload = JSON.parse(job_args["payload"])
expect(payload["id"]).to eq(reviewable.id) expect(payload["id"]).to eq(reviewable.id)
reviewable.perform(Discourse.system_user, :reject) reviewable.perform(Discourse.system_user, :reject_post)
job_args = Jobs::EmitWebHookEvent.jobs.last["args"].first job_args = Jobs::EmitWebHookEvent.jobs.last["args"].first
expect(job_args["event_name"]).to eq("rejected_post") expect(job_args["event_name"]).to eq("rejected_post")

View File

@ -808,7 +808,7 @@ describe PostsController do
expect(parsed['pending_post']['raw']).to eq("this is the test content") expect(parsed['pending_post']['raw']).to eq("this is the test content")
mod = Fabricate(:moderator) mod = Fabricate(:moderator)
rp.perform(mod, :approve) rp.perform(mod, :approve_post)
user.reload user.reload
expect(user).not_to be_silenced expect(user).not_to be_silenced

View File

@ -258,7 +258,7 @@ describe ReviewablesController do
it "can properly return errors" do it "can properly return errors" do
qp = Fabricate(:reviewable_queued_post_topic, topic_id: -100) qp = Fabricate(:reviewable_queued_post_topic, topic_id: -100)
put "/review/#{qp.id}/perform/approve.json?version=#{qp.version}" put "/review/#{qp.id}/perform/approve_post.json?version=#{qp.version}"
expect(response.code).to eq("422") expect(response.code).to eq("422")
result = ::JSON.parse(response.body) result = ::JSON.parse(response.body)
expect(result['errors']).to be_present expect(result['errors']).to be_present