From 0ca7c77c10681fa84e3d6a3f243b710fe8c8362d Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 17 Feb 2013 06:37:09 -0800 Subject: [PATCH] Integrate subscriptions with ApplicationTransactions Summary: Fixes T2214. For objects which support ApplicationTransaction, use ApplicationTransactions to apply subscription action changes. Principally, this makes clicking "Subscribe" / "Unsubscribe" appear correctly in the transaction log. Test Plan: Clicked "Subscribe" and "Unsubscribe" a on Macros and Mocks. Reviewers: chad Reviewed By: chad CC: aran Maniphest Tasks: T2214 Differential Revision: https://secure.phabricator.com/D4986 --- src/__phutil_library_map__.php | 2 + .../storage/PhabricatorFileImageMacro.php | 12 ++++- .../pholio/storage/PholioMock.php | 16 +++++- ...PhabricatorSubscriptionsEditController.php | 50 +++++++++++++++---- ...habricatorApplicationTransactionEditor.php | 17 +++++-- ...ricatorApplicationTransactionInterface.php | 8 +++ 6 files changed, 91 insertions(+), 14 deletions(-) create mode 100644 src/applications/transactions/interface/PhabricatorApplicationTransactionInterface.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index f941a6274b..b4aad3e01e 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -680,6 +680,7 @@ phutil_register_library_map(array( 'PhabricatorApplicationTransactionController' => 'applications/transactions/controller/PhabricatorApplicationTransactionController.php', 'PhabricatorApplicationTransactionEditor' => 'applications/transactions/editor/PhabricatorApplicationTransactionEditor.php', 'PhabricatorApplicationTransactionFeedStory' => 'applications/transactions/feed/PhabricatorApplicationTransactionFeedStory.php', + 'PhabricatorApplicationTransactionInterface' => 'applications/transactions/interface/PhabricatorApplicationTransactionInterface.php', 'PhabricatorApplicationTransactionNoEffectException' => 'applications/transactions/exception/PhabricatorApplicationTransactionNoEffectException.php', 'PhabricatorApplicationTransactionNoEffectResponse' => 'applications/transactions/response/PhabricatorApplicationTransactionNoEffectResponse.php', 'PhabricatorApplicationTransactionQuery' => 'applications/transactions/query/PhabricatorApplicationTransactionQuery.php', @@ -2390,6 +2391,7 @@ phutil_register_library_map(array( array( 0 => 'PhabricatorFileDAO', 1 => 'PhabricatorSubscribableInterface', + 2 => 'PhabricatorApplicationTransactionInterface', ), 'PhabricatorFileInfoController' => 'PhabricatorFileController', 'PhabricatorFileLinkListView' => 'AphrontView', diff --git a/src/applications/macro/storage/PhabricatorFileImageMacro.php b/src/applications/macro/storage/PhabricatorFileImageMacro.php index 9c59f9699f..470d1a7ab4 100644 --- a/src/applications/macro/storage/PhabricatorFileImageMacro.php +++ b/src/applications/macro/storage/PhabricatorFileImageMacro.php @@ -1,7 +1,9 @@ getID(); } + +/* -( PhabricatorApplicationTransactionInterface )------------------------- */ + + + public function getApplicationTransactionEditor() { + return new PholioMockEditor(); + } + + public function getApplicationTransactionObject() { + return new PholioTransaction(); + } + + } diff --git a/src/applications/subscriptions/controller/PhabricatorSubscriptionsEditController.php b/src/applications/subscriptions/controller/PhabricatorSubscriptionsEditController.php index c17121add6..e3f5824374 100644 --- a/src/applications/subscriptions/controller/PhabricatorSubscriptionsEditController.php +++ b/src/applications/subscriptions/controller/PhabricatorSubscriptionsEditController.php @@ -58,17 +58,49 @@ final class PhabricatorSubscriptionsEditController $handle->getURI()); } - $editor = id(new PhabricatorSubscriptionsEditor()) - ->setActor($user) - ->setObject($object); - - if ($is_add) { - $editor->subscribeExplicit(array($user->getPHID()), $explicit = true); + if ($object instanceof PhabricatorApplicationTransactionInterface) { + if ($is_add) { + $xaction_value = array( + '+' => array($user->getPHID()), + ); + } else { + $xaction_value = array( + '-' => array($user->getPHID()), + ); + } + + $xaction = id($object->getApplicationTransactionObject()) + ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS) + ->setNewValue($xaction_value); + + $editor = id($object->getApplicationTransactionEditor()) + ->setActor($user) + ->setContinueOnNoEffect(true) + ->setContentSource( + PhabricatorContentSource::newForSource( + PhabricatorContentSource::SOURCE_WEB, + array( + 'ip' => $request->getRemoteAddr(), + ))); + + $editor->applyTransactions($object, array($xaction)); } else { - $editor->unsubscribe(array($user->getPHID())); - } - $editor->save(); + // TODO: Eventually, get rid of this once everything implements + // PhabriatorApplicationTransactionInterface. + + $editor = id(new PhabricatorSubscriptionsEditor()) + ->setActor($user) + ->setObject($object); + + if ($is_add) { + $editor->subscribeExplicit(array($user->getPHID()), $explicit = true); + } else { + $editor->unsubscribe(array($user->getPHID())); + } + + $editor->save(); + } // TODO: We should just render the "Unsubscribe" action and swap it out // in the document for Ajax requests. diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php index 349bb3c453..2754f53662 100644 --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -170,9 +170,20 @@ abstract class PhabricatorApplicationTransactionEditor case PhabricatorTransactions::TYPE_SUBSCRIBERS: $subeditor = id(new PhabricatorSubscriptionsEditor()) ->setObject($object) - ->setActor($this->requireActor()) - ->subscribeExplicit($xaction->getNewValue()) - ->save(); + ->setActor($this->requireActor()); + + $old_map = array_fuse($xaction->getOldValue()); + $new_map = array_fuse($xaction->getNewValue()); + + $subeditor->unsubscribe( + array_keys( + array_diff_key($old_map, $new_map))); + + $subeditor->subscribeExplicit( + array_keys( + array_diff_key($new_map, $old_map))); + + $subeditor->save(); break; } return $this->applyCustomExternalTransaction($object, $xaction); diff --git a/src/applications/transactions/interface/PhabricatorApplicationTransactionInterface.php b/src/applications/transactions/interface/PhabricatorApplicationTransactionInterface.php new file mode 100644 index 0000000000..be9b01d95b --- /dev/null +++ b/src/applications/transactions/interface/PhabricatorApplicationTransactionInterface.php @@ -0,0 +1,8 @@ +