From c3cede697dddeacabc6410ff4e4f8425805004cb Mon Sep 17 00:00:00 2001
From: Martin Brennan <mjrbrennan@gmail.com>
Date: Wed, 14 Oct 2020 09:38:57 +1000
Subject: [PATCH] FEATURE: Add weekly bookmark cleanup code (#10899)

When posts or topics are deleted we don't want to immediately delete associated bookmarks, so we have a grace period to recover them and their reminders if the post or topic is un-deleted. This PR adds a task to the Weekly scheduled job to go and delete bookmarks attached to posts or topics deleted > 3 days ago.
---
 app/jobs/scheduled/weekly.rb |  1 +
 app/models/bookmark.rb       | 14 ++++++++++++
 spec/models/bookmark_spec.rb | 43 ++++++++++++++++++++++++++++++++++++
 3 files changed, 58 insertions(+)
 create mode 100644 spec/models/bookmark_spec.rb

diff --git a/app/jobs/scheduled/weekly.rb b/app/jobs/scheduled/weekly.rb
index c71ac7042dc..a020fa11f1d 100644
--- a/app/jobs/scheduled/weekly.rb
+++ b/app/jobs/scheduled/weekly.rb
@@ -14,6 +14,7 @@ module Jobs
       UserAuthToken.cleanup!
       Email::Cleaner.delete_rejected!
       Notification.purge_old!
+      Bookmark.cleanup!
     end
   end
 end
diff --git a/app/models/bookmark.rb b/app/models/bookmark.rb
index a8f7a37f5a0..1dc75828f78 100644
--- a/app/models/bookmark.rb
+++ b/app/models/bookmark.rb
@@ -103,6 +103,20 @@ class Bookmark < ActiveRecord::Base
       .order('date(bookmarks.created_at)')
       .count
   end
+
+  ##
+  # Deletes bookmarks that are attached to posts/topics that were deleted
+  # more than X days ago. We don't delete bookmarks instantly when a post/topic
+  # is deleted so that there is a grace period to un-delete.
+  def self.cleanup!
+    grace_time = 3.days.ago
+    DB.exec(<<~SQL, grace_time: grace_time)
+      DELETE FROM bookmarks b
+      USING topics t, posts p
+      WHERE (b.topic_id = t.id AND b.post_id = p.id)
+        AND (t.deleted_at < :grace_time OR p.deleted_at < :grace_time)
+    SQL
+  end
 end
 
 # == Schema Information
diff --git a/spec/models/bookmark_spec.rb b/spec/models/bookmark_spec.rb
new file mode 100644
index 00000000000..3d4b5a6acf1
--- /dev/null
+++ b/spec/models/bookmark_spec.rb
@@ -0,0 +1,43 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe Bookmark do
+  describe "#cleanup!" do
+    it "deletes bookmarks attached to a deleted post which has been deleted for > 3 days" do
+      post = Fabricate(:post)
+      bookmark = Fabricate(:bookmark, post: post, topic: post.topic)
+      bookmark2 = Fabricate(:bookmark, post: Fabricate(:post, topic: post.topic))
+      post.trash!
+      post.update(deleted_at: 4.days.ago)
+      Bookmark.cleanup!
+      expect(Bookmark.find_by(id: bookmark.id)).to eq(nil)
+      expect(Bookmark.find_by(id: bookmark2.id)).to eq(bookmark2)
+    end
+
+    it "deletes bookmarks attached to a deleted topic which has been deleted for > 3 days" do
+      post = Fabricate(:post)
+      bookmark = Fabricate(:bookmark, post: post, topic: post.topic)
+      bookmark2 = Fabricate(:bookmark, topic: post.topic, post: Fabricate(:post, topic: post.topic))
+      bookmark3 = Fabricate(:bookmark)
+      post.topic.trash!
+      post.topic.update(deleted_at: 4.days.ago)
+      Bookmark.cleanup!
+      expect(Bookmark.find_by(id: bookmark.id)).to eq(nil)
+      expect(Bookmark.find_by(id: bookmark2.id)).to eq(nil)
+      expect(Bookmark.find_by(id: bookmark3.id)).to eq(bookmark3)
+    end
+
+    it "does not delete bookmarks attached to posts that are not deleted or that have not met the 3 day grace period" do
+      post = Fabricate(:post)
+      bookmark = Fabricate(:bookmark, post: post, topic: post.topic)
+      bookmark2 = Fabricate(:bookmark)
+      Bookmark.cleanup!
+      expect(Bookmark.find(bookmark.id)).to eq(bookmark)
+      post.trash!
+      Bookmark.cleanup!
+      expect(Bookmark.find(bookmark.id)).to eq(bookmark)
+      expect(Bookmark.find_by(id: bookmark2.id)).to eq(bookmark2)
+    end
+  end
+end