Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Devise doesn't use default_url_options #1408

Closed
installero opened this Issue Oct 30, 2011 · 12 comments

Comments

Projects
None yet
5 participants

Hello!

I am passing locale through to the generated url using the default_url_options in application_controller.rb:

def default_url_options(options={})
  options.merge({:lang => I18n.locale})
end

But the result seems to be independent on this:

def it_should_require_login(request_method, action)
  it "should redirect to the login page" do
    send request_method, action, :id => 1
    response.should redirect_to(new_user_session_path)
  end
end

...

Admin::CompaniesController anonymous user should redirect to the login page
  Failure/Error: response.should redirect_to(new_user_session_path)
    Expected response to be a redirect to <http://test.host/users/sign_in?lang=ru> but was a redirect to <http://test.host/users/sign_in>.https://github.com/plataformatec/devise/issues/1408#

Every other helper that uses url_for (like link_to of form_for) works just fine in both specs and application controllers or views.

Owner

josevalim commented Oct 30, 2011

This is supposed to work. Which Devise and Rails versions are you using? Can you push an app that reproduces the error to Github?

@josevalim josevalim closed this Oct 30, 2011

The versions are: rails (3.0.7), devise (1.3.4). I'm afraid, I can't push the whole application to github. Why did you close that issue, did you manage to find what the problem was?

Owner

josevalim commented Oct 31, 2011

I usually close the issue when it does not have enough information to reproduce it and reopen it when the situation changes. I am not asking you to push your application to Github, but if you could please duplicate the issue in a small application from scratch and push it instead. That should be enough for me to see, understand and fix the issue. Thanks!

Thanx for your attention, Jose. I suppose this bug is not about devise. Cucumber and real human tests work just fine.

I think it's all about stubbing the default_url_options in the right place in rspec. Thanx again and sorry for bothering.

I know this is a super old issue, but I'm having the same problem with version 2.2.4 and Rails 3.2.13. All my link_to stuff works fine but redirecting via before_filter :authenticate_user! strips out the url params. Any ideas?

Owner

lucasmazza commented Jun 6, 2013

@mattfordham how are you setting your default url options? Can you push a sample app to GitHub?

Taking a first look on the issue seems that our FailureApp uses a classmethod from ApplicationController (https://github.com/plataformatec/devise/blob/master/lib/devise/failure_app.rb#L23-L29), so maybe you're link_to and Devise are using different options for the URLs.

I've got this in my ApplicationController:

def default_url_options(options={})
    if params[:state] == "fb"
      options.merge({:state => "fb"})
    else
      options
    end
  end

Again, it works fine with all the link_to stuff, but not with Devise redirects.

Contributor

deivid-rodriguez commented Sep 22, 2013

This issue seems to still be there. Normally people will override default_url_options as an instance method in ApplicationController (like @mattfordham did), but devise uses a class method.

If I change https://github.com/plataformatec/devise/blob/master/lib/devise/failure_app.rb#L25 and write instead

        ApplicationController.new.default_url_options(*args)

it seems to work.

Owner

josevalim commented Sep 23, 2013

We just use the class default_url_options. We can't use the instance one because someone may be relying on instance attributes on the controller, like request info, and your example does not pass those in. You can either change your default_url_options to be a class one or change the default_url_options in an inherited Devise failure app.

Contributor

deivid-rodriguez commented Sep 23, 2013

I see. And is there a way devise con access the actual controller instance from devise?

I run into problems when following this: https://github.com/plataformatec/devise/wiki/How-To:-Redirect-back-to-current-page-after-sign-in,-sign-out,-sign-up,-update. I finally regexp matched the urls instead of exact matching them and I didn't have to override default_url_options.

Thansks @josevalim !

Owner

josevalim commented Sep 23, 2013

I see. And is there a way devise con access the actual controller instance from devise?

No.

I run into problems when following this: https://github.com/plataformatec/devise/wiki/How-To:-Redirect-back-to-current-page-after-sign-in,-sign-out,-sign-up,-update. I finally regexp matched the urls instead of exact matching them and I didn't have to override default_url_options.

The wikis are community maintained, so they may have invalid or less than optimal code. If you find something that can be improved, please contribute back!

Contributor

deivid-rodriguez commented Sep 23, 2013

Sure.

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