From 353e708538380b0482435df601559c4d1b8e5565 Mon Sep 17 00:00:00 2001 From: evazion Date: Tue, 23 Nov 2021 22:20:01 -0600 Subject: [PATCH] votes: allow admins to remove post votes. Allow admins to remove votes on posts. This is for fixing vote abuse. Votes can be removed by going to the vote list on the /post_votes page, or by clicking on a post's score, then using the "Remove" option in the "..." dropdown menu next to the vote. Votes are soft-deleted - they're marked as deleted in the database, but not fully deleted. Removed votes are only visible to admins, not to regular users. When a vote is removed by an admin, it leaves a mod action. Technically it's possible to undelete votes, but there's no UI for it. --- .../post_votes_component.html.erb | 4 +- .../post_votes_tooltip_component.rb | 2 +- app/controllers/post_votes_controller.rb | 27 +++--- app/logical/post_query_builder.rb | 4 +- app/models/favorite.rb | 10 +-- app/models/mod_action.rb | 2 + app/models/post.rb | 12 +-- app/models/post_vote.rb | 53 ++++++++++-- app/policies/post_vote_policy.rb | 4 +- app/views/post_votes/create.js.erb | 2 +- app/views/post_votes/destroy.js+listing.erb | 2 + app/views/post_votes/index.html+compact.erb | 14 +++ app/views/post_votes/index.html.erb | 17 +++- config/routes.rb | 2 +- test/functional/favorites_controller_test.rb | 12 ++- test/functional/post_votes_controller_test.rb | 85 ++++++++++++------- test/unit/favorite_test.rb | 7 +- test/unit/post_test.rb | 29 ++++--- test/unit/post_vote_test.rb | 81 +++++++++++++++--- 19 files changed, 261 insertions(+), 108 deletions(-) create mode 100644 app/views/post_votes/destroy.js+listing.erb diff --git a/app/components/post_votes_component/post_votes_component.html.erb b/app/components/post_votes_component/post_votes_component.html.erb index 65dfdd071..f099b162a 100644 --- a/app/components/post_votes_component/post_votes_component.html.erb +++ b/app/components/post_votes_component/post_votes_component.html.erb @@ -2,7 +2,7 @@ <% if current_user.is_anonymous? %> <%= link_to upvote_icon, login_path(url: request.fullpath), class: "post-upvote-link inactive-link" %> <% elsif upvoted? %> - <%= link_to upvote_icon, post_post_votes_path(post_id: post.id), class: "post-upvote-link post-unvote-link active-link", method: :delete, remote: true %> + <%= link_to upvote_icon, post_vote_path(current_vote), class: "post-upvote-link post-unvote-link active-link", method: :delete, remote: true %> <% else %> <%= link_to upvote_icon, post_post_votes_path(post_id: post.id, score: 1), class: "post-upvote-link inactive-link", method: :post, remote: true %> <% end %> @@ -14,7 +14,7 @@ <% if current_user.is_anonymous? %> <%= link_to downvote_icon, login_path(url: request.fullpath), class: "post-downvote-link inactive-link" %> <% elsif downvoted? %> - <%= link_to downvote_icon, post_post_votes_path(post_id: post.id), class: "post-downvote-link post-unvote-link active-link", method: :delete, remote: true %> + <%= link_to downvote_icon, post_vote_path(current_vote), class: "post-downvote-link post-unvote-link active-link", method: :delete, remote: true %> <% else %> <%= link_to downvote_icon, post_post_votes_path(post_id: post.id, score: -1), class: "post-downvote-link inactive-link", method: :post, remote: true %> <% end %> diff --git a/app/components/post_votes_tooltip_component.rb b/app/components/post_votes_tooltip_component.rb index c2d971d09..8408bcdb2 100644 --- a/app/components/post_votes_tooltip_component.rb +++ b/app/components/post_votes_tooltip_component.rb @@ -12,7 +12,7 @@ class PostVotesTooltipComponent < ApplicationComponent end def votes - @post.votes.includes(:user).order(id: :desc) + @post.votes.active.includes(:user).order(id: :desc) end def vote_icon(vote) diff --git a/app/controllers/post_votes_controller.rb b/app/controllers/post_votes_controller.rb index 24484deaa..7a6232b41 100644 --- a/app/controllers/post_votes_controller.rb +++ b/app/controllers/post_votes_controller.rb @@ -2,7 +2,7 @@ class PostVotesController < ApplicationController respond_to :js, :json, :xml, :html def index - @post_votes = authorize PostVote.visible(CurrentUser.user).paginated_search(params, count_pages: true) + @post_votes = authorize PostVote.visible(CurrentUser.user).paginated_search(params) @post_votes = @post_votes.includes(:user, post: [:uploader, :media_asset]) if request.format.html? @post = Post.find(params.dig(:search, :post_id)) if params.dig(:search, :post_id).present? @@ -15,23 +15,28 @@ class PostVotesController < ApplicationController end def create - @post = Post.find(params[:post_id]) - - @post.with_lock do - @post_vote = authorize PostVote.new(post: @post, score: params[:score], user: CurrentUser.user) - PostVote.where(post: @post, user: CurrentUser.user).destroy_all - @post_vote.save - end + @post_vote = authorize PostVote.new(post_id: params[:post_id], score: params[:score], user: CurrentUser.user) + @post_vote.save + @post = @post_vote.post.reload flash.now[:notice] = @post_vote.errors.full_messages.join("; ") if @post_vote.errors.present? respond_with(@post_vote) end def destroy - @post = Post.find(params[:post_id]) - @post_vote = @post.votes.find_by(user: CurrentUser.user) + if params[:post_id].present? + @post_vote = PostVote.active.find_by(post_id: params[:post_id], user_id: CurrentUser.user) + @post = Post.find(params[:post_id]) + else + @post_vote = PostVote.find(params[:id]) + @post = @post_vote.post + end + + if @post_vote.present? + authorize(@post_vote).soft_delete(updater: CurrentUser.user) + @post.reload + end - authorize(@post_vote).destroy if @post_vote respond_with(@post_vote) end end diff --git a/app/logical/post_query_builder.rb b/app/logical/post_query_builder.rb index 56b856411..14aeb9173 100644 --- a/app/logical/post_query_builder.rb +++ b/app/logical/post_query_builder.rb @@ -215,9 +215,9 @@ class PostQueryBuilder when "noteupdater" user_subquery_matches(NoteVersion.unscoped, value, field: :updater) when "upvoter", "upvote" - user_subquery_matches(PostVote.positive.visible(current_user), value, field: :user) + user_subquery_matches(PostVote.active.positive.visible(current_user), value, field: :user) when "downvoter", "downvote" - user_subquery_matches(PostVote.negative.visible(current_user), value, field: :user) + user_subquery_matches(PostVote.active.negative.visible(current_user), value, field: :user) when *CATEGORY_COUNT_METATAGS short_category = name.delete_suffix("tags") category = TagCategory.short_name_mapping[short_category] diff --git a/app/models/favorite.rb b/app/models/favorite.rb index 6f8d472c2..732e071cc 100644 --- a/app/models/favorite.rb +++ b/app/models/favorite.rb @@ -28,15 +28,13 @@ class Favorite < ApplicationRecord end def upvote_post_on_create - if Pundit.policy!(user, PostVote).create? - PostVote.negative.destroy_by(post: post, user: user) - - # Silently ignore the error if the user has already upvoted the post. - PostVote.create(post: post, user: user, score: 1) + if Pundit.policy!(user, PostVote).create? && !PostVote.active.exists?(post: post, user: user, score: 1) + PostVote.create!(post: post, user: user, score: 1) end end def unvote_post_on_destroy - PostVote.positive.destroy_by(post: post, user: user) + vote = PostVote.active.positive.find_by(post: post, user: user) + vote&.soft_delete!(updater: user) end end diff --git a/app/models/mod_action.rb b/app/models/mod_action.rb index 5c1cb48d1..2ba3df909 100644 --- a/app/models/mod_action.rb +++ b/app/models/mod_action.rb @@ -35,6 +35,8 @@ class ModAction < ApplicationRecord post_note_lock_delete: 212, post_rating_lock_create: 220, post_rating_lock_delete: 222, + post_vote_delete: 232, + post_vote_undelete: 233, pool_delete: 62, pool_undelete: 63, artist_ban: 184, diff --git a/app/models/post.rb b/app/models/post.rb index 4ac77452d..4e89dbf9f 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -40,7 +40,7 @@ class Post < ApplicationRecord has_one :upload, :dependent => :destroy has_one :artist_commentary, :dependent => :destroy has_one :pixiv_ugoira_frame_data, class_name: "PixivUgoiraFrameData", foreign_key: :md5, primary_key: :md5 - has_one :vote_by_current_user, -> { where(user_id: CurrentUser.id) }, class_name: "PostVote" # XXX using current user here is wrong + has_one :vote_by_current_user, -> { active.where(user_id: CurrentUser.id) }, class_name: "PostVote" # XXX using current user here is wrong has_many :flags, :class_name => "PostFlag", :dependent => :destroy has_many :appeals, :class_name => "PostAppeal", :dependent => :destroy has_many :votes, :class_name => "PostVote", :dependent => :destroy @@ -701,18 +701,10 @@ class Post < ApplicationRecord return unless Pundit.policy!(voter, PostVote).create? with_lock do - votes.destroy_by(user: voter) - votes.create!(user: voter, score: score) + votes.create!(user: voter, score: score) unless votes.active.exists?(user: voter, score: score) reload # PostVote.create modifies our score. Reload to get the new score. end end - - def unvote!(voter) - return unless Pundit.policy!(voter, PostVote).create? - - votes.destroy_by(user: voter) - reload - end end module ParentMethods diff --git a/app/models/post_vote.rb b/app/models/post_vote.rb index 45bb3e2e7..36e73605f 100644 --- a/app/models/post_vote.rb +++ b/app/models/post_vote.rb @@ -1,25 +1,36 @@ class PostVote < ApplicationRecord + attr_accessor :updater + belongs_to :post belongs_to :user - validates :user_id, uniqueness: { scope: :post_id, message: "have already voted for this post" } validates :score, inclusion: { in: [1, -1], message: "must be 1 or -1" } - after_create :update_score_after_create - after_destroy :update_score_after_destroy + before_save { post.lock! } + before_save :update_score_on_delete, if: -> { !new_record? && is_deleted_changed?(from: false, to: true) } + before_save :update_score_on_undelete, if: -> { !new_record? && is_deleted_changed?(from: true, to: false) } + before_save :create_mod_action_on_delete_or_undelete + before_create :update_score_on_create + before_create :remove_conflicting_votes scope :positive, -> { where("post_votes.score > 0") } scope :negative, -> { where("post_votes.score < 0") } - scope :public_votes, -> { positive.where(user: User.has_public_favorites) } + scope :public_votes, -> { active.positive.where(user: User.has_public_favorites) } deletable def self.visible(user) - user.is_admin? ? all : where(user: user).or(public_votes) + if user.is_admin? + all + elsif user.is_anonymous? + public_votes + else + active.where(user: user).or(public_votes) + end end def self.search(params) - q = search_attributes(params, :id, :created_at, :updated_at, :score, :user, :post) + q = search_attributes(params, :id, :created_at, :updated_at, :score, :is_deleted, :user, :post) q.apply_default_order(params) end @@ -32,7 +43,19 @@ class PostVote < ApplicationRecord score < 0 end - def update_score_after_create + def remove_conflicting_votes + PostVote.active.where.not(id: id).where(post: post, user: user).each do |vote| + vote.soft_delete!(updater: updater) + end + end + + def validate_vote_is_unique + if !is_deleted? && PostVote.active.where.not(id: id).exists?(post: post, user: user) + errors.add(:user, "have already voted for this post") + end + end + + def update_score_on_create if is_positive? Post.update_counters(post_id, { score: score, up_score: score }) else @@ -40,7 +63,7 @@ class PostVote < ApplicationRecord end end - def update_score_after_destroy + def update_score_on_delete if is_positive? Post.update_counters(post_id, { score: -score, up_score: -score }) else @@ -48,6 +71,20 @@ class PostVote < ApplicationRecord end end + def update_score_on_undelete + update_score_on_create + end + + def create_mod_action_on_delete_or_undelete + return if new_record? || updater.nil? || updater == user + + if is_deleted_changed?(from: false, to: true) + ModAction.log("#{updater.name} deleted post vote ##{id} on post ##{post_id}", :post_vote_delete, updater) + elsif is_deleted_changed?(from: true, to: false) + ModAction.log("#{updater.name} undeleted post vote ##{id} on post ##{post_id}", :post_vote_undelete, updater) + end + end + def self.available_includes [:user, :post] end diff --git a/app/policies/post_vote_policy.rb b/app/policies/post_vote_policy.rb index 730cbe46c..4a89d6b8f 100644 --- a/app/policies/post_vote_policy.rb +++ b/app/policies/post_vote_policy.rb @@ -4,11 +4,11 @@ class PostVotePolicy < ApplicationPolicy end def destroy? - unbanned? && record.user == user + record.user == user || user.is_admin? end def show? - user.is_admin? || record.user == user || (record.is_positive? && !record.user.enable_private_favorites?) + user.is_admin? || record.user == user || (record.is_positive? && !record.is_deleted? && !record.user.enable_private_favorites?) end def can_see_voter? diff --git a/app/views/post_votes/create.js.erb b/app/views/post_votes/create.js.erb index 70caa471c..1932aa3a4 100644 --- a/app/views/post_votes/create.js.erb +++ b/app/views/post_votes/create.js.erb @@ -1 +1 @@ -$("span.post-votes[data-id=<%= @post.reload.id %>]").replaceWith("<%= j render_post_votes @post, current_user: CurrentUser.user %>"); +$("span.post-votes[data-id=<%= @post.id %>]").replaceWith("<%= j render_post_votes @post, current_user: CurrentUser.user %>"); diff --git a/app/views/post_votes/destroy.js+listing.erb b/app/views/post_votes/destroy.js+listing.erb new file mode 100644 index 000000000..2edde040f --- /dev/null +++ b/app/views/post_votes/destroy.js+listing.erb @@ -0,0 +1,2 @@ +Danbooru.Utility.notice("Vote removed"); +location.reload(); diff --git a/app/views/post_votes/index.html+compact.erb b/app/views/post_votes/index.html+compact.erb index cd861d79e..55b91cfe4 100644 --- a/app/views/post_votes/index.html+compact.erb +++ b/app/views/post_votes/index.html+compact.erb @@ -7,6 +7,10 @@ <%= table_for @post_votes, class: "striped autofit" do |t| %> <% t.column "Score" do |vote| %> <%= link_to sprintf("%+d", vote.score), post_votes_path(search: { score: vote.score }) %> + + <% if vote.is_deleted? %> + (deleted) + <% end %> <% end %> <% t.column "Voter" do |vote| %> @@ -17,6 +21,16 @@ <% t.column "Created" do |vote| %> <%= time_ago_in_words_tagged(vote.created_at) %> <% end %> + + <% t.column column: "control" do |vote| %> + <% if !vote.is_deleted? && policy(vote).destroy? %> + <%= render PopupMenuComponent.new do |menu| %> + <% menu.item do %> + <%= link_to "Remove", post_vote_path(vote.id, variant: "listing"), method: :delete, remote: true %> + <% end %> + <% end %> + <% end %> + <% end %> <% end %> <%= numbered_paginator(@post_votes) %> diff --git a/app/views/post_votes/index.html.erb b/app/views/post_votes/index.html.erb index 096c683d3..b7e900044 100644 --- a/app/views/post_votes/index.html.erb +++ b/app/views/post_votes/index.html.erb @@ -6,17 +6,25 @@ <% t.column "Post" do |vote| %> <%= post_preview(vote.post, show_deleted: true) %> <% end %> + <% t.column "Tags", td: {class: "col-expand"} do |vote| %> <%= render_inline_tag_list(vote.post) %> <% end %> + <% t.column "Score" do |vote| %> <%= link_to sprintf("%+d", vote.score), post_votes_path(search: { score: vote.score }) %> + + <% if vote.is_deleted? %> + (deleted) + <% end %> <% end %> + <% t.column "Uploader" do |vote| %> <%= link_to_user vote.post.uploader %> <%= link_to "ยป", post_votes_path(search: { post_tags_match: "user:#{vote.post.uploader.name}" }) %>
<%= time_ago_in_words_tagged(vote.post.created_at) %>
<% end %> + <% t.column "Voter" do |vote| %> <% if policy(vote).can_see_voter? %> <%= link_to_user vote.user %> @@ -26,9 +34,14 @@ <% end %>
<%= time_ago_in_words_tagged(vote.created_at) %>
<% end %> + <% t.column column: "control" do |vote| %> - <% if vote.user == CurrentUser.user %> - <%= link_to "unvote", post_post_votes_path(vote.post), remote: true, method: :delete %> + <% if !vote.is_deleted? && policy(vote).destroy? %> + <%= render PopupMenuComponent.new do |menu| %> + <% menu.item do %> + <%= link_to "Remove", post_vote_path(vote.id, variant: "listing"), method: :delete, remote: true %> + <% end %> + <% end %> <% end %> <% end %> <% end %> diff --git a/config/routes.rb b/config/routes.rb index 7d104b16e..e9f216909 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -189,7 +189,7 @@ Rails.application.routes.draw do end resources :post_regenerations, :only => [:create] resources :post_replacements, :only => [:index, :new, :create, :update] - resources :post_votes, only: [:index, :show] + resources :post_votes, only: [:index, :show, :create, :destroy] # XXX Use `only: []` to avoid redefining post routes defined at top of file. resources :posts, only: [] do diff --git a/test/functional/favorites_controller_test.rb b/test/functional/favorites_controller_test.rb index 936e69e48..ed6a10917 100644 --- a/test/functional/favorites_controller_test.rb +++ b/test/functional/favorites_controller_test.rb @@ -86,10 +86,14 @@ class FavoritesControllerTest < ActionDispatch::IntegrationTest context "destroy action" do should "remove the favorite for the current user" do - assert_difference [-> { @faved_post.favorites.count }, -> { @faved_post.reload.fav_count }, -> { @user.reload.favorite_count }], -1 do - delete_auth favorite_path(@faved_post.id), @user, as: :javascript - assert_response :redirect - end + delete_auth favorite_path(@faved_post.id), @user, as: :javascript + + assert_response :redirect + assert_equal(0, @faved_post.favorites.count) + assert_equal(0, @faved_post.reload.fav_count) + assert_equal(0, @faved_post.votes.active.count) + assert_equal(1, @faved_post.votes.deleted.count) + assert_equal(0, @user.reload.favorite_count) end should "allow banned users to destroy favorites" do diff --git a/test/functional/post_votes_controller_test.rb b/test/functional/post_votes_controller_test.rb index ff5c47605..3a07fa27a 100644 --- a/test/functional/post_votes_controller_test.rb +++ b/test/functional/post_votes_controller_test.rb @@ -175,51 +175,51 @@ class PostVotesControllerTest < ActionDispatch::IntegrationTest context "create action" do should "work for a JSON response" do - post_auth post_post_votes_path(post_id: @post.id), @user, params: { score: 1, format: "json" } + post_auth post_post_votes_path(post_id: @post.id, score: 1), @user, as: :json assert_response 201 assert_equal(1, @post.reload.score) end should "not allow anonymous users to vote" do - post post_post_votes_path(post_id: @post.id), params: { score: 1, format: "js" } + post post_post_votes_path(post_id: @post.id, score: 1), xhr: true assert_response 403 assert_equal(0, @post.reload.score) end should "not allow banned users to vote" do - post_auth post_post_votes_path(post_id: @post.id), create(:banned_user), params: { score: 1, format: "js"} + post_auth post_post_votes_path(post_id: @post.id, score: 1), create(:banned_user), xhr: true assert_response 403 assert_equal(0, @post.reload.score) end should "not allow restricted users to vote" do - post_auth post_post_votes_path(post_id: @post.id), create(:restricted_user), params: { score: 1, format: "js"} + post_auth post_post_votes_path(post_id: @post.id, score: 1), create(:restricted_user), xhr: true assert_response 403 assert_equal(0, @post.reload.score) end should "allow members to vote" do - post_auth post_post_votes_path(post_id: @post.id), create(:user), params: { score: 1, format: "js" } + post_auth post_post_votes_path(post_id: @post.id, score: 1), create(:user), xhr: true assert_response :success assert_equal(1, @post.reload.score) end should "not allow invalid scores" do - post_auth post_post_votes_path(post_id: @post.id), @user, params: { score: 3, format: "js" } + post_auth post_post_votes_path(post_id: @post.id, score: 3), @user, xhr: true - assert_response 200 + assert_response :success assert_equal(0, @post.reload.score) assert_equal(0, @post.up_score) assert_equal(0, @post.votes.count) end should "increment a post's score if the score is positive" do - post_auth post_post_votes_path(post_id: @post.id), @user, params: { score: 1, format: "js" } + post_auth post_post_votes_path(post_id: @post.id, score: 1), @user, xhr: true assert_response :success assert_equal(1, @post.reload.score) @@ -228,7 +228,7 @@ class PostVotesControllerTest < ActionDispatch::IntegrationTest end should "decrement a post's score if the score is negative" do - post_auth post_post_votes_path(post_id: @post.id), @user, params: { score: -1, format: "js" } + post_auth post_post_votes_path(post_id: @post.id, score: -1), @user, xhr: true assert_response :success assert_equal(-1, @post.reload.score) @@ -238,45 +238,66 @@ class PostVotesControllerTest < ActionDispatch::IntegrationTest context "for a post that has already been voted on" do should "replace the vote" do - @post.vote!(1, @user) + vote = create(:post_vote, post: @post, user: @user, score: 1) + post_auth post_post_votes_path(post_id: @post.id, score: -1), @user, xhr: true - assert_no_difference("@post.votes.count") do - post_auth post_post_votes_path(post_id: @post.id), @user, params: { score: -1, format: "js" } - - assert_response :success - assert_equal(-1, @post.reload.score) - assert_equal(0, @post.up_score) - assert_equal(-1, @post.down_score) - end + assert_response :success + assert_equal(-1, @post.reload.score) + assert_equal(0, @post.up_score) + assert_equal(-1, @post.down_score) + assert_equal(1, @post.votes.negative.active.count) + assert_equal(1, @post.votes.positive.deleted.count) + assert_equal(true, vote.reload.is_deleted?) end end end context "destroy action" do - should "do nothing for anonymous users" do - delete post_post_votes_path(post_id: @post.id), xhr: true - - assert_response 200 - assert_equal(0, @post.reload.score) + setup do + @vote = create(:post_vote, post: @post, user: @user, score: 1) end - should "do nothing if the post hasn't been voted on" do - delete_auth post_post_votes_path(post_id: @post.id), @user, xhr: true + should "allow users to remove their own votes" do + delete_auth post_post_votes_path(post_id: @vote.post_id), @user, xhr: true assert_response :success assert_equal(0, @post.reload.score) - assert_equal(0, @post.down_score) - assert_equal(0, @post.votes.count) + assert_equal(0, @post.up_score) + assert_equal(0, @post.votes.active.count) + assert_equal(true, @vote.reload.is_deleted?) end - should "remove a vote" do - @post.vote!(1, @user) - delete_auth post_post_votes_path(post_id: @post.id), @user, xhr: true + should "not allow regular users to remove votes by other users" do + delete_auth post_vote_path(@vote), create(:user), xhr: true + + assert_response 403 + assert_equal(1, @post.reload.score) + assert_equal(1, @post.up_score) + assert_equal(1, @post.votes.active.count) + assert_equal(false, @vote.reload.is_deleted?) + end + + should "allow admins to remove votes by other users" do + admin = create(:admin_user) + delete_auth post_vote_path(@vote), admin, xhr: true assert_response :success assert_equal(0, @post.reload.score) - assert_equal(0, @post.down_score) - assert_equal(0, @post.votes.count) + assert_equal(0, @post.up_score) + assert_equal(0, @post.votes.active.count) + assert_equal(true, @vote.reload.is_deleted?) + assert_match(/#{admin.name} deleted post vote #\d+ on post #\d+/, ModAction.post_vote_delete.last.description) + end + + should "not fail when attempting to remove an already removed vote" do + @vote.soft_delete! + delete_auth post_post_votes_path(post_id: @vote.post_id), @user, xhr: true + + assert_response :success + assert_equal(0, @post.reload.score) + assert_equal(0, @post.up_score) + assert_equal(0, @post.votes.active.count) + assert_equal(true, @vote.reload.is_deleted?) end end end diff --git a/test/unit/favorite_test.rb b/test/unit/favorite_test.rb index 8273dc8af..d767d47b8 100644 --- a/test/unit/favorite_test.rb +++ b/test/unit/favorite_test.rb @@ -40,7 +40,8 @@ class FavoriteTest < ActiveSupport::TestCase assert_equal(0, @user.reload.favorite_count) assert_equal(0, @p1.reload.fav_count) assert_equal(0, @p1.reload.score) - refute(PostVote.positive.exists?(post: @p1, user: @user)) + refute(PostVote.active.positive.exists?(post: @p1, user: @user)) + assert(PostVote.deleted.positive.exists?(post: @p1, user: @user)) end end @@ -75,8 +76,8 @@ class FavoriteTest < ActiveSupport::TestCase assert_equal(1, @user.reload.favorite_count) assert_equal(1, @p1.reload.fav_count) assert_equal(1, @p1.reload.score) - assert(PostVote.positive.exists?(post: @p1, user: @user)) - refute(PostVote.negative.exists?(post: @p1, user: @user)) + assert(PostVote.active.positive.exists?(post: @p1, user: @user)) + assert(PostVote.deleted.negative.exists?(post: @p1, user: @user)) end should "not allow duplicate favorites" do diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index 2721c52a9..36c1055ae 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -701,12 +701,12 @@ class PostTest < ActiveSupport::TestCase @post.update(tag_string: "aaa fav:self") assert_equal(1, @post.reload.score) assert_equal(1, @post.favorites.where(user: @user).count) - assert_equal(1, @post.votes.positive.where(user: @user).count) + assert_equal(1, @post.votes.active.positive.where(user: @user).count) @post.update(tag_string: "aaa -fav:self") assert_equal(0, @post.reload.score) assert_equal(0, @post.favorites.count) - assert_equal(0, @post.votes.positive.where(user: @user).count) + assert_equal(0, @post.votes.active.positive.where(user: @user).count) end should "not allow banned users to fav" do @@ -1433,11 +1433,12 @@ class PostTest < ActiveSupport::TestCase end end - should "not decrement the post's score for basic users" do + should "decrement the post's score for basic users" do @member = FactoryBot.create(:user) - assert_no_difference("@post.score") { create(:favorite, post: @post, user: @member) } - assert_no_difference("@post.score") { Favorite.destroy_by(post: @post, user: @member) } + assert_difference("@post.reload.score", -1) do + Favorite.destroy_by(post: @post, user: @user) + end end should "not decrement the user's favorite_count if the user did not favorite the post" do @@ -1565,7 +1566,7 @@ class PostTest < ActiveSupport::TestCase post.vote!(1, user) assert_equal(1, post.reload.score) - assert_equal(1, post.votes.count) + assert_equal(1, post.votes.active.count) end should "allow undoing of votes" do @@ -1578,31 +1579,33 @@ class PostTest < ActiveSupport::TestCase assert_equal(1, post.score) assert_equal(1, post.up_score) assert_equal(0, post.down_score) - assert_equal(1, post.votes.positive.count) + assert_equal(1, post.votes.active.positive.count) - post.unvote!(user) + post.votes.last.soft_delete! + post.reload assert_equal(0, post.score) assert_equal(0, post.up_score) assert_equal(0, post.down_score) - assert_equal(0, post.votes.count) + assert_equal(0, post.votes.active.count) post.vote!(-1, user) assert_equal(-1, post.score) assert_equal(0, post.up_score) assert_equal(-1, post.down_score) - assert_equal(1, post.votes.negative.count) + assert_equal(1, post.votes.active.negative.count) - post.unvote!(user) + post.votes.last.soft_delete! + post.reload assert_equal(0, post.score) assert_equal(0, post.up_score) assert_equal(0, post.down_score) - assert_equal(0, post.votes.count) + assert_equal(0, post.votes.active.count) post.vote!(1, user) assert_equal(1, post.score) assert_equal(1, post.up_score) assert_equal(0, post.down_score) - assert_equal(1, post.votes.positive.count) + assert_equal(1, post.votes.active.positive.count) post.reload assert_equal(1, post.score) diff --git a/test/unit/post_vote_test.rb b/test/unit/post_vote_test.rb index a53232063..53a1324ad 100644 --- a/test/unit/post_vote_test.rb +++ b/test/unit/post_vote_test.rb @@ -9,7 +9,6 @@ class PostVoteTest < ActiveSupport::TestCase context "during validation" do subject { build(:post_vote, post: @post) } - should validate_uniqueness_of(:user_id).scoped_to(:post_id).with_message("have already voted for this post") should validate_inclusion_of(:score).in_array([-1, 1]).with_message("must be 1 or -1") end @@ -21,7 +20,20 @@ class PostVoteTest < ActiveSupport::TestCase assert_equal(1, @post.reload.score) assert_equal(1, @post.up_score) assert_equal(0, @post.down_score) - assert_equal(1, @post.votes.positive.count) + assert_equal(1, @post.votes.active.positive.count) + end + + should "soft delete other votes" do + @user = create(:user) + vote1 = create(:post_vote, post: @post, user: @user, score: -1) + vote2 = create(:post_vote, post: @post, user: @user, score: 1) + + assert_equal(1, @post.reload.score) + assert_equal(1, @post.up_score) + assert_equal(0, @post.down_score) + assert_equal(1, @post.votes.active.positive.count) + assert_equal(0, @post.votes.active.negative.count) + assert_equal(true, vote1.reload.is_deleted?) end end @@ -32,25 +44,40 @@ class PostVoteTest < ActiveSupport::TestCase assert_equal(-1, @post.reload.score) assert_equal(0, @post.up_score) assert_equal(-1, @post.down_score) - assert_equal(1, @post.votes.negative.count) + assert_equal(1, @post.votes.active.negative.count) + end + + should "soft delete other votes" do + @user = create(:user) + vote1 = create(:post_vote, post: @post, user: @user, score: 1) + vote2 = create(:post_vote, post: @post, user: @user, score: -1) + + assert_equal(-1, @post.reload.score) + assert_equal(0, @post.up_score) + assert_equal(-1, @post.down_score) + assert_equal(0, @post.votes.active.positive.count) + assert_equal(1, @post.votes.active.negative.count) + assert_equal(true, vote1.reload.is_deleted?) end end end - context "destroying" do + context "soft deleting" do context "an upvote" do should "decrement the post's score" do vote = create(:post_vote, post: @post, score: 1) assert_equal(1, @post.reload.score) assert_equal(1, @post.up_score) assert_equal(0, @post.down_score) - assert_equal(1, @post.votes.count) + assert_equal(1, @post.votes.active.count) + assert_equal(0, @post.votes.deleted.count) - vote.destroy + vote.soft_delete assert_equal(0, @post.reload.score) assert_equal(0, @post.up_score) assert_equal(0, @post.down_score) - assert_equal(0, @post.votes.count) + assert_equal(0, @post.votes.active.count) + assert_equal(1, @post.votes.deleted.count) end end @@ -60,15 +87,49 @@ class PostVoteTest < ActiveSupport::TestCase assert_equal(-1, @post.reload.score) assert_equal(0, @post.up_score) assert_equal(-1, @post.down_score) - assert_equal(1, @post.votes.count) + assert_equal(1, @post.votes.active.count) + assert_equal(0, @post.votes.deleted.count) - vote.destroy + vote.soft_delete assert_equal(0, @post.reload.score) assert_equal(0, @post.up_score) assert_equal(0, @post.down_score) - assert_equal(0, @post.votes.count) + assert_equal(0, @post.votes.active.count) + assert_equal(1, @post.votes.deleted.count) end end end + + context "deleting a vote by another user" do + should "leave a mod action" do + admin = create(:admin_user, name: "admin") + vote = create(:post_vote, post: @post, score: 1) + + vote.soft_delete!(updater: admin) + assert_match(/admin deleted post vote #\d+ on post #\d+/, ModAction.post_vote_delete.last.description) + end + end + + context "undeleting a vote by another user" do + setup do + @admin = create(:admin_user, name: "admin") + @vote = create(:post_vote, post: @post, score: 1) + + @vote.soft_delete!(updater: @admin) + @vote.update!(is_deleted: false, updater: @admin) + end + + should "restore the score" do + assert_equal(1, @post.reload.score) + assert_equal(1, @post.up_score) + assert_equal(0, @post.down_score) + assert_equal(1, @post.votes.active.count) + assert_equal(0, @post.votes.deleted.count) + end + + should "leave a mod action" do + assert_match(/admin undeleted post vote #\d+ on post #\d+/, ModAction.post_vote_undelete.last.description) + end + end end end