diff --git a/app/components/post_preview_component.rb b/app/components/post_preview_component.rb index 80ea2205e..8a272613a 100644 --- a/app/components/post_preview_component.rb +++ b/app/components/post_preview_component.rb @@ -69,7 +69,6 @@ class PostPreviewComponent < ApplicationComponent "data-id" => post.id, "data-has-sound" => post.has_tag?("sound"), "data-tags" => post.tag_string, - "data-pools" => post.pool_string, "data-approver-id" => post.approver_id, "data-rating" => post.rating, "data-large-width" => post.large_image_width, diff --git a/app/controllers/pools_controller.rb b/app/controllers/pools_controller.rb index c6ed65d98..d307424ba 100644 --- a/app/controllers/pools_controller.rb +++ b/app/controllers/pools_controller.rb @@ -45,11 +45,8 @@ class PoolsController < ApplicationController end def update - # need to do this in order for synchronize! to work correctly @pool = authorize Pool.find(params[:id]) - @pool.attributes = permitted_attributes(@pool) - @pool.synchronize - @pool.save + @pool.update(permitted_attributes(@pool)) unless @pool.errors.any? flash[:notice] = "Pool updated" end diff --git a/app/models/pool.rb b/app/models/pool.rb index 01995915c..f8da79b7c 100644 --- a/app/models/pool.rb +++ b/app/models/pool.rb @@ -10,7 +10,6 @@ class Pool < ApplicationRecord validate :updater_can_edit_deleted before_validation :normalize_post_ids before_validation :normalize_name - after_create :synchronize! after_save :create_version deletable @@ -130,7 +129,7 @@ class Pool < ApplicationRecord self.post_ids = version.post_ids self.name = version.name self.description = version.description - synchronize! + save! end def contains?(post_id) @@ -161,7 +160,6 @@ class Pool < ApplicationRecord with_lock do update(post_ids: post_ids + [post.id]) - post.add_pool!(self, true) end end @@ -171,7 +169,6 @@ class Pool < ApplicationRecord with_lock do reload update(post_ids: post_ids - [post.id]) - post.remove_pool!(self) end end @@ -181,29 +178,6 @@ class Pool < ApplicationRecord Post.joins("JOIN (#{pool_posts.to_sql}) pool_posts ON pool_posts.post_id = posts.id").order("pool_posts.pool_index ASC") end - def synchronize - post_ids_before = post_ids_before_last_save || post_ids_was - added = post_ids - post_ids_before - removed = post_ids_before - post_ids - - added.each do |post_id| - post = Post.find(post_id) - post.add_pool!(self, true) - end - - removed.each do |post_id| - post = Post.find(post_id) - post.remove_pool!(self) - end - - normalize_post_ids - end - - def synchronize! - synchronize - save if will_save_change_to_post_ids? - end - def post_count post_ids.size end diff --git a/app/models/post.rb b/app/models/post.rb index 562019170..ed4315a7e 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -10,6 +10,8 @@ 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] + deletable before_validation :merge_old_changes @@ -561,23 +563,23 @@ class Post < ApplicationRecord case tag when /^-pool:(\d+)$/i pool = Pool.find_by_id($1.to_i) - remove_pool!(pool) if pool + pool&.remove!(self) when /^-pool:(.+)$/i pool = Pool.find_by_name($1) - remove_pool!(pool) if pool + pool&.remove!(self) when /^pool:(\d+)$/i pool = Pool.find_by_id($1.to_i) - add_pool!(pool) if pool + pool&.add!(self) when /^pool:(.+)$/i pool = Pool.find_by_name($1) - add_pool!(pool) if pool + pool&.add!(self) when /^newpool:(.+)$/i pool = Pool.find_by_name($1) - add_pool!(pool) if pool + pool&.add!(self) when /^fav:(.+)$/i add_favorite(CurrentUser.user) @@ -777,35 +779,6 @@ class Post < ApplicationRecord pools.undeleted.present? end - def belongs_to_pool?(pool) - pool_string =~ /(?:\A| )pool:#{pool.id}(?:\Z| )/ - end - - def belongs_to_pool_with_id?(pool_id) - pool_string =~ /(?:\A| )pool:#{pool_id}(?:\Z| )/ - end - - def add_pool!(pool, force = false) - return if belongs_to_pool?(pool) - return if pool.is_deleted? && !force - - with_lock do - self.pool_string = "#{pool_string} pool:#{pool.id}".strip - update_column(:pool_string, pool_string) unless new_record? - pool.add!(self) - end - end - - def remove_pool!(pool) - return unless belongs_to_pool?(pool) - - with_lock do - self.pool_string = pool_string.gsub(/(?:\A| )pool:#{pool.id}(?:\Z| )/, " ").strip - update_column(:pool_string, pool_string) unless new_record? - pool.remove!(self) - end - end - def remove_from_all_pools pools.find_each do |pool| pool.remove!(self) diff --git a/test/unit/pool_test.rb b/test/unit/pool_test.rb index 6bf9492d6..f13335583 100644 --- a/test/unit/pool_test.rb +++ b/test/unit/pool_test.rb @@ -68,13 +68,6 @@ class PoolTest < ActiveSupport::TestCase should "initialize the post count" do assert_equal(@posts.size, @pool.post_count) end - - should "synchronize the posts with the pool" do - assert_equal(@posts.map(&:id), @pool.post_ids) - - @posts.each(&:reload) - assert_equal(["pool:#{@pool.id}"] * @posts.size, @posts.map(&:pool_string)) - end end context "Reverting a pool" do @@ -94,19 +87,15 @@ class PoolTest < ActiveSupport::TestCase assert_equal([], @pool.versions[0].post_ids) assert_equal([@p1.id], @pool.versions[1].post_ids) assert_equal([@p1.id, @p2.id], @pool.versions[2].post_ids) - assert_equal([@p1.id, @p2.id], @pool.post_ids) + assert_equal([@p1.id, @p2.id], @pool.reload.post_ids) end should "update its post_ids" do @pool.revert_to!(@pool.versions[1]) - assert_equal([@p1.id], @pool.reload.post_ids) - assert_equal("pool:#{@pool.id}", @p1.reload.pool_string) - #assert_equal("", @p2.reload.pool_string) + assert_equal([@p1.id], @pool.post_ids) @pool.revert_to!(@pool.versions[0]) assert_equal([], @pool.reload.post_ids) - assert_equal("", @p1.reload.pool_string) - #assert_equal("", @p2.reload.pool_string) end end @@ -125,7 +114,6 @@ class PoolTest < ActiveSupport::TestCase context "by #attributes=" do setup do @pool.attributes = {post_ids: [@p1.id, @p2.id]} - @pool.synchronize @pool.save end @@ -138,10 +126,6 @@ class PoolTest < ActiveSupport::TestCase assert_equal([@p1.id], @pool.post_ids) end - should "add the pool to the post" do - assert_equal("pool:#{@pool.id}", @p1.pool_string) - end - should "increment the post count" do assert_equal(1, @pool.post_count) end @@ -155,10 +139,6 @@ class PoolTest < ActiveSupport::TestCase assert_equal([@p1.id], @pool.post_ids) end - should "not double add the pool to the post" do - assert_equal("pool:#{@pool.id}", @p1.pool_string) - end - should "not double increment the post count" do assert_equal(1, @pool.post_count) end @@ -171,7 +151,6 @@ class PoolTest < ActiveSupport::TestCase @pool.update_attribute(:is_deleted, true) @pool.post_ids += [@p2.id] - @pool.synchronize! @pool.save @pool.reload @p2.reload @@ -181,10 +160,6 @@ class PoolTest < ActiveSupport::TestCase assert_equal([@p1.id, @p2.id], @pool.post_ids) end - should "add the pool to the post" do - assert_equal("pool:#{@pool.id}", @p2.pool_string) - end - should "increment the post count" do assert_equal(2, @pool.post_count) end @@ -205,10 +180,6 @@ class PoolTest < ActiveSupport::TestCase assert_equal([], @pool.post_ids) end - should "remove the pool from the post" do - assert_equal("", @p1.pool_string) - end - should "update the post count" do assert_equal(0, @pool.post_count) end @@ -223,10 +194,6 @@ class PoolTest < ActiveSupport::TestCase assert_equal([@p1.id], @pool.post_ids) end - should "not affect the post" do - assert_equal("pool:#{@pool.id}", @p1.pool_string) - end - should "not affect the post count" do assert_equal(1, @pool.post_count) end @@ -289,29 +256,6 @@ class PoolTest < ActiveSupport::TestCase @pool.add!(@p3) end - context "that is synchronized" do - setup do - @pool.reload - @pool.post_ids = [@p2.id] - @pool.synchronize! - end - - should "update the pool" do - @pool.reload - assert_equal(1, @pool.post_count) - assert_equal([@p2.id], @pool.post_ids) - end - - should "update the posts" do - @p1.reload - @p2.reload - @p3.reload - assert_equal("", @p1.pool_string) - assert_equal("pool:#{@pool.id}", @p2.pool_string) - assert_equal("", @p3.pool_string) - end - end - should "find the neighbors for the first post" do assert_nil(@pool.previous_post_id(@p1.id)) assert_equal(@p2.id, @pool.next_post_id(@p1.id)) diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index c218168b2..46f8fdc28 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -751,7 +751,6 @@ class PostTest < ActiveSupport::TestCase @post.reload @pool.reload assert_equal([@post.id], @pool.post_ids) - assert_equal("pool:#{@pool.id}", @post.pool_string) end end @@ -759,7 +758,7 @@ class PostTest < ActiveSupport::TestCase setup do @pool = FactoryBot.create(:pool) @post = FactoryBot.create(:post, :tag_string => "aaa") - @post.add_pool!(@pool) + @pool.add!(@post) @post.tag_string = "aaa -pool:#{@pool.id}" @post.save end @@ -768,7 +767,6 @@ class PostTest < ActiveSupport::TestCase @post.reload @pool.reload assert_equal([], @pool.post_ids) - assert_equal("", @post.pool_string) end end @@ -782,7 +780,6 @@ class PostTest < ActiveSupport::TestCase @post.reload @pool.reload assert_equal([@post.id], @pool.post_ids) - assert_equal("pool:#{@pool.id}", @post.pool_string) end end @@ -797,7 +794,6 @@ class PostTest < ActiveSupport::TestCase @post.reload @pool.reload assert_equal([@post.id], @pool.post_ids) - assert_equal("pool:#{@pool.id}", @post.pool_string) end end @@ -808,7 +804,6 @@ class PostTest < ActiveSupport::TestCase @post.reload assert_not_nil(@pool) assert_equal([@post.id], @pool.post_ids) - assert_equal("pool:#{@pool.id}", @post.pool_string) end end @@ -1661,36 +1656,6 @@ class PostTest < ActiveSupport::TestCase setup do SqsService.any_instance.stubs(:send_message) end - - context "Removing a post from a pool" do - should "update the post's pool string" do - post = FactoryBot.create(:post) - pool = FactoryBot.create(:pool) - post.add_pool!(pool) - post.remove_pool!(pool) - post.reload - assert_equal("", post.pool_string) - post.remove_pool!(pool) - post.reload - assert_equal("", post.pool_string) - end - end - - context "Adding a post to a pool" do - should "update the post's pool string" do - post = FactoryBot.create(:post) - pool = FactoryBot.create(:pool) - post.add_pool!(pool) - post.reload - assert_equal("pool:#{pool.id}", post.pool_string) - post.add_pool!(pool) - post.reload - assert_equal("pool:#{pool.id}", post.pool_string) - post.remove_pool!(pool) - post.reload - assert_equal("", post.pool_string) - end - end end context "Uploading:" do diff --git a/test/unit/post_version_test.rb b/test/unit/post_version_test.rb index fa30ee0c5..d89664753 100644 --- a/test/unit/post_version_test.rb +++ b/test/unit/post_version_test.rb @@ -73,7 +73,7 @@ class PostVersionTest < ActiveSupport::TestCase should "create a version" do assert_equal("tagme", @post.tag_string) - assert_equal("pool:#{@pool.id}", @post.pool_string) + assert_equal([@pool.id], @post.pools.pluck(:id)) assert_equal(1, @post.versions.size) assert_equal("tagme", @post.versions.last.tags)