FIX: don’t use memoize in the loop (#82)
Memoize was a bad idea when we have more than one pending timers
diff --git a/app/jobs/scheduled/encrypted_post_timer_evaluator.rb b/app/jobs/scheduled/encrypted_post_timer_evaluator.rb
index 148e0b2..9a507a8 100644
--- a/app/jobs/scheduled/encrypted_post_timer_evaluator.rb
+++ b/app/jobs/scheduled/encrypted_post_timer_evaluator.rb
@@ -8,10 +8,19 @@ module Jobs
EncryptedPostTimer.pending.find_each do |encrypted_post_timer|
ActiveRecord::Base.transaction do
encrypted_post_timer.touch(:destroyed_at)
- next if posts_to_delete(encrypted_post_timer).blank?
- next unless @topic
- @topic.update_columns(deleted_at: nil)
- posts_to_delete(encrypted_post_timer).each do |post|
+
+ timer_post = Post.with_deleted.find_by(id: encrypted_post_timer.post_id)
+ next if !timer_post
+
+ timer_topic = Topic.with_deleted.find_by(id: timer_post.topic_id)
+ next if !timer_topic
+
+ posts_to_delete = find_posts_to_delete(timer_topic, timer_post)
+ next if posts_to_delete.blank?
+
+ timer_topic.update_columns(deleted_at: nil)
+
+ posts_to_delete.each do |post|
next if !post&.persisted?
PostDestroyer.new(post.user, post, permanent: true).destroy
end
@@ -19,12 +28,10 @@ module Jobs
end
end
- def posts_to_delete(encrypted_post_timer)
- @post ||= Post.with_deleted.find_by(id: encrypted_post_timer.post_id)
- return [] unless @post
- @topic ||= Topic.with_deleted.find_by(id: @post.topic_id)
- @posts_to_delete ||= @post&.is_first_post? ? @topic.posts.with_deleted.order(created_at: :desc) : [@post]
- @posts_to_delete.compact
+ private
+
+ def find_posts_to_delete(topic, post)
+ (post.is_first_post? ? topic.posts.with_deleted.order(created_at: :desc) : [post]).compact
end
end
end
diff --git a/spec/jobs/encrypted_post_timer_evaluator_spec.rb b/spec/jobs/encrypted_post_timer_evaluator_spec.rb
index 0bc7205..0652a7a 100644
--- a/spec/jobs/encrypted_post_timer_evaluator_spec.rb
+++ b/spec/jobs/encrypted_post_timer_evaluator_spec.rb
@@ -31,19 +31,22 @@ describe Jobs::EncryptedPostTimerEvaluator do
context 'explosion of consecutive posts' do
it 'when time is right, delete only one post' do
encrypted_post_timer = EncryptedPostTimer.create!(post: post2, delete_at: 1.hour.from_now)
+ encrypted_post_timer2 = EncryptedPostTimer.create!(post: post3, delete_at: 1.hour.from_now)
described_class.new.execute({})
expect(post1.reload.persisted?).to be true
expect(post2.reload.persisted?).to be true
expect(post3.reload.persisted?).to be true
expect(encrypted_post_timer.reload.destroyed_at).to be nil
+ expect(encrypted_post_timer2.reload.destroyed_at).to be nil
freeze_time 61.minutes.from_now
described_class.new.execute({})
expect(post1.reload.persisted?).to be true
expect { post2.reload }.to raise_error(ActiveRecord::RecordNotFound)
- expect(post3.reload.persisted?).to be true
+ expect { post3.reload }.to raise_error(ActiveRecord::RecordNotFound)
expect(topic.reload.persisted?).to be true
expect(encrypted_post_timer.reload.destroyed_at).not_to be nil
+ expect(encrypted_post_timer2.reload.destroyed_at).not_to be nil
end
it 'does not error when post is already deleted' do
GitHub sha: 41a62020