fix: getIsa returns undef for pending assets.

Add a new option flag to getIsa, called returnAll. If returnAll = 1, then assets are returned regardless of status. Otherwise, getIsa will only return committed assets.
plainblack · Dec 9, 2009 · 27a7c09 · 27a7c09
1 parent d1e6f30
commit 27a7c09
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 10 deletions.
diff --git a/docs/changelog/7.x.x.txt b/docs/changelog/7.x.x.txt
@@ -4,6 +4,7 @@
  - fixed #11286: Workflow Instance deleted when reaching an Activity that fails to load
  - fixed #11296: listLDAPLinks op permissions problems
  - fixed #11294: Matrix - Best/Worse Rated Calculations
+ - fixed getIsa returns undef for pending assets
 
 7.8.7
  - fixed #11278: Wrong test for Template::Toolkit in testEnvironment.pl

diff --git a/lib/WebGUI/Asset.pm b/lib/WebGUI/Asset.pm
@@ -1224,7 +1224,7 @@ sub getImportNode {
 
 =head2 getIsa ( $session, [ $offset ] )
 
-A class method to return an iterator for getting all Assets by class (and all sub-classes)
+A class method to return an iterator for getting all committed Assets by class (and all sub-classes)
 as Asset objects, one at a time.  When the end of the assets is reached, then the iterator
 will close the database handle that it uses and return undef.
 
@@ -1235,6 +1235,10 @@ while (my $product = $productIterator->()) {
   ##Do something useful with $product
 }
 
+If the iterator cannot instanciate an asset, it will not return undef.  Instead, it
+will throw an exception.  This allows the error condition to be distinguished
+from the end of the set of assets.
+
 =head3 $session
 
 A reference to a WebGUI::Session object.
@@ -1244,26 +1248,44 @@ A reference to a WebGUI::Session object.
 An offset, from the beginning of the results returned from the query, to really begin
 returning results.  This allows very large sets of results to be handled in chunks.
 
+=head3 $options
+
+A hashref of options to change how getIsa works.
+
+=head4 returnAll
+
+If set to true, then all assets will be returned, regardless of status and state.
+
 =cut
 
 sub getIsa {
     my $class    = shift;
     my $session  = shift;
     my $offset   = shift;
+    my $options  = shift;
     my $def = $class->definition($session);
     my $tableName = $def->[0]->{tableName};
-    my $sql = "select distinct(assetId) from $tableName";
+    #Strategy, generate the correct set of assetIds
+    my $sql = "select assetId from assetData as ad ";
+    if ($tableName ne 'assetData') {
+        $sql .= "join `$tableName` using (assetId, revisionDate) ";
+    }
+    $sql .= 'WHERE ';
+    if (! $options->{returnAll}) {
+        $sql .= q{(status='approved' OR status='archived') AND };
+    }
+    $sql .= q{revisionDate = (SELECT MAX(revisionDate) FROM assetData AS a WHERE a.assetId = ad.assetId) };
     if (defined $offset) {
-        $sql .= ' LIMIT '. $offset . ',1234567890';
+        $sql .= 'LIMIT '. $offset . ',1234567890 ';
     }
-    my $sth = $session->db->read($sql);
+    my $sth    = $session->db->read($sql);
     return sub {
-        my ($assetId) = $sth->array;
+        my ($assetId, $revisionDate) = $sth->array;
         if (!$assetId) {
             $sth->finish;
             return undef;
         }
-        my $asset = WebGUI::Asset->newByDynamicClass($session, $assetId);
+        my $asset = eval { WebGUI::Asset->newPending($session, $assetId); };
         if (!$asset) {
             WebGUI::Error::ObjectNotFound->throw(id => $assetId);
         }

diff --git a/t/Asset/Asset.t b/t/Asset/Asset.t
@@ -152,7 +152,7 @@ $canViewMaker->prepare(
     },
 );
 
-plan tests => 126
+plan tests => 127
             + scalar(@fixIdTests)
             + scalar(@fixTitleTests)
             + 2*scalar(@getTitleTests) #same tests used for getTitle and getMenuTitle
@@ -707,9 +707,14 @@ isnt( $rootAsset->get('title'), $funkyTitle, 'get returns a safe copy of the Ass
 #
 ################################################################
 my $node = WebGUI::Asset->getRoot($session);
-my $product1 = $node->addChild({ className => 'WebGUI::Asset::Sku::Product'});
-my $product2 = $node->addChild({ className => 'WebGUI::Asset::Sku::Product'});
-my $product3 = $node->addChild({ className => 'WebGUI::Asset::Sku::Product'});
+my $product1 = $node->addChild({ className => 'WebGUI::Asset::Sku::Product'}, undef, undef, { skipAutoCommitWorkflows => 1});
+my $product2 = $node->addChild({ className => 'WebGUI::Asset::Sku::Product'}, undef, undef, { skipAutoCommitWorkflows => 1});
+my $product3 = $node->addChild({ className => 'WebGUI::Asset::Sku::Product'}, undef, undef, { skipAutoCommitWorkflows => 1});
+my $pTag = WebGUI::VersionTag->getWorking($session);
+$pTag->commit;
+addToCleanup($pTag);
+my $product4 = $node->addChild({ className => 'WebGUI::Asset::Sku::Product'}, undef, undef, { skipAutoCommitWorkflows => 1});
+addToCleanup($product4);
 
 my $getAProduct = WebGUI::Asset::Sku::Product->getIsa($session);
 isa_ok($getAProduct, 'CODE', 'getIsa returns a sub ref');
@@ -732,9 +737,17 @@ while( my $sku = $getASku->()) {
 is($counter, 3, 'getIsa: returned only 3 Products for a parent class');
 cmp_bag($skuIds, [$product1->getId, $product2->getId, $product3->getId], 'getIsa returned the correct 3 products for a parent class');
 
+my $getAnotherSku = WebGUI::Asset::Sku->getIsa($session, 0, { returnAll => 1, });
+$counter = 0;
+while( my $sku = $getAnotherSku->()) {
+    ++$counter;
+}
+is($counter, 4, 'getIsa: returned all 4 skus with returnAll => 1');
+
 $product1->purge;
 $product2->purge;
 $product3->purge;
+$product4->purge;
 
 ################################################################
 #