From 415d9591c5619e7fc5cbc67586307465d90e5907 Mon Sep 17 00:00:00 2001 From: evazion Date: Thu, 19 Mar 2020 22:55:28 -0500 Subject: [PATCH] pundit: convert post votes to pundit. Side effects: * The data-current-user-is-voter attribute has been removed. * {{upvote:self}} no longer works. {{upvote:}} should be used instead. --- app/controllers/post_votes_controller.rb | 7 ++--- app/logical/post_query_builder.rb | 28 ++++++++----------- app/models/post.rb | 6 ++-- app/models/user.rb | 7 +---- app/policies/post_vote_policy.rb | 9 ++++++ .../comments/partials/index/_header.html.erb | 2 +- app/views/modqueue/_post.html.erb | 2 +- .../posts/partials/show/_information.html.erb | 2 +- test/functional/post_votes_controller_test.rb | 25 ++++++++++------- 9 files changed, 46 insertions(+), 42 deletions(-) create mode 100644 app/policies/post_vote_policy.rb diff --git a/app/controllers/post_votes_controller.rb b/app/controllers/post_votes_controller.rb index 1ba8e15e9..1b41d4f42 100644 --- a/app/controllers/post_votes_controller.rb +++ b/app/controllers/post_votes_controller.rb @@ -1,25 +1,24 @@ class PostVotesController < ApplicationController - before_action :voter_only skip_before_action :api_check respond_to :js, :json, :xml, :html rescue_with PostVote::Error, status: 422 def index - @post_votes = PostVote.visible(CurrentUser.user).paginated_search(params, count_pages: true) + @post_votes = authorize PostVote.visible(CurrentUser.user).paginated_search(params, count_pages: true) @post_votes = @post_votes.includes(:user, post: :uploader) if request.format.html? respond_with(@post_votes) end def create - @post = Post.find(params[:post_id]) + @post = authorize Post.find(params[:post_id]), policy_class: PostVotePolicy @post.vote!(params[:score]) respond_with(@post) end def destroy - @post = Post.find(params[:post_id]) + @post = authorize Post.find(params[:post_id]), policy_class: PostVotePolicy @post.unvote! respond_with(@post) diff --git a/app/logical/post_query_builder.rb b/app/logical/post_query_builder.rb index cf309a276..84db854f1 100644 --- a/app/logical/post_query_builder.rb +++ b/app/logical/post_query_builder.rb @@ -499,14 +499,14 @@ class PostQueryBuilder relation = relation.where(id: FavoriteGroup.where(id: favgroup.id).select("unnest(post_ids)")) end - if q[:upvote].present? - post_ids = PostVote.where(user: q[:upvote]).where("score > 0").select(:post_id) - relation = relation.where("posts.id": post_ids) + q[:upvoter].to_a.each do |voter| + votes = PostVote.positive.visible(CurrentUser.user).where(user: voter).where("post_id = posts.id").select("1") + relation = relation.where("EXISTS (#{votes.to_sql})") end - if q[:downvote].present? - post_ids = PostVote.where(user: q[:downvote]).where("score < 0").select(:post_id) - relation = relation.where("posts.id": post_ids) + q[:downvoter].to_a.each do |voter| + votes = PostVote.negative.visible(CurrentUser.user).where(user: voter).where("post_id = posts.id").select("1") + relation = relation.where("EXISTS (#{votes.to_sql})") end if q[:ordfav].present? @@ -994,18 +994,14 @@ class PostQueryBuilder end when "upvote" - if CurrentUser.user.is_admin? - q[:upvote] = User.find_by_name(g2) - elsif CurrentUser.user.is_voter? - q[:upvote] = CurrentUser.user - end + user = User.find_by_name(g2) + q[:upvoter] ||= [] + q[:upvoter] << user unless user.blank? when "downvote" - if CurrentUser.user.is_admin? - q[:downvote] = User.find_by_name(g2) - elsif CurrentUser.user.is_voter? - q[:downvote] = CurrentUser.user - end + user = User.find_by_name(g2) + q[:downvoter] ||= [] + q[:downvoter] << user unless user.blank? when *COUNT_METATAGS q[g1.to_sym] = parse_helper(g2) diff --git a/app/models/post.rb b/app/models/post.rb index 22b981935..6a0a0e8b8 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -939,7 +939,7 @@ class Post < ApplicationRecord def add_favorite!(user) Favorite.add(post: self, user: user) - vote!("up", user) if user.is_voter? + vote!("up", user) if Pundit.policy!([user, nil], PostVote).create? rescue PostVote::Error end @@ -949,7 +949,7 @@ class Post < ApplicationRecord def remove_favorite!(user) Favorite.remove(post: self, user: user) - unvote!(user) if user.is_voter? + unvote!(user) if Pundit.policy!([user, nil], PostVote).create? rescue PostVote::Error end @@ -1031,7 +1031,7 @@ class Post < ApplicationRecord end def vote!(vote, voter = CurrentUser.user) - unless voter.is_voter? + unless Pundit.policy!([voter, nil], PostVote).create? raise PostVote::Error.new("You do not have permission to vote") end diff --git a/app/models/user.rb b/app/models/user.rb index 6f03d467d..4765caf7e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -17,8 +17,7 @@ class User < ApplicationRecord # Used for `before_action :_only`. Must have a corresponding `is_?` method. Roles = Levels.constants.map(&:downcase) + [ :banned, - :approver, - :voter + :approver ] # candidates for removal: @@ -354,10 +353,6 @@ class User < ApplicationRecord level >= Levels::ADMIN end - def is_voter? - is_gold? - end - def is_approver? can_approve_posts? end diff --git a/app/policies/post_vote_policy.rb b/app/policies/post_vote_policy.rb new file mode 100644 index 000000000..4e24b4895 --- /dev/null +++ b/app/policies/post_vote_policy.rb @@ -0,0 +1,9 @@ +class PostVotePolicy < ApplicationPolicy + def create? + unbanned? && user.is_gold? + end + + def destroy? + unbanned? && user.is_gold? + end +end diff --git a/app/views/comments/partials/index/_header.html.erb b/app/views/comments/partials/index/_header.html.erb index 36867db0c..7fc70b676 100644 --- a/app/views/comments/partials/index/_header.html.erb +++ b/app/views/comments/partials/index/_header.html.erb @@ -18,7 +18,7 @@ Score <%= post.score %> - <% if CurrentUser.is_voter? %> + <% if policy(PostVote).create? %> (vote <%= link_to(content_tag("i", nil, class: "far fa-thumbs-up"), post_post_votes_path(:score => "up", :post_id => post.id), :remote => true, :method => :post) %>/<%= link_to(content_tag("i", nil, class: "far fa-thumbs-down"), post_post_votes_path(:score => "down", :post_id => post.id), :remote => true, :method => :post) %>) <% end %> diff --git a/app/views/modqueue/_post.html.erb b/app/views/modqueue/_post.html.erb index 1f035b85a..ed3c5321d 100644 --- a/app/views/modqueue/_post.html.erb +++ b/app/views/modqueue/_post.html.erb @@ -30,7 +30,7 @@ Score <%= post.score %> - <% if CurrentUser.is_voter? %> + <% if policy(PostVote).create? %> (vote <%= link_to tag.i(class: "far fa-thumbs-up"), post_post_votes_path(score: "up", post_id: post.id), remote: true, method: :post %>/<%= link_to tag.i(class: "far fa-thumbs-down"), post_post_votes_path(score: "down", post_id: post.id), remote: true, method: :post %>) <% end %> diff --git a/app/views/posts/partials/show/_information.html.erb b/app/views/posts/partials/show/_information.html.erb index b36643ae4..e4a596eae 100644 --- a/app/views/posts/partials/show/_information.html.erb +++ b/app/views/posts/partials/show/_information.html.erb @@ -19,7 +19,7 @@
  • Source: <%= post_source_tag(post) %>
  • Rating: <%= post.pretty_rating %>
  • Score: <%= post.score %> - <% if CurrentUser.is_voter? %> + <% if policy(PostVote).create? %> <%= tag.span id: "vote-links-for-post-#{post.id}", style: ("display: none;" if !@post.can_be_voted_by?(CurrentUser.user)) do %> (vote <%= link_to tag.i(class: "far fa-thumbs-up"), post_post_votes_path(post_id: post.id, score: "up"), remote: true, method: :post %> diff --git a/test/functional/post_votes_controller_test.rb b/test/functional/post_votes_controller_test.rb index e6e4af466..4024010d7 100644 --- a/test/functional/post_votes_controller_test.rb +++ b/test/functional/post_votes_controller_test.rb @@ -4,9 +4,7 @@ class PostVotesControllerTest < ActionDispatch::IntegrationTest context "The post vote controller" do setup do @user = create(:gold_user) - @user.as_current do - @post = create(:post) - end + @post = create(:post) end context "index action" do @@ -48,16 +46,23 @@ class PostVotesControllerTest < ActionDispatch::IntegrationTest end context "for a post that has already been voted on" do - setup do - @user.as_current do - @post.vote!("up") + should "not create another vote" do + @post.vote!("up", @user) + assert_no_difference("PostVote.count") do + post_auth post_post_votes_path(post_id: @post.id), @user, params: { score: "up", format: "js" } + assert_response 422 end end + end + end - should "fail silently on an error" do - assert_nothing_raised do - post_auth post_post_votes_path(post_id: @post.id), @user, params: {:score => "up", :format => "js"} - end + context "destroy action" do + should "remove a vote" do + as(@user) { create(:post_vote, post_id: @post.id, user_id: @user.id) } + + assert_difference("PostVote.count", -1) do + delete_auth post_post_votes_path(post_id: @post.id), @user, as: :javascript + assert_redirected_to @post end end end