chaplinjs / chaplin

HTML5 application architecture using Backbone.js
http://chaplinjs.org
Other
2.85k stars 231 forks source link

The beforeFilter in Controller #223

Closed starkovv closed 11 years ago

starkovv commented 11 years ago

Is there some kinda 'before_filter' like in Ruby-on-Rails Controller?

It'd awesome if Chaplin will have an ability to make before-method-call-check in Controller. For example to check whether the current user is logged in before fire some method in Controller or action in Router.

paulmillr commented 11 years ago

edit: you can override methods in https://github.com/chaplinjs/chaplin/blob/master/src/chaplin/lib/router.coffee currently.

mehcode commented 11 years ago

Definitely would like something generic to do this. The question is where to place the preconditions. Somewhere in the routes like: match 'pattern', controller: 'controllers/index', action: 'show', before: 'loginRequired' (I noticed @molily mentioning an alternative to the current controller#action in routes, I quite like that) ? Where the before option would be a method on the controller that would be called "before" the action. Next question of design would be should the before method actually call redirectTo or just return false and the route to go to if the check fails be declared elsewhere?

andriijas commented 11 years ago

@molily mentioned a blockview in movie pilot here: https://groups.google.com/forum/#!msg/chaplin-js/7f4CROUpkps/lsPx8fhi0p8J

which i think maybe would be something that boilerplate could contain something similar to..

require login is quite common thing to do.

molily commented 11 years ago
  1. It shouldn’t be part of the routes file. In Rails it isn’t either. That’s a controller duty.
  2. When a route matches, routing is stopped and the controller needs to handle the request. Returning false in the before filter shouldn’t start routing again. redirectTo is more sane. This is necessary because of the Chaplin architecture which separates Router and Dispatcher. The Router notifies the Dispatcher, whose purpose is to spawn controllers. The Dispatcher shouldn’t implicitly start routing again (this would be a stack like Router > Dispatcher > Router > Dispatcher … > Controller).
  3. Technically a before filter is just a controller method which is called before all or some actions. Chaplin already has two methods which are called before all actions: the constructor and initialize (with every route match, a new controller is instantiated and initialize is called). This is a good place to check for the login status and redirect if the user isn’t logged in.
  4. While this can be expressed manually in an imperative programming style, a more declarative syntax could be really helpful for Chaplin. I’d love to see proposals for that.
mehcode commented 11 years ago

Technically a before filter is just a controller method which is called before all or some actions. Chaplin already has two methods which are called before all actions: the constructor and initialize (with every route match, a new controller is instantiated and initialize is called). This is a good place to check for the login status and redirect if the user isn’t logged in.

While this is the current solution that I have for my login required pre-condition; chaplin doesn't account for controllers being redirected from their initialize method. Every action would need a return if @redirected. Which defeats the point of the login required check on a base controller.

I have to agree about the routes file. I would then advocate for a before (perhaps an after as well) method that is simply called before an action -- that is allowed to redirect. Example below:

Protected = 
  before: ->
    @redirectTo 'unauth#login' if  @not_logged_in

class Index extends Chaplin.Controller
  _(@::).extends Protected

  action: ->
    # Now I know we're logged in here

If you're agreeable to before and after methods on a controller being 'magic' @molily et all -- I'll submit a PR with the changes shortly. Perhaps before_method and after_method are configuration options on the dispatcher?

andriijas commented 11 years ago

Guys, keep in mind this is not stateless server side land. I think a better way than redirecting is to just trigger a render of the login view in a modal and on success continue render the requested route.

mehcode commented 11 years ago

Guys, keep in mind this is not stateless server side land. I think a better way than redirecting is to just trigger a render of the login view in a modal and on success continue render the requested route.

I agree that this should be an option. With generic before/after filters one could implement it however they wanted.

molily commented 11 years ago

I think a better way than redirecting is to just trigger a render of the login view in a modal and on success continue render the requested route.

There’s no “stop routing, show the login view and then continue routing” on the client side either. Either you redirecting to a dedicated login controller (like /login?redirect=/protected/area) or you route normally to a controller which then might trigger a login overlay.

On moviepilot.com we used an event-driven approach (don’t know if that’s still used). I already described it on the mailing list I think. The target controller was started normally and instantiated the models and views. The model fetch was wrapped in an ensureLogin call which triggered the login view if the user wasn’t logged in. It fetched the data immediately if the user was already logged in. This was implemented in a decoupled way using Pub/Sub events like !showLogin. !login and login. The view was just listening for the model data. The repo of the Facebook example contains most of the code we wrote for this architecture. The ensureLogin stuff is in utils. This is an example how it would work without “blocking” before filters.

This is a very flexible solution because the user can log in and out again and again without leaving the screen. This was important on moviepilot.com because logging in should be integrated smoothly. Nearly every screen had a public and a personalized version and the user should be able to switch between them with a click. A lot of modules were listening to login and logout events and fetched public/private data automatically and the views updated themselves accordingly.

But it’s also very complex. Most of the time we just want to hide a controller behind a login, like it’s done on the server side.

andriijas commented 11 years ago

I guess its very much up to what kind of app one are building. In my current project everything requires login, except of course the login view - so I just setup mediator event binding on the login event in my app initialization and delay the ininitialization of routing untill login is done. That way I can ensure my whole app requires authentication.

paulmillr commented 11 years ago

So, I think this can be closed. You can use Controller#initialize if you need to show different versions of views to authenticated and non-authenticated people or you can just initialize the whole application after checking the login state.

andriijas commented 11 years ago

pro tip: class MyFooController extends RequireLoginController

paulmillr commented 11 years ago

@andriijas :+1: inheritance FTW