From 55d55b4eee96112635b44e10ffe961ef066b264b Mon Sep 17 00:00:00 2001
From: Colin Kuskie <colink@perldreamer.com>
Date: Tue, 11 May 2010 16:20:47 -0700
Subject: [PATCH] Install better error handling code into
 RequestApprovalForVersionTag.  Do not generate leading commas in workflow
 instance scratch data.

---
 docs/changelog/7.x.x.txt                      |  1 +
 docs/upgrades/upgrade_7.9.4-7.9.5.pl          | 20 +++++++++-
 .../Activity/RequestApprovalForVersionTag.pm  | 39 ++++++++++++-------
 3 files changed, 45 insertions(+), 15 deletions(-)

diff --git a/docs/changelog/7.x.x.txt b/docs/changelog/7.x.x.txt
index 1d33f344f..7804874f7 100644
--- a/docs/changelog/7.x.x.txt
+++ b/docs/changelog/7.x.x.txt
@@ -12,6 +12,7 @@
  - fixed #11564: Shop: Cart does not require a state field in the address
  - fixed #11562: Missing perl modules in gotcha
  - fixed #11565: Can't clear cache on plainblack.com
+ - fixed #11540: Pending version tags are not completed on approval under certain conditions
 
 7.9.4
  - We're shipping underscore.js now for its suite of extremely handy utility
diff --git a/docs/upgrades/upgrade_7.9.4-7.9.5.pl b/docs/upgrades/upgrade_7.9.4-7.9.5.pl
index f8aaae553..d0f03e388 100644
--- a/docs/upgrades/upgrade_7.9.4-7.9.5.pl
+++ b/docs/upgrades/upgrade_7.9.4-7.9.5.pl
@@ -22,6 +22,7 @@ use Getopt::Long;
 use WebGUI::Session;
 use WebGUI::Storage;
 use WebGUI::Asset;
+use WebGUI::Workflow::Instance;
 
 
 my $toVersion = '7.9.5';
@@ -32,6 +33,7 @@ my $session = start(); # this line required
 
 # upgrade functions go here
 modifySortItems( $session );
+fixRequestForApprovalScratch($session);
 
 finish($session); # this line required
 
@@ -44,6 +46,22 @@ finish($session); # this line required
 #    print "DONE!\n" unless $quiet;
 #}
 
+#----------------------------------------------------------------------------
+# Describe what our function does
+sub fixRequestForApprovalScratch {
+    my $session = shift;
+    print "\tCorrect RequestApprovalForVersionTag workflow instance data with leading commas... " unless $quiet;
+    # and here's our code
+    my $instances = WebGUI::Workflow::Instance->getAllInstances($session);
+    INSTANCE: foreach my $instance (@{ $instances }) {
+        my $messageId = $instance->getScratch('messageId');
+        next INSTANCE unless $messageId;
+        $messageId =~ s/^,//;
+        $instance->setScratch('messageId', $messageId);
+    }
+    print "DONE!\n" unless $quiet;
+}
+
 #----------------------------------------------------------------------------
 # Changes sortItems to a SelectBox
 sub modifySortItems {
@@ -56,7 +74,7 @@ sub modifySortItems {
     my $type = WebGUI::Form::SelectBox->getDatabaseFieldType;
     $session->db->write("ALTER TABLE SyndicatedContent MODIFY sortItems $type");
 
-    print "\t\tConverting old values...\n" unless $quiet;
+    print "\t\tConverting old values..." unless $quiet;
     $session->db->write(q{
         UPDATE SyndicatedContent
         SET    sortItems = 'none'
diff --git a/lib/WebGUI/Workflow/Activity/RequestApprovalForVersionTag.pm b/lib/WebGUI/Workflow/Activity/RequestApprovalForVersionTag.pm
index 9434b927d..5b68e02b9 100644
--- a/lib/WebGUI/Workflow/Activity/RequestApprovalForVersionTag.pm
+++ b/lib/WebGUI/Workflow/Activity/RequestApprovalForVersionTag.pm
@@ -208,7 +208,9 @@ sub execute {
     # Tag is approved
     elsif ( $instance->getScratch("status") eq "approved" ) {
         # Clean up after ourselves
-        $self->setMessageCompleted( $instance );
+        if (! $self->setMessageCompleted( $instance ) ) {
+            return $self->ERROR;
+        }
         $instance->deleteScratch( "status" );
         
         # We're done here
@@ -278,7 +280,7 @@ sub sendMessage {
                 groupId => $groupId,
                 status  => 'pending',
             });
-        $messageIds = join ",", $messageIds, $message->getId;
+        $messageIds = $messageIds ? join(",", $messageIds, $message->getId) : $message->getId;
     }
 
     # Keep track of message Ids so we can complete them 
@@ -335,23 +337,32 @@ workflow instance we're part of.
 =cut
 
 sub setMessageCompleted {
-    my $self        = shift;
-    my $instance    = shift;
-    my $inbox       = WebGUI::Inbox->new( $self->session );
+    my $self     = shift;
+    my $instance = shift;
+    my $inbox    = WebGUI::Inbox->new( $self->session );
 
     # Set all messages to completed
-    for my $messageId ( split /,/, $instance->getScratch("messageId") ) { 
-        if($messageId){
-            my $message = $inbox->getMessage( $messageId );
-            $message->setCompleted if $message;
+    for my $messageId ( split /,/, $instance->getScratch("messageId") ) {
+        if ($messageId) {
+            my $message = $inbox->getMessage($messageId);
+            if ($message) {
+                $message->setCompleted;
+            }
+            else {
+                $self->session->log->error("Could not get inbox message for messageId: $messageId");
+                return 0;
+            }
         }
-    }
+        else {
+            $self->session->log->error("Malformed workflow instance scratch variable messageId for instance: ". $instance->getId);
+            return 0;
+        }
+    } ## end for my $messageId ( split...)
 
-    $instance->deleteScratch( "messageId" );
-
-    return;
-}
+    $instance->deleteScratch("messageId");
 
+    return 1;
+} ## end sub setMessageCompleted
 
 1;