From 22dffe6f6951a26b0040381548952d38cb58b203 Mon Sep 17 00:00:00 2001
From: Martin Brennan <martin@discourse.org>
Date: Fri, 8 May 2020 15:24:37 +0000
Subject: [PATCH] FIX: Do not allow null options for bookmark manager

---
 lib/bookmark_manager.rb           | 10 ++++++++--
 spec/lib/bookmark_manager_spec.rb | 11 +++++++++++
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/lib/bookmark_manager.rb b/lib/bookmark_manager.rb
index 90088181f63..44e6361f22f 100644
--- a/lib/bookmark_manager.rb
+++ b/lib/bookmark_manager.rb
@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
 class BookmarkManager
+  DEFAULT_OPTIONS = { delete_when_reminder_sent: false }
+
   include HasErrors
 
   def initialize(user)
@@ -22,7 +24,7 @@ class BookmarkManager
         reminder_type: reminder_type,
         reminder_at: reminder_at,
         reminder_set_at: Time.zone.now
-      }.merge(options)
+      }.merge(default_options(options))
     )
 
     if bookmark.errors.any?
@@ -82,7 +84,7 @@ class BookmarkManager
         reminder_at: reminder_at,
         reminder_type: reminder_type,
         reminder_set_at: Time.zone.now
-      }.merge(options)
+      }.merge(default_options(options))
     )
 
     if bookmark.errors.any?
@@ -102,4 +104,8 @@ class BookmarkManager
     return if reminder_type.blank?
     reminder_type.is_a?(Integer) ? reminder_type : Bookmark.reminder_types[reminder_type.to_sym]
   end
+
+  def default_options(options)
+    DEFAULT_OPTIONS.merge(options) { |key, old, new| new.nil? ? old : new }
+  end
 end
diff --git a/spec/lib/bookmark_manager_spec.rb b/spec/lib/bookmark_manager_spec.rb
index 24bdd6bd15c..6aabf798240 100644
--- a/spec/lib/bookmark_manager_spec.rb
+++ b/spec/lib/bookmark_manager_spec.rb
@@ -53,6 +53,17 @@ RSpec.describe BookmarkManager do
       end
     end
 
+    context "when options are provided with null values" do
+      let(:options) { { delete_when_reminder_sent: nil } }
+
+      it "saves defaults successfully" do
+        subject.create(post_id: post.id, name: name, options: options)
+        bookmark = Bookmark.find_by(user: user)
+
+        expect(bookmark.delete_when_reminder_sent).to eq(false)
+      end
+    end
+
     context "when the bookmark already exists for the user & post" do
       before do
         Bookmark.create(post: post, user: user, topic: post.topic)