From 572b61c85c70cf5e98b72d799785790bf10ff2a4 Mon Sep 17 00:00:00 2001 From: Earlopain Date: Tue, 2 Nov 2021 16:36:05 +0100 Subject: [PATCH] [Forum] Prevent voting on your own requests Removing the votes is still possible but adding new ones isn't. Closes #341 --- .../forum_post_votes_controller.rb | 5 ++ app/views/forum_post_votes/_list.html.erb | 2 +- app/views/forum_post_votes/_vote.html.erb | 2 +- .../forum_post_votes_controller_test.rb | 59 ++++++++++++++----- .../functional/forum_posts_controller_test.rb | 11 +++- test/unit/forum_topic_test.rb | 8 +-- 6 files changed, 63 insertions(+), 24 deletions(-) diff --git a/app/controllers/forum_post_votes_controller.rb b/app/controllers/forum_post_votes_controller.rb index dc684cbae..f3b8e2c34 100644 --- a/app/controllers/forum_post_votes_controller.rb +++ b/app/controllers/forum_post_votes_controller.rb @@ -3,6 +3,7 @@ class ForumPostVotesController < ApplicationController before_action :member_only before_action :load_forum_post before_action :validate_forum_post + before_action :validate_no_vote_on_own_post, only: [:create] before_action :load_vote, only: [:destroy] def create @@ -36,6 +37,10 @@ private raise User::PrivilegeError.new unless @forum_post.votable? end + def validate_no_vote_on_own_post + raise User::PrivilegeError.new if @forum_post.creator == CurrentUser.user + end + def forum_post_vote_params params.fetch(:forum_post_vote, {}).permit(:score) end diff --git a/app/views/forum_post_votes/_list.html.erb b/app/views/forum_post_votes/_list.html.erb index c4b1791b5..916b11faf 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.tag_change_request && forum_post.tag_change_request.is_pending? && !votes.by(CurrentUser.user.id).exists? %> +<% if forum_post.tag_change_request&.is_pending? && !votes.by(CurrentUser.user.id).exists? && forum_post.creator != CurrentUser.user %> <%= 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 f75b62684..c95b15cae 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.tag_change_request && forum_post.tag_change_request.is_pending? && vote.creator_id == CurrentUser.id %> + <% if forum_post.tag_change_request&.is_pending? && vote.creator_id == CurrentUser.id %> <%= link_to content_tag(:i, nil, class: "far #{vote.fa_class}"), "#", class: "forum-vote-remove", 'data-forum-id': forum_post.id %> <%= link_to_user vote.creator %> <% else %> diff --git a/test/controllers/forum_post_votes_controller_test.rb b/test/controllers/forum_post_votes_controller_test.rb index c5c1ce8d1..f1789a0af 100644 --- a/test/controllers/forum_post_votes_controller_test.rb +++ b/test/controllers/forum_post_votes_controller_test.rb @@ -3,34 +3,65 @@ require 'test_helper' class ForumPostVotesControllerTest < ActionDispatch::IntegrationTest context "The forum post votes controller" do setup do - @user = create(:user) + @user1 = create(:user) + @user2 = create(:user) - as_user do - @forum_topic = create(:forum_topic) - @forum_post = create(:forum_post, topic: @forum_topic) + as @user1 do + @forum_topic = create(:forum_topic, original_post_attributes: { body: "alias" }) + @forum_post = @forum_topic.original_post end end - should "allow voting" do - assert_difference("ForumPostVote.count") do - post_auth forum_post_votes_path(forum_post_id: @forum_post.id), @user, params: {forum_post_vote: {score: 1}, format: "js"} + context "without a tag change request" do + should "not allow voting" do + post_auth forum_post_votes_path(forum_post_id: @forum_post.id), @user1, params: { forum_post_vote: { score: 1 }, format: :json } + assert_response :forbidden end - assert_response :success end - context "when deleting" do + context "with an already accepted tag change request" do + should "not allow voting" do + @alias = create(:tag_alias, forum_post: @forum_post) + post_auth forum_post_votes_path(forum_post_id: @forum_post.id), @user1, params: { forum_post_vote: { score: 1 }, format: :json } + assert_response :forbidden + end + end + + context "with a pending tag change request" do setup do - as_user do - @forum_post_vote = @forum_post.votes.create(score: 1) + as @user1 do + create(:tag_alias, status: "pending", forum_post: @forum_post) end end - should "allow removal" do - assert_difference("ForumPostVote.count", -1) do - delete_auth forum_post_votes_path(forum_post_id: @forum_post.id), @user, params: {format: "js"} + should "allow voting" do + assert_difference(-> { ForumPostVote.count }, 1) do + post_auth forum_post_votes_path(forum_post_id: @forum_post.id), @user2, params: { forum_post_vote: { score: 1 }, format: :json } end assert_response :success end + + should "not allow voting for the user who created the request" do + assert_no_difference(-> { ForumPostVote.count }) do + post_auth forum_post_votes_path(forum_post_id: @forum_post.id), @user1, params: { forum_post_vote: { score: 1 }, format: :json } + end + assert_response :forbidden + end + + context "when deleting" do + setup do + as(@user2) do + @forum_post_vote = @forum_post.votes.create(score: 1) + end + end + + should "allow removal" do + assert_difference(-> { ForumPostVote.count }, -1) do + delete_auth forum_post_votes_path(forum_post_id: @forum_post.id), @user2, params: { format: :json } + end + assert_response :success + end + end end end end diff --git a/test/functional/forum_posts_controller_test.rb b/test/functional/forum_posts_controller_test.rb index 4801d1255..4b85536d1 100644 --- a/test/functional/forum_posts_controller_test.rb +++ b/test/functional/forum_posts_controller_test.rb @@ -7,8 +7,8 @@ class ForumPostsControllerTest < ActionDispatch::IntegrationTest @other_user = create(:user) @mod = create(:moderator_user) as_user do - @forum_topic = create(:forum_topic, :title => "my forum topic") - @forum_post = create(:forum_post, :topic_id => @forum_topic.id, :body => "alias xxx -> yyy") + @forum_topic = create(:forum_topic, title: "my forum topic", original_post_attributes: { body: "alias xxx -> yyy" }) + @forum_post = @forum_topic.original_post end end @@ -21,6 +21,11 @@ class ForumPostsControllerTest < ActionDispatch::IntegrationTest end end + should "not render the vote links for the requesting user" do + get_auth forum_topic_path(@forum_topic), @user + assert_select "a[title='Vote up']", false + end + should "render the vote links" do get_auth forum_topic_path(@forum_topic), @mod assert_select "a[title='Vote up']" @@ -78,7 +83,7 @@ class ForumPostsControllerTest < ActionDispatch::IntegrationTest context "with private topics" do setup do as(@mod) do - @mod_topic = create(:mod_up_forum_topic) + @mod_topic = create(:mod_up_forum_topic, original_post_attributes: { body: "mod only" }) @mod_posts = 2.times.map do create(:forum_post, :topic_id => @mod_topic.id) end diff --git a/test/unit/forum_topic_test.rb b/test/unit/forum_topic_test.rb index 5e5a40f7c..12cf2d477 100644 --- a/test/unit/forum_topic_test.rb +++ b/test/unit/forum_topic_test.rb @@ -7,7 +7,7 @@ class ForumTopicTest < ActiveSupport::TestCase @user = FactoryBot.create(:user) CurrentUser.user = @user CurrentUser.ip_addr = "127.0.0.1" - @topic = FactoryBot.create(:forum_topic, :title => "xxx") + @topic = FactoryBot.create(:forum_topic, title: "xxx", original_post_attributes: { body: "aaa" }) end teardown do @@ -113,9 +113,7 @@ class ForumTopicTest < ActiveSupport::TestCase context "#merge" do setup do - @topic2 = FactoryBot.create(:forum_topic, :title => "yyy") - FactoryBot.create(:forum_post, :topic_id => @topic.id, :body => "xxx") - FactoryBot.create(:forum_post, :topic_id => @topic2.id, :body => "xxx") + @topic2 = FactoryBot.create(:forum_topic, title: "yyy", original_post_attributes: { body: "bbb" }) end should "merge all the posts in one topic into the other" do @@ -166,7 +164,7 @@ class ForumTopicTest < ActiveSupport::TestCase end should "delete any associated posts" do - assert_difference("ForumPost.count", -5) do + assert_difference("ForumPost.count", -6) do @topic.destroy end end