Zach
By saying that models need the following case statement
def by_role(role
case role
when "associate"
....
when "admin"
...
end
your implying that the model is represented in a different way depending on
the role. So in your case of invoices the associate's view is different from
the administrators. If this is the case then you have identified two
distinct resources!
Currently you are representing these two resources with one url e.g.
/invoices
However if you think of these as seperate resources then you have two url's
/admin_invoices /associatie_invoices
Other url schems come fo mind /admin/invoices /invoices/admin etc.
Point is that you can implement these seperate resources in the standard
rails way and remove any need for case statements
class AdminInvoicesController
before_filter must_be_admin
def index
Invoices.find # specify admin criteria here
end
end
class AssociateInvoices
before_filter must_be_associate
def index
Invoices.find # specify associate criteria here
end
end
Andrew
2009/3/3 Zach Dennis <zach.den...@gmail.com>
> On Tue, Mar 3, 2009 at 12:32 PM, Pat Maddox <pat.mad...@gmail.com> wrote:
> > On Mon, Mar 2, 2009 at 8:35 PM, Stephen Eley <sfe...@gmail.com> wrote:
> >> On Mon, Mar 2, 2009 at 5:16 PM, Zach Dennis <zach.den...@gmail.com>
> wrote:
> >>>
> >>> Forgot to mention what we did do. We ended up with the following...
> >>>
> >>> def index
> >>> if user.has_role?("admin")
> >>> user.in_role("admin").invoices
> >>> elsif user.has_role?("associate")
> >>> user.in_role("associate").invoices
> >>> else
> >>> raise AccessDenied
> >>> end
> >>> end
> >>
> >> That seems sort of backwards to me. These aren't the user's invoices,
> >> right? They're just invoices which the user happens to be allowed to
> >> see? Chaining it this way makes it look like the invoices *belong* to
> >> the role, and seems put the user up front instead of the invoices.
> >> You also have conditional branching with hardcoded values, making the
> >> controller brittle, and some redundancy with the controller asking the
> >> model for a value and then passing the value right back to the model.
> >
> > Agreed. I have a similar example in a blog post:
> >
> http://www.patmaddox.com/blog/2008/7/23/when-duplication-in-tests-informs-design
>
> I agree I'm taking a step back to try to make two steps forward. The
> heart of the exploration is more than the conditional in the action
> (which simply states which roles are allowed to access that action, I
> just made it explicit rather than using a #restrict_to call). To me
> this discussion is about where the behaviour for a role should go and
> how-to access that behaviour.
>
> The flip side of this is that models end up with redundant conditional
> branches to do x for RoleA or y for RoleB (for every model thats
> affected by a Role). I would like to collapse these conditional
> branches and attempt a more polymorphic approach by extracting a class
> representative of each role, and simply calling the method in
> question. e.g:
>
> FiscalAssociate.new(user).invoices
> FiscalAdmin.new(user).invoices
>
> Rather than the following approach. Keep in mind that roles hardly
> ever are limited to one model. Consider having the case statement in
> 20 models. Where's the redundancy now?
>
> Invoice.by_role(<role_name>)
>
> def by_role(role
> case role
> when "associate"
> ....
> when "admin"
> ...
> end
>
>
> --
> Zach Dennis
> http://www.continuousthinking.com
> http://www.mutuallyhuman.com
> _______________________________________________
> rspec-users mailing list
> rspec-users@rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>
_______________________________________________
rspec-users mailing list
rspec-users@rubyforge.org
http://rubyforge.org/mailman/listinfo/rspec-users
