[Ovirt-devel] [PATCH server 3/3] * app/controllers/host_controller.rb: factor biz logic out

[David Lutterkort]

On Thu, 2009-04-30 at 12:11 -0400, Scott Seago wrote:
> David Lutterkort wrote:
> > (1) addhost should use PoolService somehow, need to look at Scott's
> >     latest patch
> >
> >   
> I'm not sure it should use the service -- this is one of these wui-only 
> form setup bits that I've been ignoring for now. I'm not sure how this 
> fits into the service model.

I've been following the line of 'no permission checks in the
controllers' - maybe to a fault. That's the main reason why I'd like to
have addhost use the Service layer - just for the permission check.

> > (2) the way we handle unsupported actions is strange: we define them and
> >     kick out a nice error - in reality, people can only get there either if
> >     we have a bug somewhere or if they did URL surgery; either way, they
> >     deserve a gory Rails error report. We should therefore remove the
> >     actions and associated nice error messages

Any opinions on this ?

> OK, I think I understand what you're doing here -- I may have 
> misunderstood svc_modify in my comments above -- is this meant as a 
> distinct action from svc_update -- which we're using elsewhere to 
> actually make changes? i.e. it's effectively svc_show with modify 
> permissions enforcing? Does this belong in the svc layer? This seems 
> more WUI-specific, as for the api/qmf case I'm not sure we'd be using 
> this action. Looking below this is used for multiple modify actions, so 
> perhaps it does belong here -- it just might not be exposed as a 
> QMF-level API call -- am I understading this correctly now?

My main intent with that was that all permission checking is pushed into
the service layer, and when you write a controller you can just
mindlessly call the 'right' service method - e.g., if you want to
display something for modification to the user, you'd use svc_modify
(obviously not the best name)

I doubt we'd use this in the QMF API; but the main motivation is to make
sure we have permission checks localized as much as possible so that
changes to the permission model don't require use to hunt through the
whole code base.

Do you still think it's unnecessary ?

David