From cc2b4abd09f34f4e354c5ba4bafe354d7990b908 Mon Sep 17 00:00:00 2001 From: evazion Date: Wed, 18 Mar 2020 02:43:45 -0500 Subject: [PATCH] pundit: convert forum post votes to pundit. --- .../forum_post_votes_controller.rb | 16 +++------ app/models/forum_post.rb | 4 --- app/policies/forum_post_policy.rb | 4 +++ app/policies/forum_post_vote_policy.rb | 13 +++++++ app/views/forum_post_votes/_list.html.erb | 2 +- app/views/forum_post_votes/_vote.html.erb | 2 +- app/views/forum_post_votes/create.js.erb | 4 +-- .../forum_post_votes_controller_test.rb | 36 ++++++++++++++----- 8 files changed, 54 insertions(+), 27 deletions(-) create mode 100644 app/policies/forum_post_vote_policy.rb diff --git a/app/controllers/forum_post_votes_controller.rb b/app/controllers/forum_post_votes_controller.rb index 733e3d50b..e41bc9655 100644 --- a/app/controllers/forum_post_votes_controller.rb +++ b/app/controllers/forum_post_votes_controller.rb @@ -1,29 +1,23 @@ class ForumPostVotesController < ApplicationController respond_to :html, :xml, :json, :js - before_action :member_only, only: [:create, :destroy] def index - @forum_post_votes = ForumPostVote.visible(CurrentUser.user).paginated_search(params, count_pages: true) + @forum_post_votes = authorize ForumPostVote.visible(CurrentUser.user).paginated_search(params, count_pages: true) @forum_post_votes = @forum_post_votes.includes(:creator, forum_post: [:creator, :topic]) if request.format.html? respond_with(@forum_post_votes) end def create - @forum_post = ForumPost.visible(CurrentUser.user).find(params[:forum_post_id]) - @forum_post_vote = @forum_post.votes.create(forum_post_vote_params.merge(creator: CurrentUser.user)) + @forum_post = ForumPost.find(params[:forum_post_id]) + @forum_post_vote = authorize ForumPostVote.new(creator: CurrentUser.user, forum_post: @forum_post, **permitted_attributes(ForumPostVote)) + @forum_post_vote.save respond_with(@forum_post_vote) end def destroy - @forum_post_vote = CurrentUser.user.forum_post_votes.find(params[:id]) + @forum_post_vote = authorize ForumPostVote.find(params[:id]) @forum_post_vote.destroy respond_with(@forum_post_vote) end - - private - - def forum_post_vote_params - params.fetch(:forum_post_vote, {}).permit(:score) - end end diff --git a/app/models/forum_post.rb b/app/models/forum_post.rb index ee15d1904..fde386242 100644 --- a/app/models/forum_post.rb +++ b/app/models/forum_post.rb @@ -81,10 +81,6 @@ class ForumPost < ApplicationRecord end end - def votable? - bulk_update_request.present? && bulk_update_request.is_pending? - end - def voted?(user, score) votes.where(creator_id: user.id, score: score).exists? end diff --git a/app/policies/forum_post_policy.rb b/app/policies/forum_post_policy.rb index a444b16a0..69e31fa34 100644 --- a/app/policies/forum_post_policy.rb +++ b/app/policies/forum_post_policy.rb @@ -19,6 +19,10 @@ class ForumPostPolicy < ApplicationPolicy unbanned? && show? && user.is_moderator? end + def votable? + unbanned? && show? && record.bulk_update_request.present? && record.bulk_update_request.is_pending? + end + def reportable? unbanned? && show? && record.creator_id != user.id && !record.creator.is_moderator? end diff --git a/app/policies/forum_post_vote_policy.rb b/app/policies/forum_post_vote_policy.rb new file mode 100644 index 000000000..f5b83030e --- /dev/null +++ b/app/policies/forum_post_vote_policy.rb @@ -0,0 +1,13 @@ +class ForumPostVotePolicy < ApplicationPolicy + def create? + unbanned? && policy(record.forum_post).votable? + end + + def destroy? + unbanned? && record.creator_id == user.id + end + + def permitted_attributes + [:score] + end +end diff --git a/app/views/forum_post_votes/_list.html.erb b/app/views/forum_post_votes/_list.html.erb index d866af2d0..75653e2ed 100644 --- a/app/views/forum_post_votes/_list.html.erb +++ b/app/views/forum_post_votes/_list.html.erb @@ -11,6 +11,6 @@ <%= render "forum_post_votes/vote", vote: vote, forum_post: forum_post %> <% end %> -<% if forum_post.votable? && !votes.by(CurrentUser.user.id).exists? %> +<% if policy(forum_post).votable? && !votes.by(CurrentUser.user.id).exists? %> <%= render "forum_post_votes/add_vote", vote: votes.by(CurrentUser.user.id).first, forum_post: forum_post %> <% end %> diff --git a/app/views/forum_post_votes/_vote.html.erb b/app/views/forum_post_votes/_vote.html.erb index 51dc051b1..a22e093f8 100644 --- a/app/views/forum_post_votes/_vote.html.erb +++ b/app/views/forum_post_votes/_vote.html.erb @@ -4,7 +4,7 @@ %>
  • - <% if forum_post.votable? && vote.creator_id == CurrentUser.id %> + <% if policy(forum_post).votable? && vote.creator_id == CurrentUser.id %> <%= link_to content_tag(:i, nil, class: "far #{vote.fa_class}"), forum_post_vote_path(vote, format: "js"), remote: true, method: :delete %> <%= link_to_user vote.creator %> <% else %> diff --git a/app/views/forum_post_votes/create.js.erb b/app/views/forum_post_votes/create.js.erb index 98b79da6b..766f21392 100644 --- a/app/views/forum_post_votes/create.js.erb +++ b/app/views/forum_post_votes/create.js.erb @@ -2,6 +2,6 @@ Danbooru.error(<%= raw @forum_post_vote.errors.full_messages.join("; ").to_json %>); <% else %> Danbooru.notice("Voted"); - var code = <%= raw render(partial: "forum_post_votes/list", locals: {forum_post: @forum_post, votes: @forum_post.votes}).to_json %>; - $("#forum-post-votes-for-<%= @forum_post.id %>").html(code); + var code = <%= raw render(partial: "forum_post_votes/list", locals: {forum_post: @forum_post_vote.forum_post, votes: @forum_post_vote.forum_post.votes }).to_json %>; + $("#forum-post-votes-for-<%= @forum_post_vote.forum_post.id %>").html(code); <% end %> diff --git a/test/functional/forum_post_votes_controller_test.rb b/test/functional/forum_post_votes_controller_test.rb index 3c6cf9766..7143c038d 100644 --- a/test/functional/forum_post_votes_controller_test.rb +++ b/test/functional/forum_post_votes_controller_test.rb @@ -4,10 +4,12 @@ class ForumPostVotesControllerTest < ActionDispatch::IntegrationTest context "The forum post votes controller" do setup do @user = create(:user) + @other_user = create(:user) as(@user) do @forum_topic = create(:forum_topic) @forum_post = create(:forum_post, topic: @forum_topic) + @bulk_update_request = create(:bulk_update_request, forum_post: @forum_post) end end @@ -15,26 +17,44 @@ class ForumPostVotesControllerTest < ActionDispatch::IntegrationTest should "render" do @forum_post_vote = create(:forum_post_vote, creator: @user, forum_post: @forum_post) get forum_post_votes_path - assert_response :success end end - should "allow voting" do - assert_difference("ForumPostVote.count") do - post_auth forum_post_votes_path(format: :js), @user, params: { forum_post_id: @forum_post.id, forum_post_vote: { score: 1 }} + context "create action" do + should "allow members to vote" do + assert_difference("ForumPostVote.count", 1) do + post_auth forum_post_votes_path(format: :js), @user, params: { forum_post_id: @forum_post.id, forum_post_vote: { score: 1 }} + assert_response :success + end + end + + should "not allow privileged users to vote on private forum posts" do + as(@user) { @forum_post.topic.update!(min_level: User::Levels::ADMIN) } + assert_difference("ForumPostVote.count", 0) do + post_auth forum_post_votes_path(format: :js), @user, params: { forum_post_id: @forum_post.id, forum_post_vote: { score: 1 }} + assert_response 403 + end end - assert_response :success end - context "when deleting" do - should "allow removal" do + context "destroy action" do + setup do @forum_post_vote = create(:forum_post_vote, creator: @user, forum_post: @forum_post) + end + + should "allow members to destroy their own votes" do assert_difference("ForumPostVote.count", -1) do delete_auth forum_post_vote_path(@forum_post_vote.id, format: :js), @user + assert_response :success end + end - assert_response :success + should "not allow members to destroy other people's votes" do + assert_difference("ForumPostVote.count", 0) do + delete_auth forum_post_vote_path(@forum_post_vote.id, format: :js), @other_user + assert_response 403 + end end end end