FEATURE: Promote bookmarks with reminders to core functionality (#9369)

FEATURE: Promote bookmarks with reminders to core functionality (#9369)

The main thrust of this PR is to take all the conditional checks based on the enable_bookmarks_with_reminders away and only keep the code from the true path, making bookmarks with reminders the core bookmarks feature. There is also a migration to create Bookmark records out of PostAction bookmarks for a site.

Summary

  • Remove logic based on whether enable_bookmarks_with_reminders is true. This site setting is now obsolete, the old bookmark functionality is being removed. Retain the setting and set the value to true in a migration.
  • Use the code from the rake task to create a database migration that creates bookmarks from post actions.
  • Change the bookmark report to read from the new table.
  • Get rid of old endpoints for bookmarks
  • Link to the new bookmarks list from the user summary page
diff --git a/app/assets/javascripts/discourse/controllers/keyboard-shortcuts-help.js b/app/assets/javascripts/discourse/controllers/keyboard-shortcuts-help.js
index a0d42ac..2b0df87 100644
--- a/app/assets/javascripts/discourse/controllers/keyboard-shortcuts-help.js
+++ b/app/assets/javascripts/discourse/controllers/keyboard-shortcuts-help.js
@@ -1,6 +1,5 @@
 import Controller from "@ember/controller";
 import ModalFunctionality from "discourse/mixins/modal-functionality";
-import { setting } from "discourse/lib/computed";
 
 const KEY = "keyboard_shortcuts_help";
 
@@ -57,8 +56,6 @@ export default Controller.extend(ModalFunctionality, {
     this.set("shortcuts", null);
   },
 
-  showBookmarkShortcuts: setting("enable_bookmarks_with_reminders"),
-
   _defineShortcuts() {
     this.set("shortcuts", {
       jump_to: {
diff --git a/app/assets/javascripts/discourse/controllers/topic.js b/app/assets/javascripts/discourse/controllers/topic.js
index eea22c7..b934c13 100644
--- a/app/assets/javascripts/discourse/controllers/topic.js
+++ b/app/assets/javascripts/discourse/controllers/topic.js
@@ -652,10 +652,7 @@ export default Controller.extend(bufferedProperty("model"), {
       if (!this.currentUser) {
         return bootbox.alert(I18n.t("bookmarks.not_bookmarked"));
       } else if (post) {
-        if (this.siteSettings.enable_bookmarks_with_reminders) {
-          return post.toggleBookmarkWithReminder();
-        }
-        return post.toggleBookmark().catch(popupAjaxError);
+        return post.toggleBookmarkWithReminder();
       } else {
         return this.model.toggleBookmark().then(changedIds => {
           if (!changedIds) {
diff --git a/app/assets/javascripts/discourse/controllers/user-activity.js b/app/assets/javascripts/discourse/controllers/user-activity.js
index 03828c4..12333fc 100644
--- a/app/assets/javascripts/discourse/controllers/user-activity.js
+++ b/app/assets/javascripts/discourse/controllers/user-activity.js
@@ -3,7 +3,6 @@ import { inject as service } from "@ember/service";
 import Controller, { inject as controller } from "@ember/controller";
 import { exportUserArchive } from "discourse/lib/export-csv";
 import { observes } from "discourse-common/utils/decorators";
-import { setting } from "discourse/lib/computed";
 
 export default Controller.extend({
   application: controller(),
@@ -12,7 +11,6 @@ export default Controller.extend({
   userActionType: null,
 
   canDownloadPosts: alias("user.viewingSelf"),
-  bookmarksWithRemindersEnabled: setting("enable_bookmarks_with_reminders"),
 
   @observes("userActionType", "model.stream.itemsLoaded")
   _showFooter: function() {
diff --git a/app/assets/javascripts/discourse/models/topic.js b/app/assets/javascripts/discourse/models/topic.js
index 6ad8de2..1cbab38 100644
--- a/app/assets/javascripts/discourse/models/topic.js
+++ b/app/assets/javascripts/discourse/models/topic.js
@@ -400,10 +400,8 @@ const Topic = RestModel.extend({
   afterTopicBookmarked(firstPost) {
     if (firstPost) {
       firstPost.set("bookmarked", true);
-      if (this.siteSettings.enable_bookmarks_with_reminders) {
-        this.set("bookmark_reminder_at", firstPost.bookmark_reminder_at);
-        firstPost.set("bookmarked_with_reminder", true);
-      }
+      firstPost.set("bookmarked_with_reminder", true);
+      this.set("bookmark_reminder_at", firstPost.bookmark_reminder_at);
       return [firstPost.id];
     }
   },
@@ -438,24 +436,10 @@ const Topic = RestModel.extend({
     return this.firstPost().then(firstPost => {
       const toggleBookmarkOnServer = () => {
         if (bookmark) {
-          if (this.siteSettings.enable_bookmarks_with_reminders) {
-            return firstPost.toggleBookmarkWithReminder().then(response => {
-              this.set("bookmarking", false);
-              if (response && response.closedWithoutSaving) {
-                this.set("bookmarked", false);
-              } else {
-                return this.afterTopicBookmarked(firstPost);
-              }
-            });
-          } else {
-            return ajax(`/t/${this.id}/bookmark`, { type: "PUT" })
-              .then(() => {
-                this.toggleProperty("bookmarked");
-                return this.afterTopicBookmarked(firstPost);
-              })
-              .catch(popupAjaxError)
-              .finally(() => this.set("bookmarking", false));
-          }
+          return firstPost.toggleBookmarkWithReminder().then(() => {
+            this.set("bookmarking", false);
+            return this.afterTopicBookmarked(firstPost);
+          });
         } else {
           return ajax(`/t/${this.id}/remove_bookmarks`, { type: "PUT" })
             .then(() => {
@@ -474,10 +458,7 @@ const Topic = RestModel.extend({
                     post.set("bookmarked", false);
                     updated.push(post.id);
                   }
-                  if (
-                    this.siteSettings.enable_bookmarks_with_reminders &&
-                    post.bookmarked_with_reminder
-                  ) {
+                  if (post.bookmarked_with_reminder) {
                     post.setProperties(clearedBookmarkProps);
                     updated.push(post.id);
                   }
diff --git a/app/assets/javascripts/discourse/routes/discovery.js b/app/assets/javascripts/discourse/routes/discovery.js
index 1dd2985..f7bd4a5 100644
--- a/app/assets/javascripts/discourse/routes/discovery.js
+++ b/app/assets/javascripts/discourse/routes/discovery.js
@@ -17,10 +17,7 @@ export default DiscourseRoute.extend(OpenComposer, {
     // including being able to show links to multiple posts to the same topic
     // and being based on a different model. better to just redirect
     const url = transition.intent.url;
-    if (
-      this.siteSettings.enable_bookmarks_with_reminders &&
-      url === "/bookmarks"
-    ) {
+    if (url === "/bookmarks") {
       this.transitionTo(
         "userActivity.bookmarksWithReminders",
         this.currentUser
diff --git a/app/assets/javascripts/discourse/templates/modal/keyboard-shortcuts-help.hbs b/app/assets/javascripts/discourse/templates/modal/keyboard-shortcuts-help.hbs
index 5299b52..813ab3a 100644
--- a/app/assets/javascripts/discourse/templates/modal/keyboard-shortcuts-help.hbs
+++ b/app/assets/javascripts/discourse/templates/modal/keyboard-shortcuts-help.hbs
@@ -55,24 +55,22 @@
         <li>{{html-safe shortcuts.actions.quote_post}}</li>
       </ul>
     </section>
-    {{#if showBookmarkShortcuts}}
-      <section class="keyboard-shortcuts-bookmark-section">
-        <h4>{{i18n "keyboard_shortcuts_help.bookmarks.title"}}</h4>
-        <ul>
-          <li>{{html-safe shortcuts.bookmarks.enter}}</li>
-          <li>{{html-safe shortcuts.bookmarks.later_today}}</li>
-          <li>{{html-safe shortcuts.bookmarks.later_this_week}}</li>
-          <li>{{html-safe shortcuts.bookmarks.tomorrow}}</li>
-          <li>{{html-safe shortcuts.bookmarks.next_week}}</li>
-          <li>{{html-safe shortcuts.bookmarks.next_month}}</li>
-          <li>{{html-safe shortcuts.bookmarks.next_business_week}}</li>
-          <li>{{html-safe shortcuts.bookmarks.next_business_day}}</li>
-          <li>{{html-safe shortcuts.bookmarks.custom}}</li>
-          <li>{{html-safe shortcuts.bookmarks.none}}</li>
-          <li>{{html-safe shortcuts.bookmarks.delete}}</li>
-        </ul>
-      </section>
-    {{/if}}
+    <section class="keyboard-shortcuts-bookmark-section">
+      <h4>{{i18n "keyboard_shortcuts_help.bookmarks.title"}}</h4>
+      <ul>
+        <li>{{html-safe shortcuts.bookmarks.enter}}</li>
+        <li>{{html-safe shortcuts.bookmarks.later_today}}</li>
+        <li>{{html-safe shortcuts.bookmarks.later_this_week}}</li>
+        <li>{{html-safe shortcuts.bookmarks.tomorrow}}</li>
+        <li>{{html-safe shortcuts.bookmarks.next_week}}</li>
+        <li>{{html-safe shortcuts.bookmarks.next_month}}</li>

[... diff too long, it was truncated ...]

GitHub sha: 628ba9d1

This commit appears in #9369 which was approved by eviltrout. It was merged by martin.