diff --git a/app/assets/javascripts/discourse/app/controllers/topic.js b/app/assets/javascripts/discourse/app/controllers/topic.js index 365d154b1b..0caf07b75f 100644 --- a/app/assets/javascripts/discourse/app/controllers/topic.js +++ b/app/assets/javascripts/discourse/app/controllers/topic.js @@ -4,10 +4,7 @@ import { alias, and, not, or } from "@ember/object/computed"; import discourseComputed, { observes } from "discourse-common/utils/decorators"; import { isEmpty, isPresent } from "@ember/utils"; import { later, next, schedule } from "@ember/runloop"; -import { - AUTO_DELETE_PREFERENCES, - FOR_TOPIC_POST_ID, -} from "discourse/models/bookmark"; +import { AUTO_DELETE_PREFERENCES } from "discourse/models/bookmark"; import Composer from "discourse/models/composer"; import EmberObject, { action } from "@ember/object"; import I18n from "I18n"; @@ -1196,7 +1193,7 @@ export default Controller.extend(bufferedProperty("model"), { _toggleBookmark(target) { return new Promise((resolve) => { const bookmarkingTopic = target instanceof Topic; - const postId = bookmarkingTopic ? FOR_TOPIC_POST_ID : target.id; + const postId = bookmarkingTopic ? null : target.id; const topicId = bookmarkingTopic ? target.id : target.topic_id; let modalController = showModal("bookmark", { @@ -1233,9 +1230,9 @@ export default Controller.extend(bufferedProperty("model"), { }, afterDelete: (topicBookmarked) => { this.model.set( - "bookmarked_posts", - this.model.bookmarked_posts.filter( - (bookmarkedPost) => bookmarkedPost.post_id !== postId + "bookmarked_items", + this.model.bookmarked_items.filter( + (bookmarkedItem) => bookmarkedItem.post_id !== postId ) ); target.deleteBookmark(topicBookmarked); @@ -1245,17 +1242,17 @@ export default Controller.extend(bufferedProperty("model"), { }, _addOrUpdateBookmarkedPost(postId, reminderAt) { - if (!this.model.bookmarked_posts) { - this.model.set("bookmarked_posts", []); + if (!this.model.bookmarked_items) { + this.model.set("bookmarked_items", []); } - let bookmarkedPost = this.model.bookmarked_posts.findBy("post_id", postId); - if (!bookmarkedPost) { - bookmarkedPost = { post_id: postId }; - this.model.bookmarked_posts.pushObject(bookmarkedPost); + let bookmarkedItem = this.model.bookmarked_items.findBy("post_id", postId); + if (!bookmarkedItem) { + bookmarkedItem = { post_id: postId }; + this.model.bookmarked_items.pushObject(bookmarkedItem); } - bookmarkedPost.reminder_at = reminderAt; + bookmarkedItem.reminder_at = reminderAt; }, _toggleTopicBookmark() { @@ -1263,8 +1260,8 @@ export default Controller.extend(bufferedProperty("model"), { return Promise.resolve(); } this.model.set("bookmarking", true); - const bookmarkedPostsCount = this.model.bookmarked_posts - ? this.model.bookmarked_posts.length + const bookmarkedCount = this.model.bookmarked_items + ? this.model.bookmarked_items.length : 0; const bookmarkTarget = async (target) => { @@ -1283,15 +1280,15 @@ export default Controller.extend(bufferedProperty("model"), { const toggleBookmarkOnServer = async () => { // bookmark the topic if there are no bookmarked posts - if (bookmarkedPostsCount === 0) { + if (bookmarkedCount === 0) { return bookmarkTarget(this.model); - // if there is only one bookmarked post for the topic we want to - // get it in the stream and edit that one - } else if (bookmarkedPostsCount === 1) { - const postId = this.model.bookmarked_posts[0].post_id; + // if there is only one bookmarked item for the topic we want to + // get it in the stream and edit that one (could be a post or the topic-level bookmark) + } else if (bookmarkedCount === 1) { + const postId = this.model.bookmarked_items[0].post_id; - if (postId === FOR_TOPIC_POST_ID) { + if (!postId) { return bookmarkTarget(this.model); } else { const post = await this.model.postById(postId); @@ -1299,7 +1296,7 @@ export default Controller.extend(bufferedProperty("model"), { } } else { // otherwise we want to clear all the bookmarks out if there is more - // than one bookmarked post in the topic + // than one bookmarked item in the topic return this.model .deleteBookmarks() .then(() => this.model.clearBookmarks()) @@ -1309,7 +1306,7 @@ export default Controller.extend(bufferedProperty("model"), { }; return new Promise((resolve) => { - if (bookmarkedPostsCount > 1) { + if (bookmarkedCount > 1) { bootbox.confirm( I18n.t("bookmarks.confirm_clear"), I18n.t("no_value"), diff --git a/app/assets/javascripts/discourse/app/initializers/topic-footer-buttons.js b/app/assets/javascripts/discourse/app/initializers/topic-footer-buttons.js index 46d72f2a36..d8fa9f98c0 100644 --- a/app/assets/javascripts/discourse/app/initializers/topic-footer-buttons.js +++ b/app/assets/javascripts/discourse/app/initializers/topic-footer-buttons.js @@ -67,10 +67,9 @@ export default { dependentKeys: ["topic.bookmarked", "topic.bookmarksWereChanged"], id: "bookmark", icon() { - const bookmarkedPosts = this.topic.bookmarked_posts; + const bookmarkedItems = this.topic.bookmarked_items || []; if ( - bookmarkedPosts && - bookmarkedPosts.find((bookmarkedPost) => bookmarkedPost.reminder_at) + bookmarkedItems.find((bookmarkedItem) => bookmarkedItem.reminder_at) ) { return "discourse-bookmark-clock"; } @@ -84,14 +83,14 @@ export default { }, label() { if (!this.topic.isPrivateMessage || this.site.mobileView) { - const bookmarkedPosts = this.topic.bookmarked_posts; - const bookmarkedPostsCount = bookmarkedPosts - ? bookmarkedPosts.length + const bookmarkedItems = this.topic.bookmarked_items; + const bookmarkedItemsCount = bookmarkedItems + ? bookmarkedItems.length : 0; - if (bookmarkedPostsCount === 0) { + if (bookmarkedItemsCount === 0) { return "bookmarked.title"; - } else if (bookmarkedPostsCount === 1) { + } else if (bookmarkedItemsCount === 1) { return "bookmarked.edit_bookmark"; } else { return "bookmarked.clear_bookmarks"; @@ -99,13 +98,13 @@ export default { } }, translatedTitle() { - const bookmarkedPosts = this.topic.bookmarked_posts; - if (!bookmarkedPosts || bookmarkedPosts.length === 0) { + const bookmarkedItems = this.topic.bookmarked_items; + if (!bookmarkedItems || bookmarkedItems.length === 0) { return I18n.t("bookmarked.help.bookmark"); - } else if (bookmarkedPosts.length === 1) { + } else if (bookmarkedItems.length === 1) { return I18n.t("bookmarked.help.edit_bookmark"); } else if ( - bookmarkedPosts.find((bookmarkedPost) => bookmarkedPost.reminder_at) + bookmarkedItems.find((bookmarkedItem) => bookmarkedItem.reminder_at) ) { return I18n.t("bookmarked.help.unbookmark_with_reminder"); } else { diff --git a/app/assets/javascripts/discourse/app/models/bookmark.js b/app/assets/javascripts/discourse/app/models/bookmark.js index 7a0f601152..52b032e264 100644 --- a/app/assets/javascripts/discourse/app/models/bookmark.js +++ b/app/assets/javascripts/discourse/app/models/bookmark.js @@ -18,8 +18,6 @@ export const AUTO_DELETE_PREFERENCES = { ON_OWNER_REPLY: 2, }; -export const FOR_TOPIC_POST_ID = -1; - const Bookmark = RestModel.extend({ newBookmark: none("id"), diff --git a/app/assets/javascripts/discourse/app/models/topic.js b/app/assets/javascripts/discourse/app/models/topic.js index 5ffe3cecef..f2bb520430 100644 --- a/app/assets/javascripts/discourse/app/models/topic.js +++ b/app/assets/javascripts/discourse/app/models/topic.js @@ -1,5 +1,4 @@ import { alias, and, equal, notEmpty, or } from "@ember/object/computed"; -import { FOR_TOPIC_POST_ID } from "discourse/models/bookmark"; import { fmt, propertyEqual } from "discourse/lib/computed"; import ActionSummary from "discourse/models/action-summary"; import Category from "discourse/models/category"; @@ -412,18 +411,16 @@ const Topic = RestModel.extend({ clearBookmarks() { this.toggleProperty("bookmarked"); - const postIds = this.bookmarked_posts.mapBy("post_id"); + const postIds = this.bookmarked_items.mapBy("post_id"); postIds.forEach((postId) => { - if (postId === FOR_TOPIC_POST_ID) { - return this.clearBookmark(); - } - const loadedPost = this.postStream.findLoadedPost(postId); if (loadedPost) { loadedPost.clearBookmark(); } }); - this.set("bookmarked_posts", []); + + this.clearBookmark(); + this.set("bookmarked_items", []); return postIds; }, diff --git a/app/assets/javascripts/discourse/tests/acceptance/topic-timeline-test.js b/app/assets/javascripts/discourse/tests/acceptance/topic-timeline-test.js index 9f43748553..f137ed2fc3 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/topic-timeline-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/topic-timeline-test.js @@ -266,7 +266,7 @@ acceptance("Topic Timeline", function (needs) { ], chunk_size: 20, bookmarked: false, - bookmarked_posts: null, + bookmarked_items: null, topic_timer: null, message_bus_last_id: 5, participant_count: 1, diff --git a/app/controllers/bookmarks_controller.rb b/app/controllers/bookmarks_controller.rb index 48d279eade..dfb3d124f9 100644 --- a/app/controllers/bookmarks_controller.rb +++ b/app/controllers/bookmarks_controller.rb @@ -4,9 +4,13 @@ class BookmarksController < ApplicationController requires_login def create - params.require(:post_id) - if params[:post_id].to_i == Bookmark::FOR_TOPIC_POST_ID - params.require(:topic_id) + if params[:post_id].blank? + if params[:topic_id].blank? + raise Discourse::InvalidParameters.new("Either a post_id or a topic_id is required.") + end + topic_id = params[:topic_id].to_i + else + post_id = params[:post_id]&.to_i end RateLimiter.new( @@ -15,8 +19,8 @@ class BookmarksController < ApplicationController bookmark_manager = BookmarkManager.new(current_user) bookmark = bookmark_manager.create( - topic_id: params[:topic_id].to_i, - post_id: params[:post_id].to_i, + topic_id: topic_id, + post_id: post_id, name: params[:name], reminder_type: params[:reminder_type], reminder_at: params[:reminder_at], diff --git a/app/models/bookmark.rb b/app/models/bookmark.rb index 0c15df2585..ba2546c20d 100644 --- a/app/models/bookmark.rb +++ b/app/models/bookmark.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true class Bookmark < ActiveRecord::Base - FOR_TOPIC_POST_ID = -1 - belongs_to :user belongs_to :post belongs_to :topic @@ -80,7 +78,7 @@ class Bookmark < ActiveRecord::Base end def for_topic? - self.post_id == FOR_TOPIC_POST_ID + self.post_id.blank? end def for_post? diff --git a/app/serializers/topic_view_serializer.rb b/app/serializers/topic_view_serializer.rb index 8afc5f7441..e8dcb82445 100644 --- a/app/serializers/topic_view_serializer.rb +++ b/app/serializers/topic_view_serializer.rb @@ -67,7 +67,7 @@ class TopicViewSerializer < ApplicationSerializer :bookmark_reminder_type, :bookmark_name, :bookmark_auto_delete_preference, - :bookmarked_posts, + :bookmarked_items, :message_archived, :topic_timer, :unicode_title, @@ -240,8 +240,8 @@ class TopicViewSerializer < ApplicationSerializer object.bookmark&.auto_delete_preference end - def bookmarked_posts - object.bookmarked_posts + def bookmarked_items + object.bookmarked_items end def topic_timer diff --git a/app/serializers/user_bookmark_serializer.rb b/app/serializers/user_bookmark_serializer.rb index 39a494c579..f8ae018960 100644 --- a/app/serializers/user_bookmark_serializer.rb +++ b/app/serializers/user_bookmark_serializer.rb @@ -35,7 +35,15 @@ class UserBookmarkSerializer < ApplicationSerializer end def post - @post ||= object.post || Post.unscoped.find(object.post_id) + if for_topic? + @post ||= topic.first_post + else + @post ||= object.post || Post.unscoped.find(object.post_id) + end + end + + def for_topic? + object.for_topic? end def closed diff --git a/app/serializers/web_hook_topic_view_serializer.rb b/app/serializers/web_hook_topic_view_serializer.rb index bff763f9b8..f1bc303e2c 100644 --- a/app/serializers/web_hook_topic_view_serializer.rb +++ b/app/serializers/web_hook_topic_view_serializer.rb @@ -22,7 +22,7 @@ class WebHookTopicViewSerializer < TopicViewSerializer image_url slow_mode_seconds slow_mode_enabled_until - bookmarked_posts + bookmarked_items }.each do |attr| define_method("include_#{attr}?") do false diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index b3e537218f..c9088677a0 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -298,7 +298,7 @@ en: edit_bookmark: "Edit Bookmark" clear_bookmarks: "Clear Bookmarks" help: - bookmark: "Click to bookmark the first post on this topic" + bookmark: "Click to bookmark this topic" edit_bookmark: "Click to edit the bookmark on this topic" unbookmark: "Click to remove all bookmarks in this topic" unbookmark_with_reminder: "Click to remove all bookmarks and reminders in this topic." diff --git a/db/migrate/20210907022247_make_post_id_optional_bookmarks.rb b/db/migrate/20210907022247_make_post_id_optional_bookmarks.rb index 03dc0e222f..98df3ba8e2 100644 --- a/db/migrate/20210907022247_make_post_id_optional_bookmarks.rb +++ b/db/migrate/20210907022247_make_post_id_optional_bookmarks.rb @@ -3,12 +3,15 @@ class MakePostIdOptionalBookmarks < ActiveRecord::Migration[6.1] def up remove_index :bookmarks, [:user_id, :post_id], unique: true + change_column_null :bookmarks, :post_id, true add_index :bookmarks, [:user_id, :post_id, :topic_id], unique: true end def down Bookmark.where(post_id: -1).delete_all + Bookmark.where(post_id: nil).delete_all remove_index :bookmarks, [:user_id, :post_id, :topic_id], unique: true + change_column_null :bookmarks, :post_id, false add_index :bookmarks, [:user_id, :post_id], unique: true end end diff --git a/lib/bookmark_manager.rb b/lib/bookmark_manager.rb index eedcb5f017..7e167beaef 100644 --- a/lib/bookmark_manager.rb +++ b/lib/bookmark_manager.rb @@ -8,7 +8,7 @@ class BookmarkManager end def create(post_id:, topic_id: nil, name: nil, reminder_type: nil, reminder_at: nil, options: {}) - if post_id == Bookmark::FOR_TOPIC_POST_ID + if post_id.blank? && topic_id.present? bookmark_for_topic = true topic = Topic.find_by(id: topic_id) else diff --git a/lib/bookmark_query.rb b/lib/bookmark_query.rb index 02dee3b8d2..d7d06496c1 100644 --- a/lib/bookmark_query.rb +++ b/lib/bookmark_query.rb @@ -35,13 +35,22 @@ class BookmarkQuery pms = Topic.private_messages_for_user(@user) results = results.merge(topics.or(pms)) - results = results.merge(Post.secured(@guardian)) + results = results.where( + "posts.post_type IN (?) OR bookmarks.post_id IS NULL", Topic.visible_post_types(@guardian&.user) + ) 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") + .joins(<<~SQL) + LEFT JOIN post_search_data ON post_search_data.post_id = CASE + WHEN bookmarks.post_id IS NULL THEN ( + SELECT posts.id FROM posts WHERE post_number = 1 AND topic_id = bookmarks.topic_id + ) + WHEN bookmarks.post_id IS NOT NULL THEN bookmarks.post_id + END + SQL .where( "bookmarks.name ILIKE :q OR #{bookmark_ts_query} @@ post_search_data.search_data", q: "%#{term}%" diff --git a/lib/bookmark_reminder_notification_handler.rb b/lib/bookmark_reminder_notification_handler.rb index 55c8247811..76ee532aa0 100644 --- a/lib/bookmark_reminder_notification_handler.rb +++ b/lib/bookmark_reminder_notification_handler.rb @@ -4,7 +4,7 @@ class BookmarkReminderNotificationHandler def self.send_notification(bookmark) return if bookmark.blank? Bookmark.transaction do - if bookmark.post.blank? || bookmark.post.deleted_at.present? + if bookmark.post&.deleted_at.present? clear_reminder(bookmark) elsif bookmark.topic create_notification(bookmark) @@ -31,7 +31,7 @@ class BookmarkReminderNotificationHandler user.notifications.create!( notification_type: Notification.types[:bookmark_reminder], topic_id: bookmark.topic_id, - post_number: bookmark.post.post_number, + post_number: bookmark.for_topic? ? 1 : bookmark.post.post_number, data: { topic_title: bookmark.topic.title, display_username: user.username, diff --git a/lib/topic_view.rb b/lib/topic_view.rb index 2c43c4ebcf..87a9828d06 100644 --- a/lib/topic_view.rb +++ b/lib/topic_view.rb @@ -395,19 +395,20 @@ class TopicView @topic.bookmarks.exists?(user_id: @user.id) end - def bookmarked_posts + def bookmarked_items return nil unless has_bookmarks? @topic.bookmarks.where(user: @user).pluck(:post_id, :reminder_at).map do |post_id, reminder_at| { post_id: post_id, - reminder_at: reminder_at + reminder_at: reminder_at, + topic_level: post_id.blank? } end end def bookmark return if !has_bookmarks? - @bookmark ||= @topic.bookmarks.where(user_id: @user.id, post_id: Bookmark::FOR_TOPIC_POST_ID).first + @bookmark ||= @topic.bookmarks.where(user_id: @user.id, post_id: nil).first end MAX_PARTICIPANTS = 24 diff --git a/spec/components/topic_view_spec.rb b/spec/components/topic_view_spec.rb index ac4e91d746..5cb367d870 100644 --- a/spec/components/topic_view_spec.rb +++ b/spec/components/topic_view_spec.rb @@ -408,19 +408,31 @@ describe TopicView do end end - context "#bookmarked_posts" do + context "#bookmarked_items" do let!(:user) { Fabricate(:user) } let!(:bookmark1) { Fabricate(:bookmark_next_business_day_reminder, post: topic.first_post, user: user) } let!(:bookmark2) { Fabricate(:bookmark_next_business_day_reminder, post: topic.posts[1], user: user) } + let!(:bookmark3) { Fabricate(:bookmark_next_business_day_reminder, post: nil, user: user, topic: topic) } it "gets the first post bookmark reminder at for the user" do topic_view = TopicView.new(topic.id, user) - first, second = topic_view.bookmarked_posts + first, second = topic_view.bookmarked_items expect(first[:post_id]).to eq(bookmark1.post_id) expect(first[:reminder_at]).to eq_time(bookmark1.reminder_at) + expect(first[:topic_level]).to eq(false) expect(second[:post_id]).to eq(bookmark2.post_id) expect(second[:reminder_at]).to eq_time(bookmark2.reminder_at) + expect(second[:topic_level]).to eq(false) + end + + it "gets the topic level bookmark reminder at for the user" do + topic_view = TopicView.new(topic.id, user) + + topic_level_item = topic_view.bookmarked_items.last + expect(topic_level_item[:post_id]).to eq(nil) + expect(topic_level_item[:reminder_at]).to eq(bookmark3.reminder_at) + expect(topic_level_item[:topic_level]).to eq(true) end context "when the topic is deleted" do @@ -429,7 +441,7 @@ describe TopicView do PostDestroyer.new(Fabricate(:admin), topic.first_post).destroy topic.reload - first, second = topic_view.bookmarked_posts + first, second = topic_view.bookmarked_items expect(first[:post_id]).to eq(bookmark1.post_id) expect(first[:reminder_at]).to eq_time(bookmark1.reminder_at) expect(second[:post_id]).to eq(bookmark2.post_id) diff --git a/spec/fabricators/bookmark_fabricator.rb b/spec/fabricators/bookmark_fabricator.rb index ede44dfeca..087c1165c4 100644 --- a/spec/fabricators/bookmark_fabricator.rb +++ b/spec/fabricators/bookmark_fabricator.rb @@ -3,7 +3,7 @@ Fabricator(:bookmark) do user post { Fabricate(:post) } - topic { |attrs| attrs[:post].topic } + topic { |attrs| attrs[:post].present? ? attrs[:post].topic : Fabricate(:topic) } name "This looked interesting" reminder_type { Bookmark.reminder_types[:tomorrow] } reminder_at { 1.day.from_now.iso8601 } diff --git a/spec/lib/bookmark_manager_spec.rb b/spec/lib/bookmark_manager_spec.rb index 3cfe8af080..c1cea4e89f 100644 --- a/spec/lib/bookmark_manager_spec.rb +++ b/spec/lib/bookmark_manager_spec.rb @@ -42,22 +42,22 @@ RSpec.describe BookmarkManager do end context "when creating a topic-level bookmark" do - it "allows passing Bookmark::FOR_TOPIC_POST_ID as a post ID" do - subject.create(post_id: Bookmark::FOR_TOPIC_POST_ID, topic_id: post.topic.id, name: name, reminder_type: reminder_type, reminder_at: reminder_at) + it "allows passing nil as a post ID" do + subject.create(post_id: nil, topic_id: post.topic.id, name: name, reminder_type: reminder_type, reminder_at: reminder_at) expect( - Bookmark.exists?(user: user, topic: post.topic, post_id: Bookmark::FOR_TOPIC_POST_ID) + Bookmark.exists?(user: user, topic: post.topic, post_id: nil) ).to eq(true) end it "when topic is deleted it raises invalid access from guardian check" do post.topic.trash! - expect { subject.create(post_id: Bookmark::FOR_TOPIC_POST_ID, topic_id: post.topic.id, name: name) }.to raise_error( + expect { subject.create(post_id: nil, topic_id: post.topic.id, name: name) }.to raise_error( Discourse::InvalidAccess ) end it "updates the topic user bookmarked column to true" do - subject.create(post_id: Bookmark::FOR_TOPIC_POST_ID, topic_id: post.topic.id, name: name, reminder_type: reminder_type, reminder_at: reminder_at) + subject.create(post_id: nil, topic_id: post.topic.id, name: name, reminder_type: reminder_type, reminder_at: reminder_at) tu = TopicUser.find_by(user: user) expect(tu.bookmarked).to eq(true) tu.update(bookmarked: false) diff --git a/spec/lib/bookmark_query_spec.rb b/spec/lib/bookmark_query_spec.rb index 5511164d01..cce73d173b 100644 --- a/spec/lib/bookmark_query_spec.rb +++ b/spec/lib/bookmark_query_spec.rb @@ -62,6 +62,28 @@ RSpec.describe BookmarkQuery do bookmarks = bookmark_query(params: { q: 'bugfix' }).list_all expect(bookmarks.map(&:id)).to eq([@bookmark4.id]) end + + context "for bookmarks that are topic level" do + before do + @bookmark3.update(post_id: nil) + @bookmark4.update(post_id: nil) + 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 end context "for a whispered post" do @@ -173,6 +195,7 @@ RSpec.describe BookmarkQuery do let!(:bookmark3) { Fabricate(:bookmark, user: user, updated_at: 6.days.ago, reminder_type: nil, reminder_at: nil) } let!(:bookmark4) { Fabricate(:bookmark, user: user, updated_at: 4.days.ago, reminder_type: nil, reminder_at: nil) } let!(:bookmark5) { Fabricate(:bookmark, user: user, updated_at: 3.days.ago, reminder_type: nil, reminder_at: nil) } + let!(:bookmark6) { Fabricate(:bookmark, user: user, updated_at: 7.days.ago, post: nil, reminder_type: nil, reminder_at: nil) } it "order defaults to updated_at DESC" do expect(bookmark_query.list_all.map(&:id)).to eq([ @@ -180,7 +203,8 @@ RSpec.describe BookmarkQuery do bookmark2.id, bookmark5.id, bookmark4.id, - bookmark3.id + bookmark3.id, + bookmark6.id ]) end @@ -195,7 +219,8 @@ RSpec.describe BookmarkQuery do bookmark5.id, bookmark1.id, bookmark2.id, - bookmark3.id + bookmark3.id, + bookmark6.id ]) end @@ -220,7 +245,8 @@ RSpec.describe BookmarkQuery do bookmark4.id, bookmark1.id, bookmark2.id, - bookmark5.id + bookmark5.id, + bookmark6.id ]) end end diff --git a/spec/lib/bookmark_reminder_notification_handler_spec.rb b/spec/lib/bookmark_reminder_notification_handler_spec.rb index c8e5ef806f..1c280f974b 100644 --- a/spec/lib/bookmark_reminder_notification_handler_spec.rb +++ b/spec/lib/bookmark_reminder_notification_handler_spec.rb @@ -28,6 +28,19 @@ RSpec.describe BookmarkReminderNotificationHandler do expect(data["bookmark_name"]).to eq(bookmark.name) end + it "creates a bookmark reminder notification for a topic-level bookmark" do + bookmark.update(post_id: nil) + subject.send_notification(bookmark) + notif = bookmark.user.notifications.last + expect(notif.notification_type).to eq(Notification.types[:bookmark_reminder]) + expect(notif.topic_id).to eq(bookmark.topic_id) + expect(notif.post_number).to eq(1) + data = JSON.parse(notif.data) + expect(data["topic_title"]).to eq(bookmark.topic.title) + expect(data["display_username"]).to eq(bookmark.user.username) + expect(data["bookmark_name"]).to eq(bookmark.name) + end + it "clears the reminder" do subject.send_notification(bookmark) bookmark.reload diff --git a/spec/requests/bookmarks_controller_spec.rb b/spec/requests/bookmarks_controller_spec.rb index 70906757fb..def8857eeb 100644 --- a/spec/requests/bookmarks_controller_spec.rb +++ b/spec/requests/bookmarks_controller_spec.rb @@ -25,7 +25,7 @@ describe BookmarksController do it "creates a bookmark for the topic" do post "/bookmarks.json", params: { - post_id: Bookmark::FOR_TOPIC_POST_ID, + post_id: nil, topic_id: bookmark_post.topic.id, reminder_type: "tomorrow", reminder_at: (Time.zone.now + 1.day).iso8601 @@ -34,7 +34,7 @@ describe BookmarksController do expect(response.status).to eq(200) expect( Bookmark.exists?( - user: bookmark_user, post_id: Bookmark::FOR_TOPIC_POST_ID, topic_id: bookmark_post.topic.id + user: bookmark_user, post_id: nil, topic_id: bookmark_post.topic.id ) ).to eq(true) end