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