FIX: N1 issues for bookmark list (#9236)

* Preload custom fields for BookmarkQuery and add preload callback. Copy TopicQuery preload methodology to allow plugins to preload data for the BookmarkQuery. This fixes assigned plugin custom fields N1
* Include topic tags in initial query to avoid tags N1

Related: discourse/discourse-assign#63

lib/bookmark_query.rb

@@ -5,6 +5,19 @@
 # in the user/activity/bookmarks page.
 
 class BookmarkQuery
+  cattr_accessor :preloaded_custom_fields
+  self.preloaded_custom_fields = Set.new
+
+  def self.on_preload(&blk)
+    (@preload ||= Set.new) << blk
+  end
+
+  def self.preload(bookmarks, object)
+    if @preload
+      @preload.each { |preload| preload.call(bookmarks, object) }
+    end
+  end
+
   def initialize(user, params = {})
     @user = user
     @params = params
@@ -15,18 +28,28 @@ class BookmarkQuery
       .joins('INNER JOIN topics ON topics.id = bookmarks.topic_id')
       .joins('INNER JOIN posts ON posts.id = bookmarks.post_id')
       .joins('INNER JOIN users ON users.id = posts.user_id')
-      .order('created_at DESC')
+      .order('bookmarks.created_at DESC')
 
     if @params[:limit]
       results = results.limit(@params[:limit])
     end
 
+    if BookmarkQuery.preloaded_custom_fields.any?
+      Topic.preload_custom_fields(
+        results.map(&:topic), BookmarkQuery.preloaded_custom_fields
+      )
+    end
+
+    BookmarkQuery.preload(results, self)
+
     results
   end
 
   private
 
   def user_bookmarks
-    Bookmark.where(user: @user).includes(:topic).includes(post: :user)
+    Bookmark.where(user: @user)
+      .includes(topic: :tags)
+      .includes(post: :user)
   end
 end

spec/lib/bookmark_query_spec.rb

@@ -0,0 +1,51 @@
+# frozen_string_literal: true
+
+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) { {} }
+
+  subject { described_class.new(user, params) }
+
+  describe "#list_all" do
+    it "returns all the bookmarks for a user" do
+      expect(subject.list_all.count).to eq(2)
+    end
+
+    it "runs the on_preload block provided passing in bookmarks" do
+      preloaded_bookmarks = []
+      BookmarkQuery.on_preload do |bookmarks, bq|
+        (preloaded_bookmarks << bookmarks).flatten
+      end
+      subject.list_all
+      expect(preloaded_bookmarks.any?).to eq(true)
+    end
+
+    context "when the limit param is provided" do
+      let(:params) { { limit: 1 } }
+      it "is respected" do
+        expect(subject.list_all.count).to eq(1)
+      end
+    end
+
+    context "when there are topic custom fields to preload" do
+      before do
+        TopicCustomField.create(
+          topic_id: bookmark1.topic.id, name: 'test_field', value: 'test'
+        )
+        BookmarkQuery.preloaded_custom_fields << "test_field"
+      end
+      it "preloads them" do
+        Topic.expects(:preload_custom_fields)
+        expect(
+          subject.list_all.find do |b|
+            b.topic_id = bookmark1.topic_id
+          end.topic.custom_fields['test_field']
+        ).not_to eq(nil)
+      end
+    end
+  end
+end