Add pagination to the DataForm list view. Fixes bug #10989.

Adds new code to Paginator, to handle paginating with an iterator.

docs/changelog/7.x.x.txt

@@ -17,6 +17,7 @@
  - fixed #11107: linked image with caption
  - fixed #10914: Shop: No email notifications sent when the cart has net value 0
  - fixed #11126: WebGUI database has varchar fields
+ - fixed #10989: DataForm List: No pagination
 
 7.8.1
  - mark $session->datetime->time as deprecated and remove its use from core code

lib/WebGUI/Asset/Wobject/DataForm.pm

@@ -31,6 +31,7 @@ use WebGUI::Utility;
 use WebGUI::Group;
 use WebGUI::AssetCollateral::DataForm::Entry;
 use WebGUI::Form::SelectRichEditor;
+use WebGUI::Paginator;
 use JSON ();
 
 our @ISA = qw(WebGUI::Asset::Wobject);
@@ -697,9 +698,10 @@ A hash reference.  New template variables will be appended to it.
 =cut
 
 sub getListTemplateVars {
-	my $self = shift;
+	my $self    = shift;
-	my $var = shift;
+    my $session = $self->session;
-	my $i18n = WebGUI::International->new($self->session,"Asset_DataForm");
+	my $var     = shift;
+	my $i18n    = WebGUI::International->new($session,"Asset_DataForm");
 	$var->{"back.url"} = $self->getFormUrl;
 	$var->{"back.label"} = $i18n->get('go to form');
     my $fieldConfig = $self->getFieldConfig;
@@ -713,7 +715,9 @@ sub getListTemplateVars {
     } @{ $self->getFieldOrder };
     $var->{field_loop} = \@fieldLoop;
     my @recordLoop;
-    my $entryIter = $self->entryClass->iterateAll($self);
+    my $p = WebGUI::Paginator->new($session);
+    $p->setDataByCallback(sub { return $self->entryClass->iterateAll($self, { offset => $_[0], limit => $_[1], }); });
+    my $entryIter = $p->getPageIterator();
     while ( my $entry = $entryIter->() ) {
         my $entryData = $entry->fields;
         my @dataLoop;
@@ -734,9 +738,9 @@ sub getListTemplateVars {
             %dataVars,
             "record.ipAddress"              => $entry->ipAddress,
             "record.edit.url"               => $self->getFormUrl("func=view;entryId=".$entry->getId),
-            "record.edit.icon"              => $self->session->icon->edit("func=view;entryId=".$entry->getId, $self->get('url')),
+            "record.edit.icon"              => $session->icon->edit("func=view;entryId=".$entry->getId, $self->get('url')),
             "record.delete.url"             => $self->getUrl("func=deleteEntry;entryId=".$entry->getId),
-            "record.delete.icon"            => $self->session->icon->delete("func=deleteEntry;entryId=".$entry->getId, $self->get('url'), $i18n->get('Delete entry confirmation')),
+            "record.delete.icon"            => $session->icon->delete("func=deleteEntry;entryId=".$entry->getId, $self->get('url'), $i18n->get('Delete entry confirmation')),
             "record.username"               => $entry->username,
             "record.userId"                 => $entry->userId,
             "record.submissionDate.epoch"   => $entry->submissionDate->epoch,
@@ -746,6 +750,7 @@ sub getListTemplateVars {
         };
     }
     $var->{record_loop} = \@recordLoop;
+    $p->appendTemplateVars($var);
     return $var;
 }
 

lib/WebGUI/AssetCollateral/DataForm/Entry.pm

@@ -207,7 +207,7 @@ sub getId {
 
 #-------------------------------------------------------------------
 
-=head2 iterateAll ( $asset )
+=head2 iterateAll ( $asset, [ $options ] )
 
 This class method returns an iterator set to iterate over all entries for a Dataform.
 
@@ -215,13 +215,33 @@ This class method returns an iterator set to iterate over all entries for a Data
 
 A reference to a Dataform object.
 
+=head3 $options
+
+A hashreference of options.
+
+=head4 offset
+
+The record number to start the iterator at.  Defaults to 0 if not set.
+
+=head4 limit
+
+The number of records for the iterator to return.  Defaults to a very large number if not set.
+
 =cut
 
 sub iterateAll {
-    my $class = shift;
+    my $class   = shift;
-    my $asset = shift;
+    my $asset   = shift;
-    my $sth = $asset->session->dbSlave->read("SELECT `DataForm_entryId`, `userId`, `username`, `ipAddress`, `submissionDate`, `entryData` FROM `DataForm_entry` WHERE `assetId` = ? ORDER BY `submissionDate` DESC", [$asset->getId]);
+    my $options = shift;
-    my $sub = sub {
+    my $sql = "SELECT SQL_CALC_FOUND_ROWS `DataForm_entryId`, `userId`, `username`, `ipAddress`, `submissionDate`, `entryData` FROM `DataForm_entry` WHERE `assetId` = ? ORDER BY `submissionDate` DESC LIMIT ?,?";
+    my $placeHolders = [ $asset->getId ];
+    push @{ $placeHolders }, exists $options->{offset} ? $options->{offset} : 0;
+    push @{ $placeHolders }, exists $options->{limit}  ? $options->{limit}  : 1234567890;
+    my $slave   = $asset->session->dbSlave;  ##Use the same slave to calculate the number of rows
+    my $sth     = $slave->read($sql, $placeHolders);
+    my $allRows = $slave->quickScalar('SELECT FOUND_ROWS()');
+    my $sub   = sub {
+        return $allRows if $_[0] eq 'rowCount';
         if (defined wantarray) {
             my $properties = $sth->hashRef;
             if ($properties) {

lib/WebGUI/Help/Asset_DataForm.pm

@@ -141,6 +141,9 @@ our $HELP = {
             {   namespace => "Asset_Template",
                 tag       => "template variables"
             },
+            {   namespace => "WebGUI",
+                tag       => "pagination template variables"
+            },
         ],
         fields    => [],
         variables => [

lib/WebGUI/Paginator.pm

@@ -63,7 +63,7 @@ Private method which retrieves a data set from a database and replaces whatever
 
 This method should only ever be called by the public setDataByQuery method and is only called in the case that dynamicPageNumberKey is set.
 
-The public setDataByQuery method is not capable of efficiently handling requests that dyncmically set the page number by value
+The public setDataByQuery method is not capable of efficiently handling requests that dynamically set the page number by value
 due to the fact that only one page of results is ever returned.   In this method, all the results are returned making this possible.
 
 =head3 query
@@ -405,7 +405,7 @@ sub getPage {
 
 =head2 getPageData ( [ pageNumber ] )
 
-Returns the data from the page specified as an array reference.
+Returns the data from the specified page as an array reference.
 
 =head3 pageNumber
 
@@ -414,40 +414,41 @@ Defaults to the page you're currently viewing. This is mostly here as an overrid
 =cut
 
 sub getPageData {
-	my $self = shift;
+    my $self       = shift;
-        my $pageNumber = shift || $self->getPageNumber;
+    my $pageNumber = shift || $self->getPageNumber;
-    my $allRows = $self->{_rowRef};
+    my $allRows    = $self->{_rowRef};
-    
+
-        my $pageCount = $self->getNumberOfPages;
+    my $pageCount = $self->getNumberOfPages;
-        return [] if ($pageNumber > $pageCount);
+    return [] if ($pageNumber > $pageCount);
 
     if($self->{_setByQuery}) {
         #Return the cached page
         return $allRows if($pageNumber == $self->getPageNumber);
         return [];
-	}
+    }
 
-   #Handle setByArrayRef or the old setDataByQuery method
+    #Handle setByArrayRef or the old setDataByQuery method
-	my @pageRows = ();
+    my @pageRows = ();
     my $rowsPerPage = $self->{_rpp};
     my $pageStartRow = ($pageNumber*$rowsPerPage)-$rowsPerPage;
     my $pageEndRow = $pageNumber*$rowsPerPage;
     for (my $i=$pageStartRow; $i<$pageEndRow; $i++) {
-	   $pageRows[$i-$pageStartRow] = $allRows->[$i] if ($i <= $#{$self->{_rowRef}});
+       $pageRows[$i-$pageStartRow] = $allRows->[$i] if ($i <= $#{$self->{_rowRef}});
-        }
+    }
-	return \@pageRows;
+    return \@pageRows;
 }
 
 #-------------------------------------------------------------------
 
-=head2 getPageNumber ( )
+=head2 getPageIterator (  )
 
-Returns the current page number. If no page number can be found then it returns 1.
+Returns the iterator that was created by setDataByCallback
 
 =cut
 
-sub getPageNumber {
+sub getPageIterator {
-        return $_[0]->{_pn};
+    my $self = shift;
+    return $self->{_iteratorObj};
 }
 
 #-------------------------------------------------------------------
@@ -458,7 +459,7 @@ Returns links to all pages in this paginator.
 
 =head3 limit
 
-An integer representing the maximum number of page links to return. Defaultly all page links will be returned.
+An integer representing the maximum number of page links to return. By default, all page links will be returned.
 
 =cut
 
@@ -523,6 +524,18 @@ sub getPageLinks {
 }
 
 
+#-------------------------------------------------------------------
+
+=head2 getPageNumber ( )
+
+Returns the current page number. If no page number can be found then it returns 1.
+
+=cut
+
+sub getPageNumber {
+        return $_[0]->{_pn};
+}
+
 #-------------------------------------------------------------------
 
 =head2 getPreviousPageLink ( )
@@ -589,13 +602,13 @@ By default the page number will be determined by looking at $self->session->form
 =cut
 
 sub new {
-	my $class = shift;
+    my $class       = shift;
-	my $session = shift;
+    my $session     = shift;
-	my $currentURL = shift;
+    my $currentURL  = shift;
-	my $rowsPerPage = shift || 25;
+    my $rowsPerPage = shift || 25;
-	my $formVar = shift || "pn";
+    my $formVar     = shift || "pn";
-	my $pn = shift || $session->form->process($formVar) || 1;
+    my $pn          = shift || $session->form->process($formVar) || 1;
-        bless {_session=>$session, _url => $currentURL, _rpp => $rowsPerPage, _formVar => $formVar, _pn => $pn}, $class;
+    bless {_session=>$session, _url => $currentURL, _rpp => $rowsPerPage, _formVar => $formVar, _pn => $pn}, $class;
 }
 
 #-------------------------------------------------------------------
@@ -651,6 +664,42 @@ sub setDataByArrayRef {
 }
 
 
+#-------------------------------------------------------------------
+
+=head2 setDataByCallback ( callback )
+
+Provide the paginator with data by giving it a callback.  This interface does not support
+having alphabetical keys ala C<setAlphabeticalKey> because the data is never stored in
+the Paginator object.
+
+=head3 callback
+
+A callback to invoke that returns an iterator.  The callback method should
+accept two optional parameters, an offset to start, and the rows per page
+to return.  The iterator should return the total number of rows in
+the query, without limits, when the first argument it is passed is 'rowCount'.
+
+=cut
+
+sub setDataByCallback {
+	my $self     = shift;
+	my $callback = shift;
+
+    my $pageNumber  = $self->getPageNumber;
+    my $rowsPerPage = $self->{_rpp};
+    my $start       = ( ($pageNumber - 1) * $rowsPerPage );
+
+    my $obj = $callback->($start, $rowsPerPage);
+    $self->{_totalRows} = $obj->('rowCount');
+
+    $self->{_iteratorObj}   = $obj;
+    $self->{_setByQuery}    = 0;
+    $self->{_setByArrayRef} = 0;
+    $self->{_setByCallback} = 1;
+    return '';
+}
+
+
 #-------------------------------------------------------------------
 
 =head2 setDataByQuery ( query [, dbh, unconditional, placeholders, dynamicPageNumberKey, dynamicPageNumberValue ] )

t/Asset/Wobject/DataForm/viewList.t

@@ -29,7 +29,7 @@ my $df = WebGUI::Asset->getImportNode($session)->addChild( {
     className => 'WebGUI::Asset::Wobject::DataForm', 
 } );
 
-WebGUI::Test->tagsToRollback( WebGUI::VersionTag->getWorking( $session ) );
+addToCleanup( WebGUI::VersionTag->getWorking( $session ) );
 
 # Add fields to the dataform
 $df->createField( "name",    { type => "text", } );
@@ -49,18 +49,20 @@ my @entryProperties = (
     }
 );
 
+my $birthday = WebGUI::Test->webguiBirthday;
+
 my @entries = ();
 for my $properties (@entryProperties) {
     my $entry = $df->entryClass->newFromHash( $df, $properties );
+    $entry->submissionDate(WebGUI::DateTime->new($session, $birthday++));
     $entry->save;
     push @entries, $entry;
-    sleep 1;
 }
 
 #----------------------------------------------------------------------------
 # Tests
 
-plan tests => 2;    # Increment this number for each test you create
+plan tests => 6;    # Increment this number for each test you create
 
 #----------------------------------------------------------------------------
 # Test getListTemplateVars
@@ -184,8 +186,63 @@ cmp_deeply(
     'getListTemplateVars is complete and correct',
 );
 
+is($tmplVar->{'pagination.pageCount'}, 1, '... and has pagination variables');
 
-#----------------------------------------------------------------------------
+#-------------------------------------
-# Cleanup
+#Shove in a bunch of data to test pagination
+
+my @quoteDb = (
+    { name => "Red",    message => "That tall drink of water", },
+    { name => "Norton", message => "Do you enjoy working in the laundry?", },
+    { name => "Andy",   message => "They say it has no memory", },
+    { name => "Boggs",  message => "Hey, we all need friends in here", },
+    { name => "Andy",   message => "It's my life.  Don't you understand?", },
+
+    { name => "Red",    message => "Rehabilitated?  Well, now let me see.", },
+    { name => "Red",    message => "I know what *you* think it means, sonny.", },
+    { name => "Red",    message => "I know what *you* think it means, sonny.", },
+    { name => "Andy",   message => "How can you be so obtuse?", },
+    { name => "Red",    message => "The man likes to play chess; let's get him some rocks. ", },
+
+    { name => "Brooks", message => "Easy peasy japanesey.", },
+    { name => "Hadley", message => "What is your malfunction?", },
+    { name => "Red",    message => "Hope is a dangerous thing. Hope can drive a man insane. ", },
+    { name => "Red",    message => "They send you here for life, and that's exactly what they take.", },
+    { name => "Red",    message => "Truth is, I don't want to know. Some things are best left unsaid.", },
+
+    { name => "Andy",   message => "That's the beauty of music.", },
+    { name => "Red",    message => "I played a mean harmonica as a younger man.", },
+    { name => "Tommy",  message => "I don't read so good.", },
+    { name => "Andy",   message => "You don't read so *well*.", },
+    { name => "Red",    message => "Murder, same as you.", },
+
+    { name => "Norton", message => "Salvation lies within.", },
+    { name => "Andy",   message => "Remember Red, hope is a good thing.", },
+    { name => "Hadley", message => "Drink up while it's cold, ladies.", },
+    { name => "Red",    message => "We sat and drank with the sun on our shoulders and felt like free men.", },
+    { name => "Andy",   message => "You see that's tax deductible, you can write that off. ", },
+
+    { name => "Norton", message => "Lord! It's a miracle!", },
+    { name => "Red",    message => "I don't have her stuffed down the front of my pants right now, I'm sorry to say, but I'll get her.", },
+    { name => "Andy",   message => "Get busy living, or get busy dying.", },
+    { name => "Brooks", message => "The world went and got itself in a big damn hurry.", },
+    { name => "Andy",   message => "Everybody's innocent in here. Didn't you know that?", },
+
+);
+
+for my $quote (@quoteDb) {
+    my $entry = $df->entryClass->newFromHash( $df, $quote );
+    $entry->submissionDate(WebGUI::DateTime->new($session, $birthday++));
+    $entry->save;
+    push @entries, $entry;
+}
+
+$tmplVar = $df->getListTemplateVars({});
+is @{ $tmplVar->{record_loop} }, 25, 'list variables are paginated';
+ok $tmplVar->{'pagination.pageCount.isMultiple'}, 'pagination: has multiple pages';
+
+$session->request->setup_body({ pn => 2, });
+$tmplVar = $df->getListTemplateVars({});
+is @{ $tmplVar->{record_loop} }, 7, '7 entries in the 2nd page';
 
 #vim:ft=perl

t/Paginator.t

@@ -25,7 +25,7 @@ use Data::Dumper;
 
 my $session = WebGUI::Test->session;
 
-plan tests => 26; # increment this value for each test you create
+plan tests => 32; # increment this value for each test you create
 
 my $startingRowNum =   0;
 my $endingRowNum   =  99;
@@ -107,6 +107,12 @@ is('100', $p->getPage(5), '(101) page 5 stringification okay');
 
 is($p->getPageNumber, 1, 'Default page number is 1'); ##Additional page numbers are specified at instantiation
 
+########################################################################
+#
+# getPageLinks with limits
+#
+########################################################################
+
 $expectedPages = [  map { +{ 
             'pagination.text'   => ( $_ + 1 ), 
             'pagination.range'  => ( 25 * $_ + 1 ) . "-" . ( $_ * 25 + 25 <= $endingRowNum + 1 ? $_ * 25 + 25 : $endingRowNum + 1 ), # First row number - Last row number
@@ -115,12 +121,6 @@ $expectedPages = [  map { +{
 
 $expectedPages->[0]->{'pagination.activePage'} = 'true';
 
-########################################################################
-#
-# getPageLinks with limits
-#
-########################################################################
-
 cmp_deeply(
     ($p->getPageLinks)[0], 
     $expectedPages,
@@ -217,3 +217,65 @@ cmp_deeply(
     \@pageWindow,
     'set of last 10 pages selected correctly, (20/20)',
 );
+
+########################################################################
+#
+# iterator based paginator
+#
+########################################################################
+
+my $callback = sub {
+    my ($start, $rowsPerPage) = @_;
+    my $state   = $start * 2;
+    my $counter = 0;
+    my $iterator = sub {
+        return 50 if $_[0] eq 'rowCount';
+        return if ($counter >= $rowsPerPage);
+        $state += 2;
+        ++$counter;
+        return if $state > 50;
+        return $state;
+    };
+    return $iterator;
+};
+
+my $p1 = WebGUI::Paginator->new($session, '/neighborhood', 5);
+$p1->setDataByCallback($callback);
+my $pIterator = $p1->getPageIterator;
+isa_ok($pIterator, 'CODE', 'getPageIterator');
+is($pIterator->('rowCount'), 50, 'generated iterator returns the correct maximum number of rows');
+is($p1->getNumberOfPages, 10, 'getNumberOfPages works with an iterator');
+
+cmp_deeply(drainIterator($pIterator, 10), [2, 4, 6, 8, 10], 'setDataByCallback: iterator returned page 1 data');
+
+$p1->setPageNumber(2);
+$p1->setDataByCallback($callback);
+$pIterator = $p1->getPageIterator;
+
+cmp_deeply(drainIterator($pIterator, 10), [12, 14, 16, 18, 20], '... iterator returned page 2 data');
+
+$expectedPages = [  map { +{ 
+            'pagination.text'   => ( $_ + 1 ), 
+            'pagination.range'  => ( 5 * $_ + 1 ) . "-" . ( ($_+1) * 5 ), # First row number - Last row number
+            'pagination.url'    => ( $_ != 1 ? '/neighborhood' . '?pn=' . ( $_ + 1 ) : '' ), # Current page has no URL
+        } } (0..9) ];
+
+$expectedPages->[1]->{'pagination.activePage'} = 'true';
+
+cmp_deeply(
+    ($p1->getPageLinks())[0],
+    $expectedPages,
+    'getPageLinks works with a paginator'
+);
+
+sub drainIterator {
+    my $iterator      = shift;
+    my $terminalCount = shift; 
+    my $pageData = [];
+    my $infiniteLoopCount = 0;
+    while (defined(my $item = $pIterator->())) {
+        push @{ $pageData }, $item;
+        last if ++$infiniteLoopCount >= $terminalCount;
+    }
+    return $pageData;
+}