diff --git a/app/controllers/post_disapprovals_controller.rb b/app/controllers/post_disapprovals_controller.rb index c7c5dc072..6dbd93f46 100644 --- a/app/controllers/post_disapprovals_controller.rb +++ b/app/controllers/post_disapprovals_controller.rb @@ -1,23 +1,17 @@ class PostDisapprovalsController < ApplicationController - before_action :approver_only, only: [:create] skip_before_action :api_check respond_to :js, :html, :json, :xml def create - @post_disapproval = PostDisapproval.create(user: CurrentUser.user, **post_disapproval_params) + @post_disapproval = authorize PostDisapproval.new(user: CurrentUser.user, **permitted_attributes(PostDisapproval)) + @post_disapproval.save respond_with(@post_disapproval) end def index - @post_disapprovals = PostDisapproval.paginated_search(params) + @post_disapprovals = authorize PostDisapproval.paginated_search(params) @post_disapprovals = @post_disapprovals.includes(:user) if request.format.html? respond_with(@post_disapprovals) end - - private - - def post_disapproval_params - params.require(:post_disapproval).permit(%i[post_id reason message]) - end end diff --git a/app/models/post_disapproval.rb b/app/models/post_disapproval.rb index 8fb5d04a5..24a711923 100644 --- a/app/models/post_disapproval.rb +++ b/app/models/post_disapproval.rb @@ -66,13 +66,9 @@ class PostDisapproval < ApplicationRecord super(message) end - def can_view_creator?(user) - user.is_moderator? || user_id == user.id - end - def api_attributes attributes = super - attributes -= [:creator_id] unless can_view_creator?(CurrentUser.user) + attributes -= [:creator_id] unless Pundit.policy!([CurrentUser.user, nil], self).can_view_creator? attributes end end diff --git a/app/policies/post_disapproval_policy.rb b/app/policies/post_disapproval_policy.rb new file mode 100644 index 000000000..fad211267 --- /dev/null +++ b/app/policies/post_disapproval_policy.rb @@ -0,0 +1,13 @@ +class PostDisapprovalPolicy < ApplicationPolicy + def create? + user.is_approver? + end + + def can_view_creator? + user.is_moderator? || record.user_id == user.id + end + + def permitted_attributes + [:post_id, :reason, :message] + end +end diff --git a/app/views/post_disapprovals/index.html.erb b/app/views/post_disapprovals/index.html.erb index d878024da..6b3048697 100644 --- a/app/views/post_disapprovals/index.html.erb +++ b/app/views/post_disapprovals/index.html.erb @@ -27,7 +27,7 @@ <%= link_to post_disapproval.reason.humanize, post_disapprovals_path(search: params[:search].merge(reason: post_disapproval.reason)) %> <% end %> <% t.column "Created" do |post_disapproval| %> - <% if post_disapproval.can_view_creator?(CurrentUser.user) %> + <% if policy(post_disapproval).can_view_creator? %> <%= link_to_user post_disapproval.user %> <%= link_to "ยป", post_disapprovals_path(search: params[:search].merge(creator_name: post_disapproval.user&.name)) %> <% end %> diff --git a/test/functional/post_disapprovals_controller_test.rb b/test/functional/post_disapprovals_controller_test.rb index 5a0e5f18b..f0eb17ad8 100644 --- a/test/functional/post_disapprovals_controller_test.rb +++ b/test/functional/post_disapprovals_controller_test.rb @@ -3,38 +3,45 @@ require 'test_helper' class PostDisapprovalsControllerTest < ActionDispatch::IntegrationTest context "The post disapprovals controller" do setup do - @admin = create(:admin_user) - as_user do - @post = create(:post, :is_pending => true) - end - - CurrentUser.user = @admin + @approver = create(:approver) + @post = create(:post, is_pending: true) + @post_disapproval = create(:post_disapproval, post: @post) end context "create action" do should "render" do assert_difference("PostDisapproval.count", 1) do - post_auth post_disapprovals_path, @admin, params: { post_disapproval: { post_id: @post.id, reason: "breaks_rules" }, format: "js" } + post_auth post_disapprovals_path, @approver, params: { post_disapproval: { post_id: @post.id, reason: "breaks_rules" }, format: "js" } + assert_response :success end - assert_response :success end - context "for json" do - should "render" do - assert_difference("PostDisapproval.count", 1) do - post_auth post_disapprovals_path, @admin, params: { post_disapproval: { post_id: @post.id, reason: "breaks_rules" }, format: "json" } - end + should "render for json" do + assert_difference("PostDisapproval.count", 1) do + post_auth post_disapprovals_path, @approver, params: { post_disapproval: { post_id: @post.id, reason: "breaks_rules" }, format: "json" } assert_response :success end end + + should "not allow non-approvers to create disapprovals" do + assert_difference("PostDisapproval.count", 0) do + post_auth post_disapprovals_path, create(:user), params: { post_disapproval: { post_id: @post.id, reason: "breaks_rules" }, format: "json" } + assert_response 403 + end + end end context "index action" do - should "render" do - disapproval = FactoryBot.create(:post_disapproval, post: @post) - get_auth post_disapprovals_path, @admin - + should "allow mods to see disapprover names" do + get_auth post_disapprovals_path, create(:mod_user) assert_response :success + assert_select "tr#post-disapproval-#{@post_disapproval.id} .created-column a.user-member", true + end + + should "not allow non-mods to see disapprover names" do + get post_disapprovals_path + assert_response :success + assert_select "tr#post-disapproval-#{@post_disapproval.id} .created-column a.user-member", false end end end