[SearchBuilder-devel] [RFC][PATCH] Pages fixes and tests for it functionality

[David Glasser]

On Jul 28, 2005, at 10:27 AM, Jesse Vincent wrote:

> Dave,
>
> How much code is there in "Internal Project" that we could harvest and
> hand to ruslan for the paging work?
>
> Jesse

The code is just:

=head2 pager

Returns a L<Data::Page> object associated with this collection.  This
object defaults to 10 entries per page.  You should use only use
L<Data::Page>  methods on this object to B<get> information about  
paging,
not to B<set> it; use C<set_page_info> to set paging information.

=cut

__PACKAGE__->mk_accessors(qw(pager));

=head2 set_page_info [per_page => NUMBER,] [current_page => NUMBER]

Sets the current page (one-based) and number of items per page on the
pager object, and pulls the number of elements from the collection.
This both sets up the collection's L<Data::Page> object so that you  
can use
its calculations, and sets the L<DBIx::SearchBuilder> C<first_row> and
C<rows_per_page> so that queries return values from the selected page.

=cut

sub set_page_info {
   my $self = shift;
   my %args = (
     per_page => undef,
     current_page => undef, # 1-based
     @_
   );

   $self->pager->total_entries($self->count_all)
               ->entries_per_page($args{'per_page'})
               ->current_page($args{'current_page'});

   $self->rows_per_page($args{'per_page'});
   $self->first_row($self->pager->first);

}



Some notes though:

   * This code expects users to read paging information not through  
an SB call, but through calls like
        $collection->pager->first
     This is good in that it lets us just say "read the Data::Page  
docs for what we can do" and we don't have to write ten methods that  
do nothing but delegate, but on the other hand it means that if we  
want to put our own code wrapping one of the methods we lose.

   * I don't know if my decision to make the only legal *set*  
operation on pagination be a single set-per_page-and-current_page-at- 
the-same-time operation was a good idea.  (And the name  
"set_page_info" totally sucks.)  Probably a better API would be to  
put this logic in "per_page" and "current_page" calls themselves,  
which both shell out to an internal method that does the logic  
above.  And not actually expose "first_row" as part of the public  
API, or at least say "don't use first_row if you're using our  
pagination support"

   * the class here is a Class::Accessor.  We might want to do that  
for Jifty::DBI too, although we might not like having methods named  
get and set show up.

So I wouldn't recommend just throwing this code into Jifty::DBI  
without thinking.

--dave
   Code Monkey, Best Practical Solutions
-- 
David Glasser | glasser at bestpractical.com