views:

160

answers:

6

Say I have a form_for with a select menu to assign a User on a belongs_to association:

...
form.select :user_id, @users, :prompt => "Select a User"
...

Currently I have @users in the controller as follows:

@users = User.all.map { |u| [u.full_name, u.id] }

I feel like this logic should maybe moved into a helper or even to the model. But I am confused as to where to deal with this and how.

A: 

That would be a model method since it has logic for the model. Helper methods should have UI level logic (whether to display a link or not) and HTML helpers (methods to generate links for example)

James Deville
I guess so. But couldn't this also be UI logic as it relates to how the select menu is displayed?
Cameron
I feel this would be helpful if you provided code backing up your opinion that it should be a model method; but I do agree with you.
Ryan Bigg
A: 

i think moving that to a helper is the best thing, since it just does help you with creating options for a select box, which is UI level.

But unless you would use that piece of code again, then to the model it must go! :)

paolo granada lim
+1  A: 

You should put this in a model as it's logic oriented and by the way you should never do

@users = User.all.map { |u| [u.full_name, u.id] }

but

@users = User.all(:select => "full_name, id")

and if full_name is a method, something like that :

@users = User.all(:select => "last_name, first_name, id").map{|u| [User.full_name(u.first_name, u.last_name), u.id]}
Mike
Yea but what if full_name is a method and not an actual column in the db??
Cameron
just add an example to math your comment
Mike
+3  A: 

The general answer depends on how often you're going to use it:

  • helper: used often but only in views or controllers
  • model: used often anywhere the model can be used (other models)
  • controller: used rarely and only for specific actions.

However in your case, the answer is none of the above, and stop trying to reinvent the wheel. About 95% of the things people try to do with Rails are tasks that others have already done. There's a very good chance that it's either in the Rails Core or exist in either gem or plugin form.

What you're trying to do has already been done, and is built into the Rails core. It's a ActionView::Helpers::FormOpitionsHelper method called collection_select

collection_select does exactly what you want to do, it's also much more robust than a single purpose method.

It has the form of

collection_select(object, method, collection, value_method,
  text_method, select_options = {}, html_options)

value_method and text_method are sent to each item in collection to get the select value and the display text for each select option. It is not required that either are column names.

Use it like this:

<% form_for @whatever do |form| %>
  <%= form.collection_select :user_id, User.all, :id,
    :full_name, :prompt => "Select a User" %>
<% end %>
EmFi
Never knew about collection_select, good call. Deleted my answer that home brewed this code.
cwninja
A: 

The model:

def self.select_display
  all(:select => "id, first_name, last_name").map { |u| [u.name, u.id] }
end

The view:

select :user_id, User.select_display
Ryan Bigg
A: 

I had a similar problem and ended up using a module, in order to stay as DRY as possible (most of my models had a name and an id)

The module looked like this:

#lib/all_for_select.rb
module AllForSelect

  def all_for_select(permission = :read)
    #used declarative authorization for checking permissions
    #replace first line with self.find(:all, if not using it
    with_permissions_to(permission).find( :all,
      :select =>"#{table_name}.id, #{table_name}.name",
      :order => "#{table_name}.name ASC"
    )
  end

end

On your model, you just extend the module:

class Client < ActiveRecord::Base
  extend AllForSelect
  ...
end

On your controller, you can call Client.all_for_select. I usually do this on a before_filter

class SalesController < ApplicationController
  before_filter :fill_selects, :only => [:new, :edit, :update, :create]

  ...

  private
  def fill_selects
    @clients = Client.all_for_select
  end
egarcia