From 06d426bd87d5bf87e36dd543efd298aaef94156e Mon Sep 17 00:00:00 2001 From: Vinoth Kannan Date: Sun, 5 Jul 2020 14:04:31 +0530 Subject: [PATCH] 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. --- lib/topic_view.rb | 3 ++- spec/components/topic_view_spec.rb | 15 +++++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/lib/topic_view.rb b/lib/topic_view.rb index 0ab7c7f060c..17e1d8988e3 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 d55a475c71e..8bd81460419 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