DEV: Refactor bookmark modal code (#14654)

DEV: Refactor bookmark modal code (#14654)

We had code to open the bookmark modal in two places – the bookmark list and also from within a topic. This caused the two code paths to drift, as in the bookmark list we were not passing in the forTopic or autoDeletePreferences data into the modal, and we were also not refreshing the bookmark list when the bookmark was deleted from within the modal.

This commit moves the modal opening code into an importable function from the controllers/bookmark module, and all callers have to do is pass it an instance of Bookmark and also options for what to do for the following:

  • onAfterSave
  • onAfterDelete
  • onCloseWithoutSaving
diff --git a/app/assets/javascripts/discourse/app/components/bookmark-list.js b/app/assets/javascripts/discourse/app/components/bookmark-list.js
index 30b7197..7272ab7 100644
--- a/app/assets/javascripts/discourse/app/components/bookmark-list.js
+++ b/app/assets/javascripts/discourse/app/components/bookmark-list.js
@@ -7,7 +7,7 @@ import I18n from "I18n";
 import { Promise } from "rsvp";
 import { action } from "@ember/object";
 import bootbox from "bootbox";
-import showModal from "discourse/lib/show-modal";
+import { openBookmarkModal } from "discourse/controllers/bookmark";
 
 export default Component.extend({
   classNames: ["bookmark-list-wrapper"],
@@ -51,17 +51,14 @@ export default Component.extend({
 
   @action
   editBookmark(bookmark) {
-    let controller = showModal("bookmark", {
-      model: {
-        postId: bookmark.post_id,
-        id: bookmark.id,
-        reminderAt: bookmark.reminder_at,
-        name: bookmark.name,
+    openBookmarkModal(bookmark, {
+      onAfterSave: () => {
+        this.reload();
+      },
+      onAfterDelete: () => {
+        this.reload();
       },
-      title: "post.bookmarks.edit",
-      modalClass: "bookmark-with-reminder",
     });
-    controller.set("afterSave", this.reload);
   },
 
   @action
diff --git a/app/assets/javascripts/discourse/app/controllers/bookmark.js b/app/assets/javascripts/discourse/app/controllers/bookmark.js
index 6b57d74..b47b83b 100644
--- a/app/assets/javascripts/discourse/app/controllers/bookmark.js
+++ b/app/assets/javascripts/discourse/app/controllers/bookmark.js
@@ -1,6 +1,61 @@
 import Controller from "@ember/controller";
 import ModalFunctionality from "discourse/mixins/modal-functionality";
 import { action } from "@ember/object";
+import { Promise } from "rsvp";
+import showModal from "discourse/lib/show-modal";
+
+export function openBookmarkModal(
+  bookmark,
+  callbacks = {
+    onCloseWithoutSaving: null,
+    onAfterSave: null,
+    onAfterDelete: null,
+  }
+) {
+  return new Promise((resolve) => {
+    const modalTitle = () => {
+      if (bookmark.for_topic) {
+        return bookmark.id
+          ? "post.bookmarks.edit_for_topic"
+          : "post.bookmarks.create_for_topic";
+      }
+      return bookmark.id ? "post.bookmarks.edit" : "post.bookmarks.create";
+    };
+    let modalController = showModal("bookmark", {
+      model: {
+        postId: bookmark.post_id,
+        id: bookmark.id,
+        reminderAt: bookmark.reminder_at,
+        autoDeletePreference: bookmark.auto_delete_preference,
+        name: bookmark.name,
+        forTopic: bookmark.for_topic,
+      },
+      title: modalTitle(),
+      modalClass: "bookmark-with-reminder",
+    });
+    modalController.setProperties({
+      onCloseWithoutSaving: () => {
+        if (callbacks.onCloseWithoutSaving) {
+          callbacks.onCloseWithoutSaving();
+        }
+        resolve();
+      },
+      afterSave: (savedData) => {
+        let resolveData;
+        if (callbacks.onAfterSave) {
+          resolveData = callbacks.onAfterSave(savedData);
+        }
+        resolve(resolveData);
+      },
+      afterDelete: (topicBookmarked, bookmarkId) => {
+        if (callbacks.onAfterDelete) {
+          callbacks.onAfterDelete(topicBookmarked, bookmarkId);
+        }
+        resolve();
+      },
+    });
+  });
+}
 
 export default Controller.extend(ModalFunctionality, {
   onShow() {
diff --git a/app/assets/javascripts/discourse/app/controllers/topic.js b/app/assets/javascripts/discourse/app/controllers/topic.js
index a0e0182..5ebaa90 100644
--- a/app/assets/javascripts/discourse/app/controllers/topic.js
+++ b/app/assets/javascripts/discourse/app/controllers/topic.js
@@ -4,7 +4,7 @@ import { alias, and, not, or } from "@ember/object/computed";
 import discourseComputed, { observes } from "discourse-common/utils/decorators";
 import { isEmpty, isPresent } from "@ember/utils";
 import { later, next, schedule } from "@ember/runloop";
-import { AUTO_DELETE_PREFERENCES } from "discourse/models/bookmark";
+import Bookmark, { AUTO_DELETE_PREFERENCES } from "discourse/models/bookmark";
 import Composer from "discourse/models/composer";
 import EmberObject, { action } from "@ember/object";
 import I18n from "I18n";
@@ -26,6 +26,7 @@ import { popupAjaxError } from "discourse/lib/ajax-error";
 import { inject as service } from "@ember/service";
 import showModal from "discourse/lib/show-modal";
 import { spinnerHTML } from "discourse/helpers/loading-spinner";
+import { openBookmarkModal } from "discourse/controllers/bookmark";
 
 let customPostMessageCallbacks = {};
 
@@ -1223,86 +1224,43 @@ export default Controller.extend(bufferedProperty("model"), {
   },
 
   _modifyTopicBookmark(bookmark) {
-    const title = bookmark.id
-      ? "post.bookmarks.edit_for_topic"
-      : "post.bookmarks.create_for_topic";
-    return this._openBookmarkModal(bookmark, title, {
-      onAfterSave: () => {
+    bookmark = Bookmark.create(bookmark);
+    return openBookmarkModal(bookmark, {
+      onAfterSave: (savedData) => {
+        this._syncBookmarks(savedData);
+        this.model.set("bookmarking", false);
         this.model.set("bookmarked", true);
         this.model.incrementProperty("bookmarksWereChanged");
         this.appEvents.trigger("topic:bookmark-toggled");
       },
+      onAfterDelete: (topicBookmarked, bookmarkId) => {
+        this.model.removeBookmark(bookmarkId);
+      },
     });
   },
 
   _modifyPostBookmark(bookmark, post) {
-    const title = bookmark.id ? "post.bookmarks.edit" : "post.bookmarks.create";
-    return this._openBookmarkModal(bookmark, title, {
+    bookmark = Bookmark.create(bookmark);
+    return openBookmarkModal(bookmark, {
       onCloseWithoutSaving: () => {
         post.appEvents.trigger("post-stream:refresh", {
           id: bookmark.post_id,
         });
       },
       onAfterSave: (savedData) => {
+        this._syncBookmarks(savedData);
+        this.model.set("bookmarking", false);
         post.createBookmark(savedData);
         this.model.afterPostBookmarked(post, savedData);
         return [post.id];
       },
-      onAfterDelete: (topicBookmarked) => {
+      onAfterDelete: (topicBookmarked, bookmarkId) => {
+        this.model.removeBookmark(bookmarkId);
         post.deleteBookmark(topicBookmarked);
       },
     });
   },
 
-  _openBookmarkModal(
-    bookmark,
-    title,
-    callbacks = {
-      onCloseWithoutSaving: null,
-      onAfterSave: null,
-      onAfterDelete: null,
-    }
-  ) {
-    return new Promise((resolve) => {
-      let modalController = showModal("bookmark", {
-        model: {
-          postId: bookmark.post_id,
-          id: bookmark.id,
-          reminderAt: bookmark.reminder_at,
-          autoDeletePreference: bookmark.auto_delete_preference,
-          name: bookmark.name,
-          forTopic: bookmark.for_topic,
-        },
-        title,
-        modalClass: "bookmark-with-reminder",
-      });
-      modalController.setProperties({
-        onCloseWithoutSaving: () => {
-          if (callbacks.onCloseWithoutSaving) {
-            callbacks.onCloseWithoutSaving();
-          }
-          resolve();
-        },
-        afterSave: (savedData) => {
-          this._syncBookmarks(savedData);
-          this.model.set("bookmarking", false);
-          let resolveData;
-          if (callbacks.onAfterSave) {
-            resolveData = callbacks.onAfterSave(savedData);
-          }
-          resolve(resolveData);
-        },
-        afterDelete: (topicBookmarked, bookmarkId) => {
-          this.model.removeBookmark(bookmarkId);
-          if (callbacks.onAfterDelete) {
-            callbacks.onAfterDelete(topicBookmarked);
-          }
-          resolve();
-        },
-      });
-    });
-  },
-
   _syncBookmarks(data) {
     if (!this.model.bookmarks) {
       this.model.set("bookmarks", []);

GitHub sha: 0f0388437557553c2e28c04c471e997ff5fe645c

This commit appears in #14654 which was approved by tgxworld. It was merged by martin.