From 046888ac2e56cbe20122053bce6140be4d2f1142 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 13 May 2019 09:53:49 -0700 Subject: [PATCH] In Herald, save applied transaction PHIDs in the transcript and display them in the UI Summary: Ref T13283. Since D14575, we already pass applied transactions to Herald, but they exist only as a backwards compatibility layer and have no upstream callsites. Save the applied transaction PHIDs as part of the object transcript, and show them in the UI. Test Plan: - Viewed a modern transcript, saw a list of transactions. - Viewed an older transcript, saw nothing (since there were no transactions in the transcript). {F6456431} Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13283 Differential Revision: https://secure.phabricator.com/D20518 --- .../controller/HeraldTranscriptController.php | 94 +++++++++++++++++++ .../herald/engine/HeraldEngine.php | 17 +++- .../herald/query/HeraldTranscriptQuery.php | 12 ++- .../transcript/HeraldObjectTranscript.php | 10 ++ .../storage/transcript/HeraldTranscript.php | 11 +++ 5 files changed, 138 insertions(+), 6 deletions(-) diff --git a/src/applications/herald/controller/HeraldTranscriptController.php b/src/applications/herald/controller/HeraldTranscriptController.php index dc7ca676e3..60dd6dc6ed 100644 --- a/src/applications/herald/controller/HeraldTranscriptController.php +++ b/src/applications/herald/controller/HeraldTranscriptController.php @@ -20,6 +20,8 @@ final class HeraldTranscriptController extends HeraldController { return new Aphront404Response(); } + $object = $xscript->getObject(); + require_celerity_resource('herald-test-css'); $content = array(); @@ -72,6 +74,9 @@ final class HeraldTranscriptController extends HeraldController { $content[] = array( $this->buildActionTranscriptPanel($xscript), $this->buildObjectTranscriptPanel($xscript), + $this->buildTransactionsTranscriptPanel( + $object, + $xscript), ); } @@ -502,5 +507,94 @@ final class HeraldTranscriptController extends HeraldController { return $box; } + private function buildTransactionsTranscriptPanel( + $object, + HeraldTranscript $xscript) { + $viewer = $this->getViewer(); + + $object_xscript = $xscript->getObjectTranscript(); + + $xaction_phids = $object_xscript->getAppliedTransactionPHIDs(); + + // If the value is "null", this is an older transcript or this adapter + // does not use transactions. We render nothing. + // + // If the value is "array()", this is a modern transcript which uses + // transactions, there just weren't any applied. Below, we'll render a + // "No Transactions Applied" state. + if ($xaction_phids === null) { + return null; + } + + // If this object doesn't implement the right interface, we won't be + // able to load the transactions. Just bail. + if (!($object instanceof PhabricatorApplicationTransactionInterface)) { + return null; + } + + $query = PhabricatorApplicationTransactionQuery::newQueryForObject( + $object); + + if ($xaction_phids) { + $xactions = $query + ->setViewer($viewer) + ->withPHIDs($xaction_phids) + ->execute(); + $xactions = mpull($xactions, null, 'getPHID'); + } else { + $xactions = array(); + } + + $rows = array(); + foreach ($xaction_phids as $xaction_phid) { + $xaction = idx($xactions, $xaction_phid); + + $xaction_identifier = $xaction_phid; + $xaction_date = null; + $xaction_display = null; + if ($xaction) { + $xaction_identifier = $xaction->getID(); + $xaction_date = phabricator_datetime( + $xaction->getDateCreated(), + $viewer); + + // Since we don't usually render transactions outside of the context + // of objects, some of them might depend on missing object data. Out of + // an abundance of caution, catch any rendering issues. + try { + $xaction_display = $xaction->getTitle(); + } catch (Exception $ex) { + $xaction_display = $ex->getMessage(); + } + } + + $rows[] = array( + $xaction_identifier, + $xaction_display, + $xaction_date, + ); + } + + $table_view = id(new AphrontTableView($rows)) + ->setHeaders( + array( + pht('ID'), + pht('Transaction'), + pht('Date'), + )) + ->setColumnClasses( + array( + null, + 'wide', + null, + )); + + $box_view = id(new PHUIObjectBoxView()) + ->setHeaderText(pht('Transactions')) + ->setTable($table_view); + + return $box_view; + } + } diff --git a/src/applications/herald/engine/HeraldEngine.php b/src/applications/herald/engine/HeraldEngine.php index d853e3eb9a..2d96532882 100644 --- a/src/applications/herald/engine/HeraldEngine.php +++ b/src/applications/herald/engine/HeraldEngine.php @@ -126,11 +126,18 @@ final class HeraldEngine extends Phobject { } } - $object_transcript = new HeraldObjectTranscript(); - $object_transcript->setPHID($object->getPHID()); - $object_transcript->setName($object->getHeraldName()); - $object_transcript->setType($object->getAdapterContentType()); - $object_transcript->setFields($this->fieldCache); + $xaction_phids = null; + $xactions = $object->getAppliedTransactions(); + if ($xactions !== null) { + $xaction_phids = mpull($xactions, 'getPHID'); + } + + $object_transcript = id(new HeraldObjectTranscript()) + ->setPHID($object->getPHID()) + ->setName($object->getHeraldName()) + ->setType($object->getAdapterContentType()) + ->setFields($this->fieldCache) + ->setAppliedTransactionPHIDs($xaction_phids); $this->transcript->setObjectTranscript($object_transcript); diff --git a/src/applications/herald/query/HeraldTranscriptQuery.php b/src/applications/herald/query/HeraldTranscriptQuery.php index 71789e07c5..00a9dffeaf 100644 --- a/src/applications/herald/query/HeraldTranscriptQuery.php +++ b/src/applications/herald/query/HeraldTranscriptQuery.php @@ -81,10 +81,20 @@ final class HeraldTranscriptQuery ->execute(); foreach ($transcripts as $key => $transcript) { - if (empty($objects[$transcript->getObjectPHID()])) { + $object_phid = $transcript->getObjectPHID(); + + if (!$object_phid) { + $transcript->attachObject(null); + continue; + } + + $object = idx($objects, $object_phid); + if (!$object) { $this->didRejectResult($transcript); unset($transcripts[$key]); } + + $transcript->attachObject($object); } return $transcripts; diff --git a/src/applications/herald/storage/transcript/HeraldObjectTranscript.php b/src/applications/herald/storage/transcript/HeraldObjectTranscript.php index e0b85162af..9d331b271e 100644 --- a/src/applications/herald/storage/transcript/HeraldObjectTranscript.php +++ b/src/applications/herald/storage/transcript/HeraldObjectTranscript.php @@ -6,6 +6,7 @@ final class HeraldObjectTranscript extends Phobject { protected $type; protected $name; protected $fields; + protected $appliedTransactionPHIDs; public function setPHID($phid) { $this->phid = $phid; @@ -47,6 +48,15 @@ final class HeraldObjectTranscript extends Phobject { return $this->fields; } + public function setAppliedTransactionPHIDs($phids) { + $this->appliedTransactionPHIDs = $phids; + return $this; + } + + public function getAppliedTransactionPHIDs() { + return $this->appliedTransactionPHIDs; + } + private static function truncateValue($value, $length) { if (is_string($value)) { if (strlen($value) <= $length) { diff --git a/src/applications/herald/storage/transcript/HeraldTranscript.php b/src/applications/herald/storage/transcript/HeraldTranscript.php index 474eb10b53..9539d4653d 100644 --- a/src/applications/herald/storage/transcript/HeraldTranscript.php +++ b/src/applications/herald/storage/transcript/HeraldTranscript.php @@ -18,6 +18,8 @@ final class HeraldTranscript extends HeraldDAO protected $dryRun; protected $garbageCollected = 0; + private $object = self::ATTACHABLE; + const TABLE_SAVED_HEADER = 'herald_savedheader'; public function getXHeraldRulesHeader() { @@ -194,6 +196,15 @@ final class HeraldTranscript extends HeraldDAO HeraldTranscriptPHIDType::TYPECONST); } + public function attachObject($object = null) { + $this->object = $object; + return $this; + } + + public function getObject() { + return $this->assertAttached($this->object); + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */ public function getCapabilities() {