FIX: don't clear reminder on deleted bookmarks (#35987)

When sending a bookmark notification and the user has opted to
automatically delete the bookmark, we should not be trying to clear the
reminder since the bookmark was just deleted.

Internal ref - t/146349
This commit is contained in:
Régis Hanol
2025-11-12 17:47:07 +01:00
committed by GitHub
parent abd05f2d56
commit ca3a2d2c88
2 changed files with 23 additions and 4 deletions
@@ -9,16 +9,17 @@ class BookmarkReminderNotificationHandler
def send_notification
return if bookmark.blank?
Bookmark.transaction do
if !bookmark.registered_bookmarkable.can_send_reminder?(bookmark)
bookmark.clear_reminder!
else
if bookmark.registered_bookmarkable.can_send_reminder?(bookmark)
bookmark.registered_bookmarkable.send_reminder_notification(bookmark)
if bookmark.auto_delete_when_reminder_sent?
BookmarkManager.new(bookmark.user).destroy(bookmark.id)
else
bookmark.clear_reminder!
end
else
bookmark.clear_reminder!
end
end
@@ -58,6 +58,15 @@ RSpec.describe BookmarkReminderNotificationHandler do
)
end
it "does not call clear_reminder! after deleting the bookmark" do
allow(bookmark).to receive(:clear_reminder!)
send_notification
expect(Bookmark.find_by(id: bookmark.id)).to be_nil
expect(bookmark).not_to have_received(:clear_reminder!)
end
context "if there are still other bookmarks in the topic" do
before do
Fabricate(
@@ -86,6 +95,15 @@ RSpec.describe BookmarkReminderNotificationHandler do
send_notification
expect(Bookmark.find_by(id: bookmark.id).reminder_at).to eq(nil)
end
it "calls clear_reminder! and keeps the bookmark" do
allow(bookmark).to receive(:clear_reminder!).and_call_original
send_notification
expect(Bookmark.find_by(id: bookmark.id)).not_to be_nil
expect(bookmark).to have_received(:clear_reminder!)
end
end
context "when the auto_delete_preference is never" do