[tools] testbot/cgi: Document and tweak the action error handling.

[Francois Gouget]

Fix OnAction() so it returns an error when OnItemAction() fails to apply 
an action to a selected item.
OnItemAction() should not return success on unknown item actions.
Also the action is a user-supplied parameter so a proper error message
shoulf be displayed when it is invalid instead of having OnAction()
and OnItemAction() just die.

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com>
---
 testbot/lib/ObjectModel/CGI/CollectionBlock.pm | 16 ++++++++++++----
 testbot/lib/ObjectModel/CGI/FormPage.pm        |  6 +++++-
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/testbot/lib/ObjectModel/CGI/CollectionBlock.pm b/testbot/lib/ObjectModel/CGI/CollectionBlock.pm
index 6f5c4d5fa..58bf927ca 100644
--- a/testbot/lib/ObjectModel/CGI/CollectionBlock.pm
+++ b/testbot/lib/ObjectModel/CGI/CollectionBlock.pm
@@ -603,6 +603,10 @@ This method is called to apply $Action to each selected item.
 Note that it is the responsibility of this method to validate the property
 values by calling Validate() before performing the action.
 
+Returns true on success. In case an error occurs while performing the action on
+any of the selected items, the enclosing page's error message is set and
+false is returned.
+
 =back
 =cut
 
@@ -617,7 +621,8 @@ sub OnItemAction($$$)
     return ! defined($ErrMessage);
   }
 
-  return 1;
+  $self->{EnclosingPage}->SetError(undef, "No per-Item action defined for $Action");
+  return 0;
 }
 
 
@@ -668,6 +673,9 @@ Implements the "Add" and per-item actions.
 Note that it is the responsibility of this method to validate the property
 values by calling Validate() before performing the action.
 
+Returns true on success. Otherwise the enclosing page's error message is set
+and false is returned.
+
 =back
 =cut
 
@@ -692,15 +700,15 @@ sub OnAction($$)
     exit($self->{EnclosingPage}->Redirect($Target));
   }
 
-  my $Ok = 1;
   foreach my $Key (@{$self->{Collection}->GetKeys()})
   {
-    if (defined $self->{EnclosingPage}->GetParam($self->SelName($Key)) && $Ok)
+    if (defined $self->{EnclosingPage}->GetParam($self->SelName($Key)))
     {
       my $Item = $self->{Collection}->GetItem($Key);
-      $Ok = $self->CallOnItemAction($Item, $Action);
+      return 0 if (!$self->CallOnItemAction($Item, $Action));
     }
   }
+  return 1;
 }
 
 1;
diff --git a/testbot/lib/ObjectModel/CGI/FormPage.pm b/testbot/lib/ObjectModel/CGI/FormPage.pm
index 83b078b24..0f4c5cd0e 100644
--- a/testbot/lib/ObjectModel/CGI/FormPage.pm
+++ b/testbot/lib/ObjectModel/CGI/FormPage.pm
@@ -596,6 +596,9 @@ actions.
 Note that it is the responsibility of this method to validate the property
 values by calling Validate() before performing the action.
 
+Returns true on success. Otherwise sets the page's error message and returns
+false.
+
 =back
 =cut
 
@@ -603,7 +606,8 @@ sub OnAction($$)
 {
   my ($self, $Action) = @_;
 
-  die "No action defined for $Action";
+  $self->{EnclosingPage}->SetError(undef, "No action defined for $Action");
+  return 0;
 }
 
 1;
-- 
2.30.2