Support incomplete topic urls like /t/just-a-slug; fix error when using route /t/:topic_id/:post_number

app/controllers/topics_controller.rb

@@ -21,9 +21,16 @@ class TopicsController < ApplicationController
 
   before_filter :consider_user_for_promotion, only: :show
 
-  skip_before_filter :check_xhr, only: [:avatar, :show, :feed]
+  skip_before_filter :check_xhr, only: [:avatar, :show, :feed, :redirect_to_show]
   caches_action :avatar, cache_path: Proc.new {|c| "#{c.params[:post_number]}-#{c.params[:topic_id]}" }
 
+  def redirect_to_show
+    topic_query = ((num = params[:id].to_i) > 0 and num.to_s == params[:id].to_s) ? Topic.where(id: num) : Topic.where(slug: params[:id])
+    topic = topic_query.includes(:category).first
+    raise Discourse::NotFound unless topic
+    redirect_to topic.relative_url
+  end
+
   def show
     opts = params.slice(:username_filters, :best_of, :page, :post_number, :posts_before, :posts_after, :best)
     @topic_view = TopicView.new(params[:id] || params[:topic_id], current_user, opts)

app/views/topics/show.html.erb

@@ -25,7 +25,7 @@
 <p><%= t 'powered_by_html' %></p>
 
 <% content_for :head do %>
-  <%= auto_discovery_link_tag(@topic_view, {action: :feed, format: :rss}, title: t('rss_posts_in_topic', topic: @topic_view.title), type: 'application/rss+xml') %>
+  <%= auto_discovery_link_tag(@topic_view, {action: :feed, format: :rss, slug: @topic_view.topic.slug, topic_id: @topic_view.topic.id}, title: t('rss_posts_in_topic', topic: @topic_view.title), type: 'application/rss+xml') %>
   <%= crawlable_meta_data(title: @topic_view.title,
                           description: @topic_view.summary,
                           image: @topic_view.image_url) %>

config/routes.rb

@@ -182,7 +182,7 @@ Discourse::Application.routes.draw do
   get 'search' => 'search#query'
 
   # Topics resource
-  get 't/:id' => 'topics#show'
+  get 't/:id' => 'topics#redirect_to_show'
   delete 't/:id' => 'topics#destroy'
   put 't/:id' => 'topics#update'
   post 't' => 'topics#create'

spec/controllers/topics_controller_spec.rb

@@ -369,18 +369,18 @@ describe TopicsController do
     let!(:p2) { Fabricate(:post, user: topic.user) }
 
     it 'shows a topic correctly' do
-      xhr :get, :show, id: topic.id
+      xhr :get, :show, topic_id: topic.id, slug: topic.slug
       response.should be_success
     end
 
     it 'records a view' do
-      lambda { xhr :get, :show, id: topic.id }.should change(View, :count).by(1)
+      lambda { xhr :get, :show, topic_id: topic.id, slug: topic.slug }.should change(View, :count).by(1)
     end
 
     it 'tracks a visit for all html requests' do
       current_user = log_in(:coding_horror)
       TopicUser.expects(:track_visit!).with(topic, current_user)
-      get :show, id: topic.id
+      get :show, topic_id: topic.id, slug: topic.slug
     end
 
     context 'consider for a promotion' do
@@ -394,7 +394,7 @@ describe TopicsController do
       it "reviews the user for a promotion if they're new" do
         user.update_column(:trust_level, TrustLevel.levels[:newuser])
         Promotion.any_instance.expects(:review)
-        get :show, id: topic.id
+        get :show, topic_id: topic.id, slug: topic.slug
       end
     end
 
@@ -403,40 +403,59 @@ describe TopicsController do
       it 'grabs first page when no filter is provided' do
         SiteSetting.stubs(:posts_per_page).returns(20)
         TopicView.any_instance.expects(:filter_posts_in_range).with(0, 20)
-        xhr :get, :show, id: topic.id
+        xhr :get, :show, topic_id: topic.id, slug: topic.slug
       end
 
       it 'grabs first page when first page is provided' do
         SiteSetting.stubs(:posts_per_page).returns(20)
         TopicView.any_instance.expects(:filter_posts_in_range).with(0, 20)
-        xhr :get, :show, id: topic.id, page: 1
+        xhr :get, :show, topic_id: topic.id, slug: topic.slug, page: 1
       end
 
       it 'grabs correct range when a page number is provided' do
         SiteSetting.stubs(:posts_per_page).returns(20)
         TopicView.any_instance.expects(:filter_posts_in_range).with(20, 40)
-        xhr :get, :show, id: topic.id, page: 2
+        xhr :get, :show, topic_id: topic.id, slug: topic.slug, page: 2
       end
 
       it 'delegates a post_number param to TopicView#filter_posts_near' do
         TopicView.any_instance.expects(:filter_posts_near).with(p2.post_number)
-        xhr :get, :show, id: topic.id, post_number: p2.post_number
+        xhr :get, :show, topic_id: topic.id, slug: topic.slug, post_number: p2.post_number
       end
 
       it 'delegates a posts_after param to TopicView#filter_posts_after' do
         TopicView.any_instance.expects(:filter_posts_after).with(p1.post_number)
-        xhr :get, :show, id: topic.id, posts_after: p1.post_number
+        xhr :get, :show, topic_id: topic.id, slug: topic.slug, posts_after: p1.post_number
       end
 
       it 'delegates a posts_before param to TopicView#filter_posts_before' do
         TopicView.any_instance.expects(:filter_posts_before).with(p2.post_number)
-        xhr :get, :show, id: topic.id, posts_before: p2.post_number
+        xhr :get, :show, topic_id: topic.id, slug: topic.slug, posts_before: p2.post_number
       end
 
     end
 
   end
 
+  describe 'redirect_to_show' do
+    let(:topic) { Fabricate(:topic) }
+
+    it 'raises an error if topic is not found' do
+      get :redirect_to_show, id: 121212121212
+      expect(response.status).to eq(404)
+    end
+
+    it 'redirects to the correct topic when given its id' do
+      get :redirect_to_show, id: topic.id
+      expect(response).to redirect_to(topic.relative_url)
+    end
+
+    it 'redirects to the correct topic when given its slug' do
+      get :redirect_to_show, id: topic.slug
+      expect(response).to redirect_to(topic.relative_url)
+    end
+  end
+
   describe '#feed' do
     let(:topic) { Fabricate(:post).topic }