From cc8cbed029416f4a7dc45d4a9a6b565ae76c458e Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 26 Jul 2021 11:37:35 -0700 Subject: [PATCH] Make "DifferentialDiff->properties" a proper "attachable" property Summary: See PHI498. This should be initialized to "self::ATTACHABLE" like other attachable properties, but is currently initialized to "array()". Initialize it the normal way and try to catch all code paths which may have accessed it without actually loading and attaching it. Also, remove UI for the very old "excuse" property, which "arc" has not written for well more than a year. Test Plan: Grepped for affected symbols, loaded various revision pages. Somewhat tricky to be 100% sure that every pathway is caught, but it should be obvious if I missed anything once someone hits the code path. Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam Differential Revision: https://secure.phabricator.com/D21710 --- .../DifferentialRevisionViewController.php | 8 +------ .../differential/storage/DifferentialDiff.php | 5 ++++- .../view/HarbormasterUnitSummaryView.php | 21 ------------------- 3 files changed, 5 insertions(+), 29 deletions(-) diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php index 13ab8a8954..102c7f560b 100644 --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -1282,7 +1282,7 @@ final class DifferentialRevisionViewController } private function buildUnitMessagesView( - $diff, + DifferentialDiff $diff, DifferentialRevision $revision) { $viewer = $this->getViewer(); @@ -1310,14 +1310,8 @@ final class DifferentialRevisionViewController return null; } - $excuse = null; - if ($diff->hasDiffProperty('arc:unit-excuse')) { - $excuse = $diff->getProperty('arc:unit-excuse'); - } - return id(new HarbormasterUnitSummaryView()) ->setViewer($viewer) - ->setExcuse($excuse) ->setBuildable($diff->getBuildable()) ->setUnitMessages($diff->getUnitMessages()) ->setLimit(5) diff --git a/src/applications/differential/storage/DifferentialDiff.php b/src/applications/differential/storage/DifferentialDiff.php index f5cb91c56b..78025e44ec 100644 --- a/src/applications/differential/storage/DifferentialDiff.php +++ b/src/applications/differential/storage/DifferentialDiff.php @@ -42,7 +42,7 @@ final class DifferentialDiff private $unsavedChangesets = array(); private $changesets = self::ATTACHABLE; private $revision = self::ATTACHABLE; - private $properties = array(); + private $properties = self::ATTACHABLE; private $buildable = self::ATTACHABLE; private $unitMessages = self::ATTACHABLE; @@ -338,6 +338,9 @@ final class DifferentialDiff } public function attachProperty($key, $value) { + if (!is_array($this->properties)) { + $this->properties = array(); + } $this->properties[$key] = $value; return $this; } diff --git a/src/applications/harbormaster/view/HarbormasterUnitSummaryView.php b/src/applications/harbormaster/view/HarbormasterUnitSummaryView.php index 66ba2307b6..154e1c96fc 100644 --- a/src/applications/harbormaster/view/HarbormasterUnitSummaryView.php +++ b/src/applications/harbormaster/view/HarbormasterUnitSummaryView.php @@ -5,7 +5,6 @@ final class HarbormasterUnitSummaryView extends AphrontView { private $buildable; private $messages; private $limit; - private $excuse; private $showViewAll; public function setBuildable(HarbormasterBuildable $buildable) { @@ -23,11 +22,6 @@ final class HarbormasterUnitSummaryView extends AphrontView { return $this; } - public function setExcuse($excuse) { - $this->excuse = $excuse; - return $this; - } - public function setShowViewAll($show_view_all) { $this->showViewAll = $show_view_all; return $this; @@ -88,21 +82,6 @@ final class HarbormasterUnitSummaryView extends AphrontView { $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;