From db70e7a8426947cebeeb698819559d352b9deae2 Mon Sep 17 00:00:00 2001
From: Martin Brennan <martin@discourse.org>
Date: Tue, 23 Jan 2024 16:49:41 +1000
Subject: [PATCH] FIX: Minor bookmark issues (#25358)

* FIX: Minor bookmark issues

* We were showing "missing %{name} value" when the name for the
  bookmark was undefined with title translations
* There was no way to see the bookmark details on hover in chat
  for a message where the bookmark icon was in the left gutter.
  We can show the title on the bookmark button in the chat message
  actions instead.

* Minor fix

* DEV: Test fix
---
 .../discourse/app/components/bookmark-icon.js | 18 ++---------------
 .../discourse/app/models/bookmark.js          | 20 ++++++++++++++++++-
 .../components/bookmark-icon-test.js          |  2 ++
 .../chat-message-actions-desktop.gjs          |  1 +
 4 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/app/assets/javascripts/discourse/app/components/bookmark-icon.js b/app/assets/javascripts/discourse/app/components/bookmark-icon.js
index 351304b5d73..bcde72c66ac 100644
--- a/app/assets/javascripts/discourse/app/components/bookmark-icon.js
+++ b/app/assets/javascripts/discourse/app/components/bookmark-icon.js
@@ -1,7 +1,6 @@
 import Component from "@ember/component";
 import { computed } from "@ember/object";
 import { isEmpty } from "@ember/utils";
-import { formattedReminderTime } from "discourse/lib/bookmark";
 import {
   NO_REMINDER_ICON,
   WITH_REMINDER_ICON,
@@ -32,25 +31,12 @@ export default class BookmarkIcon extends Component {
       : "bookmark-icon";
   }
 
-  @computed("bookmark.name", "bookmark.reminder_at")
+  @computed("bookmark.title")
   get title() {
     if (!this.bookmark) {
       return I18n.t("bookmarks.create");
     }
 
-    if (!isEmpty(this.bookmark.reminder_at)) {
-      const formattedTime = formattedReminderTime(
-        this.bookmark.reminder_at,
-        this.currentUser.user_option.timezone
-      );
-      return I18n.t("bookmarks.created_with_reminder_generic", {
-        date: formattedTime,
-        name: this.bookmark.name,
-      });
-    }
-
-    return I18n.t("bookmarks.created_generic", {
-      name: this.bookmark.name,
-    });
+    return this.bookmark.reminderTitle;
   }
 }
diff --git a/app/assets/javascripts/discourse/app/models/bookmark.js b/app/assets/javascripts/discourse/app/models/bookmark.js
index 68abb6b58cb..26c11f3cd9b 100644
--- a/app/assets/javascripts/discourse/app/models/bookmark.js
+++ b/app/assets/javascripts/discourse/app/models/bookmark.js
@@ -1,6 +1,7 @@
 import { computed } from "@ember/object";
 import { none } from "@ember/object/computed";
 import { capitalize } from "@ember/string";
+import { isEmpty } from "@ember/utils";
 import { Promise } from "rsvp";
 import { ajax } from "discourse/lib/ajax";
 import { formattedReminderTime } from "discourse/lib/bookmark";
@@ -102,6 +103,23 @@ const Bookmark = RestModel.extend({
     }
   },
 
+  @discourseComputed("name", "reminder_at")
+  reminderTitle(name, reminderAt) {
+    if (!isEmpty(reminderAt)) {
+      return I18n.t("bookmarks.created_with_reminder_generic", {
+        date: formattedReminderTime(
+          reminderAt,
+          this.currentUser?.user_option?.timezone || moment.tz.guess()
+        ),
+        name: name || "",
+      });
+    }
+
+    return I18n.t("bookmarks.created_generic", {
+      name: name || "",
+    });
+  },
+
   @discourseComputed("created_at")
   createdAt(created_at) {
     return new Date(created_at);
@@ -135,7 +153,7 @@ const Bookmark = RestModel.extend({
     return capitalize(
       formattedReminderTime(
         bookmarkReminderAt,
-        currentUser.user_option.timezone
+        currentUser?.user_option?.timezone || moment.tz.guess()
       )
     );
   },
diff --git a/app/assets/javascripts/discourse/tests/integration/components/bookmark-icon-test.js b/app/assets/javascripts/discourse/tests/integration/components/bookmark-icon-test.js
index 101bef1faa0..a5a1567c94d 100644
--- a/app/assets/javascripts/discourse/tests/integration/components/bookmark-icon-test.js
+++ b/app/assets/javascripts/discourse/tests/integration/components/bookmark-icon-test.js
@@ -16,6 +16,7 @@ module("Integration | Component | bookmark-icon", function (hooks) {
       bookmark: Bookmark.create({
         reminder_at: tomorrow(this.currentUser.user_option.timezone),
         name: "some name",
+        currentUser: this.currentUser,
       }),
     });
 
@@ -41,6 +42,7 @@ module("Integration | Component | bookmark-icon", function (hooks) {
       "bookmark",
       Bookmark.create({
         name: "some name",
+        currentUser: this.currentUser,
       })
     );
 
diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-message-actions-desktop.gjs b/plugins/chat/assets/javascripts/discourse/components/chat-message-actions-desktop.gjs
index bec1a747954..9c24ce399a4 100644
--- a/plugins/chat/assets/javascripts/discourse/components/chat-message-actions-desktop.gjs
+++ b/plugins/chat/assets/javascripts/discourse/components/chat-message-actions-desktop.gjs
@@ -161,6 +161,7 @@ export default class ChatMessageActionsDesktop extends Component {
             <DButton
               @action={{this.messageInteractor.toggleBookmark}}
               class="btn-flat bookmark-btn"
+              @translatedTitle={{this.message.bookmark.reminderTitle}}
             >
               <BookmarkIcon @bookmark={{this.message.bookmark}} />
             </DButton>