From 0f0388437557553c2e28c04c471e997ff5fe645c Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Thu, 21 Oct 2021 09:02:35 +1000 Subject: [PATCH] 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 --- .../discourse/app/components/bookmark-list.js | 17 ++--- .../discourse/app/controllers/bookmark.js | 55 ++++++++++++++ .../discourse/app/controllers/topic.js | 74 ++++--------------- 3 files changed, 78 insertions(+), 68 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/bookmark-list.js b/app/assets/javascripts/discourse/app/components/bookmark-list.js index 30b7197c4c0..7272ab73816 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 6b57d740c7c..b47b83b9513 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 a0e01822e41..5ebaa903e2d 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", []);