Forum: Ruby on Rails Best way to sort categories w/ pager

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.
81a3fc5c10a93d34ee0f25eb1e4227a1?d=identicon&s=25 Will Jessup (wjessup)
on 2006-04-16 07:02
Ahoy,

I made this pager,

 " def list
    @item_pages = Paginator.new self, Item.count, 10, @params['page']
    @items = Item.find :all, :conditions => "category_id =
#{params[:condition]}",
                          :limit  => @item_pages.items_per_page,
                          :offset => @item_pages.current.offset
    @categories = Category.find_all
  end"

And have this code to switch categories

"<table>
  <tr>
    <td><%= link_to 'all', :action => 'list' %></td>
  <% @categories.each do |category| %>
    <td><%= link_to category.name, :action => 'list', :condition =>
category.id %></td>
  <% end %>
    <td><%= link_to 'New category', :controller => 'categories', :action
=> 'new' %></td>
  </tr>
</table>"

It works when any of the looped categories are selected because they
pass their ID to the condition of the pager, now, how do I let the "ALL"
link work? Do I need multiple pagers?

Sorry, still on 2nd day rails.
7e9d48fccd1178c1e48e21a4c7a8667f?d=identicon&s=25 Jeff Gordon (jefers)
on 2006-04-16 07:18
check this out http://www.ruby-forum.com/topic/50290#17201

Would like to know what ppl think of this. Is it good rails practice?
81a3fc5c10a93d34ee0f25eb1e4227a1?d=identicon&s=25 Will Jessup (wjessup)
on 2006-04-16 07:23
Jeff Gordon wrote:
> check this out http://www.ruby-forum.com/topic/50290#17201
>
> Would like to know what ppl think of this. Is it good rails practice?

Thanks for pointing that out, learned a few more things. could someone
w/ experience chime in here?
59ea1b450935b9d70abfec4186b7a4d5?d=identicon&s=25 Jeff Coleman (progressions)
on 2006-04-16 09:24
Will Jessup wrote:
> Ahoy,
>
> I made this pager,
>
>  " def list
>     @item_pages = Paginator.new self, Item.count, 10, @params['page']
>     @items = Item.find :all, :conditions => "category_id =
> #{params[:condition]}",
>                           :limit  => @item_pages.items_per_page,
>                           :offset => @item_pages.current.offset
>     @categories = Category.find_all
>   end"
>
> And have this code to switch categories
>
> "<table>
>   <tr>
>     <td><%= link_to 'all', :action => 'list' %></td>
>   <% @categories.each do |category| %>
>     <td><%= link_to category.name, :action => 'list', :condition =>
> category.id %></td>
>   <% end %>
>     <td><%= link_to 'New category', :controller => 'categories', :action
> => 'new' %></td>
>   </tr>
> </table>"
>
> It works when any of the looped categories are selected because they
> pass their ID to the condition of the pager, now, how do I let the "ALL"
> link work? Do I need multiple pagers?
>
> Sorry, still on 2nd day rails.

Can I ask why you're not using the standard paginate method?

@item_pages, @items = paginate :items, :conditions => "category_id =
#{params[:condition]}"

Would do pretty much the same as what you're describing here, wouldn't
it?

You can optionally choose not to send the :conditions parameter if you
want to show all the records.

O.
81a3fc5c10a93d34ee0f25eb1e4227a1?d=identicon&s=25 Will Jessup (wjessup)
on 2006-04-16 10:29

 because i didn't see the :conditions in the documentation for that
method at the time. (>_<). How do I optionally choose not to send the
parameter?
34f5b045aec62235c17458650ea75353?d=identicon&s=25 Steve Koppelman (hatless)
on 2006-04-16 13:56
Not a seasoned expert or anything here, but wouldn't this be vulnerable
to a SQL injecton attack?

From what I've come to understand, it's better to say this as

@item_pages, @items = paginate :items, :conditions => ["category_id =
?", params[:category_id]]

And incidentally, the paginate should take pretty much any parameters
you'd want to use with a find, including :order, which is for sorting
the results.

http://api.rubyonrails.com/classes/ActiveRecord/Ba...


Jeff Coleman wrote:
>
> Can I ask why you're not using the standard paginate method?
>
> @item_pages, @items = paginate :items, :conditions => "category_id =
> #{params[:condition]}"
>
> Would do pretty much the same as what you're describing here, wouldn't
> it?
>
> You can optionally choose not to send the :conditions parameter if you
> want to show all the records.
81a3fc5c10a93d34ee0f25eb1e4227a1?d=identicon&s=25 Will Jessup (wjessup)
on 2006-04-16 22:00
Yea, one thing at a time. Still looking how to optionally not send that
parameter.
59ea1b450935b9d70abfec4186b7a4d5?d=identicon&s=25 Jeff Coleman (progressions)
on 2006-04-17 00:15
Will Jessup wrote:
> Yea, one thing at a time. Still looking how to optionally not send that
> parameter.

One simple way:

if params[:category] && params[:category] == "all"
  @item_pages, @items = paginate: items
else
  @item_pages, @items = paginate :items, :conditions => "category_id =
#{params[:category]}"
end

You'd need to amend your view so that one of the table headings includes
the category parameter "all":

<td><%= link_to 'all', :action => 'list', :category => 'all' %></td>

I'd recommend using the parameter name "category" instead of
"condition", since it's more descriptive--you're sending the name of a
category, so params[:category] would describe that perfectly.

Jeff Coleman
59ea1b450935b9d70abfec4186b7a4d5?d=identicon&s=25 Jeff Coleman (progressions)
on 2006-04-17 00:16
Jeff Coleman wrote:
> Will Jessup wrote:
>> Yea, one thing at a time. Still looking how to optionally not send that
>> parameter.
>
> One simple way:
>
> if params[:category] && params[:category] == "all"
>   @item_pages, @items = paginate: items
> else
>   @item_pages, @items = paginate :items, :conditions => "category_id =
> #{params[:category]}"
> end
>
> You'd need to amend your view so that one of the table headings includes
> the category parameter "all":
>
> <td><%= link_to 'all', :action => 'list', :category => 'all' %></td>
>
> I'd recommend using the parameter name "category" instead of
> "condition", since it's more descriptive--you're sending the name of a
> category, so params[:category] would describe that perfectly.
>
> Jeff Coleman

And yes, the previous poster was exactly right about SQL
injection--definitely use the form of the statement he recommended.

@item_pages, @items = paginate :items, :conditions => ["category_id =
?", params[:category_id]]

Jeff
81a3fc5c10a93d34ee0f25eb1e4227a1?d=identicon&s=25 Will Jessup (wjessup)
on 2006-04-17 07:27
Jeff,

Thanks a bunch. I knew about the SQL injection (read it in the API later
that night after I posted)

I wasn't sure that its OK to put that logic in the controller, so that
is fine?

THanks!
59ea1b450935b9d70abfec4186b7a4d5?d=identicon&s=25 Jeff Coleman (progressions)
on 2006-04-17 08:21
Will Jessup wrote:
> Jeff,
>
> Thanks a bunch. I knew about the SQL injection (read it in the API later
> that night after I posted)
>
> I wasn't sure that its OK to put that logic in the controller, so that
> is fine?
>
> THanks!

That kind of logic is exactly right for the controller.  Hope it works
out!

Jeff Coleman
D9094da56083f39d42f3675403d65831?d=identicon&s=25 will (Guest)
on 2006-04-28 02:44
Jeff, What about this.

I now have my paginator attached to a sorter.

  def list
    @sorter     = SortingHelper::Sorter.new self, %w(id name created_on
category_id), @params['sort'], @params['order'], 'id', 'ASC'
    @pages      = Paginator.new self, Item.count, 10, @params['page']
    if (params[:category])
       @items      = Item.find(:all, :conditions => [ "items.category_id
= ?", params[:category]]), @sorter.to_sql, @pages.current.to_sql
    else
       @items      = Item.find_all nil, @sorter.to_sql,
@pages.current.to_sql
    end
  end

This doesn't return any results, it seems. I get an error on the next
page when trying to call <%= item.name %> says 'name' doesn't exist.

How can i check how many results are returned? (like mysql_num_rows)?
This topic is locked and can not be replied to.