Revert "FIX: notification reminder is deleted with bookmark (#35069)" (#35138)

This reverts commit 74c60fef17.

There are still some outstanding questions about bookmark notification
behaviour.

app/models/bookmark.rb

@@ -28,18 +28,6 @@ class Bookmark < ActiveRecord::Base
   belongs_to :user
   belongs_to :bookmarkable, polymorphic: true
 
-  has_many :reminder_notifications,
-           ->(bookmark) do
-             where(notification_type: Notification.types[:bookmark_reminder]).where(
-               "data::jsonb->>'bookmark_id' = ?",
-               bookmark.id.to_s,
-             )
-           end,
-           class_name: "Notification",
-           foreign_key: :user_id,
-           primary_key: :user_id,
-           dependent: :destroy
-
   def self.auto_delete_preferences
     @auto_delete_preferences ||=
       Enum.new(never: 0, when_reminder_sent: 1, on_owner_reply: 2, clear_reminder: 3)

plugins/chat/spec/system/bookmark_message_spec.rb

@@ -103,37 +103,6 @@ RSpec.describe "Bookmark message", type: :system do
         )
       end
     end
-
-    context "with reminder notification cleanup" do
-      it "removes bookmark reminder notification when bookmark is deleted" do
-        chat_page.visit_channel(category_channel_1)
-        channel_page.bookmark_message(message_1)
-        bookmark_modal.fill_name("Check this out later")
-        bookmark_modal.select_preset_reminder(:tomorrow)
-
-        expect(channel_page).to have_bookmarked_message(message_1)
-
-        bookmark = Bookmark.find_by(bookmarkable: message_1, user: current_user)
-        Chat::MessageBookmarkable.send_reminder_notification(
-          bookmark,
-          data: {
-            title: bookmark.bookmarkable.chat_channel.title(current_user),
-            bookmarkable_url: bookmark.bookmarkable.url,
-          },
-        )
-
-        user_menu.open
-
-        expect(page).to have_css("#quick-access-all-notifications .bookmark-reminder")
-
-        channel_page.bookmark_message(message_1)
-        bookmark_modal.delete
-        bookmark_modal.confirm_delete
-        user_menu.open
-
-        expect(page).to have_no_css("#quick-access-all-notifications .bookmark-reminder")
-      end
-    end
   end
 
   context "when mobile", mobile: true do

spec/models/bookmark_spec.rb

@@ -178,100 +178,4 @@ RSpec.describe Bookmark do
       end
     end
   end
-
-  describe "reminder notifications cleanup" do
-    fab!(:user)
-
-    it "deletes reminder notifications when bookmark is destroyed" do
-      bookmark = Fabricate(:bookmark, user: user, bookmarkable: post)
-
-      notification =
-        Fabricate(
-          :bookmark_reminder_notification,
-          user:,
-          post:,
-          data: {
-            bookmark_id: bookmark.id,
-            bookmarkable_type: "Post",
-            bookmarkable_id: post.id,
-            title: post.topic.title,
-            bookmarkable_url: post.url,
-          }.to_json,
-        )
-
-      expect { bookmark.destroy }.to change { Notification.find_by(id: notification.id) }.from(
-        notification,
-      ).to(nil)
-    end
-
-    it "does not delete other user's reminder notifications" do
-      other_user = Fabricate(:user)
-      bookmark = Fabricate(:bookmark, user: user, bookmarkable: post)
-
-      user_notification =
-        Fabricate(
-          :bookmark_reminder_notification,
-          user:,
-          post:,
-          data: {
-            bookmark_id: bookmark.id,
-            bookmarkable_type: "Post",
-            bookmarkable_id: post.id,
-            title: post.topic.title,
-            bookmarkable_url: post.url,
-          }.to_json,
-        )
-
-      other_notification =
-        Fabricate(
-          :bookmark_reminder_notification,
-          user: other_user,
-          post: post,
-          data: {
-            bookmark_id: bookmark.id,
-            bookmarkable_type: "Post",
-            bookmarkable_id: post.id,
-            title: post.topic.title,
-            bookmarkable_url: post.url,
-          }.to_json,
-        )
-
-      expect { bookmark.destroy }.to change { Notification.find_by(id: user_notification.id) }
-        .from(user_notification)
-        .to(nil)
-        .and(not_change { Notification.find_by(id: other_notification.id) })
-    end
-
-    it "does not delete non-reminder notifications" do
-      bookmark = Fabricate(:bookmark, user: user, bookmarkable: post)
-
-      reminder_notification =
-        Fabricate(
-          :bookmark_reminder_notification,
-          user:,
-          post:,
-          data: {
-            bookmark_id: bookmark.id,
-            bookmarkable_type: "Post",
-            bookmarkable_id: post.id,
-            title: post.topic.title,
-            bookmarkable_url: post.url,
-          }.to_json,
-        )
-
-      other_notification =
-        Fabricate(
-          :notification,
-          user:,
-          post:,
-          notification_type: Notification.types[:liked],
-          data: { post_number: post.post_number }.to_json,
-        )
-
-      expect { bookmark.destroy }.to change { Notification.find_by(id: reminder_notification.id) }
-        .from(reminder_notification)
-        .to(nil)
-        .and(not_change { Notification.find_by(id: other_notification.id) })
-    end
-  end
 end

spec/requests/users_controller_spec.rb

@@ -7705,14 +7705,15 @@ RSpec.describe UsersController do
         expect(notifications.first["data"]["bookmark_id"]).to eq(bookmark_with_reminder.id)
       end
 
-      it "doesn’t show unread notifications when the bookmark has been deleted" do
+      it "shows unread notifications even if the bookmark has been deleted if they have bookmarkable data" do
         bookmark_with_reminder.destroy!
 
         get "/u/#{user.username}/user-menu-bookmarks"
         expect(response.status).to eq(200)
 
         notifications = response.parsed_body["notifications"]
-        expect(notifications.size).to eq(0)
+        expect(notifications.size).to eq(1)
+        expect(notifications.first["data"]["bookmark_id"]).to eq(bookmark_with_reminder.id)
       end
 
       it "does not show unread notifications if the bookmark has been deleted if they only have the bookmark_id data" do