[tools] testbot/cgi: Clean up FormPage's property handling.

[Francois Gouget]

Separate getting the value used by the form, from generating the HTML to
display or edit it.

Signed-off-by: Francois Gouget <fgouget at codeweavers.com>
---
 testbot/lib/ObjectModel/CGI/FormPage.pm | 191 +++++++++++++++---------
 testbot/web/Submit.pl                   |   2 +-
 2 files changed, 124 insertions(+), 69 deletions(-)

diff --git a/testbot/lib/ObjectModel/CGI/FormPage.pm b/testbot/lib/ObjectModel/CGI/FormPage.pm
index c6a8c6bf1..c41924433 100644
--- a/testbot/lib/ObjectModel/CGI/FormPage.pm
+++ b/testbot/lib/ObjectModel/CGI/FormPage.pm
@@ -222,6 +222,17 @@ sub GetPropertyValue($$)
 # Form field support
 #
 
+=pod
+=over 12
+
+=item C<GetDisplayName()>
+
+Returns the unescaped property's label / description / user-friendly name.
+Escaping the name for HTML is the responsibility of the caller.
+
+=back
+=cut
+
 sub GetDisplayName($$)
 {
   my ($self, $PropertyDescriptor) = @_;
@@ -229,17 +240,54 @@ sub GetDisplayName($$)
   return $PropertyDescriptor->GetDisplayName();
 }
 
-sub GetDisplayValue($$)
+=pod
+=over 12
+
+=item C<GetFormValue()>
+
+Returns the value to be used for this form's property.
+
+This is either the value received through the HTTP parameters, or if it was
+not specified there, the underlying property value (see GetPropertyValue()).
+
+Converting that value into a form suitable for viewing (including escaping it
+for HTML) or editing is the responsibility of GenerateValueView() and
+GenerateValueInput() respectively.
+
+=back
+=cut
+
+sub GetFormValue($$)
 {
   my ($self, $PropertyDescriptor) = @_;
 
   my $Value = $self->GetParam($PropertyDescriptor->GetName());
-  if (!defined $Value)
+  return defined $Value ? $Value :
+         $self->GetPropertyValue($PropertyDescriptor);
+}
+
+=pod
+=over 12
+
+=item C<GenerateValueView()>
+
+Generates an HTML snippet representing the value in a user-readable form.
+
+=back
+=cut
+
+sub GenerateValueView($$$)
+{
+  my ($self, $PropertyDescriptor, $Value) = @_;
+
+  if ($PropertyDescriptor->GetClass() eq "Basic")
   {
-    $Value = $self->GetPropertyValue($PropertyDescriptor);
+    if ($PropertyDescriptor->GetType() eq "B")
+    {
+      print $Value ? "Yes" : "No";
+    }
   }
-
-  return $Value;
+  print defined $Value ? $self->escapeHTML($Value) : " ";
 }
 
 sub GetInputType($$)
@@ -276,85 +324,69 @@ sub GenerateRequired($$)
   }
 }
 
-sub GenerateField($$$)
-{
-  my ($self, $PropertyDescriptor, $Display) = @_;
+=pod
+=over 12
 
-  print "<div class='ItemProperty'><label>",
-        $self->escapeHTML($self->GetDisplayName($PropertyDescriptor)),
-        "</label>";
+=item C<GenerateValueInput()>
 
-  my $Value = $self->GetDisplayValue($PropertyDescriptor);
-  if ($Display eq "rw")
+Generates an HTML snippet allowing the specified value to be edited.
+
+=back
+=cut
+
+sub GenerateValueInput($$$)
+{
+  my ($self, $PropertyDescriptor, $Value) = @_;
+
+  my $InputType = $self->GetInputType($PropertyDescriptor);
+  if ($InputType eq "checkbox")
   {
-    my $InputType = $self->GetInputType($PropertyDescriptor);
-    print "<div class='ItemValue'>";
-    if ($InputType eq "checkbox")
-    {
-      print "<input type='checkbox' name='", $PropertyDescriptor->GetName(),
-            "' ";
-      if ($Value)
-      {
-        print "checked='checked' ";
-      }
-      print "/>";
-    }
-    elsif ($InputType eq "select")
+    my $Checked = $Value ? " checked" : "";
+    print "<input type='checkbox' name='", $PropertyDescriptor->GetName(),
+          "'$Checked/>";
+  }
+  elsif ($InputType eq "select")
+  {
+    print "<select name='", $PropertyDescriptor->GetName(), "'>\n";
+    foreach my $V (@{$PropertyDescriptor->GetValues()})
     {
-      print "<select name='", $PropertyDescriptor->GetName(), "'>\n";
-      foreach my $V (@{$PropertyDescriptor->GetValues()})
-      {
-        print "  <option value='", $self->escapeHTML($V), "'";
-        print " selected='selected'" if ($Value eq $V);
-        print ">", $self->escapeHTML($V), "</option>\n";
-      }
-      print "</select>";
+      print "  <option value='", $self->escapeHTML($V), "'";
+      print " selected" if ($Value eq $V);
+      print ">", $self->escapeHTML($V), "</option>\n";
     }
-    elsif ($InputType eq "textarea")
+    print "</select>";
+    # No need to add a 'required' label since checkboxes always have a value
+  }
+  elsif ($InputType eq "textarea")
+  {
+    my $MaxLength = $PropertyDescriptor->GetMaxLength();
+    print "<textarea name='", $PropertyDescriptor->GetName(), "' cols='";
+    if ($MaxLength < 50)
     {
-      my $MaxLength = $PropertyDescriptor->GetMaxLength();
-      print "<textarea name='", $PropertyDescriptor->GetName(), "' cols='";
-      if ($MaxLength < 50)
-      {
-        print "$MaxLength' rows='1";
-      }
-      else
-      {
-        print "50' rows='", int(($MaxLength + 49) / 50);
-      }
-      print "'>";
-      print $self->escapeHTML($Value) if (defined $Value);
-      print "</textarea>";
-      $self->GenerateRequired($PropertyDescriptor);
+      print "$MaxLength' rows='1'>";
     }
     else
     {
-      my $Size=$PropertyDescriptor->GetMaxLength();
-      $Size=45 if ($Size > 45);
-      print "<input type='$InputType' name='", $PropertyDescriptor->GetName(),
-            "' maxlength='", $PropertyDescriptor->GetMaxLength(), "' size='$Size'";
-      if (defined $Value && $InputType ne "password")
-      {
-        print " value='", $self->escapeHTML($Value), "'";
-      }
-      print " />";
-      $self->GenerateRequired($PropertyDescriptor);
+      print "50' rows='", int(($MaxLength + 49) / 50),  "'>";
     }
-    print "</div>";
+    print $self->escapeHTML($Value) if (defined $Value);
+    print "</textarea>";
+    $self->GenerateRequired($PropertyDescriptor);
   }
   else
   {
-    if ($Value)
-    {
-      print $self->escapeHTML($Value);
-    }
-    else
+    my $Size=$PropertyDescriptor->GetMaxLength();
+    $Size=45 if ($Size > 45);
+    print "<input type='$InputType' name='", $PropertyDescriptor->GetName(),
+          "' maxlength='", $PropertyDescriptor->GetMaxLength(),
+          "' size='$Size'";
+    if (defined $Value and $InputType ne "password")
     {
-      print " ";
+      print " value='", $self->escapeHTML($Value), "'";
     }
+    print "/>";
+    $self->GenerateRequired($PropertyDescriptor);
   }
-
-  print "</div>\n";
 }
 
 
@@ -392,6 +424,29 @@ sub GenerateFormStart($)
        "' method='$self->{Method}' enctype='multipart/form-data'>\n";
 }
 
+sub GenerateField($$$)
+{
+  my ($self, $PropertyDescriptor, $Display) = @_;
+
+  print "<div class='ItemProperty'><label>",
+        $self->escapeHTML($self->GetDisplayName($PropertyDescriptor)),
+        "</label>";
+
+  my $Value = $self->GetFormValue($PropertyDescriptor);
+  if ($Display eq "rw")
+  {
+    print "<div class='ItemValue'>";
+    $self->GenerateValueInput($PropertyDescriptor, $Value);
+    print "</div>";
+  }
+  else
+  {
+    $self->GenerateValueView($PropertyDescriptor, $Value);
+  }
+
+  print "</div>\n";
+}
+
 sub GenerateFields($)
 {
   my ($self) = @_;
diff --git a/testbot/web/Submit.pl b/testbot/web/Submit.pl
index 8323b9ab7..1ad94e4a4 100644
--- a/testbot/web/Submit.pl
+++ b/testbot/web/Submit.pl
@@ -947,7 +947,7 @@ sub DisplayProperty($$)
   return $self->SUPER::DisplayProperty($PropertyDescriptor);
 }
 
-sub GetDisplayValue($$)
+sub GetPropertyValue($$)
 {
   my ($self, $PropertyDescriptor) = @_;
 
-- 
2.30.2