diff --git a/app/assets/javascripts/discourse/app/models/bookmark.js b/app/assets/javascripts/discourse/app/models/bookmark.js
index f28c92845ee..5d874499a81 100644
--- a/app/assets/javascripts/discourse/app/models/bookmark.js
+++ b/app/assets/javascripts/discourse/app/models/bookmark.js
@@ -159,17 +159,12 @@ const Bookmark = RestModel.extend({
});
},
- @discourseComputed(
- "post_user_username",
- "post_user_avatar_template",
- "post_user_name"
- )
- postUser(post_user_username, avatarTemplate, name) {
- return User.create({
- username: post_user_username,
- avatar_template: avatarTemplate,
- name,
- });
+ @discourseComputed("bookmarkable_type")
+ bookmarkableTopicAlike(bookmarkable_type) {
+ if (!this.siteSettings.use_polymorphic_bookmarks) {
+ return true;
+ }
+ return ["Topic", "Post"].includes(bookmarkable_type);
},
});
@@ -177,6 +172,7 @@ Bookmark.reopenClass({
create(args) {
args = args || {};
args.currentUser = args.currentUser || User.current();
+ args.user = User.create(args.user);
return this._super(args);
},
});
diff --git a/app/assets/javascripts/discourse/app/templates/components/bookmark-list.hbs b/app/assets/javascripts/discourse/app/templates/components/bookmark-list.hbs
index 819b95296ae..c7954015474 100644
--- a/app/assets/javascripts/discourse/app/templates/components/bookmark-list.hbs
+++ b/app/assets/javascripts/discourse/app/templates/components/bookmark-list.hbs
@@ -31,17 +31,29 @@
{{#if bookmark.pinned}}
{{d-icon "thumbtack" class="bookmark-pinned"}}
{{/if}}
- {{~topic-status topic=bookmark.topicStatus~}}
- {{topic-link bookmark.topicForList}}
+ {{#if bookmark.bookmarkableTopicAlike}}
+ {{~topic-status topic=bookmark.topicStatus~}}
+ {{topic-link bookmark.topicForList}}
+ {{else}}
+
+ {{bookmark.fancy_title}}
+
+ {{/if}}
-
- {{category-link bookmark.category}}
- {{discourse-tags bookmark mode="list" tagsForUser=tagsForUser}}
-
- {{#if (and site.mobileView bookmark.excerpt bookmark.post_user_avatar_template)}}
-
- {{avatar bookmark.postUser avatarTemplatePath="avatar_template" usernamePath="username" namePath="name" imageSize="small"}}
+ {{#if bookmark.bookmarkableTopicAlike}}
+
+ {{category-link bookmark.category}}
+ {{discourse-tags bookmark mode="list" tagsForUser=tagsForUser}}
+
+ {{/if}}
+ {{#if (and site.mobileView bookmark.excerpt bookmark.user.avatar_template)}}
+
+ {{avatar bookmark.bookmarkableUser avatarTemplatePath="avatar_template" usernamePath="username" namePath="name" imageSize="small"}}
{{/if}}
{{!-- template-lint-disable --}}
@@ -49,9 +61,9 @@
{{#unless site.mobileView}}
- {{#if bookmark.post_user_avatar_template}}
-
- {{avatar bookmark.postUser avatarTemplatePath="avatar_template" usernamePath="username" namePath="name" imageSize="small"}}
+ {{#if bookmark.user.avatar_template}}
+
+ {{avatar bookmark.user avatarTemplatePath="avatar_template" usernamePath="username" namePath="name" imageSize="small"}}
{{/if}}
|
diff --git a/app/models/bookmark.rb b/app/models/bookmark.rb
index 6007677cde0..aa48363ac6a 100644
--- a/app/models/bookmark.rb
+++ b/app/models/bookmark.rb
@@ -8,12 +8,101 @@ class Bookmark < ActiveRecord::Base
"reminder_type" # TODO 2021-04-01: remove
]
+ cattr_accessor :registered_bookmarkables
+ self.registered_bookmarkables = []
+
+ def self.registered_bookmarkable_from_type(type)
+ Bookmark.registered_bookmarkables.find { |bm| bm.model.name == type }
+ end
+
+ def self.register_bookmarkable(
+ model:, serializer:, list_query:, search_query:, preload_associations: []
+ )
+ Bookmark.registered_bookmarkables << Bookmarkable.new(
+ model: model,
+ serializer: serializer,
+ list_query: list_query,
+ search_query: search_query,
+ preload_associations: preload_associations
+ )
+ end
+
+ ##
+ # This is called when the app loads, similar to AdminDashboardData.reset_problem_checks,
+ # so the default Post and Topic bookmarkables are registered on
+ # boot.
+ #
+ # This method also can be used in testing to reset bookmarkables between
+ # tests. It will also fire multiple times in development mode because
+ # classes are not cached.
+ def self.reset_bookmarkables
+ self.registered_bookmarkables = []
+ Bookmark.register_bookmarkable(
+ model: Post,
+ serializer: UserPostBookmarkSerializer,
+ list_query: lambda do |user, guardian|
+ topics = Topic.listable_topics.secured(guardian)
+ pms = Topic.private_messages_for_user(user)
+ post_bookmarks = user
+ .bookmarks_of_type("Post")
+ .joins("INNER JOIN posts ON posts.id = bookmarks.bookmarkable_id AND bookmarks.bookmarkable_type = 'Post'")
+ .joins("LEFT JOIN topics ON topics.id = posts.topic_id")
+ .joins("LEFT JOIN topic_users ON topic_users.topic_id = topics.id")
+ .where("topic_users.user_id = ?", user.id)
+ guardian.filter_allowed_categories(
+ post_bookmarks.merge(topics.or(pms)).merge(Post.secured(guardian))
+ )
+ end,
+ search_query: lambda do |bookmarks, query, ts_query, &bookmarkable_search|
+ bookmarkable_search.call(
+ bookmarks.joins(
+ "LEFT JOIN post_search_data ON post_search_data.post_id = bookmarks.bookmarkable_id AND bookmarks.bookmarkable_type = 'Post'"
+ ),
+ "#{ts_query} @@ post_search_data.search_data"
+ )
+ end,
+ preload_associations: [{ topic: [:topic_users, :tags] }, :user]
+ )
+ Bookmark.register_bookmarkable(
+ model: Topic,
+ serializer: UserTopicBookmarkSerializer,
+ list_query: lambda do |user, guardian|
+ topics = Topic.listable_topics.secured(guardian)
+ pms = Topic.private_messages_for_user(user)
+ topic_bookmarks = user
+ .bookmarks_of_type("Topic")
+ .joins("INNER JOIN topics ON topics.id = bookmarks.bookmarkable_id AND bookmarks.bookmarkable_type = 'Topic'")
+ .joins("LEFT JOIN topic_users ON topic_users.topic_id = topics.id")
+ .where("topic_users.user_id = ?", user.id)
+ guardian.filter_allowed_categories(topic_bookmarks.merge(topics.or(pms)))
+ end,
+ search_query: lambda do |bookmarks, query, ts_query, &bookmarkable_search|
+ bookmarkable_search.call(
+ bookmarks
+ .joins("LEFT JOIN posts ON posts.topic_id = topics.id AND posts.post_number = 1")
+ .joins("LEFT JOIN post_search_data ON post_search_data.post_id = posts.id"),
+ "#{ts_query} @@ post_search_data.search_data"
+ )
+ end,
+ preload_associations: [:topic_users, :tags, { posts: :user }]
+ )
+ end
+ reset_bookmarkables
+
+ def self.valid_bookmarkable_types
+ Bookmark.registered_bookmarkables.map(&:model).map(&:to_s)
+ end
+
belongs_to :user
belongs_to :post
has_one :topic, through: :post
belongs_to :bookmarkable, polymorphic: true
- delegate :topic_id, to: :post
+ # TODO (martin) [POLYBOOK] Not relevant once polymorphic bookmarks are implemented.
+ def topic_id
+ return if SiteSetting.use_polymorphic_bookmarks
+ post.topic_id
+ end
def self.auto_delete_preferences
@auto_delete_preferences ||= Enum.new(
@@ -24,6 +113,10 @@ class Bookmark < ActiveRecord::Base
)
end
+ def self.select_type(bookmarks_relation, type)
+ bookmarks_relation.select { |bm| bm.bookmarkable_type == type }
+ end
+
# TODO (martin) [POLYBOOK] Not relevant once polymorphic bookmarks are implemented.
validate :unique_per_post_for_user,
on: [:create, :update],
@@ -35,6 +128,7 @@ class Bookmark < ActiveRecord::Base
if: Proc.new { |b| b.will_save_change_to_post_id? || b.will_save_change_to_for_topic? }
validate :polymorphic_columns_present, on: [:create, :update]
+ validate :valid_bookmarkable_type, on: [:create, :update]
validate :unique_per_bookmarkable,
on: [:create, :update],
@@ -106,6 +200,13 @@ class Bookmark < ActiveRecord::Base
)
end
+ def valid_bookmarkable_type
+ return if !SiteSetting.use_polymorphic_bookmarks
+ return if Bookmark.valid_bookmarkable_types.include?(self.bookmarkable_type)
+
+ self.errors.add(:base, I18n.t("bookmarks.errors.invalid_bookmarkable", type: self.bookmarkable_type))
+ end
+
# TODO (martin) [POLYBOOK] Not relevant once polymorphic bookmarks are implemented.
def is_for_first_post?
@is_for_first_post ||= new_record? ? Post.exists?(id: post_id, post_number: 1) : post.post_number == 1
diff --git a/app/models/user.rb b/app/models/user.rb
index 2e5bab0b851..3a5f3837529 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -343,6 +343,10 @@ class User < ActiveRecord::Base
end
end
+ def bookmarks_of_type(type)
+ bookmarks.where(bookmarkable_type: type)
+ end
+
EMAIL = %r{([^@]+)@([^\.]+)}
FROM_STAGED = "from_staged"
diff --git a/app/serializers/user_bookmark_base_serializer.rb b/app/serializers/user_bookmark_base_serializer.rb
new file mode 100644
index 00000000000..94ac277c79d
--- /dev/null
+++ b/app/serializers/user_bookmark_base_serializer.rb
@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+class UserBookmarkBaseSerializer < ApplicationSerializer
+ attributes :id,
+ :created_at,
+ :updated_at,
+ :name,
+ :reminder_at,
+ :pinned,
+ :title,
+ :fancy_title,
+ :excerpt,
+ :bookmarkable_id,
+ :bookmarkable_type,
+ :bookmarkable_url
+
+ def title
+ raise NotImplementedError
+ end
+
+ def fancy_title
+ raise NotImplementedError
+ end
+
+ def cooked
+ raise NotImplementedError
+ end
+
+ def bookmarkable_url
+ raise NotImplementedError
+ end
+
+ def excerpt
+ raise NotImplementedError
+ end
+
+ # Note: This assumes that the bookmarkable has a user attached to it,
+ # we may need to revisit this assumption at some point.
+ has_one :user, serializer: BasicUserSerializer, embed: :objects
+
+ def user
+ bookmarkable_user
+ end
+end
diff --git a/app/serializers/user_bookmark_list_serializer.rb b/app/serializers/user_bookmark_list_serializer.rb
index a7a4ed0fdae..25c840f9c64 100644
--- a/app/serializers/user_bookmark_list_serializer.rb
+++ b/app/serializers/user_bookmark_list_serializer.rb
@@ -1,11 +1,27 @@
# frozen_string_literal: true
class UserBookmarkListSerializer < ApplicationSerializer
- attributes :more_bookmarks_url
+ attributes :more_bookmarks_url, :bookmarks
- has_many :bookmarks, serializer: UserBookmarkSerializer, embed: :objects
+ def bookmarks
+ if SiteSetting.use_polymorphic_bookmarks
+ object.bookmarks.map do |bm|
+ serialize_registered_type(bm)
+ end
+ else
+ object.bookmarks.map { |bm| UserBookmarkSerializer.new(bm, scope: scope, root: false) }
+ end
+ end
def include_more_bookmarks_url?
@include_more_bookmarks_url ||= object.bookmarks.size == object.per_page
end
+
+ private
+
+ def serialize_registered_type(bookmark)
+ Bookmark.registered_bookmarkable_from_type(
+ bookmark.bookmarkable_type
+ ).serializer.new(bookmark, scope: scope, root: false)
+ end
end
diff --git a/app/serializers/user_bookmark_serializer.rb b/app/serializers/user_bookmark_serializer.rb
index 1edb26adb69..0f8386703ba 100644
--- a/app/serializers/user_bookmark_serializer.rb
+++ b/app/serializers/user_bookmark_serializer.rb
@@ -2,6 +2,7 @@
require_relative 'post_item_excerpt'
+# TODO (martin) [POLYBOOK] Not relevant once polymorphic bookmarks are implemented.
class UserBookmarkSerializer < ApplicationSerializer
include PostItemExcerpt
include TopicTagsMixin
@@ -30,7 +31,12 @@ class UserBookmarkSerializer < ApplicationSerializer
:post_user_username,
:post_user_avatar_template,
:post_user_name,
- :for_topic
+ :for_topic,
+ :bookmarkable_id,
+ :bookmarkable_type,
+ :bookmarkable_user_username,
+ :bookmarkable_user_avatar_template,
+ :bookmarkable_user_name,
def topic_id
post.topic_id
@@ -148,4 +154,19 @@ class UserBookmarkSerializer < ApplicationSerializer
def post_user_name
post_user.name
end
+
+ # TODO (martin) [POLYBOOK] Not relevant once polymorphic bookmarks are implemented.
+ # Note...these are just stub methods for compatability with the user-bookmark-list.hbs
+ # changes in a transition period for polymorphic bookmarks.
+ def bookmarkable_user_username
+ post_user.username
+ end
+
+ def bookmarkable_user_avatar_template
+ post_user.avatar_template
+ end
+
+ def bookmarkable_user_name
+ post_user.name
+ end
end
diff --git a/app/serializers/user_post_bookmark_serializer.rb b/app/serializers/user_post_bookmark_serializer.rb
new file mode 100644
index 00000000000..d1cafcf6ff5
--- /dev/null
+++ b/app/serializers/user_post_bookmark_serializer.rb
@@ -0,0 +1,43 @@
+# frozen_string_literal: true
+
+class UserPostBookmarkSerializer < UserPostTopicBookmarkBaseSerializer
+ attr_reader :post_id
+
+ def post_id
+ post.id
+ end
+
+ def linked_post_number
+ post.post_number
+ end
+
+ def deleted
+ topic.deleted_at.present? || post.deleted_at.present?
+ end
+
+ def hidden
+ post.hidden
+ end
+
+ def raw
+ post.raw
+ end
+
+ def cooked
+ post.cooked
+ end
+
+ def bookmarkable_user
+ @bookmarkable_user ||= post.user
+ end
+
+ private
+
+ def topic
+ post.topic
+ end
+
+ def post
+ object.bookmarkable
+ end
+end
diff --git a/app/serializers/user_post_topic_bookmark_base_serializer.rb b/app/serializers/user_post_topic_bookmark_base_serializer.rb
new file mode 100644
index 00000000000..b33898d2381
--- /dev/null
+++ b/app/serializers/user_post_topic_bookmark_base_serializer.rb
@@ -0,0 +1,77 @@
+# frozen_string_literal: true
+
+require_relative 'post_item_excerpt'
+
+class UserPostTopicBookmarkBaseSerializer < UserBookmarkBaseSerializer
+ include TopicTagsMixin
+ include PostItemExcerpt
+
+ attributes :topic_id,
+ :linked_post_number,
+ :deleted,
+ :hidden,
+ :category_id,
+ :closed,
+ :archived,
+ :archetype,
+ :highest_post_number,
+ :last_read_post_number,
+ :bumped_at,
+ :slug
+
+ def topic_id
+ topic.id
+ end
+
+ def title
+ topic.title
+ end
+
+ def fancy_title
+ topic.fancy_title
+ end
+
+ def category_id
+ topic.category_id
+ end
+
+ def archetype
+ topic.archetype
+ end
+
+ def archived
+ topic.archived
+ end
+
+ def closed
+ topic.closed
+ end
+
+ def highest_post_number
+ scope.is_staff? ? topic.highest_staff_post_number : topic.highest_post_number
+ end
+
+ def last_read_post_number
+ topic_user&.last_read_post_number
+ end
+
+ def bumped_at
+ topic.bumped_at
+ end
+
+ def slug
+ topic.slug
+ end
+
+ # Note: This is nil because in the UI there are special topic-status and
+ # topic-link components to display the topic URL, and this is not used.
+ def bookmarkable_url
+ nil
+ end
+
+ private
+
+ def topic_user
+ @topic_user ||= topic.topic_users.find { |tu| tu.user_id == scope.user.id }
+ end
+end
diff --git a/app/serializers/user_topic_bookmark_serializer.rb b/app/serializers/user_topic_bookmark_serializer.rb
new file mode 100644
index 00000000000..c6a02b8772a
--- /dev/null
+++ b/app/serializers/user_topic_bookmark_serializer.rb
@@ -0,0 +1,61 @@
+# frozen_string_literal: true
+
+class UserTopicBookmarkSerializer < UserPostTopicBookmarkBaseSerializer
+ # it does not matter what the linked post number is for topic bookmarks,
+ # on the client we always take the user to the last unread post in the
+ # topic when the bookmark URL is clicked
+ def linked_post_number
+ 1
+ end
+
+ def first_post
+ @first_post ||= topic.posts.find { |post| post.post_number == 1 }
+ end
+
+ def deleted
+ topic.deleted_at.present? || first_post.deleted_at.present?
+ end
+
+ def hidden
+ first_post.hidden
+ end
+
+ def raw
+ first_post.raw
+ end
+
+ def cooked
+ @cooked ||= \
+ if last_read_post_number.present?
+ for_topic_cooked_post
+ else
+ first_post.cooked
+ end
+ end
+
+ def for_topic_cooked_post
+ post_number = [last_read_post_number + 1, highest_post_number].min
+ sorted_regular_posts = topic.posts.sort_by(&:post_number).select do |post|
+ post.post_type == Post.types[:regular]
+ end
+ first_unread_post = sorted_regular_posts.find do |post|
+ post.post_number >= post_number
+ end
+
+ # if first_unread_cooked is blank this likely means that the last
+ # read post was either deleted or is a small action post.
+ # in this case we should just get the last regular post and
+ # use that for the cooked value so we have something to show
+ (first_unread_post || sorted_regular_posts.last).cooked
+ end
+
+ def bookmarkable_user
+ @bookmarkable_user ||= first_post.user
+ end
+
+ private
+
+ def topic
+ object.bookmarkable
+ end
+end
diff --git a/app/services/bookmarkable.rb b/app/services/bookmarkable.rb
new file mode 100644
index 00000000000..525f07ee65b
--- /dev/null
+++ b/app/services/bookmarkable.rb
@@ -0,0 +1,80 @@
+# frozen_string_literal: true
+#
+# Should only be created via the Bookmark.register_bookmarkable
+# method; this is used to let the BookmarkQuery class query and
+# search additional bookmarks for the user bookmark list, and
+# also to enumerate on the registered Bookmarkable types.
+#
+# Post and Topic bookmarkables are registered by default.
+#
+# Anything other than types registered in this way will throw an error
+# when trying to save the Bookmark record. All things that are bookmarkable
+# must be registered in this way.
+#
+# See Bookmark#reset_bookmarkables for some examples on how registering
+# bookmarkables works.
+class Bookmarkable
+ attr_reader :model, :serializer, :list_query, :search_query, :preload_associations
+ delegate :table_name, to: :@model
+
+ def initialize(model:, serializer:, list_query:, search_query:, preload_associations: [])
+ @model = model
+ @serializer = serializer
+ @list_query = list_query
+ @search_query = search_query
+ @preload_associations = preload_associations
+ end
+
+ ##
+ # This is where the main query to filter the bookmarks by the provided bookmarkable
+ # type should occur. This should join on additional tables that are required later
+ # on to preload additional data for serializers, and also is the place where the
+ # bookmarks should be filtered based on security checks, which is why the Guardian
+ # instance is provided.
+ #
+ # @param [User] user The user to perform the query for, this scopes the bookmarks returned.
+ # @param [Guardian] guardian An instance of Guardian for the user to be used for security filters.
+ def perform_list_query(user, guardian)
+ list_query.call(user, guardian)
+ end
+
+ ##
+ # Called from BookmarkQuery when the initial results have been returned by
+ # perform_list_query. The search_query should join additional tables required
+ # to filter the bookmarks further, as well as defining a string used for
+ # where_sql, which can include comparisons with the :q parameter.
+ #
+ # The block here warrants explanation -- when the search_query is called, we
+ # call the provided block with the bookmark relation with additional joins
+ # as well as the where_sql string, and then also add the additional OR bookmarks.name
+ # filter. This is so every bookmarkable is filtered by its own customized
+ # columns _as well as_ the bookmark name, because the bookmark name must always
+ # be used in the search.
+ #
+ # @param [Bookmark::ActiveRecord_Relation] bookmarks The bookmark records returned by perform_list_query
+ # @param [String] query The search query from the user surrounded by the %% wildcards
+ # @param [String] ts_query The postgres TSQUERY string used for comparisons with full text search columns
+ def perform_search_query(bookmarks, query, ts_query)
+ search_query.call(bookmarks, query, ts_query) do |bookmarks_joined, where_sql|
+ bookmarks_joined.where("#{where_sql} OR bookmarks.name ILIKE :q", q: query)
+ end
+ end
+
+ ##
+ # When displaying the bookmarks in a list for a user there is often additional
+ # information drawn from other tables joined to the bookmarkable that must
+ # be displayed. We preload these additional associations here on top of the
+ # array of bookmarks which has already been filtered, offset by page, ordered,
+ # and limited. The preload_associations array should be in the same format as
+ # used for .includes() e.g.
+ #
+ # [{ topic: [:topic_users, :tags] }, :user]
+ #
+ # @param [Array] bookmarks The array of bookmarks after initial listing and filtering, note this is
+ # array _not_ an ActiveRecord::Relation.
+ def perform_preload(bookmarks)
+ ActiveRecord::Associations::Preloader.new.preload(
+ Bookmark.select_type(bookmarks, model.to_s), { bookmarkable: preload_associations }
+ )
+ end
+end
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 7125477c1ab..dbab889524c 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -437,6 +437,7 @@ en:
time_must_be_provided: "Time must be provided for all reminders"
for_topic_must_use_first_post: "You can only use the first post to bookmark the topic."
bookmarkable_id_type_required: "The name and type of the record to bookmark is required."
+ invalid_bookmarkable: "A %{type} cannot be bookmarked."
reminders:
at_desktop: "Next time I'm at my desktop"
diff --git a/lib/bookmark_query.rb b/lib/bookmark_query.rb
index 34ba8318b6a..fea26a77e85 100644
--- a/lib/bookmark_query.rb
+++ b/lib/bookmark_query.rb
@@ -5,19 +5,29 @@
# 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 SiteSetting.use_polymorphic_bookmarks
+ preload_polymorphic_associations(bookmarks)
+ end
if @preload
@preload.each { |preload| preload.call(bookmarks, object) }
end
end
+ # These polymorphic associations are loaded to make the UserBookmarkListSerializer's
+ # life easier, which conditionally chooses the bookmark serializer to use based
+ # on the type, and we want the associations all loaded ahead of time to make
+ # sure we are not doing N+1s.
+ def self.preload_polymorphic_associations(bookmarks)
+ Bookmark.registered_bookmarkables.each do |registered_bookmarkable|
+ registered_bookmarkable.perform_preload(bookmarks)
+ end
+ end
+
def initialize(user:, guardian: nil, params: {})
@user = user
@params = params
@@ -27,49 +37,77 @@ class BookmarkQuery
end
def list_all
- results = user_bookmarks.order(
- '(CASE WHEN bookmarks.pinned THEN 0 ELSE 1 END), bookmarks.reminder_at ASC, bookmarks.updated_at DESC'
- )
+ return polymorphic_list_all if SiteSetting.use_polymorphic_bookmarks
topics = Topic.listable_topics.secured(@guardian)
pms = Topic.private_messages_for_user(@user)
- results = results.merge(topics.or(pms))
+ results = list_all_results(topics, pms)
- results = results.merge(Post.secured(@guardian))
+ results = results.order(
+ "(CASE WHEN bookmarks.pinned THEN 0 ELSE 1 END),
+ bookmarks.reminder_at ASC,
+ bookmarks.updated_at DESC"
+ )
if @params[:q].present?
- 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}%"
- )
+ results = search(results, @params[:q])
end
if @page.positive?
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
- )
- end
-
+ results = results.limit(@limit).to_a
BookmarkQuery.preload(results, self)
-
- @guardian.filter_allowed_categories(results)
+ results
end
private
- def user_bookmarks
- # There is guaranteed to be a TopicUser record if the user has bookmarked
- # a topic, see BookmarkManager
+ def polymorphic_list_all
+ search_term = @params[:q]
+ ts_query = search_term.present? ? Search.ts_query(term: search_term) : nil
+ search_term_wildcard = search_term.present? ? "%#{search_term}%" : nil
+
+ queries = Bookmark.registered_bookmarkables.map do |bookmarkable|
+ interim_results = bookmarkable.perform_list_query(@user, @guardian)
+
+ if search_term.present?
+ interim_results = bookmarkable.perform_search_query(
+ interim_results, search_term_wildcard, ts_query
+ )
+ end
+
+ # this is purely to make the query easy to read and debug, otherwise it's
+ # all mashed up into a massive ball in MiniProfiler :)
+ "---- #{bookmarkable.model.to_s} bookmarkable ---\n\n #{interim_results.to_sql}"
+ end
+
+ union_sql = queries.join("\n\nUNION\n\n")
+ results = Bookmark.select("bookmarks.*").from("(\n\n#{union_sql}\n\n) as bookmarks")
+ results = results.order(
+ "(CASE WHEN bookmarks.pinned THEN 0 ELSE 1 END),
+ bookmarks.reminder_at ASC,
+ bookmarks.updated_at DESC"
+ )
+
+ if @page.positive?
+ results = results.offset(@page * @params[:per_page])
+ end
+
+ results = results.limit(@limit).to_a
+ BookmarkQuery.preload(results, self)
+ results
+ end
+
+ def list_all_results(topics, pms)
+ results = base_bookmarks.merge(topics.or(pms))
+ results = results.merge(Post.secured(@guardian))
+ results = @guardian.filter_allowed_categories(results)
+ results
+ end
+
+ def base_bookmarks
Bookmark.where(user: @user)
.includes(post: :user)
.includes(post: { topic: :tags })
@@ -77,4 +115,11 @@ class BookmarkQuery
.references(:post)
.where(topic_users: { user_id: @user.id })
end
+
+ def search(results, term)
+ bookmark_ts_query = Search.ts_query(term: term)
+ 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}%")
+ end
end
diff --git a/spec/fabricators/bookmark_fabricator.rb b/spec/fabricators/bookmark_fabricator.rb
index d86816792a7..6cbabbfa00f 100644
--- a/spec/fabricators/bookmark_fabricator.rb
+++ b/spec/fabricators/bookmark_fabricator.rb
@@ -2,10 +2,19 @@
Fabricator(:bookmark) do
user
- post { Fabricate(:post) }
+ post {
+ if !SiteSetting.use_polymorphic_bookmarks
+ Fabricate(:post)
+ end
+ }
name "This looked interesting"
reminder_at { 1.day.from_now.iso8601 }
reminder_set_at { Time.zone.now }
+ bookmarkable {
+ if SiteSetting.use_polymorphic_bookmarks
+ Fabricate(:post)
+ end
+ }
# TODO (martin) [POLYBOOK] Not relevant once polymorphic bookmarks are implemented.
before_create do |bookmark|
diff --git a/spec/lib/bookmark_query_spec.rb b/spec/lib/bookmark_query_spec.rb
index 8bfe3cf97c2..a3563dd53f3 100644
--- a/spec/lib/bookmark_query_spec.rb
+++ b/spec/lib/bookmark_query_spec.rb
@@ -12,6 +12,21 @@ RSpec.describe BookmarkQuery do
BookmarkQuery.new(user: user || self.user, params: params || self.params)
end
+ def register_user_bookmarkable
+ Bookmark.register_bookmarkable(
+ model: User,
+ serializer: UserBookmarkSerializer,
+ list_query: lambda do |user, guardian|
+ user.bookmarks.joins(
+ "INNER JOIN users ON users.id = bookmarks.bookmarkable_id AND bookmarks.bookmarkable_type = 'User'"
+ ).where(bookmarkable_type: "User")
+ end,
+ search_query: lambda do |bookmarks, query, ts_query|
+ bookmarks.where("users.username ILIKE ?", query)
+ end
+ )
+ end
+
describe "#list_all" do
fab!(:bookmark1) { Fabricate(:bookmark, user: user) }
fab!(:bookmark2) { Fabricate(:bookmark, user: user) }
@@ -50,28 +65,104 @@ RSpec.describe BookmarkQuery do
expect(bookmark.topic.topic_users.map(&:user_id)).to contain_exactly(user.id)
end
- context "when q param is provided" do
+ context "for polymorphic bookmarks" do
before do
- @post = Fabricate(:post, raw: "Some post content here", topic: Fabricate(:topic, title: "Bugfix game for devs"))
- @bookmark3 = Fabricate(:bookmark, user: user, name: "Check up later")
- @bookmark4 = Fabricate(:bookmark, user: user, post: @post)
- Fabricate(:topic_user, user: user, topic: @bookmark3.topic)
- Fabricate(:topic_user, user: user, topic: @bookmark4.topic)
+ SiteSetting.use_polymorphic_bookmarks = true
+ Bookmark.reset_bookmarkables
+ register_user_bookmarkable
+
+ Fabricate(:topic_user, user: user, topic: post_bookmark.bookmarkable.topic)
+ Fabricate(:topic_user, user: user, topic: topic_bookmark.bookmarkable)
+ user_bookmark
end
- it "can search by bookmark name" do
- bookmarks = bookmark_query(params: { q: 'check' }).list_all
- expect(bookmarks.map(&:id)).to eq([@bookmark3.id])
+ let(:post_bookmark) { Fabricate(:bookmark, user: user, bookmarkable: Fabricate(:post)) }
+ let(:topic_bookmark) { Fabricate(:bookmark, user: user, bookmarkable: Fabricate(:topic)) }
+ let(:user_bookmark) { Fabricate(:bookmark, user: user, bookmarkable: Fabricate(:user, username: "bookmarkqueen")) }
+
+ after do
+ Bookmark.reset_bookmarkables
end
- it "can search by post content" do
- bookmarks = bookmark_query(params: { q: 'content' }).list_all
- expect(bookmarks.map(&:id)).to eq([@bookmark4.id])
+ it "returns a mixture of post, topic, and custom bookmarkable type bookmarks" do
+ bookmarks = bookmark_query.list_all
+ expect(bookmarks.map(&:id)).to match_array([post_bookmark.id, topic_bookmark.id, user_bookmark.id])
+ end
+ end
+
+ context "when q param is provided" do
+ let!(:post) { Fabricate(:post, raw: "Some post content here", topic: Fabricate(:topic, title: "Bugfix game for devs")) }
+
+ context "when not using polymorphic bookmarks" do
+ let(:bookmark3) { Fabricate(:bookmark, user: user, name: "Check up later") }
+ let(:bookmark4) { Fabricate(:bookmark, user: user, post: post) }
+
+ before do
+ Fabricate(:topic_user, user: user, topic: bookmark3.topic)
+ Fabricate(:topic_user, user: user, topic: bookmark4.topic)
+ end
+
+ it "can search by bookmark name" do
+ bookmarks = bookmark_query(params: { q: 'check' }).list_all
+ expect(bookmarks.map(&:id)).to eq([bookmark3.id])
+ end
+
+ it "can search by post content" do
+ bookmarks = bookmark_query(params: { q: 'content' }).list_all
+ expect(bookmarks.map(&:id)).to eq([bookmark4.id])
+ end
+
+ it "can search by topic title" do
+ bookmarks = bookmark_query(params: { q: 'bugfix' }).list_all
+ expect(bookmarks.map(&:id)).to eq([bookmark4.id])
+ end
end
- it "can search by topic title" do
- bookmarks = bookmark_query(params: { q: 'bugfix' }).list_all
- expect(bookmarks.map(&:id)).to eq([@bookmark4.id])
+ context "when using polymorphic bookmarks" do
+ before do
+ SiteSetting.use_polymorphic_bookmarks = true
+ Bookmark.reset_bookmarkables
+ end
+
+ after do
+ Bookmark.reset_bookmarkables
+ end
+
+ let(:bookmark3) { Fabricate(:bookmark, user: user, name: "Check up later", bookmarkable: Fabricate(:post)) }
+ let(:bookmark4) { Fabricate(:bookmark, user: user, bookmarkable: post) }
+
+ before do
+ Fabricate(:topic_user, user: user, topic: bookmark3.bookmarkable.topic)
+ Fabricate(:topic_user, user: user, topic: bookmark4.bookmarkable.topic)
+ end
+
+ it "can search by bookmark name" do
+ bookmarks = bookmark_query(params: { q: 'check' }).list_all
+ expect(bookmarks.map(&:id)).to eq([bookmark3.id])
+ end
+
+ it "can search by post content" do
+ bookmarks = bookmark_query(params: { q: 'content' }).list_all
+ expect(bookmarks.map(&:id)).to eq([bookmark4.id])
+ end
+
+ it "can search by topic title" do
+ bookmarks = bookmark_query(params: { q: 'bugfix' }).list_all
+ expect(bookmarks.map(&:id)).to eq([bookmark4.id])
+ end
+
+ context "with custom bookmarkable fitering" do
+ before do
+ register_user_bookmarkable
+ end
+
+ let!(:bookmark5) { Fabricate(:bookmark, user: user, bookmarkable: Fabricate(:user, username: "bookmarkqueen")) }
+
+ it "allows searching bookmarkables by fields in other tables" do
+ bookmarks = bookmark_query(params: { q: 'bookmarkq' }).list_all
+ expect(bookmarks.map(&:id)).to eq([bookmark5.id])
+ end
+ end
end
end
@@ -159,23 +250,6 @@ RSpec.describe BookmarkQuery do
expect(bookmark_query.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(
- bookmark_query.list_all.find do |b|
- b.topic.id = bookmark1.topic.id
- end.topic.custom_fields['test_field']
- ).not_to eq(nil)
- end
- end
end
describe "#list_all ordering" do
diff --git a/spec/models/bookmark_spec.rb b/spec/models/bookmark_spec.rb
index d42868ca919..0addabef238 100644
--- a/spec/models/bookmark_spec.rb
+++ b/spec/models/bookmark_spec.rb
@@ -43,6 +43,10 @@ describe Bookmark do
SiteSetting.use_polymorphic_bookmarks = true
end
+ after do
+ Bookmark.registered_bookmarkables = []
+ end
+
it "does not allow a user to create a bookmark with only one polymorphic column" do
user = Fabricate(:user)
bm = Bookmark.create(bookmarkable_id: post.id, user: user)
@@ -59,6 +63,25 @@ describe Bookmark do
bm = Bookmark.create(bookmarkable_type: "Post", bookmarkable_id: post.id, user: user)
expect(bm.errors.full_messages).to include(I18n.t("bookmarks.errors.already_bookmarked", type: "Post"))
end
+
+ it "does not allow a user to create a bookmarkable for a type that has not been registered" do
+ user = Fabricate(:user)
+ bm = Bookmark.create(bookmarkable_type: "User", bookmarkable: Fabricate(:user), user: user)
+ expect(bm.errors.full_messages).to include(I18n.t("bookmarks.errors.invalid_bookmarkable", type: "User"))
+ Bookmark.register_bookmarkable(
+ model: User,
+ serializer: UserBookmarkSerializer,
+ list_query: lambda do |bookmark_user, guardian|
+ bookmark_user.bookmarks.joins(
+ "INNER JOIN users ON users.id = bookmarks.bookmarkable_id AND bookmarks.bookmarkable_type = 'User'"
+ ).where(bookmarkable_type: "User")
+ end,
+ search_query: lambda do |bookmarks, query, ts_query|
+ bookmarks.where("users.username ILIKE ?", query)
+ end
+ )
+ expect(bm.valid?).to eq(true)
+ end
end
end
diff --git a/spec/models/user_bookmark_list_spec.rb b/spec/models/user_bookmark_list_spec.rb
index 43726477364..98f3ddf3fad 100644
--- a/spec/models/user_bookmark_list_spec.rb
+++ b/spec/models/user_bookmark_list_spec.rb
@@ -5,22 +5,55 @@ RSpec.describe UserBookmarkList do
fab!(:user) { Fabricate(:user) }
let(:list) { UserBookmarkList.new(user: user, guardian: Guardian.new(user), params: params) }
- before do
- 22.times do
- bookmark = Fabricate(:bookmark, user: user)
- Fabricate(:topic_user, topic: bookmark.topic, user: user)
+ context "for non-polymorphic bookmarks" do
+ before do
+ 22.times do
+ bookmark = Fabricate(:bookmark, user: user)
+ Fabricate(:topic_user, topic: bookmark.topic, user: user)
+ end
+ end
+
+ it "defaults to 20 per page" do
+ expect(list.per_page).to eq(20)
+ end
+
+ context "when the per_page param is too high" do
+ let(:params) { { per_page: 1000 } }
+
+ it "does not allow more than X bookmarks to be requested per page" do
+ expect(list.load.count).to eq(20)
+ end
end
end
- it "defaults to 20 per page" do
- expect(list.per_page).to eq(20)
- end
+ context "for polymorphic bookmarks" do
+ before do
+ SiteSetting.use_polymorphic_bookmarks = true
+ Bookmark.register_bookmarkable(
+ model: User,
+ serializer: UserBookmarkSerializer,
+ list_query: lambda do |user, guardian|
+ user.bookmarks.joins(
+ "INNER JOIN users ON users.id = bookmarks.bookmarkable_id AND bookmarks.bookmarkable_type = 'User'"
+ ).where(bookmarkable_type: "User")
+ end,
+ search_query: lambda do |bookmarks, query, ts_query|
+ bookmarks.where("users.username ILIKE ?", query)
+ end
+ )
- context "when the per_page param is too high" do
- let(:params) { { per_page: 1000 } }
+ Fabricate(:topic_user, user: user, topic: post_bookmark.bookmarkable.topic)
+ Fabricate(:topic_user, user: user, topic: topic_bookmark.bookmarkable)
+ user_bookmark
+ end
- it "does not allow more than X bookmarks to be requested per page" do
- expect(list.load.count).to eq(20)
+ let(:post_bookmark) { Fabricate(:bookmark, user: user, bookmarkable: Fabricate(:post)) }
+ let(:topic_bookmark) { Fabricate(:bookmark, user: user, bookmarkable: Fabricate(:topic)) }
+ let(:user_bookmark) { Fabricate(:bookmark, user: user, bookmarkable: Fabricate(:user)) }
+
+ it "returns all types of bookmarks" do
+ list.load
+ expect(list.bookmarks.map(&:id)).to match_array([post_bookmark.id, topic_bookmark.id, user_bookmark.id])
end
end
end
diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb
index b63eda110ae..1b633bd3117 100644
--- a/spec/rails_helper.rb
+++ b/spec/rails_helper.rb
@@ -139,6 +139,9 @@ module TestSetup
# Don't queue badge grant in test mode
BadgeGranter.disable_queue
+ # Make sure the default Post and Topic bookmarkables are registered
+ Bookmark.reset_bookmarkables
+
OmniAuth.config.test_mode = false
end
end
diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb
index 163aaf84aa8..6ad8ce3b3b6 100644
--- a/spec/requests/users_controller_spec.rb
+++ b/spec/requests/users_controller_spec.rb
@@ -5208,30 +5208,31 @@ describe UsersController do
end
describe "#bookmarks" do
- fab!(:bookmark1) { Fabricate(:bookmark, user: user1) }
- fab!(:bookmark2) { Fabricate(:bookmark, user: user1) }
- fab!(:bookmark3) { Fabricate(:bookmark) }
+ context "when polymorphic bookmarks are not used" do
+ let(:bookmark1) { Fabricate(:bookmark, user: user1) }
+ let(:bookmark2) { Fabricate(:bookmark, user: user1) }
+ let(:bookmark3) { Fabricate(:bookmark) }
+ before do
+ TopicUser.change(user1.id, bookmark1.topic_id, total_msecs_viewed: 1)
+ TopicUser.change(user1.id, bookmark2.topic_id, total_msecs_viewed: 1)
+ bookmark3
+ end
- before do
- TopicUser.change(user1.id, bookmark1.topic_id, total_msecs_viewed: 1)
- TopicUser.change(user1.id, bookmark2.topic_id, total_msecs_viewed: 1)
- end
+ it "returns a list of serialized bookmarks for the user" do
+ sign_in(user1)
+ get "/u/#{user1.username}/bookmarks.json"
+ expect(response.status).to eq(200)
+ expect(response.parsed_body['user_bookmark_list']['bookmarks'].map { |b| b['id'] }).to match_array([bookmark1.id, bookmark2.id])
+ end
- it "returns a list of serialized bookmarks for the user" do
- sign_in(user1)
- get "/u/#{user1.username}/bookmarks.json"
- expect(response.status).to eq(200)
- expect(response.parsed_body['user_bookmark_list']['bookmarks'].map { |b| b['id'] }).to match_array([bookmark1.id, bookmark2.id])
- end
+ it "returns an .ics file of bookmark reminders for the user in date order" do
+ bookmark1.update!(name: nil, reminder_at: 1.day.from_now)
+ bookmark2.update!(name: "Some bookmark note", reminder_at: 1.week.from_now)
- it "returns an .ics file of bookmark reminders for the user in date order" do
- bookmark1.update!(name: nil, reminder_at: 1.day.from_now)
- bookmark2.update!(name: "Some bookmark note", reminder_at: 1.week.from_now)
-
- sign_in(user1)
- get "/u/#{user1.username}/bookmarks.ics"
- expect(response.status).to eq(200)
- expect(response.body).to eq(<<~ICS)
+ sign_in(user1)
+ get "/u/#{user1.username}/bookmarks.ics"
+ expect(response.status).to eq(200)
+ expect(response.body).to eq(<<~ICS)
BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//Discourse//#{Discourse.current_hostname}//#{Discourse.full_version}//EN
@@ -5254,32 +5255,108 @@ describe UsersController do
URL:#{Discourse.base_url}/t/-/#{bookmark2.topic_id}
END:VEVENT
END:VCALENDAR
- ICS
+ ICS
+ end
+
+ it "does not show another user's bookmarks" do
+ sign_in(user1)
+ get "/u/#{bookmark3.user.username}/bookmarks.json"
+ expect(response.status).to eq(403)
+ end
+
+ it "shows a helpful message if no bookmarks are found" do
+ bookmark1.destroy
+ bookmark2.destroy
+ bookmark3.destroy
+ sign_in(user1)
+ get "/u/#{user1.username}/bookmarks.json"
+ expect(response.status).to eq(200)
+ expect(response.parsed_body['bookmarks']).to eq([])
+ end
+
+ it "shows a helpful message if no bookmarks are found for the search" do
+ sign_in(user1)
+ get "/u/#{user1.username}/bookmarks.json", params: {
+ q: 'badsearch'
+ }
+ expect(response.status).to eq(200)
+ expect(response.parsed_body['bookmarks']).to eq([])
+ end
end
- it "does not show another user's bookmarks" do
- sign_in(user1)
- get "/u/#{bookmark3.user.username}/bookmarks.json"
- expect(response.status).to eq(403)
- end
+ context "for polymorphic bookmarks" do
+ class UserTestBookmarkSerializer < UserBookmarkBaseSerializer
+ def title
+ fancy_title
+ end
- it "shows a helpful message if no bookmarks are found" do
- bookmark1.destroy
- bookmark2.destroy
- bookmark3.destroy
- sign_in(user1)
- get "/u/#{user1.username}/bookmarks.json"
- expect(response.status).to eq(200)
- expect(response.parsed_body['bookmarks']).to eq([])
- end
+ def fancy_title
+ @fancy_title ||= user.username
+ end
- it "shows a helpful message if no bookmarks are found for the search" do
- sign_in(user1)
- get "/u/#{user1.username}/bookmarks.json", params: {
- q: 'badsearch'
- }
- expect(response.status).to eq(200)
- expect(response.parsed_body['bookmarks']).to eq([])
+ def cooked
+ "Something cooked
"
+ end
+
+ def bookmarkable_user
+ @bookmarkable_user ||= user
+ end
+
+ def bookmarkable_url
+ "#{Discourse.base_url}/u/#{user.username}"
+ end
+
+ def excerpt
+ return nil unless cooked
+ @excerpt ||= PrettyText.excerpt(cooked, 300, keep_emoji_images: true)
+ end
+
+ private
+
+ def user
+ object.bookmarkable
+ end
+ end
+
+ before do
+ SiteSetting.use_polymorphic_bookmarks = true
+ Bookmark.register_bookmarkable(
+ model: User,
+ serializer: UserTestBookmarkSerializer,
+ list_query: lambda do |user, guardian|
+ user.bookmarks.joins(
+ "INNER JOIN users ON users.id = bookmarks.bookmarkable_id AND bookmarks.bookmarkable_type = 'User'"
+ ).where(bookmarkable_type: "User")
+ end,
+ search_query: lambda do |bookmarks, query, ts_query|
+ bookmarks.where("users.username ILIKE ?", query)
+ end
+ )
+ TopicUser.change(user1.id, bookmark1.bookmarkable.topic_id, total_msecs_viewed: 1)
+ TopicUser.change(user1.id, bookmark2.bookmarkable_id, total_msecs_viewed: 1)
+ Fabricate(:post, topic: bookmark2.bookmarkable)
+ bookmark3 && bookmark4
+ end
+
+ after do
+ Bookmark.registered_bookmarkables = []
+ end
+
+ let(:bookmark1) { Fabricate(:bookmark, user: user1, bookmarkable: Fabricate(:post)) }
+ let(:bookmark2) { Fabricate(:bookmark, user: user1, bookmarkable: Fabricate(:topic)) }
+ let(:bookmark3) { Fabricate(:bookmark, user: user1, bookmarkable: Fabricate(:user)) }
+ let(:bookmark4) { Fabricate(:bookmark) }
+
+ it "returns a list of serialized bookmarks for the user including custom registered bookmarkables" do
+ sign_in(user1)
+ get "/u/#{user1.username}/bookmarks.json"
+ expect(response.status).to eq(200)
+ response_bookmarks = response.parsed_body['user_bookmark_list']['bookmarks']
+ expect(response_bookmarks.map { |b| b['id'] }).to match_array(
+ [bookmark1.id, bookmark2.id, bookmark3.id]
+ )
+ expect(response_bookmarks.find { |b| b['id'] == bookmark3.id }['excerpt']).to eq('Something cooked')
+ end
end
end
diff --git a/spec/serializers/user_bookmark_list_serializer_spec.rb b/spec/serializers/user_bookmark_list_serializer_spec.rb
new file mode 100644
index 00000000000..8d32f2c77bc
--- /dev/null
+++ b/spec/serializers/user_bookmark_list_serializer_spec.rb
@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+RSpec.describe UserBookmarkListSerializer do
+ class UserTestBookmarkSerializer < UserBookmarkBaseSerializer; end
+
+ fab!(:user) { Fabricate(:user) }
+
+ context "for polymorphic bookmarks" do
+ before do
+ SiteSetting.use_polymorphic_bookmarks = true
+ Bookmark.register_bookmarkable(
+ model: User,
+ serializer: UserTestBookmarkSerializer,
+ list_query: lambda do |user, guardian|
+ user.bookmarks.joins(
+ "INNER JOIN users ON users.id = bookmarks.bookmarkable_id AND bookmarks.bookmarkable_type = 'User'"
+ ).where(bookmarkable_type: "User")
+ end,
+ search_query: lambda do |bookmarks, query, ts_query|
+ bookmarks.where("users.username ILIKE ?", query)
+ end
+ )
+
+ Fabricate(:topic_user, user: user, topic: post_bookmark.bookmarkable.topic)
+ Fabricate(:topic_user, user: user, topic: topic_bookmark.bookmarkable)
+ user_bookmark
+ end
+
+ let(:post_bookmark) { Fabricate(:bookmark, user: user, bookmarkable: Fabricate(:post)) }
+ let(:topic_bookmark) { Fabricate(:bookmark, user: user, bookmarkable: Fabricate(:topic)) }
+ let(:user_bookmark) { Fabricate(:bookmark, user: user, bookmarkable: Fabricate(:user)) }
+
+ def run_serializer
+ bookmark_list = UserBookmarkList.new(user: user, guardian: Guardian.new(user), params: {})
+ bookmark_list.load
+ UserBookmarkListSerializer.new(bookmark_list)
+ end
+
+ it "chooses the correct class of serializer for all the bookmarkable types" do
+ serializer = run_serializer
+ expect(serializer.bookmarks.map(&:class).map(&:to_s)).to match_array(["UserTestBookmarkSerializer", "UserTopicBookmarkSerializer", "UserPostBookmarkSerializer"])
+ end
+ end
+end
diff --git a/spec/serializers/user_post_bookmark_serializer_spec.rb b/spec/serializers/user_post_bookmark_serializer_spec.rb
new file mode 100644
index 00000000000..2e678c4e969
--- /dev/null
+++ b/spec/serializers/user_post_bookmark_serializer_spec.rb
@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+RSpec.describe UserPostBookmarkSerializer do
+ before do
+ SiteSetting.use_polymorphic_bookmarks = true
+ end
+
+ let(:user) { Fabricate(:user) }
+ let(:post) { Fabricate(:post, user: user, topic: topic) }
+ let(:topic) { Fabricate(:topic) }
+ let!(:bookmark) { Fabricate(:bookmark, name: 'Test', user: user, bookmarkable: post) }
+
+ it "uses the correct highest_post_number column based on whether the user is staff" do
+ Fabricate(:post, topic: topic)
+ Fabricate(:post, topic: topic)
+ Fabricate(:whisper, topic: topic)
+ topic.reload
+ bookmark.reload
+ serializer = UserPostBookmarkSerializer.new(bookmark, scope: Guardian.new(user))
+
+ expect(serializer.highest_post_number).to eq(3)
+
+ user.update!(admin: true)
+
+ expect(serializer.highest_post_number).to eq(4)
+ end
+end
diff --git a/spec/serializers/user_topic_bookmark_serializer_spec.rb b/spec/serializers/user_topic_bookmark_serializer_spec.rb
new file mode 100644
index 00000000000..34f332e6edb
--- /dev/null
+++ b/spec/serializers/user_topic_bookmark_serializer_spec.rb
@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+RSpec.describe UserTopicBookmarkSerializer do
+ before do
+ SiteSetting.use_polymorphic_bookmarks = true
+ end
+
+ fab!(:user) { Fabricate(:user) }
+ let!(:topic) { Fabricate(:topic, user: user) }
+ let!(:post) { Fabricate(:post, topic: topic) }
+ let!(:bookmark) { Fabricate(:bookmark, name: 'Test', user: user, bookmarkable: topic) }
+
+ it "uses the last_read_post_number + 1 for the bookmarks excerpt" do
+ next_unread_post = Fabricate(:post_with_long_raw_content, topic: bookmark.bookmarkable)
+ Fabricate(:post_with_external_links, topic: bookmark.bookmarkable)
+ bookmark.reload
+ TopicUser.change(user.id, bookmark.bookmarkable.id, { last_read_post_number: post.post_number })
+ serializer = UserTopicBookmarkSerializer.new(bookmark, scope: Guardian.new(user))
+ expect(serializer.excerpt).to eq(PrettyText.excerpt(next_unread_post.cooked, 300, keep_emoji_images: true))
+ end
+
+ it "does not use a small post for the last unread cooked post" do
+ small_action_post = Fabricate(:small_action, topic: bookmark.bookmarkable)
+ next_unread_post = Fabricate(:post_with_long_raw_content, topic: bookmark.bookmarkable)
+ Fabricate(:post_with_external_links, topic: bookmark.bookmarkable)
+ bookmark.reload
+ TopicUser.change(user.id, bookmark.bookmarkable.id, { last_read_post_number: post.post_number })
+ serializer = UserTopicBookmarkSerializer.new(bookmark, scope: Guardian.new(user))
+ expect(serializer.excerpt).to eq(PrettyText.excerpt(next_unread_post.cooked, 300, keep_emoji_images: true))
+ end
+
+ it "handles the last read post in the topic being a small post by getting the last read regular post" do
+ last_regular_post = Fabricate(:post_with_long_raw_content, topic: bookmark.bookmarkable)
+ small_action_post = Fabricate(:small_action, topic: bookmark.bookmarkable)
+ bookmark.reload
+ topic.reload
+ TopicUser.change(user.id, bookmark.bookmarkable.id, { last_read_post_number: small_action_post.post_number })
+ serializer = UserTopicBookmarkSerializer.new(bookmark, scope: Guardian.new(user))
+ expect(serializer.cooked).to eq(last_regular_post.cooked)
+ expect(serializer.excerpt).to eq(PrettyText.excerpt(last_regular_post.cooked, 300, keep_emoji_images: true))
+ end
+end