Give unit test results their own table in Differential

Summary: Ref T10457. This gives unit test results a more first-class treatment in the Differential UI, and consolidates some rendering code.

Test Plan:
Before:

{F1135536}

After:

{F1135537}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10457

Differential Revision: https://secure.phabricator.com/D15365

resources/celerity/map.php

@@ -7,7 +7,7 @@
  */
 return array(
   'names' => array(
-    'core.pkg.css' => 'f7a91f6a',
+    'core.pkg.css' => '76a3afdf',
     'core.pkg.js' => '7d8faf57',
     'darkconsole.pkg.js' => 'e7393ebb',
     'differential.pkg.css' => '2de124c9',
@@ -25,7 +25,7 @@ return array(
     'rsrc/css/aphront/notification.css' => '7f684b62',
     'rsrc/css/aphront/panel-view.css' => '8427b78d',
     'rsrc/css/aphront/phabricator-nav-view.css' => 'ac79a758',
-    'rsrc/css/aphront/table-view.css' => 'ec078a76',
+    'rsrc/css/aphront/table-view.css' => 'aba95954',
     'rsrc/css/aphront/tokenizer.css' => '056da01b',
     'rsrc/css/aphront/tooltip.css' => '1a07aea8',
     'rsrc/css/aphront/typeahead-browse.css' => 'd8581d2c',
@@ -523,7 +523,7 @@ return array(
     'aphront-list-filter-view-css' => '5d6f0526',
     'aphront-multi-column-view-css' => 'fd18389d',
     'aphront-panel-view-css' => '8427b78d',
-    'aphront-table-view-css' => 'ec078a76',
+    'aphront-table-view-css' => 'aba95954',
     'aphront-tokenizer-control-css' => '056da01b',
     'aphront-tooltip-css' => '1a07aea8',
     'aphront-typeahead-control-css' => 'd4f16145',

src/__phutil_library_map__.php

@@ -1149,6 +1149,7 @@ phutil_register_library_map(array(
     'HarbormasterUnitMessageViewController' => 'applications/harbormaster/controller/HarbormasterUnitMessageViewController.php',
     'HarbormasterUnitPropertyView' => 'applications/harbormaster/view/HarbormasterUnitPropertyView.php',
     'HarbormasterUnitStatus' => 'applications/harbormaster/constants/HarbormasterUnitStatus.php',
+    'HarbormasterUnitSummaryView' => 'applications/harbormaster/view/HarbormasterUnitSummaryView.php',
     'HarbormasterUploadArtifactBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterUploadArtifactBuildStepImplementation.php',
     'HarbormasterWaitForPreviousBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterWaitForPreviousBuildStepImplementation.php',
     'HarbormasterWorker' => 'applications/harbormaster/worker/HarbormasterWorker.php',
@@ -4624,7 +4625,7 @@ phutil_register_library_map(array(
     'DifferentialTransactionEditor' => 'PhabricatorApplicationTransactionEditor',
     'DifferentialTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
     'DifferentialTransactionView' => 'PhabricatorApplicationTransactionView',
-    'DifferentialUnitField' => 'DifferentialHarbormasterField',
+    'DifferentialUnitField' => 'DifferentialCustomField',
     'DifferentialUnitStatus' => 'Phobject',
     'DifferentialUnitTestResult' => 'Phobject',
     'DifferentialUpdateRevisionConduitAPIMethod' => 'DifferentialConduitAPIMethod',
@@ -5318,6 +5319,7 @@ phutil_register_library_map(array(
     'HarbormasterUnitMessageViewController' => 'HarbormasterController',
     'HarbormasterUnitPropertyView' => 'AphrontView',
     'HarbormasterUnitStatus' => 'Phobject',
+    'HarbormasterUnitSummaryView' => 'AphrontView',
     'HarbormasterUploadArtifactBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
     'HarbormasterWaitForPreviousBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
     'HarbormasterWorker' => 'PhabricatorWorker',

src/applications/differential/controller/DifferentialController.php

@@ -92,4 +92,130 @@ abstract class DifferentialController extends PhabricatorController {
     return $toc_view;
   }
 
+  protected function loadDiffProperties(array $diffs) {
+    $diffs = mpull($diffs, null, 'getID');
+
+    $properties = id(new DifferentialDiffProperty())->loadAllWhere(
+      'diffID IN (%Ld)',
+      array_keys($diffs));
+    $properties = mgroup($properties, 'getDiffID');
+
+    foreach ($diffs as $id => $diff) {
+      $values = idx($properties, $id, array());
+      $values = mpull($values, 'getData', 'getName');
+      $diff->attachDiffProperties($values);
+    }
+  }
+
+
+  protected function loadHarbormasterData(array $diffs) {
+    $viewer = $this->getViewer();
+
+    $diffs = mpull($diffs, null, 'getPHID');
+
+    $buildables = id(new HarbormasterBuildableQuery())
+      ->setViewer($viewer)
+      ->withBuildablePHIDs(array_keys($diffs))
+      ->withManualBuildables(false)
+      ->needBuilds(true)
+      ->needTargets(true)
+      ->execute();
+
+    $buildables = mpull($buildables, null, 'getBuildablePHID');
+    foreach ($diffs as $phid => $diff) {
+      $diff->attachBuildable(idx($buildables, $phid));
+    }
+
+    $target_map = array();
+    foreach ($diffs as $phid => $diff) {
+      $target_map[$phid] = $diff->getBuildTargetPHIDs();
+    }
+    $all_target_phids = array_mergev($target_map);
+
+    if ($all_target_phids) {
+      $unit_messages = id(new HarbormasterBuildUnitMessage())->loadAllWhere(
+        'buildTargetPHID IN (%Ls)',
+        $all_target_phids);
+      $unit_messages = mgroup($unit_messages, 'getBuildTargetPHID');
+    } else {
+      $unit_messages = array();
+    }
+
+    foreach ($diffs as $phid => $diff) {
+      $target_phids = idx($target_map, $phid, array());
+      $messages = array_select_keys($unit_messages, $target_phids);
+      $messages = array_mergev($messages);
+      $diff->attachUnitMessages($messages);
+    }
+
+    // For diffs with no messages, look for legacy unit messages stored on the
+    // diff itself.
+    foreach ($diffs as $phid => $diff) {
+      if ($diff->getUnitMessages()) {
+        continue;
+      }
+
+      if (!$diff->hasDiffProperty('arc:unit')) {
+        continue;
+      }
+
+      $legacy_messages = $diff->getProperty('arc:unit');
+      if (!$legacy_messages) {
+        continue;
+      }
+
+      // Show the top 100 legacy lint messages. Previously, we showed some
+      // by default and let the user toggle the rest. With modern messages,
+      // we can send the user to the Harbormaster detail page. Just show
+      // "a lot" of messages in legacy cases to try to strike a balance
+      // between implementation simplicitly and compatibility.
+      $legacy_messages = array_slice($legacy_messages, 0, 100);
+
+      $messages = array();
+      foreach ($legacy_messages as $message) {
+        $messages[] = HarbormasterBuildUnitMessage::newFromDictionary(
+          new HarbormasterBuildTarget(),
+          $this->getModernUnitMessageDictionary($message));
+      }
+
+      $diff->attachUnitMessages($messages);
+    }
+  }
+
+  private function getModernUnitMessageDictionary(array $map) {
+    // Strip out `null` values to satisfy stricter typechecks.
+    foreach ($map as $key => $value) {
+      if ($value === null) {
+        unset($map[$key]);
+      }
+    }
+
+    return $map;
+  }
+
+  protected function getDiffTabLabels(array $diffs) {
+    // Make sure we're only going to render unique diffs.
+    $diffs = mpull($diffs, null, 'getID');
+    $labels = array(pht('Left'), pht('Right'));
+
+    $results = array();
+
+    foreach ($diffs as $diff) {
+      if (count($diffs) == 2) {
+        $label = array_shift($labels);
+        $label = pht('%s (Diff %d)', $label, $diff->getID());
+      } else {
+        $label = pht('Diff %d', $diff->getID());
+      }
+
+      $results[] = array(
+        $label,
+        $diff,
+      );
+    }
+
+    return $results;
+  }
+
+
 }

src/applications/differential/controller/DifferentialRevisionViewController.php

@@ -102,10 +102,8 @@ final class DifferentialRevisionViewController extends DifferentialController {
       }
     }
 
-    $props = id(new DifferentialDiffProperty())->loadAllWhere(
+    $this->loadDiffProperties($diffs);
-      'diffID = %d',
+    $props = $target_manual->getDiffProperties();
-      $target_manual->getID());
-    $props = mpull($props, 'getData', 'getName');
 
     $object_phids = array_merge(
       $revision->getReviewers(),
@@ -256,23 +254,17 @@ final class DifferentialRevisionViewController extends DifferentialController {
       array($diff_vs, $target->getID()));
     $detail_diffs = mpull($detail_diffs, null, 'getPHID');
 
-    $buildables = id(new HarbormasterBuildableQuery())
+    $this->loadHarbormasterData($detail_diffs);
-      ->setViewer($viewer)
-      ->withBuildablePHIDs(array_keys($detail_diffs))
-      ->withManualBuildables(false)
-      ->needBuilds(true)
-      ->needTargets(true)
-      ->execute();
-    $buildables = mpull($buildables, null, 'getBuildablePHID');
-    foreach ($detail_diffs as $diff_phid => $detail_diff) {
-      $detail_diff->attachBuildable(idx($buildables, $diff_phid));
-    }
 
     $diff_detail_box = $this->buildDiffDetailView(
       $detail_diffs,
       $revision,
       $field_list);
 
+    $unit_box = $this->buildUnitMessagesView(
+      $target,
+      $revision);
+
     $comment_view = $this->buildTransactions(
       $revision,
       $diff_vs ? $diffs[$diff_vs] : $target,
@@ -469,6 +461,7 @@ final class DifferentialRevisionViewController extends DifferentialController {
       $operations_box,
       $revision_detail_box,
       $diff_detail_box,
+      $unit_box,
       $page_pane,
     );
 
@@ -972,18 +965,9 @@ final class DifferentialRevisionViewController extends DifferentialController {
       return null;
     }
 
-    // Make sure we're only going to render unique diffs.
-    $diffs = mpull($diffs, null, 'getID');
-    $labels = array(pht('Left'), pht('Right'));
-
     $property_lists = array();
-    foreach ($diffs as $diff) {
+    foreach ($this->getDiffTabLabels($diffs) as $tab) {
-      if (count($diffs) == 2) {
+      list($label, $diff) = $tab;
-        $label = array_shift($labels);
-        $label = pht('%s (Diff %d)', $label, $diff->getID());
-      } else {
-        $label = pht('Diff %d', $diff->getID());
-      }
 
       $property_lists[] = array(
         $label,
@@ -1083,4 +1067,44 @@ final class DifferentialRevisionViewController extends DifferentialController {
       ->setOperation($operation);
   }
 
+  private function buildUnitMessagesView(
+    $diff,
+    DifferentialRevision $revision) {
+    $viewer = $this->getViewer();
+
+    if (!$diff->getUnitMessages()) {
+      return null;
+    }
+
+    $interesting_messages = array();
+    foreach ($diff->getUnitMessages() as $message) {
+      switch ($message->getResult()) {
+        case ArcanistUnitTestResult::RESULT_PASS:
+        case ArcanistUnitTestResult::RESULT_SKIP:
+          break;
+        default:
+          $interesting_messages[] = $message;
+          break;
+      }
+    }
+
+    if (!$interesting_messages) {
+      return null;
+    }
+
+    $excuse = null;
+    if ($diff->hasDiffProperty('arc:unit-excuse')) {
+      $excuse = $diff->getProperty('arc:unit-excuse');
+    }
+
+    return id(new HarbormasterUnitSummaryView())
+      ->setUser($viewer)
+      ->setExcuse($excuse)
+      ->setBuildable($diff->getBuildable())
+      ->setUnitMessages($diff->getUnitMessages())
+      ->setLimit(5)
+      ->setShowViewAll(true);
+  }
+
+
 }

src/applications/differential/customfield/DifferentialUnitField.php

@@ -1,7 +1,7 @@
 <?php
 
 final class DifferentialUnitField
-  extends DifferentialHarbormasterField {
+  extends DifferentialCustomField {
 
   public function getFieldKey() {
     return 'differential:unit';
@@ -31,62 +31,6 @@ final class DifferentialUnitField
     return $this->getFieldName();
   }
 
-  protected function getLegacyProperty() {
-    return 'arc:unit';
-  }
-
-  protected function getDiffPropertyKeys() {
-    return array(
-      'arc:unit',
-      'arc:unit-excuse',
-    );
-  }
-
-  protected function loadHarbormasterTargetMessages(array $target_phids) {
-    return id(new HarbormasterBuildUnitMessage())->loadAllWhere(
-      'buildTargetPHID IN (%Ls)',
-      $target_phids);
-  }
-
-  protected function newModernMessage(array $message) {
-    return HarbormasterBuildUnitMessage::newFromDictionary(
-      new HarbormasterBuildTarget(),
-      $this->getModernUnitMessageDictionary($message));
-  }
-
-  protected function newHarbormasterMessageView(array $all_messages) {
-    $messages = $all_messages;
-
-    foreach ($messages as $key => $message) {
-      switch ($message->getResult()) {
-        case ArcanistUnitTestResult::RESULT_PASS:
-        case ArcanistUnitTestResult::RESULT_SKIP:
-          // Don't show "Pass" or "Skip" in the UI since they aren't very
-          // interesting. The user can click through to the full results if
-          // they want details.
-          unset($messages[$key]);
-          break;
-      }
-    }
-
-    if (!$messages) {
-      return null;
-    }
-
-    $table = id(new HarbormasterUnitPropertyView())
-      ->setLimit(5)
-      ->setUnitMessages($all_messages);
-
-    $diff = $this->getObject()->getActiveDiff();
-    $buildable = $diff->getBuildable();
-    if ($buildable) {
-      $full_results = '/harbormaster/unit/'.$buildable->getID().'/';
-      $table->setFullResultsURI($full_results);
-    }
-
-    return $table;
-  }
-
   public function getWarningsForDetailView() {
     $status = $this->getObject()->getActiveDiff()->getUnitStatus();
 
@@ -105,9 +49,7 @@ final class DifferentialUnitField
     return $warnings;
   }
 
-  protected function renderHarbormasterStatus(
+  public function renderDiffPropertyViewValue(DifferentialDiff $diff) {
-    DifferentialDiff $diff,
-    array $messages) {
 
     $colors = array(
       DifferentialUnitStatus::UNIT_NONE => 'grey',
@@ -121,42 +63,15 @@ final class DifferentialUnitField
 
     $message = DifferentialRevisionUpdateHistoryView::getDiffUnitMessage($diff);
 
-    $note = array();
-
-    $excuse = $diff->getProperty('arc:unit-excuse');
-    if (strlen($excuse)) {
-      $excuse = array(
-        phutil_tag('strong', array(), pht('Excuse:')),
-        ' ',
-        phutil_escape_html_newlines($excuse),
-      );
-      $note[] = $excuse;
-    }
-
-    $note = phutil_implode_html(" \xC2\xB7 ", $note);
-
     $status = id(new PHUIStatusListView())
       ->addItem(
         id(new PHUIStatusItemView())
           ->setIcon(PHUIStatusItemView::ICON_STAR, $icon_color)
-          ->setTarget($message)
+          ->setTarget($message));
-          ->setNote($note));
 
     return $status;
   }
 
-  private function getModernUnitMessageDictionary(array $map) {
-    // Strip out `null` values to satisfy stricter typechecks.
-    foreach ($map as $key => $value) {
-      if ($value === null) {
-        unset($map[$key]);
-      }
-    }
-
-    // TODO: Remap more stuff here?
-
-    return $map;
-  }
 
 
 }

src/applications/differential/storage/DifferentialDiff.php

@@ -40,6 +40,8 @@ final class DifferentialDiff
   private $properties = array();
   private $buildable = self::ATTACHABLE;
 
+  private $unitMessages = self::ATTACHABLE;
+
   protected function getConfiguration() {
     return array(
       self::CONFIG_AUX_PHID => true,
@@ -334,6 +336,20 @@ final class DifferentialDiff
     return $this->assertAttachedKey($this->properties, $key);
   }
 
+  public function hasDiffProperty($key) {
+    $properties = $this->getDiffProperties();
+    return array_key_exists($key, $properties);
+  }
+
+  public function attachDiffProperties(array $properties) {
+    $this->properties = $properties;
+    return $this;
+  }
+
+  public function getDiffProperties() {
+    return $this->assertAttached($this->properties);
+  }
+
   public function attachBuildable(HarbormasterBuildable $buildable = null) {
     $this->buildable = $buildable;
     return $this;
@@ -391,6 +407,17 @@ final class DifferentialDiff
   }
 
 
+  public function attachUnitMessages(array $unit_messages) {
+    $this->unitMessages = $unit_messages;
+    return $this;
+  }
+
+
+  public function getUnitMessages() {
+    return $this->assertAttached($this->unitMessages);
+  }
+
+
 /* -(  PhabricatorPolicyInterface  )----------------------------------------- */
 
 

src/applications/harbormaster/controller/HarbormasterBuildableViewController.php

@@ -336,42 +336,11 @@ final class HarbormasterBuildableViewController
     }
 
     if ($unit_data) {
-      $unit_href = $this->getApplicationURI('unit/'.$buildable->getID().'/');
+      $unit = id(new HarbormasterUnitSummaryView())
-
+        ->setBuildable($buildable)
-      $unit_table = id(new HarbormasterUnitPropertyView())
-        ->setUser($viewer)
-        ->setLimit(5)
         ->setUnitMessages($unit_data)
-        ->setFullResultsURI($unit_href);
+        ->setShowViewAll(true)
-
+        ->setLimit(5);
-      $unit_data = msort($unit_data, 'getSortKey');
-      $head_unit = head($unit_data);
-      if ($head_unit) {
-        $status = $head_unit->getResult();
-
-        $tag_text = HarbormasterUnitStatus::getUnitStatusLabel($status);
-        $tag_color = HarbormasterUnitStatus::getUnitStatusColor($status);
-        $tag_icon = HarbormasterUnitStatus::getUnitStatusIcon($status);
-
-      } else {
-        $tag_text = pht('No Unit Tests');
-        $tag_color = 'grey';
-        $tag_icon = 'fa-ban';
-      }
-
-      $unit_header = id(new PHUIHeaderView())
-        ->setHeader(pht('Unit Tests'))
-        ->setStatus($tag_icon, $tag_color, $tag_text)
-        ->addActionLink(
-          id(new PHUIButtonView())
-            ->setTag('a')
-            ->setHref($unit_href)
-            ->setIcon('fa-list-ul')
-            ->setText('View All'));
-
-      $unit = id(new PHUIObjectBoxView())
-        ->setHeader($unit_header)
-        ->setTable($unit_table);
     } else {
       $unit = null;
     }

src/applications/harbormaster/controller/HarbormasterUnitMessageListController.php

@@ -34,14 +34,10 @@ final class HarbormasterUnitMessageListController
       $unit_data = array();
     }
 
-    $unit_table = id(new HarbormasterUnitPropertyView())
+    $unit = id(new HarbormasterUnitSummaryView())
-      ->setUser($viewer)
+      ->setBuildable($buildable)
       ->setUnitMessages($unit_data);
 
-    $unit = id(new PHUIObjectBoxView())
-      ->setHeaderText(pht('Unit Tests'))
-      ->setTable($unit_table);
-
     $crumbs = $this->buildApplicationCrumbs();
     $this->addBuildableCrumb($crumbs, $buildable);
     $crumbs->addTextCrumb(pht('Unit Tests'));

src/applications/harbormaster/view/HarbormasterUnitPropertyView.php

@@ -6,6 +6,7 @@ final class HarbormasterUnitPropertyView extends AphrontView {
   private $unitMessages = array();
   private $limit;
   private $fullResultsURI;
+  private $notice;
 
   public function setPathURIMap(array $map) {
     $this->pathURIMap = $map;
@@ -28,6 +29,12 @@ final class HarbormasterUnitPropertyView extends AphrontView {
     return $this;
   }
 
+  public function setNotice($notice) {
+    $this->notice = $notice;
+    return $this;
+  }
+
+
   public function render() {
     require_celerity_resource('harbormaster-css');
 
@@ -68,12 +75,14 @@ final class HarbormasterUnitPropertyView extends AphrontView {
       $name = $message->getUnitMessageDisplayName();
       $id = $message->getID();
 
-      $name = phutil_tag(
+      if ($id) {
-        'a',
+        $name = phutil_tag(
-        array(
+          'a',
-          'href' => "/harbormaster/unit/view/{$id}/",
+          array(
-        ),
+            'href' => "/harbormaster/unit/view/{$id}/",
-        $name);
+          ),
+          $name);
+      }
 
       $details = $message->getUnitMessageDetails();
       if (strlen($details)) {
@@ -127,8 +136,8 @@ final class HarbormasterUnitPropertyView extends AphrontView {
         ))
       ->setColumnClasses(
         array(
-          'top',
+          'top center',
-          'top',
+          'top right',
           'top wide',
         ))
       ->setColumnWidths(
@@ -142,6 +151,10 @@ final class HarbormasterUnitPropertyView extends AphrontView {
           $any_duration,
         ));
 
+    if ($this->notice) {
+      $table->setNotice($this->notice);
+    }
+
     return $table;
   }
 

src/applications/harbormaster/view/HarbormasterUnitSummaryView.php

@@ -0,0 +1,104 @@
+<?php
+
+final class HarbormasterUnitSummaryView extends AphrontView {
+
+  private $buildable;
+  private $messages;
+  private $limit;
+  private $excuse;
+  private $showViewAll;
+
+  public function setBuildable(HarbormasterBuildable $buildable) {
+    $this->buildable = $buildable;
+    return $this;
+  }
+
+  public function setUnitMessages(array $messages) {
+    $this->messages = $messages;
+    return $this;
+  }
+
+  public function setLimit($limit) {
+    $this->limit = $limit;
+    return $this;
+  }
+
+  public function setExcuse($excuse) {
+    $this->excuse = $excuse;
+    return $this;
+  }
+
+  public function setShowViewAll($show_view_all) {
+    $this->showViewAll = $show_view_all;
+    return $this;
+  }
+
+  public function render() {
+    $messages = $this->messages;
+    $buildable = $this->buildable;
+
+    $id = $buildable->getID();
+    $full_uri = "/harbormaster/unit/{$id}/";
+
+    $messages = msort($messages, 'getSortKey');
+    $head_unit = head($messages);
+    if ($head_unit) {
+      $status = $head_unit->getResult();
+
+      $tag_text = HarbormasterUnitStatus::getUnitStatusLabel($status);
+      $tag_color = HarbormasterUnitStatus::getUnitStatusColor($status);
+      $tag_icon = HarbormasterUnitStatus::getUnitStatusIcon($status);
+    } else {
+      $tag_text = pht('No Unit Tests');
+      $tag_color = 'grey';
+      $tag_icon = 'fa-ban';
+    }
+
+    $header = id(new PHUIHeaderView())
+      ->setHeader(pht('Unit Tests'))
+      ->setStatus($tag_icon, $tag_color, $tag_text);
+
+    if ($this->showViewAll) {
+      $view_all = id(new PHUIButtonView())
+        ->setTag('a')
+        ->setHref($full_uri)
+        ->setIcon('fa-list-ul')
+        ->setText('View All');
+      $header->addActionLink($view_all);
+    }
+
+    $box = id(new PHUIObjectBoxView())
+      ->setHeader($header);
+
+    $table = id(new HarbormasterUnitPropertyView())
+      ->setUnitMessages($messages);
+
+    if ($this->showViewAll) {
+      $table->setFullResultsURI($full_uri);
+    }
+
+    if ($this->limit) {
+      $table->setLimit($this->limit);
+    }
+
+    $excuse = $this->excuse;
+    if (strlen($excuse)) {
+      $excuse_icon = id(new PHUIIconView())
+        ->setIcon('fa-commenting-o red');
+
+      $table->setNotice(
+        array(
+          $excuse_icon,
+          ' ',
+          phutil_tag('strong', array(), pht('Excuse:')),
+          ' ',
+          $excuse,
+        ));
+    }
+
+    $box->setTable($table);
+
+    return $box;
+  }
+
+}

src/view/control/AphrontTableView.php

@@ -157,21 +157,6 @@ final class AphrontTableView extends AphrontView {
         $sort_values[] = null;
       }
 
-      if ($this->notice) {
-        $colspan = max(count(array_filter($visibility)), 1);
-        $table[] = phutil_tag(
-          'tr',
-          array(),
-          phutil_tag(
-            'td',
-            array(
-              'colspan' => $colspan,
-              'class' => 'aphront-table-notice',
-            ),
-            $this->notice));
-
-      }
-
       $tr = array();
       foreach ($headers as $col_num => $header) {
         if (!$visibility[$col_num]) {
@@ -350,13 +335,29 @@ final class AphrontTableView extends AphrontView {
       $classes[] = 'aphront-table-view-fixed';
     }
 
+    $notice = null;
+    if ($this->notice) {
+      $notice = phutil_tag(
+        'div',
+        array(
+          'class' => 'aphront-table-notice',
+        ),
+        $this->notice);
+    }
+
     $html = phutil_tag(
       'table',
       array(
         'class' => implode(' ', $classes),
       ),
       $table);
-    return phutil_tag_div('aphront-table-wrap', $html);
+
+    return phutil_tag_div(
+      'aphront-table-wrap',
+      array(
+        $notice,
+        $html,
+      ));
   }
 
   public static function renderSingleDisplayLine($line) {

webroot/rsrc/css/aphront/table-view.css

@@ -19,7 +19,11 @@
   table-layout: fixed;
 }
 
-.aphront-table-view td.aphront-table-notice {
+.aphront-table-view-fixed th {
+  box-sizing: border-box;
+}
+
+.aphront-table-notice {
   padding: 12px 16px;
   font-size: {$normalfontsize};
   color: {$darkbluetext};