From 30f3e78834c01eaac4eddf0752d59e1371439427 Mon Sep 17 00:00:00 2001
From: Bianca Nenciu <nbianca@users.noreply.github.com>
Date: Fri, 18 Mar 2022 16:31:35 +0200
Subject: [PATCH]  FIX: Reset last sent for existent bookmarks (#16202)

The meaning of reminder_at and reminder_last_sent_at changed after
commit 6d422a8033fb31821203f2725a7fb667ef031e65. A bookmark reminder
will fire only if reminder_last_sent_at is null, but before that it
fired everytime reminder_at was set. This is no longer true because
sometimes reminder_at continues to exist even after a reminder fired.
---
 .../scheduled/bookmark_reminder_notifications.rb  |  2 +-
 app/models/bookmark.rb                            |  2 +-
 ...50247_reset_bookmarks_reminder_last_sent_at.rb | 15 +++++++++++++++
 3 files changed, 17 insertions(+), 2 deletions(-)
 create mode 100644 db/migrate/20220316150247_reset_bookmarks_reminder_last_sent_at.rb

diff --git a/app/jobs/scheduled/bookmark_reminder_notifications.rb b/app/jobs/scheduled/bookmark_reminder_notifications.rb
index bf84ff16124..0795e607bca 100644
--- a/app/jobs/scheduled/bookmark_reminder_notifications.rb
+++ b/app/jobs/scheduled/bookmark_reminder_notifications.rb
@@ -18,7 +18,7 @@ module Jobs
     end
 
     def execute(args = nil)
-      bookmarks = Bookmark.pending_reminders.includes(:user).where(reminder_last_sent_at: nil).order('reminder_at ASC')
+      bookmarks = Bookmark.pending_reminders.includes(:user).order('reminder_at ASC')
       bookmarks.limit(BookmarkReminderNotifications.max_reminder_notifications_per_run).each do |bookmark|
         BookmarkReminderNotificationHandler.send_notification(bookmark)
       end
diff --git a/app/models/bookmark.rb b/app/models/bookmark.rb
index b181d9fd339..e2f413bf13b 100644
--- a/app/models/bookmark.rb
+++ b/app/models/bookmark.rb
@@ -112,7 +112,7 @@ class Bookmark < ActiveRecord::Base
   end
 
   scope :pending_reminders, ->(before_time = Time.now.utc) do
-    with_reminders.where("reminder_at <= :before_time", before_time: before_time)
+    with_reminders.where("reminder_at <= ?", before_time).where(reminder_last_sent_at: nil)
   end
 
   scope :pending_reminders_for_user, ->(user) do
diff --git a/db/migrate/20220316150247_reset_bookmarks_reminder_last_sent_at.rb b/db/migrate/20220316150247_reset_bookmarks_reminder_last_sent_at.rb
new file mode 100644
index 00000000000..a2df61018c1
--- /dev/null
+++ b/db/migrate/20220316150247_reset_bookmarks_reminder_last_sent_at.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+class ResetBookmarksReminderLastSentAt < ActiveRecord::Migration[6.1]
+  def up
+    DB.exec <<~SQL
+      UPDATE bookmarks
+      SET reminder_last_sent_at = NULL
+      WHERE reminder_last_sent_at < reminder_at
+    SQL
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end