[tools 1/2] testbot/cgi: Improve the error handling API.

[Francois Gouget]

Add IsErrField() and ResetErrors() so web pages don't need to access the
error fields directly.
Add AddError() where the name of the error field is optional since it is
often undetermined.
Document the API, particularly with regards to the content of the error
message and whether all errors have an associated error field.

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com>
---
AddError() will later be extended to allow adding multiple error
messages and multiple invalid fields and should entirely replace
SetError() eventually.
---
 testbot/lib/ObjectModel/CGI/Page.pm | 90 ++++++++++++++++++++++++++---
 1 file changed, 82 insertions(+), 8 deletions(-)

diff --git a/testbot/lib/ObjectModel/CGI/Page.pm b/testbot/lib/ObjectModel/CGI/Page.pm
index 1b9630cc28..6e18c29ab4 100644
--- a/testbot/lib/ObjectModel/CGI/Page.pm
+++ b/testbot/lib/ObjectModel/CGI/Page.pm
@@ -89,8 +89,9 @@ sub new($$$@)
 
   my $self = {Request => $Request,
               CGIObj => CGI->new($Request),
-              ErrMessage => undef,
-              ErrField => undef};
+              # ErrMessage => undef by default
+              # ErrField => undef by default
+  };
   $self = bless $self, $class;
   $self->{PageBase} = &$PageBaseCreator($self, @_);
   $self->_initialize(@_);
@@ -293,26 +294,99 @@ sub Redirect($$)
 # Error handling framework
 #
 
-sub GetErrField($)
+=pod
+=over 12
+
+=head1 C<GetErrMessage()>
+
+Returns the error message if any, undef otherwise.
+
+=back
+=cut
+
+sub GetErrMessage($)
 {
   my ($self) = @_;
+  return $self->{ErrMessage};
+}
+
+=pod
+=over 12
 
+=head1 C<GetErrField()>
+
+Returns the name of the invalid field, undef otherwise.
+
+Note that if the error does not concern a specific field this may return
+undef even though an error has been set.
+
+=back
+=cut
+
+sub GetErrField($)
+{
+  my ($self) = @_;
   return $self->{ErrField};
 }
 
-sub GetErrMessage($)
+=pod
+=over 12
+
+=head1 C<IsErrField()>
+
+Returns true if the error concerns the specified field.
+
+=back
+=cut
+
+sub IsErrField($$)
 {
-  my ($self) = @_;
+  my ($self, $FieldName) = @_;
+  return defined $self->{ErrField} and $self->{ErrField} eq $FieldName;
+}
 
-  return $self->{ErrMessage};
+=pod
+=over 12
+
+=head1 C<AddError()>
+
+Sets the error message to be shown to the user.
+=over
+
+=item ErrMessage
+A message to be shown on the page that describes why the form data is invalid.
+This should include the display name of the field(s) that need to be fixed.
+
+=item ErrField
+If the error concerns a specific field, the name of that field. This can be
+used by the page to highlight the field that needs to be corrected.
+
+=back
+
+=back
+=cut
+
+sub AddError($$;$)
+{
+  my ($self, $ErrMessage, $ErrField) = @_;
+
+  $self->{ErrMessage} = $ErrMessage;
+  $self->{ErrField} = $ErrField;
 }
 
+# FIXME: For legacy code, to be removed.
 sub SetError($$$)
 {
   my ($self, $ErrField, $ErrMessage) = @_;
+  $self->AddError($ErrMessage, $ErrField);
+}
 
-  $self->{ErrField} = $ErrField;
-  $self->{ErrMessage} = $ErrMessage;
+sub ResetErrors($)
+{
+  my ($self) = @_;
+
+  delete $self->{ErrMessage};
+  delete $self->{ErrField};
 }
 
 sub GenerateErrorDiv($)
-- 
2.30.2

[tools 2/2] testbot/cgi: Avoid direct access to the base Page error fields.

[Francois Gouget]

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com>
---
 testbot/lib/ObjectModel/CGI/FormPage.pm | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/testbot/lib/ObjectModel/CGI/FormPage.pm b/testbot/lib/ObjectModel/CGI/FormPage.pm
index 6f3449b5b2..f2fd0f09cd 100644
--- a/testbot/lib/ObjectModel/CGI/FormPage.pm
+++ b/testbot/lib/ObjectModel/CGI/FormPage.pm
@@ -503,14 +503,17 @@ sub GenerateBody($)
   $self->GenerateFields();
   $self->GenerateRequiredLegend();
 
-  if (defined $self->{ErrMessage})
+  my $ErrMessage = $self->GetErrMessage();
+  if (defined $ErrMessage)
   {
-    if (defined $self->{ErrField})
+    my $ErrField = $self->GetErrField();
+    if (defined $ErrField)
     {
-      my $PropertyDescriptor = $self->GetPropertyDescriptorByName($self->{ErrField});
+      my $PropertyDescriptor = $self->GetPropertyDescriptorByName($ErrField);
       if ($PropertyDescriptor and !($self->DisplayProperty($PropertyDescriptor))[0])
       {
-        $self->{ErrMessage} = "Internal error?\n$self->{ErrMessage}";
+        # The hidden fields should not be invalid
+        $self->AddError("Internal error?\n$ErrMessage", $ErrField);
       }
     }
     $self->GenerateErrorPopup();
@@ -573,8 +576,7 @@ sub Validate($)
     my $ErrMessage = $PropertyDescriptor->ValidateValue($Value, 1);
     if ($ErrMessage)
     {
-      $self->{ErrMessage} = $ErrMessage;
-      $self->{ErrField} = $PropertyDescriptor->GetName();
+      $self->AddError($ErrMessage, $PropertyDescriptor->GetName());
       return !1;
     }
   }
@@ -686,7 +688,7 @@ sub OnAction($$)
 {
   my ($self, $Action) = @_;
 
-  $self->SetError(undef, "No action defined for $Action");
+  $self->AddError("No action defined for $Action");
   return 0;
 }
 
-- 
2.30.2