FIX: skip hidden posts while generating canonical url.

FIX: skip hidden posts while generating canonical url.

Previously, while generating the topic page’s canoncial url we used the current post number. It will create invalid canonical path if the topic has whsiper posts. Now we only taking the visible posts for current page index calculation.

diff --git a/lib/topic_view.rb b/lib/topic_view.rb
index 0ab7c7f..17e1d89 100644
--- a/lib/topic_view.rb
+++ b/lib/topic_view.rb
@@ -122,7 +122,8 @@ class TopicView
       if @page > 1
         "?page=#{@page}"
       else
-        page = ((@post_number - 1) / @limit) + 1
+        posts_count = unfiltered_posts.where("post_number <= ?", @post_number).count
+        page = ((posts_count - 1) / @limit) + 1
         page > 1 ? "?page=#{page}" : ""
       end
 
diff --git a/spec/components/topic_view_spec.rb b/spec/components/topic_view_spec.rb
index d55a475..8bd8146 100644
--- a/spec/components/topic_view_spec.rb
+++ b/spec/components/topic_view_spec.rb
@@ -244,8 +244,19 @@ describe TopicView do
       end
 
       it "generates a canonical correctly for paged results" do
-        expect(TopicView.new(1234, user, post_number: 10 * TopicView.chunk_size)
-          .canonical_path).to eql("/1234?page=10")
+        5.times { |i| Fabricate(:post, post_number: i + 1, topic: topic) }
+
+        expect(TopicView.new(1234, user, post_number: 5, limit: 2)
+          .canonical_path).to eql("/1234?page=3")
+      end
+
+      it "generates canonical path correctly by skipping whisper posts" do
+        2.times { |i| Fabricate(:post, post_number: i + 1, topic: topic) }
+        2.times { |i| Fabricate(:whisper, post_number: i + 3, topic: topic) }
+        Fabricate(:post, post_number: 5, topic: topic)
+
+        expect(TopicView.new(1234, user, post_number: 5, limit: 2)
+          .canonical_path).to eql("/1234?page=2")
       end
     end
 

GitHub sha: 06d426bd

1 Like

This commit has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/canonical-urls-use-pagination-that-counts-whispers/156650/6

On mega topics this can be expensive, maybe bypass accuracy on mega topics, there is a constant for that in the file @tgxworld added

2 Likes

Follow-up: PERF: use post number to create canoncial path in mega topics.

2 Likes