Delegate generating the inside of a cell to GenerateDataView() such that GenerateDataCell() only deals with the cell itself. Remove GetDisplayValue() and GetEscapedDisplayValue(): CollectionBlocks don't deal with editing so there is no need for access to the 'raw' data. Thus they are both replaced by GenerateDataView().
Signed-off-by: Francois Gouget fgouget@codeweavers.com --- .../lib/ObjectModel/CGI/CollectionBlock.pm | 251 +++++++++++------- testbot/web/JobDetails.pl | 36 +-- testbot/web/PatchesList.pl | 14 +- testbot/web/WineTestBot.css | 4 +- testbot/web/admin/UsersList.pl | 14 +- testbot/web/index.pl | 58 ++-- 6 files changed, 210 insertions(+), 167 deletions(-)
diff --git a/testbot/lib/ObjectModel/CGI/CollectionBlock.pm b/testbot/lib/ObjectModel/CGI/CollectionBlock.pm index f124e646b..435c44e7f 100644 --- a/testbot/lib/ObjectModel/CGI/CollectionBlock.pm +++ b/testbot/lib/ObjectModel/CGI/CollectionBlock.pm @@ -183,116 +183,134 @@ sub GetSortedItems($$) return $self->{Collection}->GetSortedItems($Items); }
-sub GetDisplayValue($$$) -{ - my ($self, $Item, $PropertyDescriptor) = @_; +=pod +=over 12
- my $PropertyName = $PropertyDescriptor->GetName(); - my $Value = $Item->$PropertyName; +=item C<GetDetailsLink()>
- if ($PropertyDescriptor->GetClass() eq "Itemref") - { - if (defined($Value)) - { - # Note: This only supports single-key Itemrefs - foreach $PropertyDescriptor (@{$Value->GetPropertyDescriptors()}) - { - if ($PropertyDescriptor->GetIsKey()) - { - $PropertyName = $PropertyDescriptor->GetName(); - $Value = $Value->$PropertyName; - last; - } - } - } - } +Returns the URL of the details page for the current row's object. + +See GenerateDataView() for details about the Row parameter.
- if ($PropertyDescriptor->GetClass() eq "Basic") +=back +=cut + +sub GetDetailsLink($$) +{ + my ($self, $Row) = @_; + + if (!$Row->{DetailsLink}) { - if ($PropertyDescriptor->GetType() eq "B") - { - $Value = ($Value ? "Yes" : "No"); - } - elsif ($PropertyDescriptor->GetType() eq "DT") + $Row->{DetailsLink} = "$Row->{DetailsPage}?Key=". uri_escape($Row->{Item}->GetKey()); + my ($MasterColNames, $MasterColValues) = $Row->{Item}->GetMasterCols(); + if (defined $MasterColNames) { - if (defined($Value)) + foreach my $ColIndex (0 .. @$MasterColNames - 1) { - $Value = - "<noscript><div>" . - strftime("%Y-%m-%d %H:%M:%S", localtime($Value)) . "</div></noscript>\n" . -"<script type='text/javascript'><!--\n" . - "ShowDateTime($Value);\n" . - "//--></script>\n"; + $Row->{DetailsLink} .= "&". $MasterColNames->[$ColIndex] ."=". + uri_escape($MasterColValues->[$ColIndex]); } } } - - return $Value; + return $Row->{DetailsLink}; }
-sub GetEscapedDisplayValue($$$) -{ - my ($self, $Item, $PropertyDescriptor) = @_; +=pod +=over 12
- my $PropertyName = $PropertyDescriptor->GetName(); - my $Value = $Item->$PropertyName; +=item C<GenerateHeaderView()>
- if ($PropertyDescriptor->GetClass() eq "Basic" && - $PropertyDescriptor->GetType() eq "DT") - { - if (defined($Value)) - { - $Value = "<script type='text/javascript'><!--\n" . - "ShowDateTime($Value);\n" . - "//--></script><noscript><div>" . - strftime("%Y-%m-%d %H:%M:%S", localtime($Value)) . - "</div></noscript>\n"; - } - } - else - { - $Value = $self->escapeHTML($self->GetDisplayValue($Item, $PropertyDescriptor)); - } +Generates an HTML snippet for the column title.
- return $Value; -} +The Row parameter contains the following information: +=over 12
+=item PropertyDescriptors +The list of item properties, including those that DisplayProperty() says +should be hidden.
-# -# Item cell generation -# +=item DetailsPage +The base link to the item details page if any. See GetDetailsPage(). + +=item ItemActions +The list of per-item actions. + +=cut + +=back +=cut
-sub GenerateHeaderCell($$) +sub GenerateHeaderView($$$) { - my ($self, $PropertyDescriptor) = @_; - print "<th>", $self->escapeHTML($PropertyDescriptor->GetDisplayName()), - "</th>\n"; + my ($self, $_Row, $PropertyDescriptor) = @_; + + print $self->escapeHTML($PropertyDescriptor->GetDisplayName()); }
-sub GenerateDataCell($$$$) +=pod +=over 12 + +=item C<GenerateDataView()> + +Generates an HTML snippet representing the property value in a user-readable +form. + +The Row parameter contains the same information as for GenerateHeaderView() +plus: +=over 12 + +=item Row +The row number starting at 1. + +=item Item +The object for the current row. + +=back + +Furthermore one can get the link to the current object's details page by +passing the row to GetDetailsLink(). + +=back +=cut + +sub GenerateDataView($$$) { - my ($self, $Item, $PropertyDescriptor, $DetailsPage) = @_; + my ($self, $Row, $PropertyDescriptor) = @_;
- print "<td>"; - my $NeedLink; - if ($PropertyDescriptor->GetIsKey() && $DetailsPage) + my $PropertyName = $PropertyDescriptor->GetName(); + my $Value = $Row->{Item}->$PropertyName; + + if ($PropertyDescriptor->GetClass() eq "Itemref" and defined $Value) { - $NeedLink = 1; - my $Query = "$DetailsPage?Key=" . uri_escape($Item->GetKey()); - my ($MasterColNames, $MasterColValues) = $Item->GetMasterCols(); - if (defined($MasterColNames)) + # Note: This only supports single-key Itemrefs + foreach my $ValuePD (@{$Value->GetPropertyDescriptors()}) { - foreach my $ColIndex (0 .. @$MasterColNames - 1) + if ($ValuePD->GetIsKey()) { - $Query .= "&" . $MasterColNames->[$ColIndex] . "=" . - uri_escape($MasterColValues->[$ColIndex]); + $PropertyName = $ValuePD->GetName(); + print $self->escapeHTML($Value->$PropertyName); + return; } } - print "<a href='", $self->escapeHTML($Query), "'>"; } - print $self->GetEscapedDisplayValue($Item, $PropertyDescriptor); - print "</a>" if ($NeedLink); - print "</td>\n"; + elsif ($PropertyDescriptor->GetClass() eq "Basic") + { + if ($PropertyDescriptor->GetType() eq "B") + { + print $Value ? "Yes" : "No"; + return; + } + if ($PropertyDescriptor->GetType() eq "DT" and defined $Value) + { + print "<script type='text/javascript'><!--\n", + "ShowDateTime($Value);\n", + "//--></script><noscript><div>", + strftime("%Y-%m-%d %H:%M:%S", localtime($Value)), + "</div></noscript>\n"; + return; + } + } + print defined $Value ? $self->escapeHTML($Value) : " "; }
@@ -317,23 +335,31 @@ sub GenerateFormStart($) } }
+sub GenerateHeaderCell($$$) +{ + my ($self, $Row, $PropertyDescriptor) = @_; + + print "<th>"; + $self->GenerateHeaderView($Row, $PropertyDescriptor); + print "</th>\n"; +} + sub GenerateHeaderRow($$$) { - my ($self, $PropertyDescriptors, $ItemActions) = @_; + my ($self, $Row) = @_;
print "<tr>\n"; - if (@$ItemActions != 0) + if (@{$Row->{ItemActions}}) { print "<th> </th>\n"; } - foreach my $PropertyDescriptor (@$PropertyDescriptors) + foreach my $PropertyDescriptor (@{$Row->{PropertyDescriptors}}) { if ($self->DisplayProperty($PropertyDescriptor)) { - $self->GenerateHeaderCell($PropertyDescriptor); + $self->GenerateHeaderCell($Row, $PropertyDescriptor); } } - print "</tr>\n"; }
@@ -345,21 +371,37 @@ sub SelName($$) return "sel_" . $Key; }
-sub GenerateDataRow($$$$$$) +sub GenerateDataCell($$$) +{ + my ($self, $Row, $PropertyDescriptor) = @_; + + print "<td>"; + my $CloseLink; + if ($Row->{DetailsPage} and $PropertyDescriptor->GetIsKey()) + { + print "<a href='", $self->escapeHTML($self->GetDetailsLink($Row)), "'>"; + $CloseLink = "</a>"; + } + $self->GenerateDataView($Row, $PropertyDescriptor); + print "$CloseLink</td>\n"; +} + +sub GenerateDataRow($$) { - my ($self, $Item, $PropertyDescriptors, $DetailsPage, $Class, $ItemActions) = @_; + my ($self, $Row) = @_;
+ my $Class = ($Row->{Row} % 2) == 0 ? "even" : "odd"; print "<tr class='$Class'>\n"; - if (@$ItemActions != 0) + if (@{$Row->{ItemActions}}) { - print "<td><input name='", $self->SelName($Item->GetKey()), + print "<td><input name='", $self->SelName($Row->{Item}->GetKey()), "' type='checkbox' /></td>\n"; } - foreach my $PropertyDescriptor (@$PropertyDescriptors) + foreach my $PropertyDescriptor (@{$Row->{PropertyDescriptors}}) { if ($self->DisplayProperty($PropertyDescriptor)) { - $self->GenerateDataCell($Item, $PropertyDescriptor, $DetailsPage); + $self->GenerateDataCell($Row, $PropertyDescriptor); } } print "</tr>\n"; @@ -421,20 +463,23 @@ EOF print "<table border='0' cellpadding='5' cellspacing='0' summary='" . "Overview of " . $Collection->GetCollectionName() . "'>\n"; print "<thead>\n"; - my $ItemActions = $self->{RW} ? $self->GetItemActions() : []; - $self->GenerateHeaderRow($PropertyDescriptors, $ItemActions); + my $Row = { + PropertyDescriptors => $PropertyDescriptors, + DetailsPage => $self->GetDetailsPage(), + ItemActions => $self->{RW} ? $self->GetItemActions() : [], + Row => 0, # 0 for the header ---> 1 for the first line + }; + $self->GenerateHeaderRow($Row); print "</thead>\n";
print "<tbody>\n"; - my $DetailsPage = $self->GetDetailsPage(); - my $Row = 0; my $Items = $self->{Collection}->GetSortedItems(); foreach my $Item (@$Items) { - my $Class = ($Row % 2) == 0 ? "even" : "odd"; - $self->GenerateDataRow($Item, $PropertyDescriptors, $DetailsPage, - $Class, $ItemActions); - $Row++; + $Row->{Row}++; + $Row->{Item} = $Item; + $Row->{DetailsLink} = undef; + $self->GenerateDataRow($Row); } if (@$Items == 0) { @@ -444,7 +489,7 @@ EOF print "</tbody>\n"; print "</table>\n";
- if (@$ItemActions != 0 && @$Items != 0) + if (@{$Row->{ItemActions}} and @$Items) { print <<EOF; <div class='CollectionBlockActions'> @@ -465,7 +510,7 @@ document.write("<a href='javascript:void(0)' onClick='ToggleAll();'>Toggle All<\ </script> EOF print "For selected ", $self->{Collection}->GetCollectionName() . ":"; - foreach my $Action (@$ItemActions) + foreach my $Action (@{$Row->{ItemActions}}) { print " <input type='submit' name='Action' value='" . $self->escapeHTML($Action) . "' />"; diff --git a/testbot/web/JobDetails.pl b/testbot/web/JobDetails.pl index da4ad7e96..2c7901a75 100644 --- a/testbot/web/JobDetails.pl +++ b/testbot/web/JobDetails.pl @@ -61,35 +61,36 @@ sub DisplayProperty($$) # Item cell generation #
-sub GenerateHeaderCell($$) +sub GenerateHeaderView($$$) { - my ($self, $PropertyDescriptor) = @_; + my ($self, $Row, $PropertyDescriptor) = @_;
my $PropertyName = $PropertyDescriptor->GetName(); if ($PropertyName eq "CmdLineArg") { - print "<th>Arguments / <span class='MissionHeader'>Missions</span></th>\n"; + print "Arguments / <span class='MissionHeader'>Missions</span>"; } elsif ($PropertyName eq "Ended") { - print "<th><a class='title' title='Execution ended'>Time</a></th>\n"; + print "<a class='title' title='Execution ended'>Time</a>"; } else { - return $self->SUPER::GenerateHeaderCell($PropertyDescriptor); + $self->SUPER::GenerateHeaderView($Row, $PropertyDescriptor); } }
-sub GenerateDataCell($$$$) +sub GenerateDataView($$$) { - my ($self, $StepTask, $PropertyDescriptor, $DetailsPage) = @_; + my ($self, $Row, $PropertyDescriptor) = @_;
+ my $StepTask = $Row->{Item}; my $PropertyName = $PropertyDescriptor->GetName(); if ($PropertyName eq "VM") { - print "<td><a href='#k", $self->escapeHTML($StepTask->GetKey()), "'>"; + print "<a href='#k", $self->escapeHTML($Row->{Item}->GetKey()), "'>"; print $self->escapeHTML($StepTask->VM->Name); - print "</a></td>\n"; + print "</a>"; } elsif ($PropertyName eq "FileName") { @@ -99,13 +100,13 @@ sub GenerateDataCell($$$$) my $JobId = $self->{EnclosingPage}->GetJob()->Id; my $URI = "/GetFile.pl?JobKey=" . uri_escape($JobId) . "&StepKey=" . uri_escape($StepTask->StepNo); - print "<td><a href='" . $self->escapeHTML($URI) . "'>"; + print "<a href='" . $self->escapeHTML($URI) . "'>"; print $self->escapeHTML($StepTask->FileName); - print "</a></td>\n"; + print "</a>"; } else { - $self->SUPER::GenerateDataCell($StepTask, $PropertyDescriptor, $DetailsPage); + $self->SUPER::GenerateDataView($Row, $PropertyDescriptor); } } elsif ($PropertyName eq "CmdLineArg") @@ -124,7 +125,7 @@ sub GenerateDataCell($$$$) $Args .= "<span class='Mission'>". $self->escapeHTML(GetTaskMissionDescription($Missions->[0], $StepTask->Type)) ."</span>"; } } - print "<td>$Args</td>\n"; + print $Args; } elsif ($PropertyName eq "Ended") { @@ -132,22 +133,21 @@ sub GenerateDataCell($$$$) { my $Duration = $StepTask->Ended - $StepTask->Started; my $TagId = "E". $StepTask->Id; - print "<td><a id='$TagId' class='title' title='", + print "<a id='$TagId' class='title' title='", strftime("%Y-%m-%d %H:%M:%S", localtime($StepTask->Ended)), "'>", DurationToString($Duration), "</a>\n"; print "<script type='text/javascript'><!--\n"; print " ShowDateTime(", $StepTask->Ended, ",'$TagId');\n"; - print "--></script>\n"; - print "</td>\n"; + print "--></script>"; } else { - print "<td> </td>\n"; + print " "; } } else { - $self->SUPER::GenerateDataCell($StepTask, $PropertyDescriptor, $DetailsPage); + $self->SUPER::GenerateDataView($Row, $PropertyDescriptor); } }
diff --git a/testbot/web/PatchesList.pl b/testbot/web/PatchesList.pl index 854c2768a..943b9c5bb 100644 --- a/testbot/web/PatchesList.pl +++ b/testbot/web/PatchesList.pl @@ -46,21 +46,21 @@ sub DisplayProperty($$) $PropertyName eq "FromName" || $PropertyName eq "Subject"; }
-sub GenerateDataCell($$$$$) +sub GenerateDataView($$$) { - my ($self, $Item, $PropertyDescriptor, $DetailsPage) = @_; + my ($self, $Row, $PropertyDescriptor) = @_;
- my $PropertyName = $PropertyDescriptor->GetName(); - if ($PropertyName eq "Disposition" and $Item->Disposition =~ /job ([0-9]+)$/) + if ($PropertyDescriptor->GetName() eq "Disposition" and + $Row->{Item}->Disposition =~ /job ([0-9]+)$/) { my $JobId = $1; my $URI = "/JobDetails.pl?Key=" . uri_escape($JobId); - print "<td><a href='" . $self->escapeHTML($URI) . "'>" . - "Job " . $self->escapeHTML($JobId) . "</a></td>\n"; + print "<a href='" . $self->escapeHTML($URI) . "'>" . + "Job " . $self->escapeHTML($JobId) . "</a>"; } else { - $self->SUPER::GenerateDataCell($Item, $PropertyDescriptor, $DetailsPage); + $self->SUPER::GenerateDataView($Row, $PropertyDescriptor); } }
diff --git a/testbot/web/WineTestBot.css b/testbot/web/WineTestBot.css index 53c939efd..a50125f8e 100644 --- a/testbot/web/WineTestBot.css +++ b/testbot/web/WineTestBot.css @@ -241,13 +241,13 @@ h2 border: 1px solid white; }
-.CollectionBlock tr.even +.CollectionBlock tr.odd { background-color: #fff8f8; color: black; }
-.CollectionBlock tr.odd +.CollectionBlock tr.even { background-color: #f8e8e8; color: black; diff --git a/testbot/web/admin/UsersList.pl b/testbot/web/admin/UsersList.pl index f54def261..bc5ced8a7 100644 --- a/testbot/web/admin/UsersList.pl +++ b/testbot/web/admin/UsersList.pl @@ -46,13 +46,13 @@ sub DisplayProperty($$) $PropertyName eq "Status" || $PropertyName eq "RealName"; }
-sub GenerateDataCell($$$$) +sub GenerateDataCell($$$) { - my ($self, $User, $PropertyDescriptor, $DetailsPage) = @_; + my ($self, $Row, $PropertyDescriptor) = @_;
- my $PropertyName = $PropertyDescriptor->GetName(); - if ($PropertyName eq "Status") + if ($PropertyDescriptor->GetName() eq "Status") { + my $User = $Row->{Item}; my $Status = $User->Status; my ($Class, $Label); if ($Status eq "disabled") @@ -83,12 +83,12 @@ sub GenerateDataCell($$$$) { ($Class, $Label) = ('usernone', 'none'); } - print "<td><a href='/admin/UserDetails.pl?Key=", uri_escape($User->GetKey()), - "'><span class='$Class'>$Label</span></a></td>"; + my $DetailsLink = $self->GetDetailsLink($Row); + print "<td><a href='$DetailsLink'><span class='$Class'>$Label</span></a></td>"; } else { - $self->SUPER::GenerateDataCell($User, $PropertyDescriptor, $DetailsPage); + $self->SUPER::GenerateDataCell($Row, $PropertyDescriptor); } }
diff --git a/testbot/web/index.pl b/testbot/web/index.pl index 478cb61c1..a18920533 100644 --- a/testbot/web/index.pl +++ b/testbot/web/index.pl @@ -50,49 +50,48 @@ sub DisplayProperty($$) return $self->SUPER::DisplayProperty($PropertyDescriptor); }
-sub GenerateHeaderCell($$$) +sub GenerateHeaderView($$$) { - my ($self, $PropertyDescriptor) = @_; + my ($self, $Row, $PropertyDescriptor) = @_;
my $PropertyName = $PropertyDescriptor->GetName(); if ($PropertyName eq "Priority") { - print "<th><a class='title' title='Higher values indicate a lower priority'>Nice</a></th>\n"; + print "<a class='title' title='Higher values indicate a lower priority'>Nice</a>"; } elsif ($PropertyName eq "Ended") { - print "<th><a class='title' title='Ended'>Time</a></th>\n"; + print "<a class='title' title='Ended'>Time</a>"; } else { - return $self->SUPER::GenerateHeaderCell($PropertyDescriptor); + $self->SUPER::GenerateHeaderView($Row, $PropertyDescriptor); } }
-sub GetDisplayValue($$$) +sub GenerateDataView($$$) { - my ($self, $Job, $PropertyDescriptor) = @_; + my ($self, $Row, $PropertyDescriptor) = @_;
- if ($PropertyDescriptor->GetName() eq "User" && - defined($Job->Patch) && - $Job->User->GetKey() eq GetBatchUser()->GetKey() && - defined($Job->Patch->FromName)) + my $Job = $Row->{Item}; + my $PropertyName = $PropertyDescriptor->GetName(); + if ($PropertyName eq "User") { - return $Job->Patch->FromName; + if (defined $Job->Patch and defined $Job->Patch->FromName and + $Job->User->GetKey() eq GetBatchUser()->GetKey()) + { + print $self->escapeHTML($Job->Patch->FromName); + } + else + { + print $self->escapeHTML($Job->User->Name); + } } - - return $self->SUPER::GetDisplayValue($Job, $PropertyDescriptor); -} - -sub GenerateDataCell($$$$) -{ - my ($self, $Job, $PropertyDescriptor, $DetailsPage) = @_; - - my $PropertyName = $PropertyDescriptor->GetName(); - if ($PropertyName eq "Status") + elsif ($PropertyName eq "Status") { - my $EscapedKey = uri_escape($Job->GetKey()); - print "<td id='job$EscapedKey'><a href='/JobDetails.pl?Key=$EscapedKey'>"; + my $EscapedKey = uri_escape($Row->{Item}->GetKey()); + my $DetailsLink = $self->GetDetailsLink($Row); + print "<a id='job$EscapedKey' href='$DetailsLink'>";
my %HTMLChunks = ("queued" => "<span class='queued'>queued</span>", "running" => "<span class='running'>running</span>", @@ -146,7 +145,7 @@ sub GenerateDataCell($$$$) { print $HTMLStatus; } - print "</a></td>\n"; + print "</a>"; } elsif ($PropertyName eq "Ended") { @@ -154,22 +153,21 @@ sub GenerateDataCell($$$$) { my $Duration = $Job->Ended - $Job->Submitted; my $TagId = "E". $Job->Id; - print "<td><a id='$TagId' class='title' title='", + 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>\n"; - print "</td>\n"; + print "--></script>"; } else { - print "<td> </td>\n"; + print " "; } } else { - $self->SUPER::GenerateDataCell($Job, $PropertyDescriptor, $DetailsPage); + $self->SUPER::GenerateDataView($Row, $PropertyDescriptor); } }