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
