Forum: Ruby on Rails Help needed to DRY helpers and proper MVC with Ruby

Announcement (2017-05-07): www.ruby-forum.com is now read-only since I unfortunately do not have the time to support and maintain the forum any more. Please see rubyonrails.org/community and ruby-lang.org/en/community for other Rails- und Ruby-related community platforms.
RubyTechie (Guest)
on 2007-07-27 23:45
(Received via mailing list)
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!
Ball, Donald A Jr (Library) (Guest)
on 2007-07-28 00:06
(Received via mailing list)
>             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 << '</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
RubyTechie (Guest)
on 2007-07-28 01:13
(Received via mailing list)
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!
RubyTechie (Guest)
on 2007-07-28 01:19
(Received via mailing list)
Also as an FYI, the users, roles and rights design was taken from the
Pragmatic Programmers Rails Recipes book.
Ball, Donald A Jr (Library) (Guest)
on 2007-07-28 01:29
(Received via mailing list)
> 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
RubyTechie (Guest)
on 2007-07-28 01:58
(Received via mailing list)
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.
RubyTechie (Guest)
on 2007-07-28 02:01
(Received via mailing list)
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.
RubyTechie (Guest)
on 2007-07-28 02:29
(Received via mailing list)
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.
RubyTechie (Guest)
on 2007-07-30 21:27
(Received via mailing list)
Does anyone know what needs to change?  Do I need to add back in
":throgh" and then make changes elsewhere?

Thanks for the help!
This topic is locked and can not be replied to.