From a5779a7d0bf26bd3abf972570948ac6a7bc52fc3 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Tue, 24 May 2022 13:52:42 +1000 Subject: [PATCH] DEV: Bookmark cleanup (#16899) Gets rid of old bookmark app event and deletes anything leftover from polymorphic bookmark changeover. --- .../discourse/app/controllers/topic.js | 3 - .../javascripts/discourse/app/models/post.js | 4 - .../javascripts/discourse/app/models/topic.js | 2 - app/models/topic.rb | 14 +- app/serializers/user_bookmark_serializer.rb | 175 ------------------ 5 files changed, 3 insertions(+), 195 deletions(-) delete mode 100644 app/serializers/user_bookmark_serializer.rb diff --git a/app/assets/javascripts/discourse/app/controllers/topic.js b/app/assets/javascripts/discourse/app/controllers/topic.js index f50bc723c02..3832df9372a 100644 --- a/app/assets/javascripts/discourse/app/controllers/topic.js +++ b/app/assets/javascripts/discourse/app/controllers/topic.js @@ -1263,9 +1263,6 @@ export default Controller.extend(bufferedProperty("model"), { savedData, bookmark.attachedTo() ); - - // TODO (martin) (2022-02-01) Remove these old bookmark events, replaced by bookmarks:changed. - this.appEvents.trigger("topic:bookmark-toggled"); }, onAfterDelete: (topicBookmarked, bookmarkId) => { this.model.removeBookmark(bookmarkId); diff --git a/app/assets/javascripts/discourse/app/models/post.js b/app/assets/javascripts/discourse/app/models/post.js index 7a046340899..a0033455561 100644 --- a/app/assets/javascripts/discourse/app/models/post.js +++ b/app/assets/javascripts/discourse/app/models/post.js @@ -321,8 +321,6 @@ const Post = RestModel.extend({ target: "post", targetId: this.id, }); - // TODO (martin) (2022-02-01) Remove these old bookmark events, replaced by bookmarks:changed. - this.appEvents.trigger("page:bookmark-post-toggled", this); this.appEvents.trigger("post-stream:refresh", { id: this.id }); }, @@ -344,8 +342,6 @@ const Post = RestModel.extend({ target: "post", targetId: this.id, }); - // TODO (martin) (2022-02-01) Remove these old bookmark events, replaced by bookmarks:changed. - this.appEvents.trigger("page:bookmark-post-toggled", this); }, updateActionsSummary(json) { diff --git a/app/assets/javascripts/discourse/app/models/topic.js b/app/assets/javascripts/discourse/app/models/topic.js index 9b4e5c8efcc..ccb9f1a6758 100644 --- a/app/assets/javascripts/discourse/app/models/topic.js +++ b/app/assets/javascripts/discourse/app/models/topic.js @@ -398,8 +398,6 @@ const Topic = RestModel.extend({ "bookmarks", this.bookmarks.filter((bookmark) => { if (bookmark.id === id && bookmark.bookmarkable_type === "Topic") { - // TODO (martin) (2022-02-01) Remove these old bookmark events, replaced by bookmarks:changed. - this.appEvents.trigger("topic:bookmark-toggled"); this.appEvents.trigger( "bookmarks:changed", null, diff --git a/app/models/topic.rb b/app/models/topic.rb index afd37fb7502..c3ac40ae016 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -208,17 +208,9 @@ class Topic < ActiveRecord::Base has_many :category_users, through: :category has_many :posts - # TODO (martin): - # - # When we are ready we can add as: :bookmarkable here to use the - # polymorphic association. - # - # At that time we may also want to make another association for example - # :topic_bookmarks that get all of the bookmarks for that topic's bookmarkable id - # and type, because this one gets all of the post bookmarks. - # - # Note: We can use Bookmark#for_user_in_topic for this. - has_many :bookmarks, through: :posts + # NOTE: To get all Post _and_ Topic bookmarks for a topic by user, + # use the Bookmark.for_user_in_topic scope. + has_many :bookmarks, as: :bookmarkable has_many :ordered_posts, -> { order(post_number: :asc) }, class_name: "Post" has_many :topic_allowed_users diff --git a/app/serializers/user_bookmark_serializer.rb b/app/serializers/user_bookmark_serializer.rb deleted file mode 100644 index 0992830d9b2..00000000000 --- a/app/serializers/user_bookmark_serializer.rb +++ /dev/null @@ -1,175 +0,0 @@ -# frozen_string_literal: true - -require_relative 'post_item_excerpt' - -# TODO (martin) [POLYBOOK] Not relevant once polymorphic bookmarks are implemented. -# -# This must be deleted after the plugins relying on it are updated once -# polymorphic bookmarks become the norm. -class UserBookmarkSerializer < ApplicationSerializer - include PostItemExcerpt - include TopicTagsMixin - - attributes :id, - :created_at, - :updated_at, - :topic_id, - :linked_post_number, - :post_id, - :name, - :reminder_at, - :pinned, - :title, - :fancy_title, - :deleted, - :hidden, - :category_id, - :closed, - :archived, - :archetype, - :highest_post_number, - :last_read_post_number, - :bumped_at, - :slug, - :post_user_username, - :post_user_avatar_template, - :post_user_name, - :for_topic, - :bookmarkable_id, - :bookmarkable_type, - :bookmarkable_user_username, - :bookmarkable_user_avatar_template, - :bookmarkable_user_name, - - def topic_id - post.topic_id - end - - def topic - @topic ||= object.topic - end - - def post - @post ||= object.post - end - - def closed - topic.closed - end - - def archived - topic.archived - end - - def linked_post_number - post.post_number - end - - def title - topic.title - end - - def fancy_title - topic.fancy_title - end - - def deleted - topic.deleted_at.present? || post.deleted_at.present? - end - - def hidden - post.hidden - 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 topic_user - @topic_user ||= topic.topic_users.find { |tu| tu.user_id == scope.user.id } - end - - def bumped_at - topic.bumped_at - end - - def raw - post.raw - end - - def cooked - @cooked ||= \ - if object.for_topic && last_read_post_number.present? - for_topic_cooked_post - else - post.cooked - end - end - - def for_topic_cooked_post - post_number = [last_read_post_number + 1, highest_post_number].min - posts = Post.where(topic: topic, post_type: Post.types[:regular]).order(:post_number) - first_unread_cooked = posts.where("post_number >= ?", post_number).pluck_first(:cooked) - - # 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_cooked || posts.last.cooked - end - - def slug - topic.slug - end - - def post_user - @post_user ||= post.user - end - - def post_user_username - post_user.username - end - - def post_user_avatar_template - post_user.avatar_template - end - - 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