favoritescontroller#destroy should work even if the post doesn't exist, remove from favorites on expunge (fixes #3222)

app/controllers/favorites_controller.rb

@@ -47,12 +47,19 @@ class FavoritesController < ApplicationController
   end
 
   def destroy
-    @post = Post.find(params[:id])
+    @post = Post.find_by_id(params[:id])
-    @post.remove_favorite!(CurrentUser.user)
+
+    if @post
+      @post.remove_favorite!(CurrentUser.user)
+      path = mobile_post_path(@post)
+    else
+      Favorite.remove(post_id: params[:id], user: CurrentUser.user)
+      path = mobile_posts_path
+    end
 
     respond_with(@post) do |format|
       format.html do
-        redirect_to(mobile_post_path(@post))
+        redirect_to(path)
       end
       format.js
       format.json do

app/logical/user_deletion.rb

@@ -7,7 +7,7 @@ class UserDeletion
     user = User.find(user_id)
     Post.without_timeout do
       Post.raw_tag_match("fav:#{user_id}").where("true /* UserDeletion.remove_favorites_for */").find_each do |post|
-        Favorite.remove(post, user)
+        Favorite.remove(post: post, user: user)
       end
     end
   end

app/models/favorite.rb

@@ -5,15 +5,19 @@ class Favorite < ApplicationRecord
   attr_accessible :user_id, :post_id
 
   # this is necessary because there's no trigger for deleting favorites
-  def self.destroy_all(hash)
+  def self.destroy_all(user_id: nil, post_id: nil)
-    if hash[:user_id] && hash[:post_id]
+    if user_id && post_id
-      connection.execute("delete from favorites_#{hash[:user_id] % 100} where user_id = #{hash[:user_id]} and post_id = #{hash[:post_id]}")
+      connection.execute("delete from favorites_#{user_id % 100} where user_id = #{user_id} and post_id = #{post_id}")
-    elsif hash[:user_id]
+    elsif user_id
-      connection.execute("delete from favorites_#{hash[:user_id] % 100} where user_id = #{hash[:user_id]}")
+      connection.execute("delete from favorites_#{user_id % 100} where user_id = #{user_id}")
+    elsif post_id
+      0.upto(99) do |uid|
+        connection.execute("delete from favorites_#{uid} where post_id = #{post_id}")
+      end
     end
   end
 
-  def self.add(post, user)
+  def self.add(post:, user:)
     Favorite.transaction do
       User.where(:id => user.id).select("id").lock("FOR UPDATE NOWAIT").first
 
@@ -27,17 +31,21 @@ class Favorite < ApplicationRecord
     end
   end
 
-  def self.remove(post, user)
+  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 NOWAIT").first
 
-      return unless Favorite.for_user(user.id).where(:user_id => user.id, :post_id => post.id).exists?
+      return unless Favorite.for_user(user.id).where(:user_id => user.id, :post_id => post_id).exists?
-      Favorite.destroy_all(user_id: user.id, post_id: post.id)
+      Favorite.destroy_all(user_id: user.id, post_id: post_id)
-      Post.where(:id => post.id).update_all("fav_count = fav_count - 1")
+      Post.where(:id => post_id).update_all("fav_count = fav_count - 1")
-      post.delete_user_from_fav_string(user.id)
+      post.delete_user_from_fav_string(user.id) if post
       User.where(:id => user.id).update_all("favorite_count = favorite_count - 1")
       user.favorite_count -= 1
-      post.fav_count -= 1
+      post.fav_count -= 1 if post
     end
   end
 end

app/models/post.rb

@@ -982,7 +982,7 @@ class Post < ApplicationRecord
     end
 
     def add_favorite!(user)
-      Favorite.add(self, user)
+      Favorite.add(post: self, user: user)
       vote!("up", user) if user.is_voter?
     rescue PostVote::Error
     end
@@ -992,7 +992,7 @@ class Post < ApplicationRecord
     end
 
     def remove_favorite!(user)
-      Favorite.remove(self, user)
+      Favorite.remove(post: self, user: user)
       unvote!(user) if user.is_voter?
     rescue PostVote::Error
     end
@@ -1022,6 +1022,10 @@ class Post < ApplicationRecord
       end
     end
 
+    def remove_from_favorites
+      Favorite.destroy_all(post_id: self.id)
+    end
+
     def remove_from_fav_groups
       FavoriteGroup.for_post(id).find_each do |group|
         group.remove!(self)
@@ -1383,6 +1387,7 @@ class Post < ApplicationRecord
         decrement_tag_post_counts
         remove_from_all_pools
         remove_from_fav_groups
+        remove_from_favorites
         destroy
         update_parent_on_destroy
       end

app/models/user.rb

@@ -289,12 +289,12 @@ class User < ApplicationRecord
     end
 
     def add_favorite!(post)
-      Favorite.add(post, self)
+      Favorite.add(post: post, user: self)
       clean_favorite_count! if clean_favorite_count?
     end
 
     def remove_favorite!(post)
-      Favorite.remove(post, self)
+      Favorite.remove(post: post, user: self)
     end
 
     def favorite_groups

test/unit/post_test.rb

@@ -36,6 +36,7 @@ class PostTest < ActiveSupport::TestCase
         @upload = FactoryGirl.create(:jpg_upload)
         @upload.process!
         @post = @upload.post
+        Favorite.add(post: @post, user: @user)
       end
 
       should "delete the files" do
@@ -52,6 +53,14 @@ class PostTest < ActiveSupport::TestCase
         assert_equal(false, File.exists?(@post.file_path))
       end
 
+      should "remove all favorites" do
+        TestAfterCommit.with_commits(true) do
+          @post.expunge!
+        end
+
+        assert_equal(0, Favorite.for_user(@user.id).where("post_id = ?", @post.id).count)
+      end
+
       should "remove the post from iqdb" do
         mock_iqdb_service!
         Post.iqdb_sqs_service.expects(:send_message).with("remove\n#{@post.id}")

test/unit/user_deletion_test.rb

@@ -40,7 +40,7 @@ class UserDeletionTest < ActiveSupport::TestCase
       CurrentUser.ip_addr = "127.0.0.1"
 
       @post = FactoryGirl.create(:post)
-      Favorite.add(@post, @user)
+      Favorite.add(post: @post, user: @user)
 
       @user.update_attributes(:email => "ted@danbooru.com")