FEATURE: Open the edit bookmark modal when clicking on the topic level bookmark button (#13407)

FEATURE: Open the edit bookmark modal when clicking on the topic level bookmark button (#13407)

If you click on a bookmark in the post stream you get an Edit Bookmark modal. This does not happen if you click the topic bookmark button.

We want to open the Edit modal too if there is only one bookmark on a topic (it doesn’t matter on the first post or not). The other behaviour if there are > 1 bookmarks in the topic is to prompt the user to confirm delete of all the bookmarks in the topic. This behaviour will stay as-is.

I have done some refactoring in this PR, and still, there is a place for improvement. For example, we don’t call post.deleteBookmark() method when deleting several bookmarks. I just don’t want to refactor too much in one PR.

diff --git a/app/assets/javascripts/discourse/app/controllers/topic.js b/app/assets/javascripts/discourse/app/controllers/topic.js
index f9b2b93..f8427f5 100644
--- a/app/assets/javascripts/discourse/app/controllers/topic.js
+++ b/app/assets/javascripts/discourse/app/controllers/topic.js
@@ -1219,57 +1219,43 @@ export default Controller.extend(bufferedProperty("model"), {
       return Promise.resolve();
     }
     this.model.set("bookmarking", true);
-    const bookmark = !this.model.bookmarked;
-    let posts = this.model.postStream.posts;
+    const alreadyBookmarkedPosts = this.model.bookmarkedPosts;
 
     return this.model.firstPost().then((firstPost) => {
+      const bookmarkPost = async (post) => {
+        const opts = await this._togglePostBookmark(post);
+        this.model.set("bookmarking", false);
+        if (opts.closedWithoutSaving) {
+          return;
+        }
+        this.model.afterPostBookmarked(post);
+        return [post.id];
+      };
+
       const toggleBookmarkOnServer = () => {
-        if (bookmark) {
-          return this._togglePostBookmark(firstPost).then((opts) => {
-            this.model.set("bookmarking", false);
-            if (opts && opts.closedWithoutSaving) {
-              return;
-            }
-            return this.model.afterTopicBookmarked(firstPost);
-          });
+        if (alreadyBookmarkedPosts.length === 0) {
+          return bookmarkPost(firstPost);
+        } else if (alreadyBookmarkedPosts.length === 1) {
+          const post = alreadyBookmarkedPosts[0];
+          return bookmarkPost(post);
         } else {
           return this.model
             .deleteBookmark()
             .then(() => {
               this.model.toggleProperty("bookmarked");
               this.model.set("bookmark_reminder_at", null);
-              let clearedBookmarkProps = {
-                bookmarked: false,
-                bookmark_id: null,
-                bookmark_name: null,
-                bookmark_reminder_at: null,
-              };
-              if (posts) {
-                const updated = [];
-                posts.forEach((post) => {
-                  if (post.bookmarked) {
-                    post.setProperties(clearedBookmarkProps);
-                    updated.push(post.id);
-                  }
-                });
-                firstPost.setProperties(clearedBookmarkProps);
-                return updated;
-              }
+              alreadyBookmarkedPosts.forEach((post) => {
+                post.clearBookmark();
+              });
+              return alreadyBookmarkedPosts.mapBy("id");
             })
             .catch(popupAjaxError)
             .finally(() => this.model.set("bookmarking", false));
         }
       };
 
-      const unbookmarkedPosts = [];
-      if (!bookmark && posts) {
-        posts.forEach(
-          (post) => post.bookmarked && unbookmarkedPosts.push(post)
-        );
-      }
-
       return new Promise((resolve) => {
-        if (unbookmarkedPosts.length > 1) {
+        if (alreadyBookmarkedPosts.length > 1) {
           bootbox.confirm(
             I18n.t("bookmarks.confirm_clear"),
             I18n.t("no_value"),
diff --git a/app/assets/javascripts/discourse/app/initializers/topic-footer-buttons.js b/app/assets/javascripts/discourse/app/initializers/topic-footer-buttons.js
index 6cbc34c..d2c365b 100644
--- a/app/assets/javascripts/discourse/app/initializers/topic-footer-buttons.js
+++ b/app/assets/javascripts/discourse/app/initializers/topic-footer-buttons.js
@@ -66,7 +66,7 @@ export default {
     });
 
     registerTopicFooterButton({
-      dependentKeys: ["topic.bookmarked"],
+      dependentKeys: ["topic.bookmarked", "topic.bookmarkedPosts"],
       id: "bookmark",
       icon() {
         if (this.get("topic.bookmark_reminder_at")) {
@@ -81,8 +81,15 @@ export default {
       },
       label() {
         if (!this.get("topic.isPrivateMessage") || this.site.mobileView) {
-          const bookmarked = this.get("topic.bookmarked");
-          return bookmarked ? "bookmarked.clear_bookmarks" : "bookmarked.title";
+          const bookmarkedPostsCount = this.get("topic.bookmarkedPosts").length;
+
+          if (bookmarkedPostsCount === 0) {
+            return "bookmarked.title";
+          } else if (bookmarkedPostsCount === 1) {
+            return "bookmarked.edit_bookmark";
+          } else {
+            return "bookmarked.clear_bookmarks";
+          }
         }
       },
       translatedTitle() {
diff --git a/app/assets/javascripts/discourse/app/models/post.js b/app/assets/javascripts/discourse/app/models/post.js
index 97642f0..8a936b5 100644
--- a/app/assets/javascripts/discourse/app/models/post.js
+++ b/app/assets/javascripts/discourse/app/models/post.js
@@ -314,6 +314,7 @@ const Post = RestModel.extend({
       bookmark_name: data.name,
       bookmark_id: data.id,
     });
+    this.topic.incrementProperty("bookmarksWereChanged");
     this.appEvents.trigger("page:bookmark-post-toggled", this);
     this.appEvents.trigger("post-stream:refresh", { id: this.id });
   },
@@ -333,6 +334,7 @@ const Post = RestModel.extend({
       bookmarked: false,
       bookmark_auto_delete_preference: null,
     });
+    this.topic.incrementProperty("bookmarksWereChanged");
   },
 
   updateActionsSummary(json) {
diff --git a/app/assets/javascripts/discourse/app/models/topic.js b/app/assets/javascripts/discourse/app/models/topic.js
index deef4b8..d52f9e1 100644
--- a/app/assets/javascripts/discourse/app/models/topic.js
+++ b/app/assets/javascripts/discourse/app/models/topic.js
@@ -322,6 +322,11 @@ const Topic = RestModel.extend({
     return Site.currentProp("archetypes").findBy("id", archetype);
   },
 
+  @discourseComputed("bookmarksWereChanged")
+  bookmarkedPosts() {
+    return this.postStream.posts.filterBy("bookmarked", true);
+  },
+
   isPrivateMessage: equal("archetype", "private_message"),
   isBanner: equal("archetype", "banner"),
 
@@ -356,12 +361,9 @@ const Topic = RestModel.extend({
     }).then(() => this.set("archetype", "regular"));
   },
 
-  afterTopicBookmarked(firstPost) {
-    if (firstPost) {
-      firstPost.set("bookmarked", true);
-      this.set("bookmark_reminder_at", firstPost.bookmark_reminder_at);
-      return [firstPost.id];
-    }
+  afterPostBookmarked(post) {
+    post.set("bookmarked", true);
+    this.set("bookmark_reminder_at", post.bookmark_reminder_at);
   },
 
   firstPost() {
diff --git a/app/assets/javascripts/discourse/tests/acceptance/bookmarks-test.js b/app/assets/javascripts/discourse/tests/acceptance/bookmarks-test.js
index f2e0dfc..e0dc6de 100644
--- a/app/assets/javascripts/discourse/tests/acceptance/bookmarks-test.js
+++ b/app/assets/javascripts/discourse/tests/acceptance/bookmarks-test.js
@@ -2,6 +2,7 @@ import {
   acceptance,
   exists,
   loggedInUser,
+  query,
   queryAll,
 } from "discourse/tests/helpers/qunit-helpers";
 import { click, fillIn, visit } from "@ember/test-helpers";
@@ -315,4 +316,42 @@ acceptance("Bookmarking", function (needs) {
       "the second bookmark is deleted"
     );
   });
+
+  test("The topic level bookmark button opens the edit modal if only the first post on the topic is bookmarked", async function (assert) {
+    await visit("/t/internationalization-localization/280");
+    await openBookmarkModal(1);
+    await click("#save-bookmark");
+
+    assert.equal(
+      query("#topic-footer-button-bookmark").innerText,
+      I18n.t("bookmarked.edit_bookmark"),
+      "A topic level bookmark button has a label 'Edit Bookmark'"
+    );
+
+    await click("#topic-footer-button-bookmark");
+
+    assert.ok(
+      exists("div.modal.bookmark-with-reminder"),
+      "The edit modal is opened"
+    );
+  });
+
+  test("The topic level bookmark button opens the edit modal if only one post in the post stream is bookmarked", async function (assert) {
+    await visit("/t/internationalization-localization/280");
+    await openBookmarkModal(2);

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

GitHub sha: 3b87271647aebb7e2ade427a9fe5cda5d74f35c6

This commit appears in #13407 which was approved by jjaffeux and ZogStriP. It was merged by AndrewPrigorshnev.