Rails - Jul 2007 - Help needed to DRY helpers and proper MVC with Ruby

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 = ''<table><tr><td>''

  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 << ''</td></tr></table>''
  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!


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---

> 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
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
> In my application_helper.rb file for the layout and display
> 
> def show_sub_nav(rights)
>   html_out = ''<table><tr><td>''
> 
>   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 <<
''</td></tr></table>''
>   html_out
> end
def show_sub_nav(rights)
  html_out = ''<table><tr><td>''
  rights.each do |right|
    html_out << link_to_unless_current(h(right.name),
{:controller=>right.controller, :action=>right.action})
  end
  html_out << ''</td></tr></table>''
  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

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---

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!


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---

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


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---

> Thanks so much for taking the time to work with me on this.  
> I''m learning so much from user groups!
No problem.
> 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.
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

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---

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.


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---

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.


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---

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.


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---

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

Thanks for the help!


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---