Help needed to DRY helpers and proper MVC with Ruby


#1

Hello.

I’m still fumbling learning RoR, so I apologize in advance.

I’ve got working code, but I don’t think that it’s done properly so
any
help to make this more efficient and most of all, correct, would be so
very appreciated.

In my users_helper.rb file

def generate_sub_nav
if session[:user]
sub_nav_menu = []
user = User.find(session[:user])
for role in user.roles
for right in role.rights
if right.on_menu
sub_nav_menu << right.id
end
end # for right
end # for role
end # if
sub_nav_menu.sort!
end # def

In my application_helper.rb file for the layout and display

def show_sub_nav(rights)
html_out = ‘

for right in rights
right_array = Right.find(:all, :conditions => [“id = ?”, right])
for menu in right_array
html_out << link_to_unless_current (menu.name,
:controller => menu.controller, :action => menu.action)
end
end #for right in rights

html_out << ‘


html_out
end

In my controllers (not including the user_controller)

helper :users

There’s got to be a better, more Ruby like way to do this.

THANKS!


#2
        end
    end # for right
  end # for role
end # if
 sub_nav_menu.sort!

end # def

First, you might map rights to users through roles:

class User < ActiveDirectory::Base
has_many :rights, :through :roles
end

Then you could reduce that method to:

def generate_sub_nav
if session[:user] && user = User.find(session[:user])
user.rights.select {|right| right.on_menu}.sort
end
end

note that I’m returning an enumeration of Rights objects, not an
enumeration of Right object ids

end

end #for right in rights

html_out << ‘’
html_out
end

def show_sub_nav(rights)
html_out = ‘


rights.each do |right|
html_out << link_to_unless_current(h(right.name),
{:controller=>right.controller, :action=>right.action})
end
html_out << ‘

html_out
end

Other things you could do include including the roles and/or rights in
the User.find method call, limiting the rights to those with the on_menu
attribute, and ordering the results by right id instead of sorting the
Rights objects.

You might also want to consider loading the User object from the
session[:user] id in a filter instead of doing it in the helper, since
you’re likely to want to use your User object in a variety of different
places.

I also wonder if show_sub_nav really merits a helper method at all. If
it were me, I’d just put this chunk of code in your standard layout
wrapper.

  • donald

#3

Thanks so much for taking the time to work with me on this. I’m
learning so much from user groups!

Here are a couple of things that I tried with results and questions.

I have this in my user.rb file

has_many :roles
has_many :rights, :through => :roles

But now I get a MySQL error because it’s looking for a column that
does not exist

Mysql::Error: #42S22Unknown column ‘roles.user_id’

The roles table doesn’t have a user_id field. I have roles and users
connected through the roles_users table.

The show_sub_nav is part of the menu and was put into the
application_helper.rb file so that it could be accessed by the entire
application. I wasn’t able to figure out where else to put it.

I’ll keep working through the rest of your suggestions to see how I
can create better code.

THANKS!


#4

Also as an FYI, the users, roles and rights design was taken from the
Pragmatic Programmers Rails Recipes book.


#5

Thanks so much for taking the time to work with me on this.
I’m learning so much from user groups!

No problem.

Mysql::Error: #42S22Unknown column ‘roles.user_id’

The roles table doesn’t have a user_id field. I have roles
and users connected through the roles_users table.

Oops, I’ve led you astray then. I’m actually a bit surprised what you
have works then, I thought when you did habtm you needed to tag both
ends of the relationship as has_and_belongs_to_many. C’e la vie,
disregard that portion of my suggestions.

It’s perhaps worth your while to try using the :include feature of
Base.find. Something like:

User.find(session[:user], :include=>{:roles=>{:rights}})

should load the user data along with its roles and rights all in one
query. I may have the syntax wrong, this is from memory.

The show_sub_nav is part of the menu and was put into the
application_helper.rb file so that it could be accessed by
the entire application. I wasn’t able to figure out where
else to put it.

Most folks would use ActionView’s layout feature for this, I suspect.

  • donald

#6

Would I incorporate this into this block here?

def generate_sub_nav
if session[:user] && user =
User.find(session[:user], :include=>{:roles=> :rights})
user.rights.select {|right| right.on_menu}.sort
end

If so, then I get this error

undefined method `rights’ for #User:0xc966b4c

But, I also removed this line

has_many :rights, :through => :roles

Basically, replaced one error with a new one.

I’m getting really lost now.


#7

Would I incorporate this into this block here?

def generate_sub_nav
if session[:user] && user =
User.find(session[:user], :include=>{:roles=> :rights})
user.rights.select {|right| right.on_menu}.sort
end

If so, then I get this error

undefined method `rights’ for #User:0xc966b4c

But, I also removed this line

has_many :rights, :through => :roles

Basically, replaced one error with a new one.

I’m getting really lost now.


#8

Would I incorporate this into this block here?

def generate_sub_nav
if session[:user] && user =
User.find(session[:user], :include=>{:roles=> :rights})
user.rights.select {|right| right.on_menu}.sort
end

If so, then I get this error

undefined method `rights’ for #User:0xc966b4c

But, I also removed this line

has_many :rights, :through => :roles

Basically, replaced one error with a new one.

I’m getting really lost now.


#9

Does anyone know what needs to change? Do I need to add back in
“:throgh” and then make changes elsewhere?

Thanks for the help!