diff --git a/app/controllers/favorites_controller.rb b/app/controllers/favorites_controller.rb index 2f086b6b8..cd740ca72 100644 --- a/app/controllers/favorites_controller.rb +++ b/app/controllers/favorites_controller.rb @@ -1,6 +1,5 @@ class FavoritesController < ApplicationController respond_to :html, :xml, :json, :js - rescue_with Favorite::Error, status: 422 def index authorize Favorite @@ -18,23 +17,18 @@ class FavoritesController < ApplicationController end def create - authorize Favorite - @post = Post.find(params[:post_id]) - @post.add_favorite!(CurrentUser.user) - flash.now[:notice] = "You have favorited this post" + @favorite = authorize Favorite.new(post_id: params[:post_id], user: CurrentUser.user) + @favorite.save + @post = @favorite.post.reload + flash.now[:notice] = "You have favorited this post" respond_with(@post) end def destroy - authorize Favorite - @post = Post.find_by_id(params[:id]) - - if @post - @post.remove_favorite!(CurrentUser.user) - else - Favorite.remove(post_id: params[:id], user: CurrentUser.user) - end + @favorite = authorize Favorite.find_by!(post_id: params[:id], user: CurrentUser.user) + @favorite.destroy + @post = @favorite.post.reload flash.now[:notice] = "You have unfavorited this post" respond_with(@post) diff --git a/app/jobs/delete_favorites_job.rb b/app/jobs/delete_favorites_job.rb index add23dae5..83890e84d 100644 --- a/app/jobs/delete_favorites_job.rb +++ b/app/jobs/delete_favorites_job.rb @@ -5,9 +5,7 @@ class DeleteFavoritesJob < ApplicationJob def perform(user) Post.without_timeout do - user.favorites.find_each do |favorite| - Favorite.remove(post: favorite.post, user: user) - end + user.favorites.destroy_all end end end diff --git a/app/logical/post_query_builder.rb b/app/logical/post_query_builder.rb index db2976dff..67c2b45bf 100644 --- a/app/logical/post_query_builder.rb +++ b/app/logical/post_query_builder.rb @@ -441,7 +441,7 @@ class PostQueryBuilder user = User.find_by_name(username) if user.present? && Pundit.policy!(current_user, user).can_see_favorites? - Post.joins(:favorites).merge(Favorite.for_user(user.id)).order("favorites.id DESC") + Post.joins(:favorites).merge(Favorite.where(user: user)).order("favorites.id DESC") else Post.none end diff --git a/app/models/favorite.rb b/app/models/favorite.rb index 061f07c6d..aa1c63aa0 100644 --- a/app/models/favorite.rb +++ b/app/models/favorite.rb @@ -1,23 +1,19 @@ class Favorite < ApplicationRecord - class Error < StandardError; end + belongs_to :post, counter_cache: :fav_count + belongs_to :user, counter_cache: :favorite_count - belongs_to :post - belongs_to :user + validates :user_id, uniqueness: { scope: :post_id, message: "have already favorited this post" } + after_create :upvote_post_on_create + after_destroy :unvote_post_on_destroy - scope :for_user, ->(user_id) { where(user_id: user_id) } scope :public_favorites, -> { where(user: User.bit_prefs_match(:enable_private_favorites, false)) } def self.visible(user) - user.is_admin? ? all : for_user(user.id).or(public_favorites) + user.is_admin? ? all : where(user: user).or(public_favorites) end def self.search(params) - q = search_attributes(params, :id, :post) - - if params[:user_id].present? - q = q.for_user(params[:user_id]) - end - + q = search_attributes(params, :id, :post, :user) q.apply_default_order(params) end @@ -25,37 +21,16 @@ class Favorite < ApplicationRecord [:post, :user] end - def self.add(post:, user:) - Favorite.transaction do - User.where(id: user.id).select("id").lock("FOR UPDATE").first + def upvote_post_on_create + if Pundit.policy!(user, PostVote).create? + PostVote.negative.destroy_by(post: post, user: user) - if Favorite.for_user(user.id).where(:user_id => user.id, :post_id => post.id).exists? - raise Error, "You have already favorited this post" - end - - Favorite.create!(:user_id => user.id, :post_id => post.id) - Post.where(:id => post.id).update_all("fav_count = fav_count + 1") - post.append_user_to_fav_string(user.id) - User.where(:id => user.id).update_all("favorite_count = favorite_count + 1") - user.favorite_count += 1 + # Silently ignore the error if the user has already upvoted the post. + PostVote.create(post: post, user: user, score: 1) end end - def self.remove(user:, post: nil, post_id: nil) - Favorite.transaction do - if post && post_id.nil? - post_id = post.id - end - - User.where(id: user.id).select("id").lock("FOR UPDATE").first - - return unless Favorite.for_user(user.id).where(:user_id => user.id, :post_id => post_id).exists? - Favorite.for_user(user.id).where(post_id: post_id).delete_all - Post.where(:id => post_id).update_all("fav_count = fav_count - 1") - post&.delete_user_from_fav_string(user.id) - User.where(:id => user.id).update_all("favorite_count = favorite_count - 1") - user.favorite_count -= 1 - post.fav_count -= 1 if post - end + def unvote_post_on_destroy + PostVote.positive.destroy_by(post: post, user: user) end end diff --git a/app/models/post.rb b/app/models/post.rb index ed4315a7e..5d6a0db68 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -10,7 +10,7 @@ class Post < ApplicationRecord NOTE_COPY_TAGS = %w[translated partially_translated check_translation translation_request reverse_translation annotated partially_annotated check_annotation annotation_request] - self.ignored_columns = [:pool_string] + self.ignored_columns = [:pool_string, :fav_string] deletable @@ -53,7 +53,7 @@ class Post < ApplicationRecord has_many :children, -> {order("posts.id")}, :class_name => "Post", :foreign_key => "parent_id" has_many :approvals, :class_name => "PostApproval", :dependent => :destroy has_many :disapprovals, :class_name => "PostDisapproval", :dependent => :destroy - has_many :favorites + has_many :favorites, dependent: :destroy has_many :favorited_users, through: :favorites, source: :user has_many :replacements, class_name: "PostReplacement", :dependent => :destroy @@ -582,10 +582,10 @@ class Post < ApplicationRecord pool&.add!(self) when /^fav:(.+)$/i - add_favorite(CurrentUser.user) + Favorite.create(post: self, user: CurrentUser.user) when /^-fav:(.+)$/i - remove_favorite(CurrentUser.user) + Favorite.destroy_by(post: self, user: CurrentUser.user) when /^(up|down)vote:(.+)$/i score = ($1 == "up" ? 1 : -1) @@ -695,56 +695,11 @@ class Post < ApplicationRecord end module FavoriteMethods - def clean_fav_string? - true - end - - def clean_fav_string! - array = fav_string.split.uniq - self.fav_string = array.join(" ") - self.fav_count = array.size - update_column(:fav_string, fav_string) - update_column(:fav_count, fav_count) - end - def favorited_by?(user) return false if user.is_anonymous? Favorite.exists?(post: self, user: user) end - def append_user_to_fav_string(user_id) - update_column(:fav_string, (fav_string + " fav:#{user_id}").strip) - clean_fav_string! if clean_fav_string? - end - - def add_favorite(user) - add_favorite!(user) - true - rescue Favorite::Error - false - end - - def add_favorite!(user) - Favorite.add(post: self, user: user) - vote!(1, user) - end - - def delete_user_from_fav_string(user_id) - update_column(:fav_string, fav_string.gsub(/(?:\A| )fav:#{user_id}(?:\Z| )/, " ").strip) - end - - def remove_favorite!(user) - Favorite.remove(post: self, user: user) - unvote!(user) - end - - def remove_favorite(user) - remove_favorite!(user) - true - rescue Favorite::Error - false - end - # Users who publicly favorited this post, ordered by time of favorite. def visible_favorited_users(viewer) favorited_users.order("favorites.id DESC").select do |fav_user| @@ -756,13 +711,6 @@ class Post < ApplicationRecord FavoriteGroup.for_post(id) end - def remove_from_favorites - Favorite.where(post_id: id).delete_all - user_ids = fav_string.scan(/\d+/) - User.where(:id => user_ids).update_all("favorite_count = favorite_count - 1") - PostVote.where(post_id: id).delete_all - end - def remove_from_fav_groups FavoriteGroup.for_post(id).find_each do |favgroup| favgroup.remove!(self) @@ -857,8 +805,8 @@ class Post < ApplicationRecord transaction do favorites.each do |fav| - remove_favorite!(fav.user) - parent.add_favorite(fav.user) + fav.destroy! + Favorite.create(post: parent, user: fav.user) end end @@ -887,7 +835,6 @@ class Post < ApplicationRecord decrement_tag_post_counts remove_from_all_pools remove_from_fav_groups - remove_from_favorites destroy update_parent_on_destroy end diff --git a/app/policies/favorite_policy.rb b/app/policies/favorite_policy.rb index be6630e3f..b604f1ea1 100644 --- a/app/policies/favorite_policy.rb +++ b/app/policies/favorite_policy.rb @@ -4,6 +4,6 @@ class FavoritePolicy < ApplicationPolicy end def destroy? - !user.is_anonymous? + record.user_id == user.id end end diff --git a/app/policies/post_policy.rb b/app/policies/post_policy.rb index ed5905ea1..5612e1ce5 100644 --- a/app/policies/post_policy.rb +++ b/app/policies/post_policy.rb @@ -81,7 +81,6 @@ class PostPolicy < ApplicationPolicy attributes += TagCategory.categories.map {|x| "tag_string_#{x}".to_sym} attributes += [:file_url, :large_file_url, :preview_file_url] if visible? attributes -= [:id, :md5] if !visible? - attributes -= [:fav_string] attributes end diff --git a/app/views/favorites/_update.js.erb b/app/views/favorites/_update.js.erb index 75051f018..b87eabc06 100644 --- a/app/views/favorites/_update.js.erb +++ b/app/views/favorites/_update.js.erb @@ -1,18 +1,22 @@ -$("#add-to-favorites, #add-fav-button, #remove-from-favorites, #remove-fav-button").toggle(); -$("#remove-fav-button").addClass("animate"); -$("span.post-votes[data-id=<%= @post.id %>]").replaceWith("<%= j render_post_votes @post, current_user: CurrentUser.user %>"); -$("#favcount-for-post-<%= @post.id %>").text(<%= @post.fav_count %>); -$(".fav-buttons").toggleClass("fav-buttons-false").toggleClass("fav-buttons-true"); +<% if @favorite.errors.any? %> + Danbooru.Utility.error("You have already favorited this post"); +<% else %> + $("#add-to-favorites, #add-fav-button, #remove-from-favorites, #remove-fav-button").toggle(); + $("#remove-fav-button").addClass("animate"); + $("span.post-votes[data-id=<%= @post.id %>]").replaceWith("<%= j render_post_votes @post, current_user: CurrentUser.user %>"); + $("#favcount-for-post-<%= @post.id %>").text(<%= @post.fav_count %>); + $(".fav-buttons").toggleClass("fav-buttons-false").toggleClass("fav-buttons-true"); -<% if policy(@post).can_view_favlist? %> - var fav_count = <%= @post.fav_count %>; - $("#favlist").html("<%= j render "posts/partials/show/favorite_list", post: @post %>"); + <% if policy(@post).can_view_favlist? %> + var fav_count = <%= @post.fav_count %>; + $("#favlist").html("<%= j render "posts/partials/show/favorite_list", post: @post %>"); - if (fav_count === 0) { - $("#show-favlist-link, #hide-favlist-link, #favlist").hide(); - } else if (!$("#favlist").is(":visible")) { - $("#show-favlist-link").show(); - } + if (fav_count === 0) { + $("#show-favlist-link, #hide-favlist-link, #favlist").hide(); + } else if (!$("#favlist").is(":visible")) { + $("#show-favlist-link").show(); + } + <% end %> + + Danbooru.Utility.notice("<%= j flash[:notice] %>"); <% end %> - -Danbooru.Utility.notice("<%= j flash[:notice] %>"); diff --git a/config/locales/en.yml b/config/locales/en.yml index eee817cb7..bf885fde2 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -35,6 +35,9 @@ en: attributes: artist_url: url: "" + favorite: + user: "You" + user_id: "You" forum_post_vote: creator_id: "Your vote" moderation_report: diff --git a/db/populate.rb b/db/populate.rb index 76a517ed3..4195cc6cf 100644 --- a/db/populate.rb +++ b/db/populate.rb @@ -186,8 +186,8 @@ if Favorite.count == 0 Post.order("random()").limit(50).each do |post| user = User.order("random()").first - post.add_favorite!(user) - post.add_favorite!(CurrentUser.user) + Favorite.create!(post: post, user: user) + Favorite.create!(post: post, user: Currentuser.user) end else puts "Skipping favorites" diff --git a/test/functional/explore/posts_controller_test.rb b/test/functional/explore/posts_controller_test.rb index f553bc594..b2f35813b 100644 --- a/test/functional/explore/posts_controller_test.rb +++ b/test/functional/explore/posts_controller_test.rb @@ -22,7 +22,7 @@ module Explore context "#curated" do should "render" do @builder = create(:builder_user) - @post.add_favorite!(@builder) + Favorite.create!(post: @post, user: @builder) get curated_explore_posts_path assert_response :success end diff --git a/test/functional/favorites_controller_test.rb b/test/functional/favorites_controller_test.rb index 3c576f733..61e94cffe 100644 --- a/test/functional/favorites_controller_test.rb +++ b/test/functional/favorites_controller_test.rb @@ -6,7 +6,7 @@ class FavoritesControllerTest < ActionDispatch::IntegrationTest @user = create(:user) @post = create(:post) @faved_post = create(:post) - @faved_post.add_favorite!(@user) + create(:favorite, post: @faved_post, user: @user) end context "index action" do @@ -33,30 +33,48 @@ class FavoritesControllerTest < ActionDispatch::IntegrationTest context "create action" do should "create a favorite for the current user" do - assert_difference("Favorite.count", 1) do + assert_difference [-> { @post.favorites.count }, -> { @post.reload.fav_count }, -> { @user.reload.favorite_count }], 1 do + post_auth favorites_path(post_id: @post.id), @user, as: :javascript + assert_response :redirect + end + end + + should "not allow creating duplicate favorites" do + create(:favorite, post: @post, user: @user) + + assert_no_difference [-> { @post.favorites.count }, -> { @post.reload.fav_count }, -> { @user.reload.favorite_count }] do post_auth favorites_path(post_id: @post.id), @user, as: :javascript assert_response :redirect end end should "allow banned users to create favorites" do - assert_difference("Favorite.count", 1) do - post_auth favorites_path(post_id: @post.id), create(:banned_user), as: :javascript + @banned_user = create(:banned_user) + + assert_difference [-> { @post.favorites.count }, -> { @post.reload.fav_count }, -> { @banned_user.reload.favorite_count }], 1 do + post_auth favorites_path(post_id: @post.id), @banned_user, as: :javascript assert_response :redirect end end + + should "not allow anonymous users to create favorites" do + assert_no_difference [-> { @post.favorites.count }, -> { @post.reload.fav_count }] do + post favorites_path(post_id: @post.id), as: :javascript + assert_response 403 + end + end end context "destroy action" do - should "remove the favorite from the current user" do - assert_difference("Favorite.count", -1) 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 end should "allow banned users to destroy favorites" do - assert_difference("Favorite.count", -1) 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 diff --git a/test/functional/moderator/post/posts_controller_test.rb b/test/functional/moderator/post/posts_controller_test.rb index 9e5ead019..7b13dc163 100644 --- a/test/functional/moderator/post/posts_controller_test.rb +++ b/test/functional/moderator/post/posts_controller_test.rb @@ -34,7 +34,7 @@ module Moderator end users = FactoryBot.create_list(:user, 2) users.each do |u| - @child.add_favorite!(u) + Favorite.create!(post: @child, user: u) @child.reload end diff --git a/test/jobs/delete_favorites_job_test.rb b/test/jobs/delete_favorites_job_test.rb index a7b4fb9d7..1081610ba 100644 --- a/test/jobs/delete_favorites_job_test.rb +++ b/test/jobs/delete_favorites_job_test.rb @@ -5,17 +5,17 @@ class DeleteFavoritesJobTest < ActiveJob::TestCase should "delete all favorites" do user = create(:user) posts = create_list(:post, 3) - favorites = posts.each { |post| post.add_favorite!(user) } + favorites = posts.each { |post| Favorite.create!(post: post, user: user) } assert_equal(3, user.favorite_count) assert_equal(3, user.favorites.count) - assert_equal(3, Post.raw_tag_match("fav:#{user.id}").count) + assert_equal(3, Post.user_tag_match("fav:#{user.name}", user).count) DeleteFavoritesJob.perform_now(user) assert_equal(0, user.reload.favorite_count) assert_equal(0, user.favorites.count) - assert_equal(0, Post.raw_tag_match("fav:#{user.id}").count) + assert_equal(0, Post.user_tag_match("fav:#{user.name}", user).count) end end end diff --git a/test/unit/favorite_test.rb b/test/unit/favorite_test.rb index e86d42568..9c0bed69b 100644 --- a/test/unit/favorite_test.rb +++ b/test/unit/favorite_test.rb @@ -2,50 +2,95 @@ require 'test_helper' class FavoriteTest < ActiveSupport::TestCase setup do - @user1 = FactoryBot.create(:user) - @user2 = FactoryBot.create(:user) - @p1 = FactoryBot.create(:post) - @p2 = FactoryBot.create(:post) - - CurrentUser.user = @user1 - CurrentUser.ip_addr = "127.0.0.1" + @user1 = create(:user) + @user2 = create(:user) + @p1 = create(:post) + @p2 = create(:post) end - teardown do - CurrentUser.user = nil - CurrentUser.ip_addr = nil - end + context "Favorites: " do + context "removing a favorite" do + should "update the post and user favorite counts" do + fav = Favorite.create!(post: @p1, user: @user1) - context "A favorite" do - should "delete from all tables" do - @p1.add_favorite!(@user1) - assert_equal(1, @user1.favorite_count) + assert_equal(1, @user1.reload.favorite_count) + assert_equal(1, @p1.reload.fav_count) - Favorite.where(:user_id => @user1.id, :post_id => @p1.id).delete_all - assert_equal(0, Favorite.count) + Favorite.destroy_by(post: @p1, user: @user1) + + assert_equal(0, @user1.reload.favorite_count) + assert_equal(0, @p1.reload.fav_count) + end + + should "remove the upvote if the user could vote" do + @user = create(:gold_user) + @vote = create(:post_vote, post: @p1, user: @user, score: 1) + fav = Favorite.create!(post: @p1, user: @user) + + 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)) + + Favorite.destroy_by(post: @p1, user: @user) + + 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)) + end end - should "know which table it belongs to" do - @p1.add_favorite!(@user1) - @p2.add_favorite!(@user1) - @p1.add_favorite!(@user2) + context "adding a favorite" do + should "update the post and user favorite counts" do + Favorite.create!(post: @p1, user: @user1) - favorites = @user1.favorites.order("id desc") - assert_equal(2, favorites.count) - assert_equal(@p2.id, favorites[0].post_id) - assert_equal(@p1.id, favorites[1].post_id) + assert_equal(1, @user1.reload.favorite_count) + assert_equal(1, @p1.reload.fav_count) + end - favorites = @user2.favorites.order("id desc") - assert_equal(1, favorites.count) - assert_equal(@p1.id, favorites[0].post_id) - end + should "not upvote the post if the user can't vote" do + Favorite.create!(post: @p1, user: @user1) - should "not allow duplicates" do - @p1.add_favorite!(@user1) - error = assert_raises(Favorite::Error) { @p1.add_favorite!(@user1) } + assert_equal(1, @user1.reload.favorite_count) + assert_equal(1, @p1.reload.fav_count) + assert_equal(0, @p1.reload.score) + refute(PostVote.positive.exists?(post: @p1, user: @user1)) + end - assert_equal("You have already favorited this post", error.message) - assert_equal(1, @user1.favorite_count) + should "upvote the post if the user can vote" do + @user = create(:gold_user) + Favorite.create!(post: @p1, user: @user) + + 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)) + end + + should "convert a downvote into an upvote if the post was downvoted" do + @user = create(:gold_user) + @vote = create(:post_vote, post: @p1, user: @user, score: -1) + + assert_equal(-1, @p1.reload.score) + Favorite.create!(post: @p1, user: @user) + + 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)) + end + + should "not allow duplicate favorites" do + @f1 = Favorite.create(post: @p1, user: @user1) + @f2 = Favorite.create(post: @p1, user: @user1) + + assert_equal(["You have already favorited this post"], @f2.errors.full_messages) + assert_equal(1, @user1.reload.favorite_count) + assert_equal(1, @p1.reload.fav_count) + assert_equal(0, @p1.reload.score) + end end end end diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index 46f8fdc28..8ca018128 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -33,7 +33,7 @@ class PostTest < ActiveSupport::TestCase setup do @upload = UploadService.new(FactoryBot.attributes_for(:jpg_upload)).start! @post = @upload.post - Favorite.add(post: @post, user: @user) + Favorite.create!(post: @post, user: @user) create(:favorite_group, post_ids: [@post.id]) perform_enqueued_jobs # perform IqdbAddPostJob end @@ -51,7 +51,9 @@ class PostTest < ActiveSupport::TestCase should "remove all favorites" do @post.expunge! - assert_equal(0, Favorite.for_user(@user.id).where("post_id = ?", @post.id).count) + assert_equal(0, @post.favorites.count) + assert_equal(0, @user.favorites.count) + assert_equal(0, @user.reload.favorite_count) end should "remove all favgroups" do @@ -248,7 +250,7 @@ class PostTest < ActiveSupport::TestCase p1 = FactoryBot.create(:post) c1 = FactoryBot.create(:post, :parent_id => p1.id) user = FactoryBot.create(:gold_user) - c1.add_favorite!(user) + create(:favorite, post: c1, user: user) c1.delete!("test") p1.reload assert(Favorite.exists?(:post_id => c1.id, :user_id => user.id)) @@ -259,7 +261,7 @@ class PostTest < ActiveSupport::TestCase p1 = FactoryBot.create(:post) c1 = FactoryBot.create(:post, :parent_id => p1.id) user = FactoryBot.create(:gold_user) - c1.add_favorite!(user) + create(:favorite, post: c1, user: user) c1.delete!("test", :move_favorites => true) p1.reload assert(!Favorite.exists?(:post_id => c1.id, :user_id => user.id), "Child should not still have favorites") @@ -278,7 +280,7 @@ class PostTest < ActiveSupport::TestCase user = FactoryBot.create(:gold_user) p1 = FactoryBot.create(:post) c1 = FactoryBot.create(:post, :parent_id => p1.id) - c1.add_favorite!(user) + create(:favorite, post: c1, user: user) assert_equal(true, p1.reload.has_active_children?) c1.delete!("test", :move_favorites => true) @@ -837,18 +839,18 @@ class PostTest < ActiveSupport::TestCase context "for a fav" do should "add/remove the current user to the post's favorite listing" do @post.update(tag_string: "aaa fav:self") - assert_equal("fav:#{@user.id}", @post.fav_string) + assert_equal(1, @post.favorites.where(user: @user).count) @post.update(tag_string: "aaa -fav:self") - assert_equal("", @post.fav_string) + assert_equal(0, @post.favorites.count) end should "not fail when the fav: metatag is used twice" do @post.update(tag_string: "aaa fav:self fav:me") - assert_equal("fav:#{@user.id}", @post.fav_string) + assert_equal(1, @post.favorites.where(user: @user).count) @post.update(tag_string: "aaa -fav:self -fav:me") - assert_equal("", @post.fav_string) + assert_equal(0, @post.favorites.count) end end @@ -1531,33 +1533,33 @@ class PostTest < ActiveSupport::TestCase setup do @user = FactoryBot.create(:contributor_user) @post = FactoryBot.create(:post) - @post.add_favorite!(@user) + create(:favorite, post: @post, user: @user) @user.reload end should "decrement the user's favorite_count" do - assert_difference("@user.favorite_count", -1) do - @post.remove_favorite!(@user) + assert_difference("@user.reload.favorite_count", -1) do + Favorite.destroy_by(post: @post, user: @user) end end should "decrement the post's score for gold users" do - assert_difference("@post.score", -1) do - @post.remove_favorite!(@user) + assert_difference("@post.reload.score", -1) do + Favorite.destroy_by(post: @post, user: @user) end end should "not decrement the post's score for basic users" do @member = FactoryBot.create(:user) - assert_no_difference("@post.score") { @post.add_favorite!(@member) } - assert_no_difference("@post.score") { @post.remove_favorite!(@member) } + assert_no_difference("@post.score") { create(:favorite, post: @post, user: @member) } + assert_no_difference("@post.score") { Favorite.destroy_by(post: @post, user: @member) } end should "not decrement the user's favorite_count if the user did not favorite the post" do @post2 = FactoryBot.create(:post) assert_no_difference("@user.favorite_count") do - @post2.remove_favorite!(@user) + Favorite.destroy_by(post: @post2, user: @user) end end end @@ -1568,53 +1570,22 @@ class PostTest < ActiveSupport::TestCase @post = FactoryBot.create(:post) end - should "periodically clean the fav_string" do - @post.update_column(:fav_string, "fav:1 fav:1 fav:1") - @post.update_column(:fav_count, 3) - @post.stubs(:clean_fav_string?).returns(true) - @post.append_user_to_fav_string(2) - assert_equal("fav:1 fav:2", @post.fav_string) - assert_equal(2, @post.fav_count) - end - should "increment the user's favorite_count" do assert_difference("@user.favorite_count", 1) do - @post.add_favorite!(@user) + create(:favorite, post: @post, user: @user) end end should "increment the post's score for gold users" do - @post.add_favorite!(@user) - assert_equal(1, @post.score) + create(:favorite, post: @post, user: @user) + assert_equal(1, @post.reload.score) end should "not increment the post's score for basic users" do @member = FactoryBot.create(:user) - @post.add_favorite!(@member) + create(:favorite, post: @post, user: @member) assert_equal(0, @post.score) end - - should "update the fav strings on the post" do - @post.add_favorite!(@user) - @post.reload - assert_equal("fav:#{@user.id}", @post.fav_string) - assert(Favorite.exists?(:user_id => @user.id, :post_id => @post.id)) - - assert_raises(Favorite::Error) { @post.add_favorite!(@user) } - @post.reload - assert_equal("fav:#{@user.id}", @post.fav_string) - assert(Favorite.exists?(:user_id => @user.id, :post_id => @post.id)) - - @post.remove_favorite!(@user) - @post.reload - assert_equal("", @post.fav_string) - assert(!Favorite.exists?(:user_id => @user.id, :post_id => @post.id)) - - @post.remove_favorite!(@user) - @post.reload - assert_equal("", @post.fav_string) - assert(!Favorite.exists?(:user_id => @user.id, :post_id => @post.id)) - end end context "Moving favorites to a parent post" do @@ -1625,8 +1596,8 @@ class PostTest < ActiveSupport::TestCase @user1 = FactoryBot.create(:user, enable_private_favorites: true) @gold1 = FactoryBot.create(:gold_user) - @child.add_favorite!(@user1) - @child.add_favorite!(@gold1) + create(:favorite, post: @child, user: @user1) + create(:favorite, post: @child, user: @gold1) @child.give_favorites_to_parent @child.reload @@ -1636,12 +1607,10 @@ class PostTest < ActiveSupport::TestCase should "move the favorites" do assert_equal(0, @child.fav_count) assert_equal(0, @child.favorites.count) - assert_equal("", @child.fav_string) assert_equal([], @child.favorites.pluck(:user_id)) assert_equal(2, @parent.fav_count) assert_equal(2, @parent.favorites.count) - assert_equal("fav:#{@user1.id} fav:#{@gold1.id}", @parent.fav_string) assert_equal([@user1.id, @gold1.id], @parent.favorites.pluck(:user_id)) end diff --git a/test/unit/user_deletion_test.rb b/test/unit/user_deletion_test.rb index 9c386ae7f..505aa69d1 100644 --- a/test/unit/user_deletion_test.rb +++ b/test/unit/user_deletion_test.rb @@ -69,13 +69,12 @@ class UserDeletionTest < ActiveSupport::TestCase should "remove any favorites" do @post = create(:post) - Favorite.add(post: @post, user: @user) + Favorite.create!(post: @post, user: @user) perform_enqueued_jobs { @deletion.delete! } assert_equal(0, Favorite.count) - assert_equal("", @post.reload.fav_string) - assert_equal(0, @post.fav_count) + assert_equal(0, @post.reload.fav_count) end end end