Francois Gouget : testbot/cgi: Document and tweak the action error handling.

[Alexandre Julliard]

Module: tools
Branch: master
Commit: 6491c1c982927442dfca75b9e1668e5b2e9e32fc
URL:    https://source.winehq.org/git/tools.git/?a=commit;h=6491c1c982927442dfca75b9...

Author: Francois Gouget 
Date:   Tue Mar 29 17:58:33 2022 +0200

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

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 
Signed-off-by: Alexandre Julliard

---

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 d0f44b9..053b225 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;
 }

@@ -664,6 +669,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

@@ -688,15 +696,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 f47d628..b85ccb2 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;