From ca49fffc1bad400c2fbfbff23400b543ab1b1d1e Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 13 Apr 2018 06:45:46 -0700 Subject: [PATCH] Fix the legacy "25, 50, 100, unlimited" Harbormaster log links to respect generation selection Summary: See PHI565. Ref T13120. Although this older log is on the chopping block (see T13088), there's some migration guidance and other complexity around just replacing it. Until it gets replaced, make clicking the "number of lines" elements respect the current "Build Generation" setting. Prior to this change, clicking the links would lose the generation information and jump you to the most recent build generation. Also fix some collateral damage from T13105 where we ended up with white text on a white background in some cases. Test Plan: - Restarted a build to get multiple generations. - On each generation, clicked the various "25", "50", etc., links. - Saw generation and log window sizes both respected by the links. Reviewers: amckinley Reviewed By: amckinley Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam Maniphest Tasks: T13120 Differential Revision: https://secure.phabricator.com/D19367 --- .../HarbormasterBuildViewController.php | 76 +++++++++++-------- .../harbormaster/view/ShellLogView.php | 3 - 2 files changed, 44 insertions(+), 35 deletions(-) diff --git a/src/applications/harbormaster/controller/HarbormasterBuildViewController.php b/src/applications/harbormaster/controller/HarbormasterBuildViewController.php index 0646bd9309..ac3a7101fd 100644 --- a/src/applications/harbormaster/controller/HarbormasterBuildViewController.php +++ b/src/applications/harbormaster/controller/HarbormasterBuildViewController.php @@ -77,7 +77,6 @@ final class HarbormasterBuildViewController pht('View Current Build'))); } - $curtain = $this->buildCurtainView($build); $properties = $this->buildPropertyList($build); $history = $this->buildHistoryTable( @@ -292,7 +291,7 @@ final class HarbormasterBuildViewController $targets[] = $target_box; - $targets[] = $this->buildLog($build, $build_target); + $targets[] = $this->buildLog($build, $build_target, $generation); } $timeline = $this->buildTransactionTimeline( @@ -371,7 +370,8 @@ final class HarbormasterBuildViewController private function buildLog( HarbormasterBuild $build, - HarbormasterBuildTarget $build_target) { + HarbormasterBuildTarget $build_target, + $generation) { $request = $this->getRequest(); $viewer = $request->getUser(); @@ -410,6 +410,8 @@ final class HarbormasterBuildViewController $log_view->setLines($lines); $log_view->setStart($start); + $subheader = $this->createLogHeader($build, $log, $limit, $generation); + $prototype_view = id(new PHUIButtonView()) ->setTag('a') ->setHref($log->getURI()) @@ -423,7 +425,7 @@ final class HarbormasterBuildViewController $log->getLogSource(), $log->getLogType())) ->addActionLink($prototype_view) - ->setSubheader($this->createLogHeader($build, $log)) + ->setSubheader($subheader) ->setUser($viewer); $log_box = id(new PHUIObjectBoxView()) @@ -479,43 +481,53 @@ final class HarbormasterBuildViewController return $log_boxes; } - private function createLogHeader($build, $log) { - $request = $this->getRequest(); - $limit = $request->getInt('l', 25); + private function createLogHeader($build, $log, $limit, $generation) { + $options = array( + array( + 'n' => 25, + ), + array( + 'n' => 50, + ), + array( + 'n' => 100, + ), + array( + 'n' => 0, + 'label' => pht('Unlimited'), + ), + ); - $lines_25 = $this->getApplicationURI('/build/'.$build->getID().'/?l=25'); - $lines_50 = $this->getApplicationURI('/build/'.$build->getID().'/?l=50'); - $lines_100 = - $this->getApplicationURI('/build/'.$build->getID().'/?l=100'); - $lines_0 = $this->getApplicationURI('/build/'.$build->getID().'/?l=0'); + $base_uri = id(new PhutilURI($build->getURI().$generation.'/')); - $link_25 = phutil_tag('a', array('href' => $lines_25), pht('25')); - $link_50 = phutil_tag('a', array('href' => $lines_50), pht('50')); - $link_100 = phutil_tag('a', array('href' => $lines_100), pht('100')); - $link_0 = phutil_tag('a', array('href' => $lines_0), pht('Unlimited')); + $links = array(); + foreach ($options as $option) { + $n = $option['n']; + $label = idx($option, 'label', $n); - if ($limit === 25) { - $link_25 = phutil_tag('strong', array(), $link_25); - } else if ($limit === 50) { - $link_50 = phutil_tag('strong', array(), $link_50); - } else if ($limit === 100) { - $link_100 = phutil_tag('strong', array(), $link_100); - } else if ($limit === 0) { - $link_0 = phutil_tag('strong', array(), $link_0); + $is_selected = ($limit == $n); + if ($is_selected) { + $links[] = phutil_tag( + 'strong', + array(), + $label); + } else { + $links[] = phutil_tag( + 'a', + array( + 'href' => (string)$base_uri->alter('l', $n), + ), + $label); + } } return phutil_tag( 'span', array(), array( - $link_25, - ' - ', - $link_50, - ' - ', - $link_100, - ' - ', - $link_0, - ' Lines', + phutil_implode_html(' - ', $links), + ' ', + pht('Lines'), )); } diff --git a/src/applications/harbormaster/view/ShellLogView.php b/src/applications/harbormaster/view/ShellLogView.php index a78840741c..c61237db10 100644 --- a/src/applications/harbormaster/view/ShellLogView.php +++ b/src/applications/harbormaster/view/ShellLogView.php @@ -65,7 +65,6 @@ final class ShellLogView extends AphrontView { 'th', array( 'class' => 'phabricator-source-line', - 'style' => 'background-color: #fff;', ), $content_number); @@ -95,13 +94,11 @@ final class ShellLogView extends AphrontView { 'div', array( 'class' => 'phabricator-source-code-container', - 'style' => 'background-color: black; color: white;', ), phutil_tag( 'table', array( 'class' => implode(' ', $classes), - 'style' => 'background-color: black', ), phutil_implode_html('', $rows))); }