From 029cfcfc193933e1d43b81c69f62e9c334065157 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 21 Nov 2012 17:38:57 -0800 Subject: [PATCH] Add subscriber/mention support to Pholio Summary: - Show subscribers. - When a user is mentioned in the description or a comment, subscribe them explicitly. Test Plan: {F22370} Reviewers: btrahan, chad Reviewed By: btrahan CC: aran Maniphest Tasks: T2097 Differential Revision: https://secure.phabricator.com/D3838 --- .../constants/PholioTransactionType.php | 1 + .../controller/PholioMockViewController.php | 72 +++++++++++++-- .../pholio/editor/PholioMockEditor.php | 89 ++++++++++++++++++- .../pholio/storage/PholioTransaction.php | 11 +++ ...PhabricatorSubscriptionsEditController.php | 1 + .../query/PhabricatorSubscribersQuery.php | 4 + 6 files changed, 168 insertions(+), 10 deletions(-) diff --git a/src/applications/pholio/constants/PholioTransactionType.php b/src/applications/pholio/constants/PholioTransactionType.php index f540adc586..ddfd1e6a35 100644 --- a/src/applications/pholio/constants/PholioTransactionType.php +++ b/src/applications/pholio/constants/PholioTransactionType.php @@ -22,5 +22,6 @@ final class PholioTransactionType extends PholioConstants { const TYPE_NAME = 'name'; const TYPE_DESCRIPTION = 'description'; const TYPE_VIEW_POLICY = 'viewPolicy'; + const TYPE_SUBSCRIBERS = 'subscribers'; } diff --git a/src/applications/pholio/controller/PholioMockViewController.php b/src/applications/pholio/controller/PholioMockViewController.php index 609435b46d..8b29c7aec1 100644 --- a/src/applications/pholio/controller/PholioMockViewController.php +++ b/src/applications/pholio/controller/PholioMockViewController.php @@ -44,11 +44,19 @@ final class PholioMockViewController extends PholioController { ->withMockIDs(array($mock->getID())) ->execute(); + $subscribers = PhabricatorSubscribersQuery::loadSubscribersForPHID( + $mock->getPHID()); $phids = array(); $phids[] = $mock->getAuthorPHID(); foreach ($xactions as $xaction) { $phids[] = $xaction->getAuthorPHID(); + foreach ($xaction->getRequiredHandlePHIDs() as $hphid) { + $phids[] = $hphid; + } + } + foreach ($subscribers as $subscriber) { + $phids[] = $subscriber; } $this->loadHandles($phids); @@ -67,15 +75,11 @@ final class PholioMockViewController extends PholioController { ->setHeader($title); $actions = $this->buildActionView($mock); - $properties = $this->buildPropertyView($mock, $engine); + $properties = $this->buildPropertyView($mock, $engine, $subscribers); $carousel = '

'. 'Carousel Goes Here

'; - $comments = - '

'. - 'Comments/Transactions Go Here

'; - $xaction_view = $this->buildTransactionView($xactions, $engine); @@ -123,7 +127,8 @@ final class PholioMockViewController extends PholioController { private function buildPropertyView( PholioMock $mock, - PhabricatorMarkupEngine $engine) { + PhabricatorMarkupEngine $engine, + array $subscribers) { $user = $this->getRequest()->getUser(); @@ -145,6 +150,20 @@ final class PholioMockViewController extends PholioController { pht('Visible To'), $descriptions[PhabricatorPolicyCapability::CAN_VIEW]); + if ($subscribers) { + $sub_view = array(); + foreach ($subscribers as $subscriber) { + $sub_view[] = $this->getHandle($subscriber)->renderLink(); + } + $sub_view = implode(', ', $sub_view); + } else { + $sub_view = ''.pht('None').''; + } + + $properties->addProperty( + pht('Subscribers'), + $sub_view); + $properties->addTextContent( $engine->getOutput($mock, PholioMock::MARKUP_FIELD_DESCRIPTION)); @@ -201,8 +220,9 @@ final class PholioMockViewController extends PholioController { $xaction_visible = true; $title = null; + $type = $xaction->getTransactionType(); - switch ($xaction->getTransactionType()) { + switch ($type) { case PholioTransactionType::TYPE_NONE: $title = pht( '%s added a comment.', @@ -243,6 +263,44 @@ final class PholioMockViewController extends PholioController { phutil_escape_html($old), phutil_escape_html($new)); break; + case PholioTransactionType::TYPE_SUBSCRIBERS: + $rem = array_diff($old, $new); + $add = array_diff($new, $old); + + $add_l = array(); + foreach ($add as $phid) { + $add_l[] = $this->getHandle($phid)->renderLink(); + } + $add_l = implode(', ', $add_l); + + $rem_l = array(); + foreach ($rem as $phid) { + $rem_l[] = $this->getHandle($phid)->renderLink(); + } + $rem_l = implode(', ', $rem_l); + + if ($add && $rem) { + $title = pht( + '%s edited subscriber(s), added %d: %s; removed %d: %s.', + $author->renderLink(), + $add_l, + count($add), + $rem_l, + count($rem)); + } else if ($add) { + $title = pht( + '%s added %d subscriber(s): %s.', + $author->renderLink(), + count($add), + $add_l); + } else if ($rem) { + $title = pht( + '%s removed %d subscribers: %s.', + $author->renderLink(), + count($rem), + $rem_l); + } + break; default: throw new Exception("Unknown transaction type '{$type}'!"); } diff --git a/src/applications/pholio/editor/PholioMockEditor.php b/src/applications/pholio/editor/PholioMockEditor.php index da6950be62..a1f727a65f 100644 --- a/src/applications/pholio/editor/PholioMockEditor.php +++ b/src/applications/pholio/editor/PholioMockEditor.php @@ -41,6 +41,36 @@ final class PholioMockEditor extends PhabricatorEditor { "Call setContentSource() before applyTransactions()!"); } + $comments = array(); + foreach ($xactions as $xaction) { + if (strlen($xaction->getComment())) { + $comments[] = $xaction->getComment(); + } + $type = $xaction->getTransactionType(); + if ($type == PholioTransactionType::TYPE_DESCRIPTION) { + $comments[] = $xaction->getNewValue(); + } + } + + $mentioned_phids = PhabricatorMarkupEngine::extractPHIDsFromMentions( + $comments); + + if ($mentioned_phids) { + if ($mock->getID()) { + $old_subs = PhabricatorSubscribersQuery::loadSubscribersForPHID( + $mock->getPHID()); + } else { + $old_subs = array(); + } + + $new_subs = array_merge($old_subs, $mentioned_phids); + $xaction = id(new PholioTransaction()) + ->setTransactionType(PholioTransactionType::TYPE_SUBSCRIBERS) + ->setOldValue($old_subs) + ->setNewValue($new_subs); + array_unshift($xactions, $xaction); + } + foreach ($xactions as $xaction) { $xaction->setContentSource($this->contentSource); $xaction->setAuthorPHID($actor->getPHID()); @@ -64,6 +94,21 @@ final class PholioMockEditor extends PhabricatorEditor { $xaction->setMockID($mock->getID()); $xaction->save(); } + + // Apply ID/PHID-dependent transactions. + foreach ($xactions as $xaction) { + $type = $xaction->getTransactionType(); + switch ($type) { + case PholioTransactionType::TYPE_SUBSCRIBERS: + $subeditor = id(new PhabricatorSubscriptionsEditor()) + ->setObject($mock) + ->setActor($this->requireActor()) + ->subscribeExplicit($xaction->getNewValue()) + ->save(); + break; + } + } + $mock->saveTransaction(); PholioIndexer::indexMock($mock); @@ -91,13 +136,17 @@ final class PholioMockEditor extends PhabricatorEditor { case PholioTransactionType::TYPE_VIEW_POLICY: $old = $mock->getViewPolicy(); break; + case PholioTransactionType::TYPE_SUBSCRIBERS: + $old = PhabricatorSubscribersQuery::loadSubscribersForPHID( + $mock->getPHID()); + break; default: throw new Exception("Unknown transaction type '{$type}'!"); } $xaction->setOldValue($old); - if (!$this->transactionHasEffect($xaction)) { + if (!$this->transactionHasEffect($mock, $xaction)) { return false; } @@ -113,6 +162,9 @@ final class PholioMockEditor extends PhabricatorEditor { case PholioTransactionType::TYPE_VIEW_POLICY: $mock->setViewPolicy($xaction->getNewValue()); break; + case PholioTransactionType::TYPE_SUBSCRIBERS: + // This applies later. + break; default: throw new Exception("Unknown transaction type '{$type}'!"); } @@ -120,16 +172,47 @@ final class PholioMockEditor extends PhabricatorEditor { return true; } - private function transactionHasEffect(PholioTransaction $xaction) { + private function transactionHasEffect( + PholioMock $mock, + PholioTransaction $xaction) { + $effect = false; + $old = $xaction->getOldValue(); + $new = $xaction->getNewValue(); + $type = $xaction->getTransactionType(); switch ($type) { case PholioTransactionType::TYPE_NONE: case PholioTransactionType::TYPE_NAME: case PholioTransactionType::TYPE_DESCRIPTION: case PholioTransactionType::TYPE_VIEW_POLICY: - $effect = ($xaction->getOldValue() !== $xaction->getNewValue()); + $effect = ($old !== $new); + break; + case PholioTransactionType::TYPE_SUBSCRIBERS: + $old = nonempty($old, array()); + $old_map = array_fill_keys($old, true); + $filtered = $old; + + foreach ($new as $phid) { + if ($mock->getAuthorPHID() == $phid) { + // The author may not be explicitly subscribed. + continue; + } + if (isset($old_map[$phid])) { + // This PHID was already subscribed. + continue; + } + $filtered[] = $phid; + } + + $old = array_keys($old_map); + $new = array_values($filtered); + + $xaction->setOldValue($old); + $xaction->setNewValue($new); + + $effect = ($old !== $new); break; default: throw new Exception("Unknown transaction type '{$type}'!"); diff --git a/src/applications/pholio/storage/PholioTransaction.php b/src/applications/pholio/storage/PholioTransaction.php index a1fd28dfd2..d7f3bbff4e 100644 --- a/src/applications/pholio/storage/PholioTransaction.php +++ b/src/applications/pholio/storage/PholioTransaction.php @@ -52,6 +52,17 @@ final class PholioTransaction extends PholioDAO return PhabricatorContentSource::newFromSerialized($this->contentSource); } + public function getRequiredHandlePHIDs() { + switch ($this->getTransactionType()) { + case PholioTransactionType::TYPE_SUBSCRIBERS: + return array_merge( + $this->getOldValue(), + $this->getNewValue()); + default: + return array(); + } + } + /* -( PhabricatorSubscribableInterface Implementation )-------------------- */ diff --git a/src/applications/subscriptions/controller/PhabricatorSubscriptionsEditController.php b/src/applications/subscriptions/controller/PhabricatorSubscriptionsEditController.php index 80bd0c4239..c17121add6 100644 --- a/src/applications/subscriptions/controller/PhabricatorSubscriptionsEditController.php +++ b/src/applications/subscriptions/controller/PhabricatorSubscriptionsEditController.php @@ -40,6 +40,7 @@ final class PhabricatorSubscriptionsEditController } $objects = id(new PhabricatorObjectHandleData(array($phid))) + ->setViewer($user) ->loadObjects(); $object = idx($objects, $phid); diff --git a/src/applications/subscriptions/query/PhabricatorSubscribersQuery.php b/src/applications/subscriptions/query/PhabricatorSubscribersQuery.php index c3a78a6900..0870fe27ee 100644 --- a/src/applications/subscriptions/query/PhabricatorSubscribersQuery.php +++ b/src/applications/subscriptions/query/PhabricatorSubscribersQuery.php @@ -6,6 +6,10 @@ final class PhabricatorSubscribersQuery extends PhabricatorQuery { private $subscriberPHIDs; public static function loadSubscribersForPHID($phid) { + if (!$phid) { + return array(); + } + $subscribers = id(new PhabricatorSubscribersQuery()) ->withObjectPHIDs(array($phid)) ->execute();