[tools 1/5] testbot/cgi: Pass a column descriptor to Generate{Header,Data}View().

[Francois Gouget]

Instead of being called for every row, DisplayProperty() is now called
only once and the result is stored in the corresponding column
descriptor. This also allows passing extra information such as the
column index to Generate{Header,Data}View().
And this allows GenerateList() to iterate only on the properties that
are actually being displayed.

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com>
---
 .../lib/ObjectModel/CGI/CollectionBlock.pm    | 81 +++++++++++--------
 testbot/web/JobDetails.pl                     | 14 ++--
 testbot/web/PatchesList.pl                    |  6 +-
 testbot/web/admin/UsersList.pl                |  6 +-
 testbot/web/index.pl                          | 12 +--
 5 files changed, 68 insertions(+), 51 deletions(-)

diff --git a/testbot/lib/ObjectModel/CGI/CollectionBlock.pm b/testbot/lib/ObjectModel/CGI/CollectionBlock.pm
index 03f1d18cd..14e3cfe54 100644
--- a/testbot/lib/ObjectModel/CGI/CollectionBlock.pm
+++ b/testbot/lib/ObjectModel/CGI/CollectionBlock.pm
@@ -227,6 +227,20 @@ sub GetDetailsLink($$)
 
 Generates an HTML snippet for the column title.
 
+The Col parameter describes the column to generate the header for.
+It contains the following information:
+=over 12
+=item Descriptor
+The corresponding PropertyDescriptor object.
+
+=item Display
+The value returned by DisplayProperty(), normally "ro".
+
+=item Index
+The index of the column (see $Row->{Cols}).
+
+=back
+
 The Row parameter contains the following information:
 =over 12
 
@@ -234,6 +248,10 @@ The Row parameter contains the following information:
 The list of item properties, including those that DisplayProperty() says
 should be hidden.
 
+=item Cols
+The collection block's list of columns. This excludes any hidden property.
+The following relation holds: $Row->{Cols}->[$Col->{Index}] == $Col
+
 =item DetailsPage
 The base link to the item details page if any. See GetDetailsPage().
 
@@ -253,9 +271,9 @@ The total number of rows (excluding the header).
 
 sub GenerateHeaderView($$$)
 {
-  my ($self, $_Row, $PropertyDescriptor) = @_;
+  my ($self, $_Row, $Col) = @_;
 
-  print $self->escapeHTML($PropertyDescriptor->GetDisplayName());
+  print $self->escapeHTML($Col->{Descriptor}->GetDisplayName());
 }
 
 =pod
@@ -266,8 +284,8 @@ sub GenerateHeaderView($$$)
 Generates an HTML snippet representing the property value in a user-readable
 form.
 
-The Row parameter contains the same information as for GenerateHeaderView()
-plus:
+See GenerateHeaderView() for a description of the Row and Col parameters.
+The Row parameter has the following extra fields:
 =over 12
 
 =item Row
@@ -286,11 +304,11 @@ passing the row to GetDetailsLink().
 
 sub GenerateDataView($$$)
 {
-  my ($self, $Row, $PropertyDescriptor) = @_;
+  my ($self, $Row, $Col) = @_;
 
-  my $PropertyName = $PropertyDescriptor->GetName();
+  my $PropertyName = $Col->{Descriptor}->GetName();
   my $Value = $Row->{Item}->$PropertyName;
-  GenerateValueHTML($self, $PropertyDescriptor, $Value);
+  GenerateValueHTML($self, $Col->{Descriptor}, $Value);
 }
 
 
@@ -317,10 +335,10 @@ sub GenerateFormStart($)
 
 sub GenerateHeaderCell($$$)
 {
-  my ($self, $Row, $PropertyDescriptor) = @_;
+  my ($self, $Row, $Col) = @_;
 
   print "<th>";
-  $self->GenerateHeaderView($Row, $PropertyDescriptor);
+  $self->GenerateHeaderView($Row, $Col);
   print "</th>\n";
 }
 
@@ -336,12 +354,9 @@ sub GenerateHeaderRow($$)
     GenerateMasterCheckbox("block$self->{Unique}");
     print "</th>\n";
   }
-  foreach my $PropertyDescriptor (@{$Row->{PropertyDescriptors}})
+  foreach my $Col (@{$Row->{Cols}})
   {
-    if ($self->DisplayProperty($PropertyDescriptor))
-    {
-      $self->GenerateHeaderCell($Row, $PropertyDescriptor);
-    }
+    $self->GenerateHeaderCell($Row, $Col);
   }
   print "</tr>\n";
 }
@@ -356,16 +371,16 @@ sub SelName($$)
 
 sub GenerateDataCell($$$)
 {
-  my ($self, $Row, $PropertyDescriptor) = @_;
+  my ($self, $Row, $Col) = @_;
 
   print "<td>";
   my $CloseLink;
-  if ($Row->{DetailsPage} and $PropertyDescriptor->GetIsKey())
+  if ($Row->{DetailsPage} and $Col->{Descriptor}->GetIsKey())
   {
     print "<a href='", $self->escapeHTML($self->GetDetailsLink($Row)), "'>";
     $CloseLink = "</a>";
   }
-  $self->GenerateDataView($Row, $PropertyDescriptor);
+  $self->GenerateDataView($Row, $Col);
   print "$CloseLink</td>\n";
 }
 
@@ -380,12 +395,9 @@ sub GenerateDataRow($$)
     print "<td><input name='", $self->SelName($Row->{Item}->GetKey()),
           "' type='checkbox' cbgroup='block$self->{Unique}'/></td>\n";
   }
-  foreach my $PropertyDescriptor (@{$Row->{PropertyDescriptors}})
+  foreach my $Col (@{$Row->{Cols}})
   {
-    if ($self->DisplayProperty($PropertyDescriptor))
-    {
-      $self->GenerateDataCell($Row, $PropertyDescriptor);
-    }
+    $self->GenerateDataCell($Row, $Col);
   }
   print "</tr>\n";
 }
@@ -403,18 +415,22 @@ sub GenerateList($)
   my $Collection = $self->{Collection};
   my $PropertyDescriptors = $Collection->GetPropertyDescriptors();
 
+  my $ColIndex = 0;
+  my (@Cols, $HasDT);
+  foreach my $PropertyDescriptor (@$PropertyDescriptors)
+  {
+    my $Display = $self->DisplayProperty($PropertyDescriptor);
+    next if (!$Display);
+    push @Cols, {Descriptor => $PropertyDescriptor,
+                 Display => $Display,
+                 Index => $ColIndex++};
+    $HasDT ||= ($PropertyDescriptor->GetClass() eq "Basic" and
+                $PropertyDescriptor->GetType() eq "DT");
+  }
   my $Items = $self->{Collection}->GetSortedItems();
-  if (@$Items != 0)
+  if ($HasDT and @$Items != 0)
   {
-    foreach my $PropertyDescriptor (@$PropertyDescriptors)
-    {
-      if ($PropertyDescriptor->GetClass() eq "Basic" and
-          $PropertyDescriptor->GetType() eq "DT")
-      {
-        $self->{EnclosingPage}->GenerateImportJS(GetDateTimeJSFile());
-        last;
-      }
-    }
+    $self->{EnclosingPage}->GenerateImportJS(GetDateTimeJSFile());
   }
 
   print "<div class='CollectionBlock'>\n";
@@ -427,6 +443,7 @@ sub GenerateList($)
   print "<thead>\n";
   my $Row = {
     PropertyDescriptors => $PropertyDescriptors,
+    Cols => \@Cols,
     DetailsPage => $self->GetDetailsPage(),
     ItemActions => $self->{RW} ? $self->GetItemActions() : [],
     Row => 0, # 0 for the header ---> 1 for the first line
diff --git a/testbot/web/JobDetails.pl b/testbot/web/JobDetails.pl
index e39df41b5..591a47ee9 100644
--- a/testbot/web/JobDetails.pl
+++ b/testbot/web/JobDetails.pl
@@ -60,9 +60,9 @@ sub DisplayProperty($$)
 
 sub GenerateHeaderView($$$)
 {
-  my ($self, $Row, $PropertyDescriptor) = @_;
+  my ($self, $Row, $Col) = @_;
 
-  my $PropertyName = $PropertyDescriptor->GetName();
+  my $PropertyName = $Col->{Descriptor}->GetName();
   if ($PropertyName eq "CmdLineArg")
   {
     print "Arguments / <span class='MissionHeader'>Missions</span>";
@@ -73,16 +73,16 @@ sub GenerateHeaderView($$$)
   }
   else
   {
-    $self->SUPER::GenerateHeaderView($Row, $PropertyDescriptor);
+    $self->SUPER::GenerateHeaderView($Row, $Col);
   }
 }
 
 sub GenerateDataView($$$)
 {
-  my ($self, $Row, $PropertyDescriptor) = @_;
+  my ($self, $Row, $Col) = @_;
 
   my $StepTask = $Row->{Item};
-  my $PropertyName = $PropertyDescriptor->GetName();
+  my $PropertyName = $Col->{Descriptor}->GetName();
   if ($PropertyName eq "VM")
   {
     print "<a href='#k", $self->escapeHTML($Row->{Item}->GetKey()), "'>";
@@ -103,7 +103,7 @@ sub GenerateDataView($$$)
     }
     else
     {
-      $self->SUPER::GenerateDataView($Row, $PropertyDescriptor);
+      $self->SUPER::GenerateDataView($Row, $Col);
     }
   }
   elsif ($PropertyName eq "CmdLineArg")
@@ -144,7 +144,7 @@ sub GenerateDataView($$$)
   }
   else
   {
-    $self->SUPER::GenerateDataView($Row, $PropertyDescriptor);
+    $self->SUPER::GenerateDataView($Row, $Col);
   }
 }
 
diff --git a/testbot/web/PatchesList.pl b/testbot/web/PatchesList.pl
index a1c5171df..30cfc7547 100644
--- a/testbot/web/PatchesList.pl
+++ b/testbot/web/PatchesList.pl
@@ -46,9 +46,9 @@ sub DisplayProperty($$)
 
 sub GenerateDataView($$$)
 {
-  my ($self, $Row, $PropertyDescriptor) = @_;
+  my ($self, $Row, $Col) = @_;
 
-  if ($PropertyDescriptor->GetName() eq "Disposition" and
+  if ($Col->{Descriptor}->GetName() eq "Disposition" and
       $Row->{Item}->Disposition =~ /job ([0-9]+)$/)
   {
     my $JobId = $1;
@@ -56,7 +56,7 @@ sub GenerateDataView($$$)
   }
   else
   {
-    $self->SUPER::GenerateDataView($Row, $PropertyDescriptor);
+    $self->SUPER::GenerateDataView($Row, $Col);
   }
 }
 
diff --git a/testbot/web/admin/UsersList.pl b/testbot/web/admin/UsersList.pl
index ec6d2822e..5f9f47b46 100644
--- a/testbot/web/admin/UsersList.pl
+++ b/testbot/web/admin/UsersList.pl
@@ -47,9 +47,9 @@ sub DisplayProperty($$)
 
 sub GenerateDataView($$$)
 {
-  my ($self, $Row, $PropertyDescriptor) = @_;
+  my ($self, $Row, $Col) = @_;
 
-  if ($PropertyDescriptor->GetName() eq "Status")
+  if ($Col->{Descriptor}->GetName() eq "Status")
   {
     my $User = $Row->{Item};
     my $Status = $User->Status;
@@ -87,7 +87,7 @@ sub GenerateDataView($$$)
   }
   else
   {
-    $self->SUPER::GenerateDataView($Row, $PropertyDescriptor);
+    $self->SUPER::GenerateDataView($Row, $Col);
   }
 }
 
diff --git a/testbot/web/index.pl b/testbot/web/index.pl
index 1c506c9b1..331ee85ea 100644
--- a/testbot/web/index.pl
+++ b/testbot/web/index.pl
@@ -51,9 +51,9 @@ sub DisplayProperty($$)
 
 sub GenerateHeaderView($$$)
 {
-  my ($self, $Row, $PropertyDescriptor) = @_;
+  my ($self, $Row, $Col) = @_;
 
-  my $PropertyName = $PropertyDescriptor->GetName();
+  my $PropertyName = $Col->{Descriptor}->GetName();
   if ($PropertyName eq "Priority")
   {
     print "<a class='title' title='Higher values indicate a lower priority'>Nice</a>";
@@ -64,16 +64,16 @@ sub GenerateHeaderView($$$)
   }
   else
   {
-    $self->SUPER::GenerateHeaderView($Row, $PropertyDescriptor);
+    $self->SUPER::GenerateHeaderView($Row, $Col);
   }
 }
 
 sub GenerateDataView($$$)
 {
-  my ($self, $Row, $PropertyDescriptor) = @_;
+  my ($self, $Row, $Col) = @_;
 
   my $Job = $Row->{Item};
-  my $PropertyName = $PropertyDescriptor->GetName();
+  my $PropertyName = $Col->{Descriptor}->GetName();
   if ($PropertyName eq "User")
   {
     if (defined $Job->Patch and defined $Job->Patch->FromName and
@@ -166,7 +166,7 @@ sub GenerateDataView($$$)
   }
   else
   {
-    $self->SUPER::GenerateDataView($Row, $PropertyDescriptor);
+    $self->SUPER::GenerateDataView($Row, $Col);
   }
 }
 
-- 
2.30.2

[tools 2/5] testbot/cgi: Add a format parameter to GenerateHTMLValue().

[Francois Gouget]

This mostly allows specifying how it should format timestamps, though it
could also be used to force intepreting an integer property as a
boolean, etc.

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com>
---
 testbot/lib/ObjectModel/CGI/ValueFormatter.pm | 31 ++++++++++++++++---
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/testbot/lib/ObjectModel/CGI/ValueFormatter.pm b/testbot/lib/ObjectModel/CGI/ValueFormatter.pm
index 62c1499ab..638504d32 100644
--- a/testbot/lib/ObjectModel/CGI/ValueFormatter.pm
+++ b/testbot/lib/ObjectModel/CGI/ValueFormatter.pm
@@ -163,27 +163,48 @@ Generates an HTML snippet representing the value in a user-readable form.
 The Parent parameter must point to a class that implements the escapeHTML()
 method.
 
+The format parameter can be used to specify how to display the specified
+value and should have one of the basic property descriptor types with a couple
+of extensions for timestamps:
+
+=over
+
+=item "DT"
+Show the date and time mostly in ISO 8601 format. This is the default format
+for timestamp properties.
+
+=item "timetipdate"
+Same as above but put the date part in a tooltip.
+
+=back
+
 =back
 =cut
 
-sub GenerateValueHTML($$$);
+sub GenerateValueHTML($$$;$);
 
-sub GenerateValueHTML($$$)
+sub GenerateValueHTML($$$;$)
 {
-  my ($Parent, $PropertyDescriptor, $Value) = @_;
+  my ($Parent, $PropertyDescriptor, $Value, $Format) = @_;
 
   if ($PropertyDescriptor->GetClass() eq "Basic")
   {
-    if ($PropertyDescriptor->GetType() eq "B")
+    $Format ||= $PropertyDescriptor->GetType();
+    if ($Format eq "B")
     {
       print $Value ? "Yes" : "No";
       return;
     }
-    if ($PropertyDescriptor->GetType() eq "DT")
+    if ($Format eq "DT")
     {
       GenerateDateTime($Value);
       return;
     }
+    if ($Format eq "timetipdate")
+    {
+      GenerateTimeTipDate($Value);
+      return;
+    }
   }
   elsif ($PropertyDescriptor->GetClass() eq "Itemref" and defined $Value)
   {
-- 
2.30.2

[tools 3/5] testbot/cgi: Let DisplayProperty() specify how to format the property.

[Francois Gouget]

This applies to CollectionBlocks as well as FormPages.

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

diff --git a/testbot/lib/ObjectModel/CGI/CollectionBlock.pm b/testbot/lib/ObjectModel/CGI/CollectionBlock.pm
index 14e3cfe54..b2ebb2eec 100644
--- a/testbot/lib/ObjectModel/CGI/CollectionBlock.pm
+++ b/testbot/lib/ObjectModel/CGI/CollectionBlock.pm
@@ -166,10 +166,15 @@ sub GenerateErrorPopup($)
 
 =item C<DisplayProperty()>
 
-Specifies whether to hide the given property.
+Specifies whether to hide the given property and how to format it.
 
-Any value evaluating to false specifies that the property should be hidden and
-all others that it should be shown.
+Returns a (display, format) array where the format value is optional.
+If the display value is false the property is hidden. Otherwise it should be
+"ro" to get a read-only view.
+
+The format value is passed to GenerateDataView() which can use it to decide
+how to format the property. By default it is simply passed to
+ValueFormatter::GenerateValueHTML().
 
 =back
 =cut
@@ -299,6 +304,14 @@ The object for the current row.
 Furthermore one can get the link to the current object's details page by
 passing the row to GetDetailsLink().
 
+The Col parameter has the following extra field:
+=over 12
+=item Format
+The format to be used to display the property.
+If undefined the default formatting is used.
+
+=back
+
 =back
 =cut
 
@@ -308,7 +321,7 @@ sub GenerateDataView($$$)
 
   my $PropertyName = $Col->{Descriptor}->GetName();
   my $Value = $Row->{Item}->$PropertyName;
-  GenerateValueHTML($self, $Col->{Descriptor}, $Value);
+  GenerateValueHTML($self, $Col->{Descriptor}, $Value, $Col->{Format});
 }
 
 
@@ -419,10 +432,11 @@ sub GenerateList($)
   my (@Cols, $HasDT);
   foreach my $PropertyDescriptor (@$PropertyDescriptors)
   {
-    my $Display = $self->DisplayProperty($PropertyDescriptor);
+    my ($Display, $Format) = $self->DisplayProperty($PropertyDescriptor);
     next if (!$Display);
     push @Cols, {Descriptor => $PropertyDescriptor,
                  Display => $Display,
+                 Format => $Format,
                  Index => $ColIndex++};
     $HasDT ||= ($PropertyDescriptor->GetClass() eq "Basic" and
                 $PropertyDescriptor->GetType() eq "DT");
diff --git a/testbot/lib/ObjectModel/CGI/FormPage.pm b/testbot/lib/ObjectModel/CGI/FormPage.pm
index 7fd2f561e..e286880fd 100644
--- a/testbot/lib/ObjectModel/CGI/FormPage.pm
+++ b/testbot/lib/ObjectModel/CGI/FormPage.pm
@@ -156,16 +156,18 @@ sub GetPropertyDescriptorByName($$)
 
 =item C<DisplayProperty()>
 
-Specifies whether to hide, display or allow editing the given property.
+Specifies whether to hide, display or allow editing the given property, and
+optionally how to format it.
 
-Return values:
+Returns a (display, format) array where the display value is one of the
+following:
 =over
 =item ""
 Any value evaluating to false specifies that the property should be hidden.
 
 =item "ro"
 Specifies that the property is read-only, i.e. GenerateField() should display
-it instead of generating an edit field.
+it using the default format instead of generating an edit field.
 
 =item "rw"
 Specifies that GenerateField() should allow editing the property.
@@ -174,6 +176,11 @@ HTTP parameter name.
 
 =back
 
+The optional format value is passed to GenerateValueInput() and
+GenerateValueView() which can use it to decide how to present the value.
+In the default implementation the format is only used for read-only properties
+and is passed to ValueFormatter::GenerateValueHTML().
+
 =back
 =cut
 
@@ -278,11 +285,11 @@ Generates an HTML snippet representing the value in a user-readable form.
 =back
 =cut
 
-sub GenerateValueView($$$)
+sub GenerateValueView($$$$)
 {
-  my ($self, $PropertyDescriptor, $Value) = @_;
+  my ($self, $PropertyDescriptor, $Value, $Format) = @_;
 
-  GenerateValueHTML($self, $PropertyDescriptor, $Value);
+  GenerateValueHTML($self, $PropertyDescriptor, $Value, $Format);
 }
 
 sub GetInputType($$)
@@ -329,9 +336,9 @@ Generates an HTML snippet allowing the specified value to be edited.
 =back
 =cut
 
-sub GenerateValueInput($$$)
+sub GenerateValueInput($$$$)
 {
-  my ($self, $PropertyDescriptor, $Value) = @_;
+  my ($self, $PropertyDescriptor, $Value, $_Format) = @_;
 
   my $InputType = $self->GetInputType($PropertyDescriptor);
   if ($InputType eq "checkbox")
@@ -419,9 +426,9 @@ sub GenerateFormStart($)
        "' method='$self->{Method}' enctype='multipart/form-data'>\n";
 }
 
-sub GenerateField($$$)
+sub GenerateField($$$$)
 {
-  my ($self, $PropertyDescriptor, $Display) = @_;
+  my ($self, $PropertyDescriptor, $Display, $Format) = @_;
 
   print "<div class='ItemProperty'><label>",
         $self->escapeHTML($self->GetDisplayName($PropertyDescriptor)),
@@ -431,12 +438,12 @@ sub GenerateField($$$)
   if ($Display eq "rw")
   {
     print "<div class='ItemValue'>";
-    $self->GenerateValueInput($PropertyDescriptor, $Value);
+    $self->GenerateValueInput($PropertyDescriptor, $Value, $Format);
     print "</div>";
   }
   else
   {
-    $self->GenerateValueView($PropertyDescriptor, $Value);
+    $self->GenerateValueView($PropertyDescriptor, $Value, $Format);
   }
 
   print "</div>\n";
@@ -448,8 +455,8 @@ sub GenerateFields($)
 
   foreach my $PropertyDescriptor (@{$self->GetPropertyDescriptors()})
   {
-    my $Display = $self->DisplayProperty($PropertyDescriptor);
-    $self->GenerateField($PropertyDescriptor, $Display) if ($Display);
+    my ($Display, $Format) = $self->DisplayProperty($PropertyDescriptor);
+    $self->GenerateField($PropertyDescriptor, $Display, $Format) if ($Display);
   }
 }
 
@@ -498,7 +505,7 @@ sub GenerateBody($)
     if (defined $self->{ErrField})
     {
       my $PropertyDescriptor = $self->GetPropertyDescriptorByName($self->{ErrField});
-      if ($PropertyDescriptor and !$self->DisplayProperty($PropertyDescriptor))
+      if ($PropertyDescriptor and !($self->DisplayProperty($PropertyDescriptor))[0])
       {
         $self->{ErrMessage} = "Internal error?\n$self->{ErrMessage}";
       }
@@ -599,7 +606,7 @@ sub Save($)
   {
     if ($PropertyDescriptor->GetClass() eq "Basic" &&
         $PropertyDescriptor->GetType() eq "B" &&
-        $self->DisplayProperty($PropertyDescriptor) eq "rw" &&
+        ($self->DisplayProperty($PropertyDescriptor))[0] eq "rw" &&
         ! defined($self->GetParam($PropertyDescriptor->GetName())))
     {
       if (! $self->SaveProperty($PropertyDescriptor, ""))
-- 
2.30.2

[tools 4/5] testbot/web: Show the job submission date as a tooltip on the main page.

[Francois Gouget]

Only the jobs of the past few days are shown so the date is not very
useful. Delegating it to a tooltip avoids much line-wrapping in the
Submitted column, allowing more lines to fit on-screen.

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com>
---
 testbot/web/index.pl | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/testbot/web/index.pl b/testbot/web/index.pl
index 331ee85ea..4048cacd3 100644
--- a/testbot/web/index.pl
+++ b/testbot/web/index.pl
@@ -46,7 +46,8 @@ sub DisplayProperty($$)
     return !1;
   }
 
-  return $self->SUPER::DisplayProperty($PropertyDescriptor);
+  return $PropertyName eq "Submitted" ? ("ro", "timetipdate") :
+         $self->SUPER::DisplayProperty($PropertyDescriptor);
 }
 
 sub GenerateHeaderView($$$)
@@ -58,6 +59,10 @@ sub GenerateHeaderView($$$)
   {
     print "<a class='title' title='Higher values indicate a lower priority'>Nice</a>";
   }
+  elsif ($PropertyName eq "Submitted")
+  {
+    print "<a class='title' title='Date'>Submitted</a>";
+  }
   elsif ($PropertyName eq "Ended")
   {
     print "<a class='title' title='Ended'>Time</a>";
-- 
2.30.2

[tools 5/5] testbot/web: Fix and share the main page's job completion date formatting.

[Francois Gouget]

The main page still depended on the pre-datetime.js ShowDateTime()
JavaScript function, which the new one is not compatible with.
Add GenerateTipDateTime() so a timestamp can easily be added as a
tooltip to an arbitrary HTML chunk.
Tweak the column title to better reflect its content (contrast duration
with elapsed time).

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com>
---
The tooltip for this column was in fact broken by commit 78f2c0fd53ea
"Handle the timestamp formatting in ValueFormatter".

This is because the main page was implicitly depending on the
ShowDateTime() JavaScript function defined by the collection block.
However that commit changed the way ShowDateTime() works, making it
incompatible with the main page.
What masked the issue is the fallback plain-HTML case. It just caused
the completion timestamps to be shown in the server's timezone instead
of the user's.

Also this is a case where the formatting cannot usefully be delegated
to DisplayProperty() because this actually makes use of two different
values:
- The timestamp to be shown as the tooltip.
- And a totally unrelated value to be shown as the base text (here the
  elapsed time from job submission to completion).

The framework and GenerateValueHTML() could be extended to expect two
values for the 'tipdatetime' format but this would require providing
a way to override a column's default value (or providing a custom
GetPropertyValue() implementation in FormPage's case) which would be no
simpler nor maintainable than simply doing it in
Generate{Data,Value}View().
---
 testbot/lib/ObjectModel/CGI/ValueFormatter.pm | 47 ++++++++++++++++++-
 testbot/web/WineTestBot.css                   |  1 +
 testbot/web/index.pl                          | 12 ++---
 testbot/web/js/datetime.js                    | 19 ++++++--
 4 files changed, 65 insertions(+), 14 deletions(-)

diff --git a/testbot/lib/ObjectModel/CGI/ValueFormatter.pm b/testbot/lib/ObjectModel/CGI/ValueFormatter.pm
index 638504d32..d987cee55 100644
--- a/testbot/lib/ObjectModel/CGI/ValueFormatter.pm
+++ b/testbot/lib/ObjectModel/CGI/ValueFormatter.pm
@@ -22,7 +22,7 @@ use strict;
 package ObjectModel::CGI::ValueFormatter;
 
 use Exporter 'import';
-our @EXPORT = qw(GetDateTimeJSFile GenerateDateTime
+our @EXPORT = qw(GetDateTimeJSFile GenerateDateTime GenerateTipDateTime
                  GenerateTimeTipDate GenerateValueHTML);
 
 use POSIX qw(strftime);
@@ -106,6 +106,51 @@ sub GenerateDateTime($;$$)
 =pod
 =over 12
 
+=item C<GenerateTipDateTime()>
+
+Adds a tooltip containing the timestamp to the specified HTML chunk.
+
+The timestamp is converted to the ISO 8601 format in the user's timezone if
+JavaScript is available and in the server's timezone otherwise.
+
+The default for TagAttrs is 'a' and it should always be a tag that shows the
+content of the title attribute as a tooltip.
+
+See GenerateDateTime() for more details.
+
+=back
+=cut
+
+sub GenerateTipDateTime($$;$$)
+{
+  my ($Sec1970, $Html, $Class, $TagAttrs) = @_;
+
+  if (defined $Sec1970)
+  {
+    my $Tag;
+    if ($TagAttrs)
+    {
+      $Tag = $TagAttrs;
+      $Tag =~ s/ .*$//;
+    }
+    else
+    {
+      $TagAttrs = $Tag = "a";
+    }
+    $Class = "$Class " if ($Class);
+    print "<$TagAttrs class='${Class}tipdatetime' timestamp='$Sec1970' ",
+          strftime("title='%Y-%m-%d %H:%M:%S'>", localtime($Sec1970)),
+          "$Html</$Tag>";
+  }
+  else
+  {
+    print $Html;
+  }
+}
+
+=pod
+=over 12
+
 =item C<GenerateTimeTipDate()>
 
 Show the timestamp's time with the date as a tooltip.
diff --git a/testbot/web/WineTestBot.css b/testbot/web/WineTestBot.css
index 0955f58cf..350ab64a4 100644
--- a/testbot/web/WineTestBot.css
+++ b/testbot/web/WineTestBot.css
@@ -408,6 +408,7 @@ pre
 .log-fullnew { color: red; font-weight: bold; }
 
 a.title { color:inherit; text-decoration: none; }
+a.tipdatetime { color:inherit; text-decoration: none; }
 a.timetipdate { color:inherit; text-decoration: none; }
 
 th.Record { text-align: center; }
diff --git a/testbot/web/index.pl b/testbot/web/index.pl
index 4048cacd3..b07b39d22 100644
--- a/testbot/web/index.pl
+++ b/testbot/web/index.pl
@@ -25,7 +25,7 @@ package JobStatusBlock;
 use ObjectModel::CGI::CollectionBlock;
 our @ISA = qw(ObjectModel::CGI::CollectionBlock);
 
-use POSIX qw(strftime);
+use ObjectModel::CGI::ValueFormatter;
 
 use WineTestBot::Branches;
 use WineTestBot::Users;
@@ -65,7 +65,7 @@ sub GenerateHeaderView($$$)
   }
   elsif ($PropertyName eq "Ended")
   {
-    print "<a class='title' title='Ended'>Time</a>";
+    print "<a class='title' title='Completion timestamp'>Elapsed</a>";
   }
   else
   {
@@ -156,13 +156,7 @@ sub GenerateDataView($$$)
     if (defined $Job->Ended)
     {
       my $Duration = $Job->Ended - $Job->Submitted;
-      my $TagId = "E". $Job->Id;
-      print "<a id='$TagId' class='title' title='",
-            strftime("%Y-%m-%d %H:%M:%S", localtime($Job->Ended)),
-            "'>", DurationToString($Duration), "</a>\n";
-      print "<script type='text/javascript'><!--\n";
-      print "  ShowDateTime(", $Job->Ended, ",'$TagId');\n";
-      print "--></script>";
+      GenerateTipDateTime($Job->Ended, DurationToString($Duration));
     }
     else
     {
diff --git a/testbot/web/js/datetime.js b/testbot/web/js/datetime.js
index 47b0feb0f..23b7b2718 100644
--- a/testbot/web/js/datetime.js
+++ b/testbot/web/js/datetime.js
@@ -23,13 +23,21 @@ function Pad2(n)
     return n < 10 ? '0' + n : n;
 }
 
-function ShowDateTime(dom)
+function ShowDateTime(dom, attr)
 {
     const sec1970 = dom.getAttribute("timestamp");
     const dt = new Date(sec1970 * 1000);
-    dom.innerHTML = dt.getFullYear() +'-'+ Pad2(dt.getMonth() + 1) +'-'+
-                    Pad2(dt.getDate()) +' '+ Pad2(dt.getHours()) +':'+
-                    Pad2(dt.getMinutes()) +':'+ Pad2(dt.getSeconds())
+    const pretty = dt.getFullYear() +'-'+ Pad2(dt.getMonth() + 1) +'-'+
+                   Pad2(dt.getDate()) +' '+ Pad2(dt.getHours()) +':'+
+                   Pad2(dt.getMinutes()) +':'+ Pad2(dt.getSeconds());
+    if (attr == null)
+    {
+        dom.innerHTML = pretty;
+    }
+    else
+    {
+        dom.setAttribute(attr, pretty);
+    }
 }
 
 function ShowTimeTipDate(dom)
@@ -47,6 +55,9 @@ function init()
     document.querySelectorAll(".datetime").forEach(dom => {
         ShowDateTime(dom);
     });
+    document.querySelectorAll(".tipdatetime").forEach(dom => {
+        ShowDateTime(dom, 'title');
+    });
     document.querySelectorAll(".timetipdate").forEach(dom => {
         ShowTimeTipDate(dom);
     });
-- 
2.30.2