FEATURE: Add "delete on owner reply" bookmark functionality (#10231)

This adds an option to "delete on owner reply" to bookmarks. If you select this option in the modal, then reply to the topic the bookmark is in, the bookmark will be deleted on reply.

This PR also changes the checkboxes for these additional bookmark options to an Integer column in the DB with a combobox to select the option you want.

The use cases are:

* Sometimes I will bookmark the topics to read it later. In this case we definitely don’t need to keep the bookmark after I replied to it.
* Sometimes I will read the topic in mobile and I will prefer to reply in PC later. Or I may have to do some research before reply. So I will bookmark it for reply later.

app/assets/javascripts/discourse/app/controllers/bookmark.js

@@ -27,6 +27,20 @@ const LATER_TODAY_CUTOFF_HOUR = 17;
 const LATER_TODAY_MAX_HOUR = 18;
 const MOMENT_MONDAY = 1;
 const MOMENT_THURSDAY = 4;
+const AUTO_DELETE_PREFERENCES = [
+  {
+    id: 0,
+    name: I18n.t("bookmarks.auto_delete_preference.never")
+  },
+  {
+    id: 1,
+    name: I18n.t("bookmarks.auto_delete_preference.when_reminder_sent")
+  },
+  {
+    id: 2,
+    name: I18n.t("bookmarks.auto_delete_preference.on_owner_reply")
+  }
+];
 
 const BOOKMARK_BINDINGS = {
   enter: { handler: "saveAndClose" },
@@ -65,7 +79,6 @@ export default Controller.extend(ModalFunctionality, {
   mouseTrap: null,
   userTimezone: null,
   showOptions: false,
-  options: null,
 
   onShow() {
     this.setProperties({
@@ -79,7 +92,6 @@ export default Controller.extend(ModalFunctionality, {
       lastCustomReminderTime: null,
       userTimezone: this.currentUser.resolvedTimezone(this.currentUser),
       showOptions: false,
-      options: {},
       model: this.model || {}
     });
 
@@ -143,19 +155,26 @@ export default Controller.extend(ModalFunctionality, {
 
   _loadBookmarkOptions() {
     this.set(
-      "options.deleteWhenReminderSent",
+      "autoDeletePreference",
-      this.model.deleteWhenReminderSent ||
+      this.model.autoDeletePreference || this._preferredDeleteOption() || 0
-        localStorage.bookmarkOptionsDeleteWhenReminderSent === "true"
     );
 
     // we want to make sure the options panel opens so the user
     // knows they have set these options previously. run next otherwise
     // the modal is not visible when it tries to slide down the options
-    if (this.options.deleteWhenReminderSent) {
+    if (this.autoDeletePreference) {
       next(() => this.toggleOptionsPanel());
     }
   },
 
+  _preferredDeleteOption() {
+    let preferred = localStorage.bookmarkDeleteOption;
+    if (preferred && preferred !== "") {
+      preferred = parseInt(preferred, 10);
+    }
+    return preferred;
+  },
+
   _loadLastUsedCustomReminderDatetime() {
     let lastTime = localStorage.lastCustomBookmarkReminderTime;
     let lastDate = localStorage.lastCustomBookmarkReminderDate;
@@ -217,6 +236,11 @@ export default Controller.extend(ModalFunctionality, {
     return REMINDER_TYPES;
   },
 
+  @discourseComputed()
+  autoDeletePreferences: () => {
+    return AUTO_DELETE_PREFERENCES;
+  },
+
   showLastCustom: and("lastCustomReminderTime", "lastCustomReminderDate"),
 
   get showLaterToday() {
@@ -294,7 +318,7 @@ export default Controller.extend(ModalFunctionality, {
       localStorage.lastCustomBookmarkReminderDate = this.customReminderDate;
     }
 
-    localStorage.bookmarkOptionsDeleteWhenReminderSent = this.options.deleteWhenReminderSent;
+    localStorage.bookmarkDeleteOption = this.autoDeletePreference;
 
     let reminderType;
     if (this.selectedReminderType === REMINDER_TYPES.NONE) {
@@ -311,7 +335,7 @@ export default Controller.extend(ModalFunctionality, {
       name: this.model.name,
       post_id: this.model.postId,
       id: this.model.id,
-      delete_when_reminder_sent: this.options.deleteWhenReminderSent
+      auto_delete_preference: this.autoDeletePreference
     };
 
     if (this._editingExistingBookmark()) {
@@ -323,7 +347,7 @@ export default Controller.extend(ModalFunctionality, {
           this.afterSave({
             reminderAt: reminderAtISO,
             reminderType: this.selectedReminderType,
-            deleteWhenReminderSent: this.options.deleteWhenReminderSent,
+            autoDeletePreference: this.autoDeletePreference,
             id: this.model.id,
             name: this.model.name
           });
@@ -335,7 +359,7 @@ export default Controller.extend(ModalFunctionality, {
           this.afterSave({
             reminderAt: reminderAtISO,
             reminderType: this.selectedReminderType,
-            deleteWhenReminderSent: this.options.deleteWhenReminderSent,
+            autoDeletePreference: this.autoDeletePreference,
             id: response.id,
             name: this.model.name
           });

app/assets/javascripts/discourse/app/controllers/topic.js

@@ -110,6 +110,10 @@ export default Controller.extend(bufferedProperty("model"), {
     this._super(...arguments);
 
     this.appEvents.on("post:show-revision", this, "_showRevision");
+    this.appEvents.on("post:created", this, () => {
+      this._removeDeleteOnOwnerReplyBookmarks();
+      this.appEvents.trigger("post-stream:refresh", { force: true });
+    });
 
     this.setProperties({
       selectedPostIds: [],
@@ -183,6 +187,15 @@ export default Controller.extend(bufferedProperty("model"), {
       : null;
   },
 
+  _removeDeleteOnOwnerReplyBookmarks() {
+    let posts = this.model.get("postStream").posts;
+    posts
+      .filter(p => p.bookmarked && p.bookmark_auto_delete_preference === 2) // 2 is on_owner_reply
+      .forEach(p => {
+        p.clearBookmark();
+      });
+  },
+
   _forceRefreshPostStream() {
     this.appEvents.trigger("post-stream:refresh", { force: true });
   },

app/assets/javascripts/discourse/app/models/post.js

@@ -305,7 +305,7 @@ const Post = RestModel.extend({
           postId: this.id,
           id: this.bookmark_id,
           reminderAt: this.bookmark_reminder_at,
-          deleteWhenReminderSent: this.bookmark_delete_when_reminder_sent,
+          autoDeletePreference: this.bookmark_auto_delete_preference,
           name: this.bookmark_name
         },
         title: this.bookmark_id
@@ -324,8 +324,7 @@ const Post = RestModel.extend({
             bookmarked: true,
             bookmark_reminder_at: savedData.reminderAt,
             bookmark_reminder_type: savedData.reminderType,
-            bookmark_delete_when_reminder_sent:
+            bookmark_auto_delete_preference: savedData.autoDeletePreference,
-              savedData.deleteWhenReminderSent,
             bookmark_name: savedData.name,
             bookmark_id: savedData.id
           });
@@ -334,19 +333,24 @@ const Post = RestModel.extend({
         },
         afterDelete: topicBookmarked => {
           this.set("topic.bookmarked", topicBookmarked);
-          this.setProperties({
+          this.clearBookmark();
-            bookmark_reminder_at: null,
-            bookmark_reminder_type: null,
-            bookmark_name: null,
-            bookmark_id: null,
-            bookmarked: false
-          });
           this.appEvents.trigger("page:bookmark-post-toggled", this);
         }
       });
     });
   },
 
+  clearBookmark() {
+    this.setProperties({
+      bookmark_reminder_at: null,
+      bookmark_reminder_type: null,
+      bookmark_name: null,
+      bookmark_id: null,
+      bookmarked: false,
+      bookmark_auto_delete_preference: null
+    });
+  },
+
   updateActionsSummary(json) {
     if (json && json.id === this.id) {
       json = Post.munge(json);

app/assets/javascripts/discourse/app/routes/topic-from-params.js

@@ -73,12 +73,7 @@ export default DiscourseRoute.extend({
         // completely clear out all the bookmark related attributes
         // because they are not in the response if bookmarked == false
         if (closestPost && !closestPost.bookmarked) {
-          closestPost.setProperties({
+          closestPost.clearBookmark();
-            bookmark_reminder_at: null,
-            bookmark_reminder_type: null,
-            bookmark_name: null,
-            bookmark_id: null
-          });
         }
 
         if (!isEmpty(topic.draft)) {

app/assets/javascripts/discourse/app/templates/modal/bookmark.hbs

@@ -14,10 +14,13 @@
     </div>
 
     <div class="bookmark-options-panel">
-      <label for="delete_when_reminder_sent">
+      <label class="control-label" for="bookmark_auto_delete_preference">{{i18n "bookmarks.auto_delete_preference.label"}}</label>
-        {{i18n "bookmarks.delete_when_reminder_sent"}}
+      {{combo-box
-        {{input id="delete_when_reminder_sent" type="checkbox" checked=options.deleteWhenReminderSent}}
+        content=autoDeletePreferences
-      </label>
+        value=autoDeletePreference
+        class="bookmark-option-selector"
+        onChange=(action (mut autoDeletePreference))
+      }}
     </div>
 
     {{#if showExistingReminderAt }}

app/assets/javascripts/discourse/app/templates/user/bookmarks.hbs

@@ -35,9 +35,9 @@
               {{#if site.mobileView}}
                 <td>
                   {{#if bookmark.post_user_avatar_template}}
-                  <a href={{bookmark.postUser.path}} data-user-card={{bookmark.post_user_username}}>
+                    <a href={{bookmark.postUser.path}} data-user-card={{bookmark.post_user_username}}>
-                    {{avatar bookmark.postUser avatarTemplatePath="avatar_template" usernamePath="username" namePath="name" imageSize="small"}}
+                      {{avatar bookmark.postUser avatarTemplatePath="avatar_template" usernamePath="username" namePath="name" imageSize="small"}}
-                  </a>
+                    </a>
                   {{/if}}
                 </td>
               {{/if}}

app/assets/stylesheets/common/components/bookmark-modal.scss

@@ -75,14 +75,24 @@
     margin-left: 0.5em;
     margin-bottom: 0.5em;
     background: transparent;
+    padding-right: 6px;
   }
 
   .bookmark-options-panel {
+    .select-kit {
+      width: 100%;
+    }
+
     margin-bottom: 18px;
     display: none;
 
-    input[type="checkbox"] {
+    label {
-      margin-right: 0.85em;
+      display: flex;
+
+      span {
+        display: block;
+        flex: 1;
+      }
     }
   }
 }

app/controllers/bookmarks_controller.rb

@@ -13,7 +13,7 @@ class BookmarksController < ApplicationController
       reminder_type: params[:reminder_type],
       reminder_at: params[:reminder_at],
       options: {
-        delete_when_reminder_sent: params[:delete_when_reminder_sent]
+        auto_delete_preference: params[:auto_delete_preference] || 0
       }
     )
 
@@ -40,7 +40,7 @@ class BookmarksController < ApplicationController
       reminder_type: params[:reminder_type],
       reminder_at: params[:reminder_at],
       options: {
-        delete_when_reminder_sent: params[:delete_when_reminder_sent]
+        auto_delete_preference: params[:auto_delete_preference] || 0
       }
     )
 

app/models/bookmark.rb

@@ -44,6 +44,14 @@ class Bookmark < ActiveRecord::Base
     self.reminder_at.blank? && self.reminder_type.blank?
   end
 
+  def delete_when_reminder_sent?
+    self.auto_delete_preference == Bookmark.auto_delete_preferences[:when_reminder_sent]
+  end
+
+  def delete_on_owner_reply?
+    self.auto_delete_preference == Bookmark.auto_delete_preferences[:on_owner_reply]
+  end
+
   scope :pending_reminders, ->(before_time = Time.now.utc) do
     where("reminder_at IS NOT NULL AND reminder_at <= :before_time", before_time: before_time)
   end
@@ -53,7 +61,7 @@ class Bookmark < ActiveRecord::Base
   end
 
   def self.reminder_types
-    @reminder_type = Enum.new(
+    @reminder_types ||= Enum.new(
       later_today: 1,
       next_business_day: 2,
       tomorrow: 3,
@@ -65,6 +73,14 @@ class Bookmark < ActiveRecord::Base
     )
   end
 
+  def self.auto_delete_preferences
+    @auto_delete_preferences ||= Enum.new(
+      never: 0,
+      when_reminder_sent: 1,
+      on_owner_reply: 2
+    )
+  end
+
   def self.count_per_day(opts = nil)
     opts ||= {}
     result = where('bookmarks.created_at >= ?', opts[:start_date] || (opts[:since_days_ago] || 30).days.ago)
@@ -91,7 +107,7 @@ end
 #  updated_at                :datetime         not null
 #  reminder_last_sent_at     :datetime
 #  reminder_set_at           :datetime
-#  delete_when_reminder_sent :boolean          default(FALSE), not null
+#  auto_delete_preference             :integer
 #
 # Indexes
 #

app/serializers/post_serializer.rb

@@ -54,7 +54,7 @@ class PostSerializer < BasicPostSerializer
              :bookmark_id,
              :bookmark_reminder_type,
              :bookmark_name,
-             :bookmark_delete_when_reminder_sent,
+             :bookmark_auto_delete_preference,
              :raw,
              :actions_summary,
              :moderator?,
@@ -335,7 +335,11 @@ class PostSerializer < BasicPostSerializer
     bookmarked
   end
 
-  def include_bookmark_delete_when_reminder_sent?
+  def include_bookmark_auto_delete_preference?
+    bookmarked
+  end
+
+  def include_bookmark_delete_on_owner_reply?
     bookmarked
   end
 
@@ -361,8 +365,8 @@ class PostSerializer < BasicPostSerializer
     post_bookmark&.name
   end
 
-  def bookmark_delete_when_reminder_sent
+  def bookmark_auto_delete_preference
-    post_bookmark&.delete_when_reminder_sent
+    post_bookmark&.auto_delete_preference
   end
 
   def bookmark_id

config/locales/client.en.yml

@@ -319,7 +319,11 @@ en:
       no_timezone: 'You have not set a timezone yet. You will not be able to set reminders. Set one up <a href="%{basePath}/my/preferences/profile">in your profile</a>.'
       invalid_custom_datetime: "The date and time you provided is invalid, please try again."
       list_permission_denied: "You do not have permission to view this user's bookmarks."
-      delete_when_reminder_sent: "Delete this bookmark when the reminder notification is sent."
+      auto_delete_preference:
+        label: "Automatically delete"
+        never: "Never"
+        when_reminder_sent: "Once the reminder is sent"
+        on_owner_reply: "After I reply to this topic"
       search_placeholder: "Search bookmarks by name, topic title, or post content"
       search: "Search"
       reminders:

db/migrate/20200715044833_add_delete_option_to_bookmarks.rb

@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+class AddDeleteOptionToBookmarks < ActiveRecord::Migration[6.0]
+  def up
+    add_column :bookmarks, :auto_delete_preference, :integer, index: true, null: false, default: 0
+    DB.exec("UPDATE bookmarks SET auto_delete_preference = 1 WHERE delete_when_reminder_sent")
+  end
+
+  def down
+    remove_column :bookmarks, :auto_delete_preference
+  end
+end

db/post_migrate/20200715045152_remove_bookmarks_delete_when_reminder_sent.rb

@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+class RemoveBookmarksDeleteWhenReminderSent < ActiveRecord::Migration[6.0]
+  def up
+    remove_column :bookmarks, :delete_when_reminder_sent
+  end
+
+  def down
+    add_column :bookmarks, :delete_when_reminder_sent, :boolean, default: false
+  end
+end

lib/bookmark_manager.rb

@@ -1,8 +1,6 @@
 # frozen_string_literal: true
 
 class BookmarkManager
-  DEFAULT_OPTIONS = { delete_when_reminder_sent: false }
-
   include HasErrors
 
   def initialize(user)
@@ -24,7 +22,7 @@ class BookmarkManager
         reminder_type: reminder_type,
         reminder_at: reminder_at,
         reminder_set_at: Time.zone.now
-      }.merge(default_options(options))
+      }.merge(options)
     )
 
     if bookmark.errors.any?
@@ -84,7 +82,7 @@ class BookmarkManager
         reminder_at: reminder_at,
         reminder_type: reminder_type,
         reminder_set_at: Time.zone.now
-      }.merge(default_options(options))
+      }.merge(options)
     )
 
     if bookmark.errors.any?
@@ -104,8 +102,4 @@ 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

lib/post_creator.rb

@@ -202,6 +202,7 @@ class PostCreator
         create_embedded_topic
         @post.link_post_uploads
         update_uploads_secure_status
+        delete_owned_bookmarks
         ensure_in_allowed_users if guardian.is_staff?
         unarchive_message if !@opts[:import_mode]
         DraftSequence.next!(@user, draft_key) if !@opts[:import_mode]
@@ -402,6 +403,14 @@ class PostCreator
     @post.update_uploads_secure_status
   end
 
+  def delete_owned_bookmarks
+    return if !@post.topic_id
+    @user.bookmarks.where(
+      topic_id: @post.topic_id,
+      auto_delete_preference: Bookmark.auto_delete_preferences[:on_owner_reply]
+    ).destroy_all
+  end
+
   def handle_spam
     if @spam
       GroupMessage.create(Group[:moderators].name,

spec/components/post_creator_spec.rb

@@ -654,6 +654,19 @@ describe PostCreator do
       end
     end
 
+    context "when the user has bookmarks with auto_delete_preference on_owner_reply" do
+      before do
+        Fabricate(:bookmark, topic: topic, user: user, auto_delete_preference: Bookmark.auto_delete_preferences[:on_owner_reply])
+        Fabricate(:bookmark, topic: topic, user: user, auto_delete_preference: Bookmark.auto_delete_preferences[:on_owner_reply])
+        Fabricate(:bookmark, topic: topic, user: user)
+        Fabricate(:bookmark, user: user)
+      end
+      it "deletes the bookmarks" do
+        creator.create
+        expect(Bookmark.where(user: user).count).to eq(2)
+      end
+    end
+
     context "topic stats" do
       before do
         PostCreator.new(

spec/lib/bookmark_manager_spec.rb

@@ -43,24 +43,13 @@ RSpec.describe BookmarkManager do
     end
 
     context "when options are provided" do
-      let(:options) { { delete_when_reminder_sent: true } }
+      let(:options) { { auto_delete_preference: Bookmark.auto_delete_preferences[:when_reminder_sent] } }
 
       it "saves any additional options 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(true)
+        expect(bookmark.auto_delete_preference).to eq(1)
-      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
 
@@ -192,12 +181,12 @@ RSpec.describe BookmarkManager do
     end
 
     context "when options are provided" do
-      let(:options) { { delete_when_reminder_sent: true } }
+      let(:options) { { auto_delete_preference: Bookmark.auto_delete_preferences[:when_reminder_sent] } }
 
       it "saves any additional options successfully" do
         update_bookmark
         bookmark.reload
-        expect(bookmark.delete_when_reminder_sent).to eq(true)
+        expect(bookmark.auto_delete_preference).to eq(1)
       end
     end
 

spec/lib/bookmark_reminder_notification_handler_spec.rb

@@ -34,9 +34,9 @@ RSpec.describe BookmarkReminderNotificationHandler do
       expect(bookmark.reload.no_reminder?).to eq(true)
     end
 
-    context "when the delete_when_reminder_sent boolean is true " do
+    context "when the auto_delete_preference is when_reminder_sent" do
       it "deletes the bookmark after the reminder gets sent" do
-        bookmark.update(delete_when_reminder_sent: true)
+        bookmark.update(auto_delete_preference: Bookmark.auto_delete_preferences[:when_reminder_sent])
         subject.send_notification(bookmark)
         expect(Bookmark.find_by(id: bookmark.id)).to eq(nil)
       end

test/javascripts/acceptance/bookmarks-test.js

@@ -1,4 +1,5 @@
 import I18n from "I18n";
+import selectKit from "helpers/select-kit-helper";
 import {
   acceptance,
   loggedInUser,
@@ -115,7 +116,8 @@ test("Opening the options panel and remembering the option", async assert => {
     exists(".bookmark-options-panel"),
     "it should open the options panel"
   );
-  await click("#delete_when_reminder_sent");
+  await selectKit(".bookmark-option-selector").expand();
+  await selectKit(".bookmark-option-selector").selectRowByValue(1);
   await click("#save-bookmark");
   await openEditBookmarkModal();
 
@@ -123,9 +125,11 @@ test("Opening the options panel and remembering the option", async assert => {
     exists(".bookmark-options-panel"),
     "it should reopen the options panel"
   );
-  assert.ok(
+  assert.equal(
-    exists(".bookmark-options-panel #delete_when_reminder_sent:checked"),
+    selectKit(".bookmark-option-selector")
-    "it should pre-check delete when reminder sent option"
+      .header()
+      .value(),
+    1
   );
   assert.verifySteps(["none"]);
 });