Redirects to canonical URLs #337

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
Contributor

glebm commented Nov 23, 2012

This will redirect users to the current slug URL for URLs with id in them

@@ -24,7 +23,7 @@ def new
def create
authorize! :create_topic, @forum
- @topic = @forum.topics.build(params[:topic], :as => :default)
+ @topic = @forum.topics.build(params[:topic], :as => :default)
@parndt

parndt Dec 6, 2012

Contributor

Please don't do this :-)

@glebm

glebm Dec 9, 2012

Contributor

Oops, RubyMine autoformat

@parndt

parndt Dec 9, 2012

Contributor

You should disable that ;-)

+ topic = FactoryGirl.create(:topic, :approved)
+ get :show, :forum_id => topic.forum.id, :id => topic.id
+ response.code.to_i.should == 301
+ response.body.should include forum_topic_path(topic.forum, topic)
@radar

radar Dec 6, 2012

Collaborator

Why don't you just check the URL rather than the entire body?

@glebm

glebm Dec 9, 2012

Contributor

How?

@radar

radar Dec 10, 2012

Collaborator

response.url.should include(forum_topic_path(topic.forum, topic), I think.

+ rescue ActiveRecord::RecordNotFound
+ # Todo: We are responding with 301 to pages that should be 404
+ # This probably isn't right
+ redirect_to @forum, :alert => t("forem.topic.not_found")
@parndt

parndt Dec 6, 2012

Contributor

Why don't we respond with 404 then? :-)

@glebm

glebm Dec 9, 2012

Contributor

If you guys are OK with it, I can make a separate pull request
I am not sure why it's there in the first place

@parndt

parndt Dec 9, 2012

Contributor

Yep a 404 seems reasonable to me -- @radar @knewter ?

@knewter

knewter Dec 9, 2012

Contributor

yeah, 404 on recordnotfound is standard and without argument against can't imagine having an issue with it :) :)

@radar

radar Dec 10, 2012

Collaborator

Agree, a 404 is best.

Contributor

parndt commented Dec 6, 2012

Thanks - just reviewed it and added a couple pieces of feedback.

@parndt parndt closed this Apr 27, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment