diff --git a/app/assets/javascripts/discourse/app/controllers/topic.js b/app/assets/javascripts/discourse/app/controllers/topic.js index f8427f5347b..c27f4bab822 100644 --- a/app/assets/javascripts/discourse/app/controllers/topic.js +++ b/app/assets/javascripts/discourse/app/controllers/topic.js @@ -1204,75 +1204,89 @@ export default Controller.extend(bufferedProperty("model"), { post.appEvents.trigger("post-stream:refresh", { id: post.id }); }, afterSave: (savedData) => { + this._addOrUpdateBookmarkedPost(post.id, savedData.reminderAt); post.createBookmark(savedData); resolve({ closedWithoutSaving: false }); }, afterDelete: (topicBookmarked) => { + this.model.set( + "bookmarked_posts", + this.model.bookmarked_posts.filter((x) => x.post_id !== post.id) + ); post.deleteBookmark(topicBookmarked); }, }); }); }, + _addOrUpdateBookmarkedPost(postId, reminderAt) { + if (!this.model.bookmarked_posts) { + this.model.set("bookmarked_posts", []); + } + + let bookmarkedPost = this.model.bookmarked_posts.findBy("post_id", postId); + if (!bookmarkedPost) { + bookmarkedPost = { post_id: postId }; + this.model.bookmarked_posts.pushObject(bookmarkedPost); + } + + bookmarkedPost.reminder_at = reminderAt; + }, + _toggleTopicBookmark() { if (this.model.bookmarking) { return Promise.resolve(); } this.model.set("bookmarking", true); - const alreadyBookmarkedPosts = this.model.bookmarkedPosts; + const bookmarkedPostsCount = this.model.bookmarked_posts + ? this.model.bookmarked_posts.length + : 0; - return this.model.firstPost().then((firstPost) => { - const bookmarkPost = async (post) => { - const opts = await this._togglePostBookmark(post); - this.model.set("bookmarking", false); - if (opts.closedWithoutSaving) { - return; - } - this.model.afterPostBookmarked(post); - return [post.id]; - }; + const bookmarkPost = async (post) => { + const opts = await this._togglePostBookmark(post); + this.model.set("bookmarking", false); + if (opts.closedWithoutSaving) { + return; + } + this.model.afterPostBookmarked(post); + return [post.id]; + }; - const toggleBookmarkOnServer = () => { - if (alreadyBookmarkedPosts.length === 0) { - return bookmarkPost(firstPost); - } else if (alreadyBookmarkedPosts.length === 1) { - const post = alreadyBookmarkedPosts[0]; - return bookmarkPost(post); - } else { - return this.model - .deleteBookmark() - .then(() => { - this.model.toggleProperty("bookmarked"); - this.model.set("bookmark_reminder_at", null); - alreadyBookmarkedPosts.forEach((post) => { - post.clearBookmark(); - }); - return alreadyBookmarkedPosts.mapBy("id"); - }) - .catch(popupAjaxError) - .finally(() => this.model.set("bookmarking", false)); - } - }; + const toggleBookmarkOnServer = async () => { + if (bookmarkedPostsCount === 0) { + const firstPost = await this.model.firstPost(); + return bookmarkPost(firstPost); + } else if (bookmarkedPostsCount === 1) { + const postId = this.model.bookmarked_posts[0].post_id; + const post = await this.model.postById(postId); + return bookmarkPost(post); + } else { + return this.model + .deleteBookmarks() + .then(() => this.model.clearBookmarks()) + .catch(popupAjaxError) + .finally(() => this.model.set("bookmarking", false)); + } + }; - return new Promise((resolve) => { - if (alreadyBookmarkedPosts.length > 1) { - bootbox.confirm( - I18n.t("bookmarks.confirm_clear"), - I18n.t("no_value"), - I18n.t("yes_value"), - (confirmed) => { - if (confirmed) { - toggleBookmarkOnServer().then(resolve); - } else { - this.model.set("bookmarking", false); - resolve(); - } + return new Promise((resolve) => { + if (bookmarkedPostsCount > 1) { + bootbox.confirm( + I18n.t("bookmarks.confirm_clear"), + I18n.t("no_value"), + I18n.t("yes_value"), + (confirmed) => { + if (confirmed) { + toggleBookmarkOnServer().then(resolve); + } else { + this.model.set("bookmarking", false); + resolve(); } - ); - } else { - toggleBookmarkOnServer().then(resolve); - } - }); + } + ); + } else { + toggleBookmarkOnServer().then(resolve); + } }); }, 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 d2c365b18ad..344b4b0c811 100644 --- a/app/assets/javascripts/discourse/app/initializers/topic-footer-buttons.js +++ b/app/assets/javascripts/discourse/app/initializers/topic-footer-buttons.js @@ -1,5 +1,4 @@ import I18n from "I18n"; -import { formattedReminderTime } from "discourse/lib/bookmark"; import { registerTopicFooterButton } from "discourse/lib/register-topic-footer-button"; import showModal from "discourse/lib/show-modal"; @@ -12,8 +11,7 @@ const DEFER_PRIORITY = 500; export default { name: "topic-footer-buttons", - initialize(container) { - const currentUser = container.lookup("current-user:main"); + initialize() { registerTopicFooterButton({ id: "share-and-invite", icon: "link", @@ -66,22 +64,27 @@ export default { }); registerTopicFooterButton({ - dependentKeys: ["topic.bookmarked", "topic.bookmarkedPosts"], + dependentKeys: ["topic.bookmarked", "topic.bookmarksWereChanged"], id: "bookmark", icon() { - if (this.get("topic.bookmark_reminder_at")) { + const bookmarkedPosts = this.topic.bookmarked_posts; + if (bookmarkedPosts && bookmarkedPosts.find((x) => x.reminder_at)) { return "discourse-bookmark-clock"; } return "bookmark"; }, priority: BOOKMARK_PRIORITY, classNames() { - const bookmarked = this.get("topic.bookmarked"); - return bookmarked ? ["bookmark", "bookmarked"] : ["bookmark"]; + return this.topic.bookmarked + ? ["bookmark", "bookmarked"] + : ["bookmark"]; }, label() { - if (!this.get("topic.isPrivateMessage") || this.site.mobileView) { - const bookmarkedPostsCount = this.get("topic.bookmarkedPosts").length; + if (!this.topic.isPrivateMessage || this.site.mobileView) { + const bookmarkedPosts = this.topic.bookmarked_posts; + const bookmarkedPostsCount = bookmarkedPosts + ? bookmarkedPosts.length + : 0; if (bookmarkedPostsCount === 0) { return "bookmarked.title"; @@ -93,20 +96,16 @@ export default { } }, translatedTitle() { - const bookmarked = this.get("topic.bookmarked"); - const bookmark_reminder_at = this.get("topic.bookmark_reminder_at"); - if (bookmarked) { - if (bookmark_reminder_at) { - return I18n.t("bookmarked.help.unbookmark_with_reminder", { - reminder_at: formattedReminderTime( - bookmark_reminder_at, - currentUser.resolvedTimezone(currentUser) - ), - }); - } + const bookmarkedPosts = this.topic.bookmarked_posts; + if (!bookmarkedPosts || bookmarkedPosts.length === 0) { + return I18n.t("bookmarked.help.bookmark"); + } else if (bookmarkedPosts.length === 1) { + return I18n.t("bookmarked.help.edit_bookmark"); + } else if (bookmarkedPosts.find((x) => x.reminder_at)) { + return I18n.t("bookmarked.help.unbookmark_with_reminder"); + } else { return I18n.t("bookmarked.help.unbookmark"); } - return I18n.t("bookmarked.help.bookmark"); }, action: "toggleBookmark", dropdown() { diff --git a/app/assets/javascripts/discourse/app/models/post.js b/app/assets/javascripts/discourse/app/models/post.js index 8a936b55967..6b352f1c87b 100644 --- a/app/assets/javascripts/discourse/app/models/post.js +++ b/app/assets/javascripts/discourse/app/models/post.js @@ -322,6 +322,7 @@ const Post = RestModel.extend({ deleteBookmark(bookmarked) { this.set("topic.bookmarked", bookmarked); this.clearBookmark(); + this.topic.incrementProperty("bookmarksWereChanged"); this.appEvents.trigger("page:bookmark-post-toggled", this); }, diff --git a/app/assets/javascripts/discourse/app/models/topic.js b/app/assets/javascripts/discourse/app/models/topic.js index d52f9e16eaa..0720289df88 100644 --- a/app/assets/javascripts/discourse/app/models/topic.js +++ b/app/assets/javascripts/discourse/app/models/topic.js @@ -322,11 +322,6 @@ const Topic = RestModel.extend({ return Site.currentProp("archetypes").findBy("id", archetype); }, - @discourseComputed("bookmarksWereChanged") - bookmarkedPosts() { - return this.postStream.posts.filterBy("bookmarked", true); - }, - isPrivateMessage: equal("archetype", "private_message"), isBanner: equal("archetype", "banner"), @@ -363,7 +358,6 @@ const Topic = RestModel.extend({ afterPostBookmarked(post) { post.set("bookmarked", true); - this.set("bookmark_reminder_at", post.bookmark_reminder_at); }, firstPost() { @@ -376,22 +370,40 @@ const Topic = RestModel.extend({ const postId = postStream.findPostIdForPostNumber(1); if (postId) { - // try loading from identity map first - firstPost = postStream.findLoadedPost(postId); - if (firstPost) { - return Promise.resolve(firstPost); - } - - return this.postStream.loadPost(postId); + return this.postById(postId); } else { return this.postStream.loadPostByPostNumber(1); } }, - deleteBookmark() { + postById(id) { + const loaded = this.postStream.findLoadedPost(id); + if (loaded) { + return Promise.resolve(loaded); + } + + return this.postStream.loadPost(id); + }, + + deleteBookmarks() { return ajax(`/t/${this.id}/remove_bookmarks`, { type: "PUT" }); }, + clearBookmarks() { + this.toggleProperty("bookmarked"); + + const postIds = this.bookmarked_posts.mapBy("post_id"); + postIds.forEach((postId) => { + const loadedPost = this.postStream.findLoadedPost(postId); + if (loadedPost) { + loadedPost.clearBookmark(); + } + }); + this.set("bookmarked_posts", []); + + return postIds; + }, + createGroupInvite(group) { return ajax(`/t/${this.id}/invite-group`, { type: "POST", diff --git a/app/assets/javascripts/discourse/tests/acceptance/bookmarks-test.js b/app/assets/javascripts/discourse/tests/acceptance/bookmarks-test.js index e0dc6dea964..60978895f8f 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/bookmarks-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/bookmarks-test.js @@ -22,6 +22,39 @@ async function openEditBookmarkModal() { await click(".topic-post:first-child button.bookmarked"); } +async function testTopicLevelBookmarkButtonIcon(assert, postNumber) { + const iconWithoutClock = "d-icon-bookmark"; + const iconWithClock = "d-icon-discourse-bookmark-clock"; + + await visit("/t/internationalization-localization/280"); + assert.ok( + query("#topic-footer-button-bookmark svg").classList.contains( + iconWithoutClock + ), + "Shows an icon without a clock when there is no a bookmark" + ); + + await openBookmarkModal(postNumber); + await click("#save-bookmark"); + + assert.ok( + query("#topic-footer-button-bookmark svg").classList.contains( + iconWithoutClock + ), + "Shows an icon without a clock when there is a bookmark without a reminder" + ); + + await openBookmarkModal(postNumber); + await click("#tap_tile_tomorrow"); + + assert.ok( + query("#topic-footer-button-bookmark svg").classList.contains( + iconWithClock + ), + "Shows an icon with a clock when there is a bookmark with a reminder" + ); +} + acceptance("Bookmarking", function (needs) { needs.user(); let steps = []; @@ -64,6 +97,7 @@ acceptance("Bookmarking", function (needs) { } server.post("/bookmarks", handleRequest); server.put("/bookmarks/1", handleRequest); + server.put("/bookmarks/2", handleRequest); server.delete("/bookmarks/1", () => helper.response({ success: "OK", topic_bookmarked: false }) ); @@ -354,4 +388,14 @@ acceptance("Bookmarking", function (needs) { "The edit modal is opened" ); }); + + test("The topic level bookmark button shows an icon with a clock if there is a bookmark with a reminder on the first post", async function (assert) { + const postNumber = 1; + await testTopicLevelBookmarkButtonIcon(assert, postNumber); + }); + + test("The topic level bookmark button shows an icon with a clock if there is a bookmark with a reminder on the second post", async function (assert) { + const postNumber = 2; + await testTopicLevelBookmarkButtonIcon(assert, postNumber); + }); }); diff --git a/app/serializers/post_serializer.rb b/app/serializers/post_serializer.rb index 8be001d5a4d..3d6f81b44c3 100644 --- a/app/serializers/post_serializer.rb +++ b/app/serializers/post_serializer.rb @@ -358,8 +358,11 @@ class PostSerializer < BasicPostSerializer end def post_bookmark - return nil if @topic_view.blank? - @post_bookmark ||= @topic_view.user_post_bookmarks.find { |bookmark| bookmark.post_id == object.id } + if @topic_view.present? + @post_bookmark ||= @topic_view.user_post_bookmarks.find { |bookmark| bookmark.post_id == object.id } + else + @post_bookmark ||= object.bookmarks.find_by(user: scope.user) + end end def bookmark_reminder_at diff --git a/app/serializers/topic_view_serializer.rb b/app/serializers/topic_view_serializer.rb index 3a5e1d747dd..141fc7fc2cd 100644 --- a/app/serializers/topic_view_serializer.rb +++ b/app/serializers/topic_view_serializer.rb @@ -62,7 +62,7 @@ class TopicViewSerializer < ApplicationSerializer :is_warning, :chunk_size, :bookmarked, - :bookmark_reminder_at, + :bookmarked_posts, :message_archived, :topic_timer, :unicode_title, @@ -193,12 +193,8 @@ class TopicViewSerializer < ApplicationSerializer object.has_bookmarks? end - def include_bookmark_reminder_at? - bookmarked - end - - def bookmark_reminder_at - object.first_post_bookmark_reminder_at + def bookmarked_posts + object.bookmarked_posts end def topic_timer diff --git a/app/serializers/web_hook_topic_view_serializer.rb b/app/serializers/web_hook_topic_view_serializer.rb index 13159fdf208..bff763f9b89 100644 --- a/app/serializers/web_hook_topic_view_serializer.rb +++ b/app/serializers/web_hook_topic_view_serializer.rb @@ -22,6 +22,7 @@ class WebHookTopicViewSerializer < TopicViewSerializer image_url slow_mode_seconds slow_mode_enabled_until + bookmarked_posts }.each do |attr| define_method("include_#{attr}?") do false diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index ca725af3940..a8d0ac280a7 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -298,8 +298,9 @@ en: clear_bookmarks: "Clear Bookmarks" help: bookmark: "Click to bookmark the first post on 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. You have a reminder set %{reminder_at} for this topic." + unbookmark_with_reminder: "Click to remove all bookmarks and reminders in this topic." bookmarks: created: "You've bookmarked this post. %{name}" diff --git a/lib/topic_view.rb b/lib/topic_view.rb index 99efecfdb1e..c3d5d2e95ba 100644 --- a/lib/topic_view.rb +++ b/lib/topic_view.rb @@ -395,13 +395,14 @@ class TopicView @topic.bookmarks.exists?(user_id: @user.id) end - def first_post_bookmark_reminder_at - @first_post_bookmark_reminder_at ||= \ - begin - first_post = @topic.posts.with_deleted.find_by(post_number: 1) - return if !first_post - first_post.bookmarks.where(user: @user).pluck_first(:reminder_at) - end + def bookmarked_posts + 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 + } + end end MAX_PARTICIPANTS = 24 diff --git a/spec/components/topic_view_spec.rb b/spec/components/topic_view_spec.rb index 87385de49a7..0a6e07b657f 100644 --- a/spec/components/topic_view_spec.rb +++ b/spec/components/topic_view_spec.rb @@ -413,9 +413,17 @@ describe TopicView do context "#first_post_bookmark_reminder_at" 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) } it "gets the first post bookmark reminder at for the user" do - expect(TopicView.new(topic.id, user).first_post_bookmark_reminder_at).to eq_time(bookmark1.reminder_at) + topic_view = TopicView.new(topic.id, user) + + bookmarked_posts = topic_view.bookmarked_posts + first, second = bookmarked_posts + 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) + expect(second[:reminder_at]).to eq_time(bookmark1.reminder_at) end context "when the topic is deleted" do @@ -423,7 +431,13 @@ describe TopicView do topic_view = TopicView.new(topic, user) PostDestroyer.new(Fabricate(:admin), topic.first_post).destroy topic.reload - expect(topic_view.first_post_bookmark_reminder_at).to eq_time(bookmark1.reminder_at) + + bookmarked_posts = topic_view.bookmarked_posts + first, second = bookmarked_posts + 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) + expect(second[:reminder_at]).to eq_time(bookmark1.reminder_at) end end end diff --git a/spec/serializers/post_serializer_spec.rb b/spec/serializers/post_serializer_spec.rb index 1009542bce4..8c020985801 100644 --- a/spec/serializers/post_serializer_spec.rb +++ b/spec/serializers/post_serializer_spec.rb @@ -245,14 +245,6 @@ describe PostSerializer do it "returns the reminder_at for the bookmark" do expect(serialized.as_json[:bookmark_reminder_at]).to eq(bookmark.reminder_at.iso8601) end - - context "if topic_view is blank" do - let(:topic_view) { nil } - - it "the bookmarked attribute will be false" do - expect(serialized.as_json[:bookmarked]).to eq(false) - end - end end end end