Should Sorcery rename redirect_back_or_to?

[martinzamuner]

Rails 7 released a new method called redirect_back_or_to added by DHH as a replacement for the now soft-deprecated redirect_back: https://github.com/rails/rails/pull/40671.

That may conflict with the method by the same name defined by Sorcery. Should it be renamed? I'm opening an issue instead of a PR because I guess it's not a trivial decision to make.

[joshbuker]

Hmm. Yeah, that'll require some thought. The most robust solution would probably be to namespace the method, but that wouldn't be backwards compatible. Could dynamically define it based on what version of rails is defined, but I prefer avoiding that when possible just because it introduces non-trivial amounts of complexity.

Thanks for bringing this up. Rails 7 has a lot of nice additions, so I want to support it as soon as possible.

I'm currently knee deep in both a personal and work project, but I'll tackle Rails 7 myself after those are both settled. In the meantime, any Rails 7 related issues that can be fixed in a backwards compatible way I'll do my best to merge in quickly.

[lorenzk]

FYI I am using this workaround for the time being:

# config/initializers/sorcery_workaround_for_redirect_back_or_to.rb
module Sorcery
  module Controller
    module InstanceMethods
      define_method :sorcery_redirect_back_or_to, instance_method(:redirect_back_or_to)
      remove_method :redirect_back_or_to
    end
  end
end

And then use sorcery_redirect_back_or_to in my controllers when I want to use the session[:return_to] feature and rails' redirect_back_or_to in place of the soft-deprecated redirect_back.

Maybe sorcery could hook into rails' mechanism by just favoring session[:return_to] over request.referer over fallback_location?

[Exterm1nate]

A good way to solve this is to define another method with same functionality (something like redirect_to_before_login_path), add a warning message to the original method that it is deprecated and will be soon removed, and finally don't define it at all for rails >= 7, because it breaks the functionality of passed options (they are all passed to the flash kwargs).

[kleinjm]

Bump 🙏 I went down a bit of a debugging rabbit hole due to this naming conflict

[PedroAugustoRamalhoDuarte]

I got this error in production today, strange that in my development workspace is calling the rails redirect_back_or_to in and in production the sorcery one. Is there anyone looking to handle this problem?

[joshbuker]

That is weird; a loading race condition or something to do with production preloading, perhaps?

I like @lorenzk's suggestion as a solution for the v0 codebase; if someone would be willing to open a PR implementing that solution, I can merge it and push a release.