FEATURE: Add lazy loading to user bookmarks list (#9317)

This is so users with huge amount of bookmarks do not have to wait a long time to see results.

* Add a bookmark list and list serializer to server-side to be able to handle paging and load more URL
* Use load-more component to load more bookmark items, 20 at a time in user activity
* Change the way current user is loaded for bookmark ember models because it was breaking/losing resolvedTimezone when loading more items

app/assets/javascripts/discourse/controllers/user-activity-bookmarks-with-reminders.js

@@ -1,4 +1,5 @@
 import Controller from "@ember/controller";
+import { Promise } from "rsvp";
 import { inject } from "@ember/controller";
 import discourseComputed from "discourse-common/utils/decorators";
 import Bookmark from "discourse/models/bookmark";
@@ -21,17 +22,7 @@ export default Controller.extend({
     return this.model
       .loadItems()
       .then(response => {
-        if (response && response.no_results_help) {
+        this.processLoadResponse(response);
-          this.set("noResultsHelp", response.no_results_help);
-        }
-
-        if (response && response.bookmarks) {
-          let bookmarks = [];
-          response.bookmarks.forEach(bookmark => {
-            bookmarks.push(Bookmark.create(bookmark));
-          });
-          this.content.pushObjects(bookmarks);
-        }
       })
       .catch(() => {
         this.set("noResultsHelp", I18n.t("bookmarks.list_permission_denied"));
@@ -49,9 +40,46 @@ export default Controller.extend({
     return loaded && contentLength === 0;
   },
 
+  processLoadResponse(response) {
+    response = response.user_bookmark_list;
+
+    if (response && response.no_results_help) {
+      this.set("noResultsHelp", response.no_results_help);
+    }
+
+    this.model.more_bookmarks_url = response.more_bookmarks_url;
+
+    if (response && response.bookmarks) {
+      let bookmarks = [];
+      response.bookmarks.forEach(bookmark => {
+        bookmarks.push(Bookmark.create(bookmark));
+      });
+      this.content.pushObjects(bookmarks);
+    }
+  },
+
   actions: {
     removeBookmark(bookmark) {
       return bookmark.destroy().then(() => this.loadItems());
+    },
+
+    loadMore() {
+      if (this.loadingMore) {
+        return Promise.resolve();
+      }
+      this.set("loadingMore", true);
+
+      return this.model
+        .loadMore()
+        .then(response => this.processLoadResponse(response))
+        .catch(() => {
+          this.set("noResultsHelp", I18n.t("bookmarks.list_permission_denied"));
+        })
+        .finally(() =>
+          this.setProperties({
+            loadingMore: false
+          })
+        );
     }
   }
 });

app/assets/javascripts/discourse/models/bookmark.js

@@ -117,6 +117,22 @@ const Bookmark = RestModel.extend({
 
   loadItems() {
     return ajax(`/u/${this.user.username}/bookmarks.json`, { cache: "false" });
+  },
+
+  loadMore() {
+    if (!this.more_bookmarks_url) {
+      return Promise.resolve();
+    }
+
+    let moreUrl = this.more_bookmarks_url;
+    if (moreUrl) {
+      let [url, params] = moreUrl.split("?");
+      moreUrl = url;
+      if (params) {
+        moreUrl += "?" + params;
+      }
+    }
+    return ajax({ url: moreUrl });
   }
 });
 

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

@@ -2,50 +2,53 @@
   <div class='alert alert-info'>{{noResultsHelp}}</div>
 {{else}}
   {{#conditional-loading-spinner condition=loading}}
-  <table class="topic-list">
+    {{#load-more selector=".bookmark-list tr" action=(action "loadMore")}}
-    <thead>
+      <table class="topic-list bookmark-list">
-      <th>{{i18n "topic.title"}}</th>
+        <thead>
-      <th>{{i18n "post.bookmarks.created"}}</th>
+          <th>{{i18n "topic.title"}}</th>
-      <th>{{i18n "activity"}}</th>
+          <th>{{i18n "post.bookmarks.created"}}</th>
-      <th>&nbsp;</th>
+          <th>{{i18n "activity"}}</th>
-    </thead>
+          <th>&nbsp;</th>
-    <tbody>
+        </thead>
-    {{#each content as |bookmark|}}
+        <tbody>
-      <tr class="topic-list-item bookmark-list-item">
+        {{#each content as |bookmark|}}
-        <td class="main-link">
+          <tr class="topic-list-item bookmark-list-item">
-          <span class="link-top-line">
+            <td class="main-link">
-            <div class="bookmark-metadata">
+              <span class="link-top-line">
-              {{#if bookmark.name}}
+                <div class="bookmark-metadata">
-                <span class="bookmark-metadata-item">
+                  {{#if bookmark.name}}
-                  {{d-icon "info-circle"}}{{bookmark.name}}
+                    <span class="bookmark-metadata-item">
-                </span>
+                      {{d-icon "info-circle"}}{{bookmark.name}}
-              {{/if}}
+                    </span>
-              {{#if bookmark.reminder_at}}
+                  {{/if}}
-                <span class="bookmark-metadata-item">
+                  {{#if bookmark.reminder_at}}
-                  {{d-icon "far-clock"}}{{bookmark.formattedReminder}}
+                    <span class="bookmark-metadata-item">
-                </span>
+                      {{d-icon "far-clock"}}{{bookmark.formattedReminder}}
-              {{/if}}
+                    </span>
-            </div>
+                  {{/if}}
+                </div>
 
-            {{topic-status topic=bookmark}}
+                {{topic-status topic=bookmark}}
-            {{topic-link bookmark}}
+                {{topic-link bookmark}}
-          </span>
+              </span>
-          {{#if bookmark.excerpt}}
+              {{#if bookmark.excerpt}}
-          <p class="post-excerpt">{{html-safe bookmark.excerpt}}</p>
+              <p class="post-excerpt">{{html-safe bookmark.excerpt}}</p>
-          {{/if}}
+              {{/if}}
-          <div class="link-bottom-line">
+              <div class="link-bottom-line">
-            {{category-link bookmark.category}}
+                {{category-link bookmark.category}}
-            {{discourse-tags bookmark mode="list" tagsForUser=tagsForUser}}
+                {{discourse-tags bookmark mode="list" tagsForUser=tagsForUser}}
-         </div>
+             </div>
-        </td>
+            </td>
-        <td>{{format-date bookmark.created_at format="tiny"}}</td>
+            <td>{{format-date bookmark.created_at format="tiny"}}</td>
-        {{raw "list/activity-column" topic=bookmark class="num" tagName="td"}}
+            {{raw "list/activity-column" topic=bookmark class="num" tagName="td"}}
-        <td>
+            <td>
-          {{bookmark-actions-dropdown bookmark=bookmark removeBookmark=(action "removeBookmark")}}
+              {{bookmark-actions-dropdown bookmark=bookmark removeBookmark=(action "removeBookmark")}}
-        </td>
+            </td>
-      </tr>
+          </tr>
-    {{/each}}
+        {{/each}}
-    </tbody>
+        </tbody>
-  </table>
+      </table>
+      {{conditional-loading-spinner condition=loadingMore}}
+    {{/load-more}}
   {{/conditional-loading-spinner}}
 {{/if}}

app/assets/javascripts/discourse/widgets/quick-access-bookmarks.js

@@ -62,6 +62,8 @@ createWidgetFrom(QuickAccessPanel, "quick-access-bookmarks", {
         limit: this.estimateItemLimit()
       }
     }).then(result => {
+      result = result.user_bookmark_list;
+
       // The empty state help text for bookmarks page is localized on the
       // server.
       if (result.no_results_help) {

app/controllers/users_controller.rb

@@ -1404,15 +1404,18 @@ class UsersController < ApplicationController
 
     respond_to do |format|
       format.json do
-        bookmarks = BookmarkQuery.new(user: user, guardian: guardian, params: params).list_all
+        bookmark_list = UserBookmarkList.new(user: user, guardian: guardian, params: params)
+        bookmark_list.load
 
-        if bookmarks.empty?
+        if bookmark_list.bookmarks.empty?
           render json: {
             bookmarks: [],
             no_results_help: I18n.t("user_activity.no_bookmarks.self")
           }
         else
-          render_serialized(bookmarks, UserBookmarkSerializer, root: 'bookmarks')
+          page = params[:page].to_i + 1
+          bookmark_list.more_bookmarks_url = "#{Discourse.base_path}/u/#{params[:username]}/bookmarks.json?page=#{page}"
+          render_serialized(bookmark_list, UserBookmarkListSerializer)
         end
       end
       format.ics do

app/models/user.rb

@@ -14,6 +14,7 @@ class User < ActiveRecord::Base
   has_many :tag_users, dependent: :destroy
   has_many :user_api_keys, dependent: :destroy
   has_many :topics
+  has_many :bookmarks
 
   # dependent deleting handled via before_destroy
   has_many :user_actions

app/models/user_bookmark_list.rb

@@ -0,0 +1,26 @@
+# frozen_string_literal: true
+
+class UserBookmarkList
+  include ActiveModel::Serialization
+
+  PER_PAGE = 20
+
+  attr_reader :bookmarks
+  attr_accessor :more_bookmarks_url
+
+  def initialize(user: user, guardian: guardian, params: params)
+    @user = user
+    @guardian = guardian
+    @params = params.merge(per_page: PER_PAGE)
+    @bookmarks = []
+  end
+
+  def load
+    @bookmarks = BookmarkQuery.new(user: @user, guardian: @guardian, params: @params).list_all
+    @bookmarks
+  end
+
+  def per_page
+    @per_page ||= PER_PAGE
+  end
+end

app/serializers/user_bookmark_list_serializer.rb

@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+class UserBookmarkListSerializer < ApplicationSerializer
+  attributes :more_bookmarks_url
+
+  has_many :bookmarks, serializer: UserBookmarkSerializer, embed: :objects
+
+  def include_more_bookmarks_url?
+    object.bookmarks.size == object.per_page
+  end
+end

lib/bookmark_query.rb

@@ -22,6 +22,8 @@ class BookmarkQuery
     @user = user
     @params = params
     @guardian = guardian || Guardian.new(@user)
+    @page = @params[:page].to_i
+    @limit = @params[:limit].present? ? @params[:limit].to_i : @params[:per_page]
   end
 
   def list_all
@@ -34,10 +36,12 @@ class BookmarkQuery
 
     results = results.merge(Post.secured(@guardian))
 
-    if @params[:limit]
+    if @page.positive?
-      results = results.limit(@params[:limit])
+      results = results.offset(@page * @params[:per_page])
     end
 
+    results = results.limit(@limit)
+
     if BookmarkQuery.preloaded_custom_fields.any?
       Topic.preload_custom_fields(
         results.map(&:topic), BookmarkQuery.preloaded_custom_fields

spec/requests/users_controller_spec.rb

@@ -4198,7 +4198,7 @@ describe UsersController do
       sign_in(user)
       get "/u/#{user.username}/bookmarks.json"
       expect(response.status).to eq(200)
-      expect(JSON.parse(response.body)['bookmarks'].map { |b| b['id'] }).to match_array([bookmark1.id, bookmark2.id])
+      expect(JSON.parse(response.body)['user_bookmark_list']['bookmarks'].map { |b| b['id'] }).to match_array([bookmark1.id, bookmark2.id])
     end
 
     it "does not show another user's bookmarks" do