From fbee7f6912080fab47221d4668a6adc73e3e74b2 Mon Sep 17 00:00:00 2001 From: evazion Date: Thu, 20 Jul 2017 21:06:37 -0500 Subject: [PATCH] Post#expunge!: fix remove_pool! to remove posts from deleted pools. Don't silently ignore attempts to remove posts from deleted pools. Remove the restriction on removing posts from deleted pools instead (ref: #1109). Fixes failure to remove posts from deleted pools during expungement. --- app/models/pool.rb | 5 ++--- app/models/post.rb | 3 +-- test/unit/post_test.rb | 13 +++++++++++-- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/app/models/pool.rb b/app/models/pool.rb index eaae96384..42a84c5b2 100644 --- a/app/models/pool.rb +++ b/app/models/pool.rb @@ -239,11 +239,10 @@ class Pool < ApplicationRecord def remove!(post) return unless contains?(post.id) return unless CurrentUser.user.can_remove_from_pools? - return if is_deleted? with_lock do update_attributes(:post_ids => remove_number_from_string(post.id, post_ids), :post_count => post_count - 1) - post.remove_pool!(self, true) + post.remove_pool!(self) clear_post_id_array end end @@ -284,7 +283,7 @@ class Pool < ApplicationRecord removed.each do |post_id| post = Post.find(post_id) - post.remove_pool!(self, true) + post.remove_pool!(self) end normalize_post_ids diff --git a/app/models/post.rb b/app/models/post.rb index f095f3092..04f974c82 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -1079,10 +1079,9 @@ class Post < ApplicationRecord end end - def remove_pool!(pool, force = false) + def remove_pool!(pool) return unless belongs_to_pool?(pool) return unless CurrentUser.user.can_remove_from_pools? - return if pool.is_deleted? && !force with_lock do self.pool_string = pool_string.gsub(/(?:\A| )pool:#{pool.id}(?:\Z| )/, " ").strip diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index a1d122492..d0b760fb2 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -86,15 +86,24 @@ class PostTest < ActiveSupport::TestCase SqsService.any_instance.stubs(:send_message) @pool = FactoryGirl.create(:pool) @pool.add!(@post) - @post.reload + + @deleted_pool = FactoryGirl.create(:pool) + @deleted_pool.add!(@post) + @deleted_pool.update_columns(is_deleted: true) + @post.expunge! + @pool.reload + @deleted_pool.reload end should "remove the post from all pools" do - @pool.reload assert_equal("", @pool.post_ids) end + should "remove the post from deleted pools" do + assert_equal("", @deleted_pool.post_ids) + end + should "destroy the record" do assert_equal([], @post.errors.full_messages) assert_equal(0, Post.where("id = ?", @post.id).count)