DEV: Allow params to be passed on topic redirects (#16218)

* DEV: Allow params to be passed on topic redirects

There are several places where we redirect a url to a standard topic url
like `/t/:slug/:topic_id` but we weren't always passing query parameters
to the new url.

This change allows a few more query params to be included on the
redirect. The new params that are permitted are page, print, and
filter_top_level_replies. Any new params will need to be specified.

This also prevents the odd trailing empty page param that would
sometimes appear on a redirect. `/t/:slug/:id.json?page=`

* rubocop: fix missing space after comma

* fix another page= reference

app/controllers/topics_controller.rb

@@ -1118,12 +1118,17 @@ class TopicsController < ApplicationController
       raise(SiteSetting.detailed_404 ? ex : Discourse::NotFound)
     end
 
+    opts = params.slice(:page, :print, :filter_top_level_replies)
+    opts.delete(:page) if params[:page] == 0
+
     url = topic.relative_url
     url << "/#{post_number}" if post_number.to_i > 0
     url << ".json" if request.format.json?
 
-    page = params[:page]
-    url << "?page=#{page}" if page != 0
+    opts.each do |k, v|
+      s = url.include?('?') ? '&' : '?'
+      url << "#{s}#{k}=#{v}"
+    end
 
     redirect_to url, status: 301
   end

spec/requests/api/topics_spec.rb

@@ -649,7 +649,7 @@ describe 'topics' do
         let(:external_id) { topic.external_id }
 
         run_test! do |response|
-          expect(response).to redirect_to(topic.relative_url + ".json?page=")
+          expect(response).to redirect_to(topic.relative_url + ".json")
         end
       end
     end

spec/requests/topics_controller_spec.rb

@@ -1818,7 +1818,7 @@ RSpec.describe TopicsController do
     it 'returns 301 when found' do
       get "/t/external_id/asdf.json"
       expect(response.status).to eq(301)
-      expect(response).to redirect_to(topic.relative_url + ".json?page=")
+      expect(response).to redirect_to(topic.relative_url + ".json")
     end
 
     it 'returns right response when not found' do
@@ -1826,6 +1826,34 @@ RSpec.describe TopicsController do
       expect(response.status).to eq(404)
     end
 
+    it 'preserves only select query params' do
+      get "/t/external_id/asdf.json", params: {
+        filter_top_level_replies: true
+      }
+      expect(response.status).to eq(301)
+      expect(response).to redirect_to(topic.relative_url + ".json?filter_top_level_replies=true")
+
+      get "/t/external_id/asdf.json", params: {
+        not_valid: true
+      }
+      expect(response.status).to eq(301)
+      expect(response).to redirect_to(topic.relative_url + ".json")
+
+      get "/t/external_id/asdf.json", params: {
+        filter_top_level_replies: true,
+        post_number: 9999
+      }
+      expect(response.status).to eq(301)
+      expect(response).to redirect_to(topic.relative_url + "/9999.json?filter_top_level_replies=true")
+
+      get "/t/external_id/asdf.json", params: {
+        filter_top_level_replies: true,
+        print: true
+      }
+      expect(response.status).to eq(301)
+      expect(response).to redirect_to(topic.relative_url + ".json?print=true&filter_top_level_replies=true")
+    end
+
     describe 'when user does not have access to the topic' do
       it 'should return the right response' do
         sign_in(user)