Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow redirects with symbols #4

Closed
jodosha opened this issue Jun 12, 2014 · 10 comments
Closed

Allow redirects with symbols #4

jodosha opened this issue Jun 12, 2014 · 10 comments

Comments

@jodosha
Copy link
Member

jodosha commented Jun 12, 2014

As now we have two ways to specify an URL for a redirect in an action:

By hardcoding the path:

redirect_to '/'

By using the helper:

redirect_to MyApp::Routes.url(:root)

I would like to introduce a third way:

redirect_to :root
@pkurek
Copy link
Contributor

pkurek commented Jun 20, 2014

@jodosha I can give it a try. Could you explain how the process looks from here ?

@jodosha
Copy link
Member Author

jodosha commented Jun 20, 2014

@pkurek Thanks for looking at this.

Action#redirect_to will trust that the argument is a valid URL, and directly put the argument in the HTTP Location header (see impl).

Now we can override this behavior in frameworks.rb, and let the right Routes object to capture that symbol argument, resolve it as an url, and pass back the control to the Action.

I haven't implemented it yet because there is one thing to solve: get the right Routes object. Because Lotus has a multi application architecture, we had to namespace with the module of the application. So that MyApp::Application (which inherits from Lotus::Application) will have MyApp::Routes.

Because that override is at the higher level Lotus::Action, instead of MyApp::Action, we can't easily implement like this, because it would only work for MyApp.

def redirect_to(url, status: 302)
  headers.merge!(LOCATION => MyApp::Routes.url(url))
  self.status = status
end

This is a nice to have feature, but has some complications, so I would postpone after the first release.

@pkurek
Copy link
Contributor

pkurek commented Jun 20, 2014

@jodosha

This is a nice to have feature, but has some complications, so I would postpone after the first release

That's your call :)

Are there any things I could help with? Cause the other 3 opened issues labeled as help-wanted miss requirements.

@jodosha
Copy link
Member Author

jodosha commented Jun 20, 2014

@pkurek they are all meant to be implemented for the next release.

@fuadsaud
Copy link

I was playing with this and was wondering if, instead of simply mokeypatching redirect_to in this gem (like Lotus::Action::Rack is) it wouldn't be better to make Lotus::Action itself provide an extension point, something like a route_resolver object or even a method (not sure if better though) that would be given the symbol or string and decide how to handle it. Have any thoughts on this?

@zlw
Copy link

zlw commented Jun 20, 2014

@fuadsaud 👍

I was thinking about patching Lotus::Action (propably Lotus::Action::Callable) in lotusrb and passing <application_module>::Routes into the action's instance, but your solution is much cleaner ;)

We could rely on #call interface. This will allow passing object, method or even a proc

@fuadsaud
Copy link

How can we dynamically get the routes (or the application) object from within the action? We would need it to be injected somehow, right?

@jodosha
Copy link
Member Author

jodosha commented Jul 25, 2014

@fuadsaud Now we have a constant that acts as a router wrapper. By convention, for a Bookshelf application, we'll have a Bookshelf::Routes, which is an instance of Lotus::Routes.

We could implement this feature by injecting a custom module into Bookshelf::Action, when the application is loaded, by using Lotus::Controller::Configuration#modules.

# lib/lotus/frameworks.rb
module Lotus::Frameworks::Action::NamedRedirect
  private
  def redirect_to(name)
    # ...
  end
end

@jodosha jodosha modified the milestone: v0.2.0 Nov 10, 2014
@jodosha jodosha modified the milestone: v0.2.0 Dec 19, 2014
@runlevel5
Copy link
Member

@fuadsaud ping, any update?

@jodosha
Copy link
Member Author

jodosha commented Oct 16, 2015

This opens an inconsistency with routing helpers in views. Not a good idea.

@jodosha jodosha closed this as completed Oct 16, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants