From 6c9026c33a45aaec7cab5d805a116e4c3e71cc78 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 24 Aug 2017 14:40:11 -0700 Subject: [PATCH] Allow ModularTransactions to opt in to providing data to Conduit Summary: Ref T5873. See PHI14. I don't want to just expose internal transaction data to Conduit by default, since it's often: unstable, unusable, sensitive, or some combination of the three. Instead, let ModularTransactions opt in to providing additional data to Conduit, similar to other infrastructure. If a transaction doesn't, the API returns an empty skeleton for it. This is generally fine since most transactions have no real use cases, and I think we can fill them in as we go. This also probably builds toward T5726, which would likely use the same format, and perhaps simply not publish stuff which did not opt in. This doesn't actually cover "comment" or "inline comment", which are presumably what PHI14 is after, since neither is modular. I'll probably just put a hack in place for this until they can modularize since I suspect modularizing them here is difficult. Test Plan: Ran `transaction.search` on a revision, saw some transactions (title and status transactions) populate with values. Reviewers: chad Reviewed By: chad Maniphest Tasks: T5873 Differential Revision: https://secure.phabricator.com/D18467 --- .../DifferentialRevisionStatusTransaction.php | 11 +++++ .../DifferentialRevisionTitleTransaction.php | 11 +++++ .../TransactionSearchConduitAPIMethod.php | 43 +++++++++++++++++++ .../PhabricatorModularTransactionType.php | 12 ++++++ 4 files changed, 77 insertions(+) diff --git a/src/applications/differential/xaction/DifferentialRevisionStatusTransaction.php b/src/applications/differential/xaction/DifferentialRevisionStatusTransaction.php index c08eb9d187..615ce38bcf 100644 --- a/src/applications/differential/xaction/DifferentialRevisionStatusTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionStatusTransaction.php @@ -70,4 +70,15 @@ final class DifferentialRevisionStatusTransaction return DifferentialRevisionStatus::newForStatus($new); } + public function getTransactionTypeForConduit($xaction) { + return 'status'; + } + + public function getFieldValuesForConduit($object, $data) { + return array( + 'old' => $object->getOldValue(), + 'new' => $object->getNewValue(), + ); + } + } diff --git a/src/applications/differential/xaction/DifferentialRevisionTitleTransaction.php b/src/applications/differential/xaction/DifferentialRevisionTitleTransaction.php index 9b763c53ca..812464b26d 100644 --- a/src/applications/differential/xaction/DifferentialRevisionTitleTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionTitleTransaction.php @@ -55,4 +55,15 @@ final class DifferentialRevisionTitleTransaction return $errors; } + public function getTransactionTypeForConduit($xaction) { + return 'title'; + } + + public function getFieldValuesForConduit($object, $data) { + return array( + 'old' => $object->getOldValue(), + 'new' => $object->getNewValue(), + ); + } + } diff --git a/src/applications/transactions/conduit/TransactionSearchConduitAPIMethod.php b/src/applications/transactions/conduit/TransactionSearchConduitAPIMethod.php index 0f655199fa..b3065b0d9c 100644 --- a/src/applications/transactions/conduit/TransactionSearchConduitAPIMethod.php +++ b/src/applications/transactions/conduit/TransactionSearchConduitAPIMethod.php @@ -89,6 +89,36 @@ final class TransactionSearchConduitAPIMethod $comment_map = array(); } + $modular_classes = array(); + $modular_objects = array(); + $modular_xactions = array(); + foreach ($xactions as $xaction) { + if (!$xaction instanceof PhabricatorModularTransaction) { + continue; + } + + $modular_template = $xaction->getModularType(); + $modular_class = get_class($modular_template); + if (!isset($modular_objects[$modular_class])) { + try { + $modular_object = newv($modular_class, array()); + $modular_objects[$modular_class] = $modular_object; + } catch (Exception $ex) { + continue; + } + } + + $modular_classes[$xaction->getPHID()] = $modular_class; + $modular_xactions[$modular_class][] = $xaction; + } + + $modular_data_map = array(); + foreach ($modular_objects as $class => $modular_type) { + $modular_data_map[$class] = $modular_type + ->setViewer($viewer) + ->loadTransactionTypeConduitData($modular_xactions[$class]); + } + $data = array(); foreach ($xactions as $xaction) { $comments = idx($comment_map, $xaction->getPHID()); @@ -126,6 +156,18 @@ final class TransactionSearchConduitAPIMethod } $fields = array(); + $type = null; + + if (isset($modular_classes[$xaction->getPHID()])) { + $modular_class = $modular_classes[$xaction->getPHID()]; + $modular_object = $modular_objects[$modular_class]; + $modular_data = $modular_data_map[$modular_class]; + + $type = $modular_object->getTransactionTypeForConduit($xaction); + $fields = $modular_object->getFieldValuesForConduit( + $xaction, + $modular_data); + } if (!$fields) { $fields = (object)$fields; @@ -134,6 +176,7 @@ final class TransactionSearchConduitAPIMethod $data[] = array( 'id' => (int)$xaction->getID(), 'phid' => (string)$xaction->getPHID(), + 'type' => $type, 'authorPHID' => (string)$xaction->getAuthorPHID(), 'objectPHID' => (string)$xaction->getObjectPHID(), 'dateCreated' => (int)$xaction->getDateCreated(), diff --git a/src/applications/transactions/storage/PhabricatorModularTransactionType.php b/src/applications/transactions/storage/PhabricatorModularTransactionType.php index 128b5c7c19..119bfedd32 100644 --- a/src/applications/transactions/storage/PhabricatorModularTransactionType.php +++ b/src/applications/transactions/storage/PhabricatorModularTransactionType.php @@ -332,4 +332,16 @@ abstract class PhabricatorModularTransactionType return $this->getStorage()->getMetadataValue($key, $default); } + public function loadTransactionTypeConduitData(array $xactions) { + return null; + } + + public function getTransactionTypeForConduit($xaction) { + return null; + } + + public function getFieldValuesForConduit($xaction, $data) { + return array(); + } + }