FIX: Bookmark UI tweaks (#9604)

* When hovering over the bookmark icon for a post, show the name of the bookmark at the end of the tooltip _if_ it has been set.
* Order bookmarks by `updated_at DESC` in the user list and show that instead of created at.

app/assets/javascripts/discourse/app/lib/transform-post.js

@@ -36,6 +36,7 @@ export function transformBasicPost(post) {
     avatar_template: post.avatar_template,
     bookmarked: post.bookmarked,
     bookmarkReminderAt: post.bookmark_reminder_at,
+    bookmarkName: post.bookmark_name,
     bookmarkReminderType: post.bookmark_reminder_type,
     yours: post.yours,
     shareUrl: post.get("shareUrl"),

app/assets/javascripts/discourse/app/templates/user/bookmarks.hbs

@@ -6,7 +6,7 @@
       <table class="topic-list bookmark-list">
         <thead>
           <th>{{i18n "topic.title"}}</th>
-          <th>{{i18n "post.bookmarks.created"}}</th>
+          <th>{{i18n "post.bookmarks.updated"}}</th>
           <th>{{i18n "activity"}}</th>
           <th>&nbsp;</th>
         </thead>
@@ -39,7 +39,7 @@
                   {{discourse-tags bookmark mode="list" tagsForUser=tagsForUser}}
                 </div>
               </td>
-              <td>{{format-date bookmark.created_at format="tiny"}}</td>
+              <td>{{format-date bookmark.updated_at format="tiny"}}</td>
               {{raw "list/activity-column" topic=bookmark class="num" tagName="td"}}
               <td>
                 {{bookmark-actions-dropdown

app/assets/javascripts/discourse/app/widgets/post-menu.js

@@ -289,7 +289,7 @@ registerButton("bookmark", attrs => {
 
   let classNames = ["bookmark", "with-reminder"];
   let title = "bookmarks.not_bookmarked";
-  let titleOptions = {};
+  let titleOptions = { name: "" };
 
   if (attrs.bookmarked) {
     classNames.push("bookmarked");
@@ -300,14 +300,16 @@ registerButton("bookmark", attrs => {
         Discourse.currentUser.resolvedTimezone()
       );
       title = "bookmarks.created_with_reminder";
-      titleOptions = {
+      titleOptions.date = formattedReminder;
-        date: formattedReminder
-      };
     } else if (attrs.bookmarkReminderType === "at_desktop") {
       title = "bookmarks.created_with_at_desktop_reminder";
     } else {
       title = "bookmarks.created";
     }
+
+    if (attrs.bookmarkName) {
+      titleOptions.name = `. ${attrs.bookmarkName}`;
+    }
   }
 
   return {

app/serializers/user_bookmark_serializer.rb

@@ -8,6 +8,7 @@ class UserBookmarkSerializer < ApplicationSerializer
 
   attributes :id,
              :created_at,
+             :updated_at,
              :topic_id,
              :linked_post_number,
              :post_id,

config/locales/client.en.yml

@@ -307,10 +307,10 @@ en:
         unbookmark_with_reminder: "Click to remove all bookmarks and reminders in this topic. You have a reminder set %{reminder_at} for this topic."
 
     bookmarks:
-      created: "you've bookmarked this post"
+      created: "you've bookmarked this post%{name}"
       not_bookmarked: "bookmark this post"
-      created_with_reminder: "you've bookmarked this post with a reminder %{date}"
+      created_with_reminder: "you've bookmarked this post with a reminder %{date}%{name}"
-      created_with_at_desktop_reminder: "you've bookmarked this post and will be reminded next time you are at your desktop"
+      created_with_at_desktop_reminder: "you've bookmarked this post and will be reminded next time you are at your desktop%{name}"
       remove: "Remove Bookmark"
       delete: "Delete Bookmark"
       confirm_delete: "Are you sure you want to delete this bookmark? The reminder will also be deleted."
@@ -2724,6 +2724,7 @@ en:
         create: "Create bookmark"
         edit: "Edit bookmark"
         created: "Created"
+        updated: "Updated"
         name: "Name"
         name_placeholder: "What is this bookmark for?"
         set_reminder: "Remind me"

lib/bookmark_query.rb

@@ -27,8 +27,7 @@ class BookmarkQuery
   end
 
   def list_all
-    results = user_bookmarks
+    results = user_bookmarks.order('bookmarks.updated_at DESC')
-      .order('bookmarks.created_at DESC')
 
     topics = Topic.listable_topics.secured(@guardian)
     pms = Topic.private_messages_for_user(@user)

spec/lib/bookmark_query_spec.rb

@@ -4,20 +4,16 @@ require 'rails_helper'
 
 RSpec.describe BookmarkQuery do
   fab!(:user) { Fabricate(:user) }
-  fab!(:bookmark1) { Fabricate(:bookmark, user: user) }
-  fab!(:bookmark2) { Fabricate(:bookmark, user: user) }
   let(:params) { {} }
 
   def bookmark_query(user: nil, params: nil)
     BookmarkQuery.new(user: user || self.user, params: params || self.params)
   end
 
-  before do
-    TopicUser.change(user.id, bookmark1.topic_id, total_msecs_viewed: 1)
-    TopicUser.change(user.id, bookmark2.topic_id, total_msecs_viewed: 1)
-  end
-
   describe "#list_all" do
+    fab!(:bookmark1) { Fabricate(:bookmark, user: user) }
+    fab!(:bookmark2) { Fabricate(:bookmark, user: user) }
+
     it "returns all the bookmarks for a user" do
       expect(bookmark_query.list_all.count).to eq(2)
     end
@@ -143,4 +139,21 @@ RSpec.describe BookmarkQuery do
       end
     end
   end
+
+  describe "#list_all ordering" do
+    let!(:bookmark1) { Fabricate(:bookmark, user: user, updated_at: 1.day.ago) }
+    let!(:bookmark2) { Fabricate(:bookmark, user: user, updated_at: 2.days.ago) }
+    let!(:bookmark3) { Fabricate(:bookmark, user: user, updated_at: 6.days.ago) }
+    let!(:bookmark4) { Fabricate(:bookmark, user: user, updated_at: 4.days.ago) }
+    let!(:bookmark5) { Fabricate(:bookmark, user: user, updated_at: 3.days.ago) }
+    it "orders by updated_at" do
+      expect(bookmark_query.list_all.map(&:id)).to eq([
+        bookmark1.id,
+        bookmark2.id,
+        bookmark5.id,
+        bookmark4.id,
+        bookmark3.id
+      ])
+    end
+  end
 end