FIX: calculate page if page param is not given to TopicView (#10953)

Currently, when page param is not given to TopicView we calculate page for canonical_path, however, it is skipped for next_path.

We should use the same calculation to define page, so next page URL will be accurate. Currently if you [view source of meta post](view-source:https://meta.discourse.org/t/post-rate-limit-trigger-for-a-topic-thats-heating-up/98294/46) you will see:

```
<link rel="canonical" href="https://meta.discourse.org/t/post-rate-limit-trigger-for-a-topic-thats-heating-up/98294?page=3" />
<link rel="next" href="/t/post-rate-limit-trigger-for-a-topic-thats-heating-up/98294?page=2">
```
This commit is contained in:
Krzysztof Kotlarek 2020-10-19 17:11:49 +11:00 committed by GitHub
parent f84261b530
commit 2ad4fc39b6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 11 additions and 11 deletions

View File

@ -66,7 +66,6 @@ class TopicView
end
@post_number = [@post_number.to_i, 1].max
@page = [@page.to_i, 1].max
@include_suggested = options.fetch(:include_suggested) { true }
@include_related = options.fetch(:include_related) { true }
@ -79,6 +78,8 @@ class TopicView
@limit ||= @chunk_size
@page = @page.to_i > 1 ? @page.to_i : calculate_page
setup_filtered_posts
@initial_load = true
@ -118,15 +119,7 @@ class TopicView
return topic_embed.embed_url if topic_embed
end
path = relative_url.dup
path <<
if @page > 1
"?page=#{@page}"
else
posts_count = is_mega_topic? ? @post_number : unfiltered_posts.where("post_number <= ?", @post_number).count
page = ((posts_count - 1) / @limit) + 1
page > 1 ? "?page=#{page}" : ""
end
path << ((@page > 1) ? "?page=#{@page}" : "")
path
end
@ -640,6 +633,11 @@ class TopicView
private
def calculate_page
posts_count = is_mega_topic? ? @post_number : unfiltered_posts.where("post_number <= ?", @post_number).count
((posts_count - 1) / @limit) + 1
end
def get_sort_order(post_number)
sql = <<~SQL
SELECT posts.sort_order

View File

@ -273,13 +273,15 @@ describe TopicView do
let!(:post) { Fabricate(:post, topic: topic, user: user) }
let!(:post2) { Fabricate(:post, topic: topic, user: user) }
let!(:post3) { Fabricate(:post, topic: topic, user: user) }
let!(:post4) { Fabricate(:post, topic: topic, user: user) }
let!(:post5) { Fabricate(:post, topic: topic, user: user) }
before do
TopicView.stubs(:chunk_size).returns(2)
end
it "should return the next page" do
expect(TopicView.new(topic.id, user).next_page).to eql(2)
expect(TopicView.new(topic.id, user, { post_number: post.post_number }).next_page).to eql(3)
end
end