Fix bug #11286: Tell the difference between end of Workflow, and unable to load WorkflowActivity.

Tests are added for new methods.  The changes to Workflow::Instance->run are peripherally
tested in Workflow Activity tests.

docs/changelog/7.x.x.txt

@@ -1,6 +1,7 @@
 7.8.8
  - fixed #11289: Gallery with pending version tag causes search engine indexer to puke.
  - fixed #11292: Search function limited to onje search?
+ - fixed #11286: Workflow Instance deleted when reaching an Activity that fails to load
 
 7.8.7
  - fixed #11278: Wrong test for Template::Toolkit in testEnvironment.pl

lib/WebGUI/Workflow.pm

@@ -314,7 +314,8 @@ sub getList {
 
 =head2 getNextActivity ( [ activityId ] )
 
-Returns the next activity in the workflow after the activity specified. If no activity id is specified, then the first workflow will be returned.
+Returns the next activity in the workflow after the activity specified.
+If no activity id is specified, then the first workflow will be returned.
 
 =head3 activityId
 
@@ -323,13 +324,53 @@ The unique id of an activity in this workflow.
 =cut
 
 sub getNextActivity {
-	my $self = shift;
+	my $self       = shift;
+	my $activityId = shift;
+    my $id         = $self->getNextActivityId($activityId);
+	return $self->getActivity($id);
+}
+
+
+#-------------------------------------------------------------------
+
+=head2 getNextActivityId ( [ activityId ] )
+
+Returns the ID of the next activity in the workflow after the activity specified.
+If no activity id is specified, then the first workflow will be returned.
+
+=head3 activityId
+
+The unique id of an activity in this workflow.
+
+=cut
+
+sub getNextActivityId {
+	my $self       = shift;
 	my $activityId = shift;
 	my ($sequenceNumber) = $self->session->db->quickArray("select sequenceNumber from WorkflowActivity where activityId=?", [$activityId]);
 	$sequenceNumber++;
 	my ($id) = $self->session->db->quickArray("select activityId from WorkflowActivity where workflowId=? 
 		and sequenceNumber>=? order by sequenceNumber", [$self->getId, $sequenceNumber]);
-	return $self->getActivity($id);
+    return $id;
+}
+
+
+#-------------------------------------------------------------------
+
+=head2 hasNextActivity ( [ activityId ] )
+
+Returns true if there is an activity after the specified activity.
+If no activity id is specified, then the first workflow will be returned.
+
+=head3 activityId
+
+The unique id of an activity in this workflow.
+
+=cut
+
+sub hasNextActivity {
+	my $self       = shift;
+    return $self->getNextActivityId(@_) ? 1 : 0;
 }
 
 

lib/WebGUI/Workflow/Instance.pm

@@ -260,6 +260,21 @@ sub getWorkflow {
 
 #-------------------------------------------------------------------
 
+=head2 hasNextActivity ( )
+
+Returns true if the instance has a workflow activity after the current one.
+
+=cut
+
+sub hasNextActivity {
+	my $self = shift;
+	my $workflow = $self->getWorkflow;
+	return undef unless defined $workflow;
+	return $workflow->hasNextActivity($self->get("currentActivityId"));
+}
+
+#-------------------------------------------------------------------
+
 =head2 new ( session, instanceId, [isNew] )
 
 Constructor.
@@ -315,8 +330,9 @@ B>NOTE:> You should normally never run this method. The workflow engine will use
 =cut
 
 sub run {
-	my $self = shift;
+	my $self     = shift;
 	my $workflow = $self->getWorkflow;
+    my $session  = $self->session;
 	unless (defined $workflow) {
 		$self->set({lastStatus=>"undefined"}, 1);
 		return "undefined";
@@ -326,7 +342,7 @@ sub run {
 		return "disabled";
 	}
 	if ($workflow->isSerial) {
-		my ($firstId) = $self->session->db->quickArray(
+		my ($firstId) = $session->db->quickArray(
             "select instanceId from WorkflowInstance where workflowId=? order by runningSince",
             [$workflow->getId]
         );
@@ -335,22 +351,33 @@ sub run {
 			return "waiting";
 		}
 	}
+    ##Undef if returned if there is an error, or if there is not a next activity.
+    ##Use hasNextActivity to tell the difference and handle the cases differently.
+    if (! $self->hasNextActivity) {
+        $self->delete(1);
+        return "done";
+    }
     my $activity = $self->getNextActivity;
 	unless  (defined $activity)  {
-		$self->delete(1);
-		return "done";
+        $session->errorHandler->error(
+            sprintf q{Unable to load Workflow Activity for activity after id %s in workflow %s},
+                $self->get('currentActivityId'),
+                $workflow->getId
+        );
+        $self->set({lastStatus=>"error"}, 1);
+        return "error";
 	}
-	$self->session->errorHandler->info("Running workflow activity ".$activity->getId.", which is a ".(ref $activity).", for instance ".$self->getId.".");
+	$session->errorHandler->info("Running workflow activity ".$activity->getId.", which is a ".(ref $activity).", for instance ".$self->getId.".");
     my $object = eval { $self->getObject };
     if ( my $e = WebGUI::Error::ObjectNotFound->caught ) {
-        $self->session->log->warn(
+        $session->log->warn(
             q{The object for this workflow does not exist.  Type: } . $self->get('className') . q{, ID: } . $e->id
         );
         $self->delete(1);
         return "done";
     }
     elsif ($@) {
-        $self->session->errorHandler->error(
+        $session->errorHandler->error(
             q{Error on workflow instance '} . $self->getId . q{': }. $@
         );
         $self->set({lastStatus=>"error"}, 1);
@@ -359,7 +386,7 @@ sub run {
 
 	my $status = eval { $activity->execute($object, $self) };
 	if ($@) {
-		$self->session->errorHandler->error("Caught exception executing workflow activity ".$activity->getId." for instance ".$self->getId." which reported ".$@);
+		$session->errorHandler->error("Caught exception executing workflow activity ".$activity->getId." for instance ".$self->getId." which reported ".$@);
 		$self->set({lastStatus=>"error"}, 1);
 		return "error";
 	}

t/Workflow.t

@@ -16,7 +16,7 @@ use WebGUI::Session;
 use WebGUI::Workflow;
 use WebGUI::Workflow::Cron;
 use WebGUI::Utility qw/isIn/;
-use Test::More tests => 67; # increment this value for each test you create
+use Test::More tests => 75; # increment this value for each test you create
 use Test::Deep;
 
 my $session = WebGUI::Test->session;
@@ -144,6 +144,28 @@ my $decayKarma = $wf3->addActivity('WebGUI::Workflow::Activity::DecayKarma');
 my $cleanTemp  = $wf3->addActivity('WebGUI::Workflow::Activity::CleanTempStorage');
 my $oldTrash   = $wf3->addActivity('WebGUI::Workflow::Activity::PurgeOldTrash');
 
+#####################################################################
+#
+# getNextActivityId
+#
+#####################################################################
+
+is $wf3->getNextActivityId($decayKarma->getId), $cleanTemp->getId,  'getNextActivityId: first activity to second';
+is $wf3->getNextActivityId($cleanTemp->getId),  $oldTrash->getId,   '... second activity to third';
+is $wf3->getNextActivityId($oldTrash->getId),   undef,              '... last returns undef';
+is $wf3->getNextActivityId(),                   $decayKarma->getId, '... with no activityId, returns the first';
+
+#####################################################################
+#
+# hasNextActivity
+#
+#####################################################################
+
+ok  $wf3->hasNextActivity($decayKarma->getId), 'hasNextActivityId: first activity to second';
+ok  $wf3->hasNextActivity($cleanTemp->getId),  '... second activity to third';
+ok  $wf3->hasNextActivity(),                   '... with no activityId, returns the first';
+ok !$wf3->hasNextActivity($oldTrash->getId),   '... last returns undef';
+
 #####################################################################
 #
 # Activity tests, promote, demote, reorder, accessing, deleting

t/Workflow/Instance.t

@@ -69,6 +69,7 @@ my $wf = WebGUI::Workflow->create(
     }
 );
 isa_ok($wf, 'WebGUI::Workflow', 'workflow created for test');
+addToCleanup($wf);
 
 # create an instance of $wfId
 my $properties = {
@@ -179,6 +180,7 @@ my $wf2 = WebGUI::Workflow->create(
         type => 'None',
     }
 );
+addToCleanup($wf2);
 
 my $wf2Instance = WebGUI::Workflow::Instance->create($session, {workflowId => $wf2->getId});
 cmp_deeply($wf2Instance->get('parameters'), {}, 'get returns {} for parameters when there are no parameters stored');
@@ -222,10 +224,3 @@ cmp_deeply($wf2Instance->get('parameters'), {}, 'get returns {} for parameters w
         is $wf3Instance->getObject, $return;
     } 'getObject is able to retrieve correct object';
 }
-
-#----------------------------------------------------------------------------
-# Cleanup
-END {
-    $wf->delete;  ##Deleting a Workflow deletes its instances, too.
-    $wf2->delete;
-}