Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Carry over pagination info on map #232

Open
wants to merge 2 commits into from

3 participants

@mmangino

This change will make calls to map on a paginated collection return paginated info. This can be really handy if you return a paginated list of OrderItems and want to map that to a list of Products in your controller. Before, you had to map and create a new collection. This makes it just work.

@mmangino

It turns out this isn't a great solution since some operations like [] with a range will lose the pagination. I'm going to try to work that out as well.

@mmangino

This modified code handles the case when an array is copied via C.

@midas

What is the status of this pull? It would be a great addition. It is kind of a pain if you need to map the models in your collection into a presenter, etc.

@mislav
Owner

Nice hack. However in Active Record, paginate returns a Relation which is a lazy-loading Array. Calling map on that won't result in the same behavior as you've added. Could you look into that?

I would be only interested in this feature if it was consistent. Still not completely sold, however. Wondering if it's too much magic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 15, 2012
  1. @mmangino
Commits on Mar 21, 2012
  1. @mmangino

    Handle copied arrays

    mmangino authored
This page is out of date. Refresh to see the latest.
Showing with 37 additions and 0 deletions.
  1. +16 −0 lib/will_paginate/collection.rb
  2. +21 −0 spec/collection_spec.rb
View
16 lib/will_paginate/collection.rb
@@ -132,5 +132,21 @@ def replace(array)
result
end
+
+ # We want to return paginated collections when mapped. Unfortunately
+ # when arrays are copied in C code for slices or other operations,
+ # they are initialized using the array initializer, not ours. This
+ # means that current page is lost and creating a new array is impossible.
+ # we treat copied arrays as non-paginated collections and just delegate to super.
+
+ def map(&block)
+ if current_page.nil?
+ super(&block)
+ else
+ self.class.create(current_page, per_page, total_entries) do |pager|
+ pager.replace(super(&block))
+ end
+ end
+ end
end
end
View
21 spec/collection_spec.rb
@@ -123,6 +123,27 @@
collection.per_page.should == 30
end
+ describe "map" do
+ it "should correctly map the values" do
+ collection = create {|p| p.replace([1,2,3])}
+ collection.map {|v| v*2}.should == [2,4,6]
+ end
+
+ it "should maintain pager methods" do
+ collection = create(2,5,100) {|p| p.replace([1,2,3])}
+ mapped = collection.map {|v| v*2 }
+ mapped.current_page.should == 2
+ mapped.per_page.should == 5
+ mapped.total_entries.should == 100
+ end
+
+ it "should not raise an error when the array is copied by C code" do
+ collection = create(2,5,100) {|p| p.replace([1,2,3])}
+ copied = collection[0..1]
+ copied.map {|a| a*2}
+ end
+ end
+
private
def create(page = 2, limit = 5, total = nil, &block)
Something went wrong with that request. Please try again.