FIX: Post menu bookmark icon and attributes not refreshing on notification click (#10214)

FIX: Post menu bookmark icon and attributes not refreshing on notification click (#10214)

When creating a bookmark reminder that deletes the bookmark on reminder, if the user clicked on the notification and got taken to the post in the topic the bookmark icon still showed as blue with the reminder clock indicator. This was because the response JSON for reloading a topic post was not including the bookmark attributes, not even the bookmarked boolean.

We now return the correct attributes in the serializer, and if bookmarked is false we clear all the bookmark related attributes on the post for the notification to make sure nothing of the old bookmark remains in the UI.

This was only a problem if the user did not refresh the app completely inbetween setting the reminder and receiving the notification.

diff --git a/app/assets/javascripts/discourse/app/routes/topic-from-params.js b/app/assets/javascripts/discourse/app/routes/topic-from-params.js
index 2c028e4..1d70023 100644
--- a/app/assets/javascripts/discourse/app/routes/topic-from-params.js
+++ b/app/assets/javascripts/discourse/app/routes/topic-from-params.js
@@ -70,6 +70,17 @@ export default DiscourseRoute.extend({
         }
         DiscourseURL.jumpToPost(closest, opts);
 
+        // completely clear out all the bookmark related attributes
+        // because they are not in the response if bookmarked == false
+        if (closestPost && !closestPost.bookmarked) {
+          closestPost.setProperties({
+            bookmark_reminder_at: null,
+            bookmark_reminder_type: null,
+            bookmark_name: null,
+            bookmark_id: null
+          });
+        }
+
         if (!isEmpty(topic.draft)) {
           composerController.open({
             draft: Draft.getLocal(topic.draft_key, topic.draft),
diff --git a/app/serializers/post_serializer.rb b/app/serializers/post_serializer.rb
index 873db6d..2372a16 100644
--- a/app/serializers/post_serializer.rb
+++ b/app/serializers/post_serializer.rb
@@ -319,34 +319,28 @@ class PostSerializer < BasicPostSerializer
     !(SiteSetting.suppress_reply_when_quoting && object.reply_quoted?) && object.reply_to_user
   end
 
-  # this atrtribute is not even included unless include_bookmarked? is true,
-  # which is why it is always true if included
   def bookmarked
-    true
-  end
-
-  def include_bookmarked?
-    post_bookmark.present?
+    @bookmarked ||= post_bookmark.present?
   end
 
   def include_bookmark_reminder_at?
-    include_bookmarked?
+    bookmarked
   end
 
   def include_bookmark_reminder_type?
-    include_bookmarked?
+    bookmarked
   end
 
   def include_bookmark_name?
-    include_bookmarked?
+    bookmarked
   end
 
   def include_bookmark_delete_when_reminder_sent?
-    include_bookmarked?
+    bookmarked
   end
 
   def include_bookmark_id?
-    include_bookmarked?
+    bookmarked
   end
 
   def post_bookmark
diff --git a/spec/serializers/post_serializer_spec.rb b/spec/serializers/post_serializer_spec.rb
index dc633ba..06ca8d4 100644
--- a/spec/serializers/post_serializer_spec.rb
+++ b/spec/serializers/post_serializer_spec.rb
@@ -250,8 +250,8 @@ describe PostSerializer do
         context "if topic_view is blank" do
           let(:topic_view) { nil }
 
-          it "does not return the bookmarked attribute" do
-            expect(serialized.as_json.key?(:bookmarked)).to eq(false)
+          it "the bookmarked attribute will be false" do
+            expect(serialized.as_json[:bookmarked]).to eq(false)
           end
         end
       end
diff --git a/spec/serializers/web_hook_post_serializer_spec.rb b/spec/serializers/web_hook_post_serializer_spec.rb
index 9e5c1f6..4366311 100644
--- a/spec/serializers/web_hook_post_serializer_spec.rb
+++ b/spec/serializers/web_hook_post_serializer_spec.rb
@@ -12,7 +12,7 @@ RSpec.describe WebHookPostSerializer do
 
   it 'should only include the required keys' do
     count = serialized_for_user(admin).keys.count
-    difference = count - 40
+    difference = count - 41
 
     expect(difference).to eq(0), lambda {
       message = +""

GitHub sha: 56f42d89

1 Like

This commit appears in #10214 which was merged by martin.