From 5d444be72b94664059b41dd57708ec364bed6ac6 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Wed, 29 May 2013 18:01:25 -0400 Subject: [PATCH] Support incomplete topic urls like /t/just-a-slug; fix error when using route /t/:topic_id/:post_number --- app/controllers/topics_controller.rb | 9 ++++- app/views/topics/show.html.erb | 2 +- config/routes.rb | 2 +- spec/controllers/topics_controller_spec.rb | 39 ++++++++++++++++------ 4 files changed, 39 insertions(+), 13 deletions(-) diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index 50ccc67ab..5f19c2c6d 100644 --- a/app/controllers/topics_controller.rb +++ b/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) diff --git a/app/views/topics/show.html.erb b/app/views/topics/show.html.erb index 9bdb07349..e6747bcc4 100644 --- a/app/views/topics/show.html.erb +++ b/app/views/topics/show.html.erb @@ -25,7 +25,7 @@

<%= t 'powered_by_html' %>

<% 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) %> diff --git a/config/routes.rb b/config/routes.rb index c26799d47..3db7e346e 100644 --- a/config/routes.rb +++ b/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' diff --git a/spec/controllers/topics_controller_spec.rb b/spec/controllers/topics_controller_spec.rb index ff790c9fc..4d5e0afe4 100644 --- a/spec/controllers/topics_controller_spec.rb +++ b/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 }