FIX: Bookmark search fixes (#10239)

FIX: Bookmark search fixes (#10239)

  • Remove unneeded bookmark name index.
  • Change bookmark search query to use post_search_data. This allows searching on topic title and post content
  • Tweak the style/layout of the bookmark list so the search looks better and the whole page fits better on mobile.
diff --git a/app/assets/javascripts/discourse/app/templates/user/bookmarks.hbs b/app/assets/javascripts/discourse/app/templates/user/bookmarks.hbs
index b6a169b..c4a0425 100644
--- a/app/assets/javascripts/discourse/app/templates/user/bookmarks.hbs
+++ b/app/assets/javascripts/discourse/app/templates/user/bookmarks.hbs
@@ -1,4 +1,4 @@
-<div class="form-horizontal" style="margin-bottom: 1em">
+<div class="form-horizontal bookmark-search-form">
   {{input type="text"
           value=searchTerm
           placeholder=(i18n "bookmarks.search_placeholder")
@@ -8,7 +8,7 @@
       class="btn-primary"
       action=(action "search")
       type="button"
-      label="bookmarks.search"}}
+      icon="search"}}
 </div>
 {{#if noContent}}
   <div class="alert alert-info">{{noResultsHelp}}</div>
@@ -17,15 +17,30 @@
     {{#load-more selector=".bookmark-list tr" action=(action "loadMore")}}
       <table class="topic-list bookmark-list">
         <thead>
-          <th>{{i18n "topic.title"}}</th>
-          <th>&nbsp;</th>
-          <th class="post-metadata">{{i18n "post.bookmarks.updated"}}</th>
-          <th class="post-metadata">{{i18n "activity"}}</th>
-          <th>&nbsp;</th>
+          {{#if site.mobileView}}
+            <th>&nbsp;</th>
+            <th>{{i18n "topic.title"}}</th>
+            <th>&nbsp;</th>
+          {{else}}
+            <th>{{i18n "topic.title"}}</th>
+            <th>&nbsp;</th>
+            <th class="post-metadata">{{i18n "post.bookmarks.updated"}}</th>
+            <th class="post-metadata">{{i18n "activity"}}</th>
+            <th>&nbsp;</th>
+          {{/if}}
         </thead>
         <tbody>
           {{#each content as |bookmark|}}
             <tr class="topic-list-item bookmark-list-item">
+              {{#if site.mobileView}}
+                <td>
+                  {{#if bookmark.post_user_avatar_template}}
+                  <a href={{bookmark.postUser.path}} data-user-card={{bookmark.post_user_username}}>
+                    {{avatar bookmark.postUser avatarTemplatePath="avatar_template" usernamePath="username" namePath="name" imageSize="small"}}
+                  </a>
+                  {{/if}}
+                </td>
+              {{/if}}
               <td class="main-link">
                 <span class="link-top-line">
                   <div class="bookmark-metadata">
@@ -52,15 +67,17 @@
                   {{discourse-tags bookmark mode="list" tagsForUser=tagsForUser}}
                 </div>
               </td>
-              <td>
-                {{#if bookmark.post_user_avatar_template}}
-                  <a href={{bookmark.postUser.path}} data-user-card={{bookmark.post_user_username}}>
-                    {{avatar bookmark.postUser avatarTemplatePath="avatar_template" usernamePath="username" namePath="name" imageSize="small"}}
-                  </a>
-                {{/if}}
-              </td>
-              <td class="post-metadata">{{format-date bookmark.updated_at format="tiny"}}</td>
-              {{raw "list/activity-column" topic=bookmark class="num post-metadata" tagName="td"}}
+              {{#unless site.mobileView}}
+                <td>
+                  {{#if bookmark.post_user_avatar_template}}
+                    <a href={{bookmark.postUser.path}} data-user-card={{bookmark.post_user_username}}>
+                      {{avatar bookmark.postUser avatarTemplatePath="avatar_template" usernamePath="username" namePath="name" imageSize="small"}}
+                    </a>
+                  {{/if}}
+                </td>
+                <td class="post-metadata">{{format-date bookmark.updated_at format="tiny"}}</td>
+                {{raw "list/activity-column" topic=bookmark class="num post-metadata" tagName="td"}}
+              {{/unless}}
               <td>
                 {{bookmark-actions-dropdown
                   bookmark=bookmark
diff --git a/app/assets/stylesheets/common/components/bookmark-list-item.scss b/app/assets/stylesheets/common/components/bookmark-list-item.scss
deleted file mode 100644
index 4710f9b..0000000
--- a/app/assets/stylesheets/common/components/bookmark-list-item.scss
+++ /dev/null
@@ -1,29 +0,0 @@
-.bookmark-list {
-  th.post-metadata {
-    text-align: center;
-  }
-}
-
-.bookmark-list-item {
-  td.post-metadata {
-    text-align: center;
-  }
-  .bookmark-metadata {
-    font-size: $font-down-2;
-    display: flex;
-    margin-bottom: 0.2em;
-
-    &-item {
-      margin-right: 0.5em;
-      display: flex;
-      align-items: center;
-    }
-
-    .d-icon {
-      // not aligning center because of multi-line notes
-      align-self: flex-start;
-      margin-right: 0.2em;
-      padding-top: 0.12em;
-    }
-  }
-}
diff --git a/app/assets/stylesheets/common/components/bookmark-list.scss b/app/assets/stylesheets/common/components/bookmark-list.scss
new file mode 100644
index 0000000..5d7d6c5
--- /dev/null
+++ b/app/assets/stylesheets/common/components/bookmark-list.scss
@@ -0,0 +1,61 @@
+$mobile-breakpoint: 700px;
+
+.bookmark-list {
+  th.post-metadata {
+    text-align: center;
+  }
+}
+
+.bookmark-search-form {
+  margin-bottom: 1em;
+  display: flex;
+
+  input[type="text"] {
+    flex: 1;
+    margin-right: 1em;
+  }
+
+  @media (max-width: $mobile-breakpoint) {
+    input[type="text"] {
+      margin-bottom: 0;
+    }
+  }
+}
+
+.bookmark-list-item {
+  td.post-metadata {
+    text-align: center;
+  }
+  @media (max-width: $mobile-breakpoint) {
+    .main-link {
+      padding-left: 0.5em;
+      padding-right: 0.5em;
+    }
+  }
+  .bookmark-metadata {
+    font-size: $font-down-2;
+    display: flex;
+    margin-bottom: 0.2em;
+
+    &-item {
+      margin-right: 0.5em;
+      display: flex;
+      align-items: center;
+    }
+
+    .d-icon {
+      // not aligning center because of multi-line notes
+      align-self: flex-start;
+      margin-right: 0.2em;
+      padding-top: 0.12em;
+    }
+
+    @media (max-width: $mobile-breakpoint) {
+      flex-direction: column;
+      &-item {
+        display: block;
+        margin-bottom: 0.2em;
+      }
+    }
+  }
+}
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 66bb549..d4eee0c 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -320,7 +320,7 @@ en:
       invalid_custom_datetime: "The date and time you provided is invalid, please try again."
       list_permission_denied: "You do not have permission to view this user's bookmarks."
       delete_when_reminder_sent: "Delete this bookmark when the reminder notification is sent."
-      search_placeholder: "Search bookmarks by name or post content"
+      search_placeholder: "Search bookmarks by name, topic title, or post content"
       search: "Search"
       reminders:
         at_desktop: "Next time I'm at my desktop"
diff --git a/db/migrate/20200715030908_remove_unneccessary_bookmark_name_index.rb b/db/migrate/20200715030908_remove_unneccessary_bookmark_name_index.rb
new file mode 100644
index 0000000..460b448
--- /dev/null
+++ b/db/migrate/20200715030908_remove_unneccessary_bookmark_name_index.rb
@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class RemoveUnneccessaryBookmarkNameIndex < ActiveRecord::Migration[6.0]
+  def change
+    remove_index :bookmarks, :name
+  end
+end
diff --git a/lib/bookmark_query.rb b/lib/bookmark_query.rb
index 2f9daf5..f2122f7 100644
--- a/lib/bookmark_query.rb
+++ b/lib/bookmark_query.rb
@@ -36,7 +36,14 @@ class BookmarkQuery
     results = results.merge(Post.secured(@guardian))
 
     if @params[:q].present?
-      results = results.where("bookmarks.name ILIKE :q OR posts.raw ILIKE :q", q: "%#{@params[:q]}%")
+      term = @params[:q]
+      bookmark_ts_query = Search.ts_query(term: term)
+      results = results
+        .joins("LEFT JOIN post_search_data ON post_search_data.post_id = bookmarks.post_id")
+        .where(
+          "bookmarks.name ILIKE :q OR #{bookmark_ts_query} @@ post_search_data.search_data",
+          q: "%#{term}%"

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

GitHub sha: 716ccf7f

This commit appears in #10239 which was merged by martin.