From 47276ce4ad93da84ba0ef00408adeb2ad33e0cde Mon Sep 17 00:00:00 2001 From: Colin Kuskie Date: Tue, 12 Jan 2010 11:07:43 -0800 Subject: [PATCH] Fix exception handling by the NotifyAboutLowStock workflow activity. Fixes bug #11343. --- docs/changelog/7.x.x.txt | 1 + .../Workflow/Activity/NotifyAboutLowStock.pm | 13 ++- t/Workflow/Activity/NotifyAboutLowStock.t | 79 ++++++++++++++++--- 3 files changed, 80 insertions(+), 13 deletions(-) diff --git a/docs/changelog/7.x.x.txt b/docs/changelog/7.x.x.txt index 53802dcc4..52bdcd6a5 100644 --- a/docs/changelog/7.x.x.txt +++ b/docs/changelog/7.x.x.txt @@ -2,6 +2,7 @@ - fixed #11332: Pagination in webgui.org forum urls - fixed #11341: tmpl_var forum.threads - fixed #11337: Parameters persist + - fixed #11343: Notify About Low Stock workflow activity not sending out emails 7.8.9 - fixed #11235: wiki search diff --git a/lib/WebGUI/Workflow/Activity/NotifyAboutLowStock.pm b/lib/WebGUI/Workflow/Activity/NotifyAboutLowStock.pm index 77255ec17..ab6dd8eb6 100644 --- a/lib/WebGUI/Workflow/Activity/NotifyAboutLowStock.pm +++ b/lib/WebGUI/Workflow/Activity/NotifyAboutLowStock.pm @@ -91,14 +91,21 @@ See WebGUI::Workflow::Activity::execute() for details. sub execute { my ($self, undef, $instance) = @_; + my $session = $self->session; my $message = $instance->getScratch('LowStockMessage') || ''; my $counter = $instance->getScratch('LowStockLast') || 0; my $belowThreshold = $instance->getScratch('LowStockBelow') || 0; - my $productIterator = WebGUI::Asset::Sku::Product->getIsa($self->session, $counter); + my $productIterator = WebGUI::Asset::Sku::Product->getIsa($session, $counter); my $warningLimit = $self->get('warningLimit'); my $finishTime = time() + $self->getTTL; my $expired = 0; - PRODUCT: while (my $product = $productIterator->()) { + PRODUCT: while (1) { + my $product = eval { $productIterator->() }; + if (my $e = Exception::Class->caught()) { + $session->log->error($@); + next PRODUCT; + } + last PRODUCT unless $product; VARIANT: foreach my $collateral ( @{ $product->getAllCollateral('variantsJSON') }) { if ($collateral->{quantity} <= $warningLimit) { ##Build message @@ -126,7 +133,7 @@ sub execute { $instance->deleteScratch('LowStockLast'); $instance->deleteScratch('LowStockBelow'); if ($belowThreshold) { - my $inbox = WebGUI::Inbox->new($self->session); + my $inbox = WebGUI::Inbox->new($session); $inbox->addMessage({ status => 'unread', subject => $self->get('subject'), diff --git a/t/Workflow/Activity/NotifyAboutLowStock.t b/t/Workflow/Activity/NotifyAboutLowStock.t index 39b564f65..1361d57bb 100644 --- a/t/Workflow/Activity/NotifyAboutLowStock.t +++ b/t/Workflow/Activity/NotifyAboutLowStock.t @@ -20,9 +20,10 @@ use WebGUI::Inbox; use Data::Dumper; use Test::More; +use Test::Exception; use URI; -plan tests => 15; # increment this value for each test you create +plan tests => 20; # increment this value for each test you create my $session = WebGUI::Test->session; $session->user({userId => 3}); @@ -35,7 +36,7 @@ my $posters = $import->addChild({ className => 'WebGUI::Asset::Sku::Product', url => 'cell_posters', title => "Red's Posters", -}, undef, undef, { skipAutoCommitWorkflows => 1, }); +}, undef, time()-15, { skipAutoCommitWorkflows => 1, }); my $versionTag = WebGUI::VersionTag->getWorking($session); $versionTag->commit(); @@ -67,7 +68,7 @@ my $marilynVarId = $posters->setCollateral('variantsJSON', 'variantId', 'new', varSku => 'subway-skirt', price => 50, weight => 1, - quantity => 5, + quantity => 10, }, ); @@ -83,8 +84,8 @@ addToCleanup($workflow); my $threshold = $workflow->addActivity('WebGUI::Workflow::Activity::NotifyAboutLowStock'); $threshold->set('className' , 'WebGUI::Activity::NotifyAboutLowStock'); $threshold->set('toGroup' , 3); -$threshold->set('subject' , 'Threshold=10'); -$threshold->set('warningLimit' , 10); +$threshold->set('subject' , 'Threshold=15'); +$threshold->set('warningLimit' , 15); my $instance1 = WebGUI::Workflow::Instance->create($session, { @@ -109,7 +110,7 @@ is(scalar @{$messages}, 1, 'Received one message'); my $message = $messages->[0]; my $body = $message->get('message'); -is($message->get('subject'), 'Threshold=10', 'Message has the right subject'); +is($message->get('subject'), 'Threshold=15', 'Message has the right subject'); my @urls = split /\n/, $body; is (scalar @urls, 1, 'Only one variant is below the threshold'); my $url = pop @urls; @@ -140,14 +141,72 @@ is(scalar @{$inbox->getMessagesForUser($admin)}, 0, 'No messages sent since thre $message = $inbox->getMessagesForUser($admin)->[0]; +note "Test that the workflow does not die when encountering bad assets"; + +my $otherPosters = $posters->duplicate; +my $movie_posters = $import->addChild({ + className => 'WebGUI::Asset::Sku::Product', + url => 'movie_posters', + title => "Movie Posters", +}, undef, undef, { skipAutoCommitWorkflows => 1, }); + +my $movieVarId = $movie_posters->setCollateral('variantsJSON', 'variantId', 'new', + { + shortdesc => 'Shawshank Redemption', + varSku => 'shawshank-1', + price => 10, + weight => 1, + quantity => 5, + }, +); +my $otherTag = WebGUI::VersionTag->getWorking($session); +addToCleanup($otherTag); +$otherTag->commit; + +$threshold->set('warningLimit' , 10); +my $instance3 = WebGUI::Workflow::Instance->create($session, + { + workflowId => $workflow->getId, + skipSpectreNotification => 1, + } +); + +$retVal = $instance3->run(); +$retVal = $instance3->run(); +is($retVal, 'done', 'Workflow is done'); + +$messages = $inbox->getMessagesForUser($admin); +is(scalar @{$messages}, 1, 'Received one message'); +wipeMessages($inbox, $admin); + +my $instance4 = WebGUI::Workflow::Instance->create($session, + { + workflowId => $workflow->getId, + skipSpectreNotification => 1, + } +); +#break the asset +$session->db->write('delete from asset where assetId=?', [$otherPosters->getId]); +is(WebGUI::Asset->new($session, $otherPosters->getId), undef, 'middle asset broken'); + +$retVal = $instance4->run(); +$retVal = $instance4->run(); +is($retVal, 'done', 'Workflow is done'); + +$messages = $inbox->getMessagesForUser($admin); +is(scalar @{$messages}, 1, 'Still received one message'); +wipeMessages($inbox, $admin); + END { - $workflow->delete; - $posters->purge; - my $i = 0; wipeMessages($inbox, $admin); $messages = $inbox->getMessagesForUser($admin); is(scalar @{$messages}, 0, 'Inbox cleaned up'); - $session->db->write("delete from mailQueue where message like '%Threshold=10%'"); + $session->db->write("delete from mailQueue where message like '%Threshold=15%'"); + $session->db->write("delete from asset where assetId=?",[$otherPosters->getId]); + $session->db->write("delete from assetData where assetId=?",[$otherPosters->getId]); + $session->db->write("delete from sku where assetId=?",[$otherPosters->getId]); + $session->db->write("delete from Product where assetId=?",[$otherPosters->getId]); + $session->db->write("delete from assetIndex where assetId=?",[$otherPosters->getId]); } sub wipeMessages {