From 51485a1f82578d9e8ef7877e13e2c2ed03b9d1bb Mon Sep 17 00:00:00 2001 From: Chad Little Date: Wed, 19 Apr 2017 11:43:40 -0700 Subject: [PATCH] Add participants ModularTransactions to Conpherence Summary: Moves participants over to ModularTransactions, simplified a lot of the code. Fixes T12550 Test Plan: Create a new room with just myself and myself + fake accounts. Remove a person. Remove myself. Edit a room, topic. Type some messages. ??? Profit Reviewers: chad Reviewed By: chad Subscribers: Korvin Maniphest Tasks: T12550 Differential Revision: https://secure.phabricator.com/D17685 --- src/__phutil_library_map__.php | 2 + .../__tests__/ConpherenceRoomTestCase.php | 3 +- .../__tests__/ConpherenceTestCase.php | 6 +- ...onpherenceUpdateThreadConduitAPIMethod.php | 4 +- .../ConpherenceNewRoomController.php | 3 +- .../ConpherenceUpdateController.php | 6 +- .../conpherence/editor/ConpherenceEditor.php | 224 ++---------------- .../mail/ConpherenceReplyHandler.php | 3 +- .../storage/ConpherenceTransaction.php | 79 ------ ...npherenceThreadParticipantsTransaction.php | 117 +++++++++ ...habricatorApplicationTransactionEditor.php | 5 +- .../PhabricatorModularTransactionType.php | 6 + 12 files changed, 158 insertions(+), 300 deletions(-) create mode 100644 src/applications/conpherence/xaction/ConpherenceThreadParticipantsTransaction.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 33481ee880..3b56e463b7 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -321,6 +321,7 @@ phutil_register_library_map(array( 'ConpherenceThreadListView' => 'applications/conpherence/view/ConpherenceThreadListView.php', 'ConpherenceThreadMailReceiver' => 'applications/conpherence/mail/ConpherenceThreadMailReceiver.php', 'ConpherenceThreadMembersPolicyRule' => 'applications/conpherence/policyrule/ConpherenceThreadMembersPolicyRule.php', + 'ConpherenceThreadParticipantsTransaction' => 'applications/conpherence/xaction/ConpherenceThreadParticipantsTransaction.php', 'ConpherenceThreadPictureTransaction' => 'applications/conpherence/xaction/ConpherenceThreadPictureTransaction.php', 'ConpherenceThreadQuery' => 'applications/conpherence/query/ConpherenceThreadQuery.php', 'ConpherenceThreadRemarkupRule' => 'applications/conpherence/remarkup/ConpherenceThreadRemarkupRule.php', @@ -5121,6 +5122,7 @@ phutil_register_library_map(array( 'ConpherenceThreadListView' => 'AphrontView', 'ConpherenceThreadMailReceiver' => 'PhabricatorObjectMailReceiver', 'ConpherenceThreadMembersPolicyRule' => 'PhabricatorPolicyRule', + 'ConpherenceThreadParticipantsTransaction' => 'ConpherenceThreadTransactionType', 'ConpherenceThreadPictureTransaction' => 'ConpherenceThreadTransactionType', 'ConpherenceThreadQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'ConpherenceThreadRemarkupRule' => 'PhabricatorObjectRemarkupRule', diff --git a/src/applications/conpherence/__tests__/ConpherenceRoomTestCase.php b/src/applications/conpherence/__tests__/ConpherenceRoomTestCase.php index 85c4b9c1f1..7ef908c315 100644 --- a/src/applications/conpherence/__tests__/ConpherenceRoomTestCase.php +++ b/src/applications/conpherence/__tests__/ConpherenceRoomTestCase.php @@ -107,7 +107,8 @@ final class ConpherenceRoomTestCase extends ConpherenceTestCase { $xactions = array(); $xactions[] = id(new ConpherenceTransaction()) - ->setTransactionType(ConpherenceTransaction::TYPE_PARTICIPANTS) + ->setTransactionType( + ConpherenceThreadParticipantsTransaction::TRANSACTIONTYPE) ->setNewValue(array('+' => $participant_phids)); $xactions[] = id(new ConpherenceTransaction()) ->setTransactionType( diff --git a/src/applications/conpherence/__tests__/ConpherenceTestCase.php b/src/applications/conpherence/__tests__/ConpherenceTestCase.php index 1ad87d8af8..6c8397c7d8 100644 --- a/src/applications/conpherence/__tests__/ConpherenceTestCase.php +++ b/src/applications/conpherence/__tests__/ConpherenceTestCase.php @@ -9,7 +9,8 @@ abstract class ConpherenceTestCase extends PhabricatorTestCase { $xactions = array( id(new ConpherenceTransaction()) - ->setTransactionType(ConpherenceTransaction::TYPE_PARTICIPANTS) + ->setTransactionType( + ConpherenceThreadParticipantsTransaction::TRANSACTIONTYPE) ->setNewValue(array('+' => $participant_phids)), ); $editor = id(new ConpherenceEditor()) @@ -26,7 +27,8 @@ abstract class ConpherenceTestCase extends PhabricatorTestCase { $xactions = array( id(new ConpherenceTransaction()) - ->setTransactionType(ConpherenceTransaction::TYPE_PARTICIPANTS) + ->setTransactionType( + ConpherenceThreadParticipantsTransaction::TRANSACTIONTYPE) ->setNewValue(array('-' => $participant_phids)), ); $editor = id(new ConpherenceEditor()) diff --git a/src/applications/conpherence/conduit/ConpherenceUpdateThreadConduitAPIMethod.php b/src/applications/conpherence/conduit/ConpherenceUpdateThreadConduitAPIMethod.php index c92cc464ed..e7d927905d 100644 --- a/src/applications/conpherence/conduit/ConpherenceUpdateThreadConduitAPIMethod.php +++ b/src/applications/conpherence/conduit/ConpherenceUpdateThreadConduitAPIMethod.php @@ -69,7 +69,7 @@ final class ConpherenceUpdateThreadConduitAPIMethod if ($add_participant_phids) { $xactions[] = id(new ConpherenceTransaction()) ->setTransactionType( - ConpherenceTransaction::TYPE_PARTICIPANTS) + ConpherenceThreadParticipantsTransaction::TRANSACTIONTYPE) ->setNewValue(array('+' => $add_participant_phids)); } if ($remove_participant_phid) { @@ -78,7 +78,7 @@ final class ConpherenceUpdateThreadConduitAPIMethod } $xactions[] = id(new ConpherenceTransaction()) ->setTransactionType( - ConpherenceTransaction::TYPE_PARTICIPANTS) + ConpherenceThreadParticipantsTransaction::TRANSACTIONTYPE) ->setNewValue(array('-' => array($remove_participant_phid))); } if ($title) { diff --git a/src/applications/conpherence/controller/ConpherenceNewRoomController.php b/src/applications/conpherence/controller/ConpherenceNewRoomController.php index d70395dbc9..6cbe86aaa2 100644 --- a/src/applications/conpherence/controller/ConpherenceNewRoomController.php +++ b/src/applications/conpherence/controller/ConpherenceNewRoomController.php @@ -23,7 +23,8 @@ final class ConpherenceNewRoomController extends ConpherenceController { $participants[] = $user->getPHID(); $participants = array_unique($participants); $xactions[] = id(new ConpherenceTransaction()) - ->setTransactionType(ConpherenceTransaction::TYPE_PARTICIPANTS) + ->setTransactionType( + ConpherenceThreadParticipantsTransaction::TRANSACTIONTYPE) ->setNewValue(array('+' => $participants)); $xactions[] = id(new ConpherenceTransaction()) ->setTransactionType(ConpherenceThreadTopicTransaction::TRANSACTIONTYPE) diff --git a/src/applications/conpherence/controller/ConpherenceUpdateController.php b/src/applications/conpherence/controller/ConpherenceUpdateController.php index d95696b68f..1d17134a53 100644 --- a/src/applications/conpherence/controller/ConpherenceUpdateController.php +++ b/src/applications/conpherence/controller/ConpherenceUpdateController.php @@ -61,7 +61,7 @@ final class ConpherenceUpdateController case ConpherenceUpdateActions::JOIN_ROOM: $xactions[] = id(new ConpherenceTransaction()) ->setTransactionType( - ConpherenceTransaction::TYPE_PARTICIPANTS) + ConpherenceThreadParticipantsTransaction::TRANSACTIONTYPE) ->setNewValue(array('+' => array($user->getPHID()))); $delete_draft = true; $message = $request->getStr('text'); @@ -95,7 +95,7 @@ final class ConpherenceUpdateController if (!empty($person_phids)) { $xactions[] = id(new ConpherenceTransaction()) ->setTransactionType( - ConpherenceTransaction::TYPE_PARTICIPANTS) + ConpherenceThreadParticipantsTransaction::TRANSACTIONTYPE) ->setNewValue(array('+' => $person_phids)); } break; @@ -108,7 +108,7 @@ final class ConpherenceUpdateController if ($person_phid) { $xactions[] = id(new ConpherenceTransaction()) ->setTransactionType( - ConpherenceTransaction::TYPE_PARTICIPANTS) + ConpherenceThreadParticipantsTransaction::TRANSACTIONTYPE) ->setNewValue(array('-' => array($person_phid))); $response_mode = 'go-home'; } diff --git a/src/applications/conpherence/editor/ConpherenceEditor.php b/src/applications/conpherence/editor/ConpherenceEditor.php index 59d854cdc3..29ffc22251 100644 --- a/src/applications/conpherence/editor/ConpherenceEditor.php +++ b/src/applications/conpherence/editor/ConpherenceEditor.php @@ -37,7 +37,8 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor { if (!$errors) { $xactions = array(); $xactions[] = id(new ConpherenceTransaction()) - ->setTransactionType(ConpherenceTransaction::TYPE_PARTICIPANTS) + ->setTransactionType( + ConpherenceThreadParticipantsTransaction::TRANSACTIONTYPE) ->setNewValue(array('+' => $participant_phids)); if ($title) { $xactions[] = id(new ConpherenceTransaction()) @@ -87,8 +88,6 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor { public function getTransactionTypes() { $types = parent::getTransactionTypes(); - $types[] = ConpherenceTransaction::TYPE_PARTICIPANTS; - $types[] = PhabricatorTransactions::TYPE_COMMENT; $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY; $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY; @@ -100,29 +99,6 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor { return pht('%s created this room.', $author); } - protected function getCustomTransactionOldValue( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case ConpherenceTransaction::TYPE_PARTICIPANTS: - if ($this->getIsNewObject()) { - return array(); - } - return $object->getParticipantPHIDs(); - } - } - - protected function getCustomTransactionNewValue( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case ConpherenceTransaction::TYPE_PARTICIPANTS: - return $this->getPHIDTransactionNewValue($xaction); - } - } - /** * We really only need a read lock if we have a comment. In that case, we * must update the messagesCount field on the conpherence and @@ -142,66 +118,6 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor { return $lock; } - /** - * We need to apply initial effects IFF the conpherence is new. We must - * save the conpherence first thing to make sure we have an id and a phid, as - * well as create the initial set of participants so that we pass policy - * checks. - */ - protected function shouldApplyInitialEffects( - PhabricatorLiskDAO $object, - array $xactions) { - - return $this->getIsNewObject(); - } - - protected function applyInitialEffects( - PhabricatorLiskDAO $object, - array $xactions) { - - $object->save(); - - foreach ($xactions as $xaction) { - switch ($xaction->getTransactionType()) { - case ConpherenceTransaction::TYPE_PARTICIPANTS: - // Since this is a new ConpherenceThread, we have to create the - // participation data asap to pass policy checks. For existing - // ConpherenceThreads, the existing participation is correct - // at this stage. Note that later in applyCustomExternalTransaction - // this participation data will be updated. - $participants = array(); - $phids = $this->getPHIDTransactionNewValue($xaction, array()); - foreach ($phids as $phid) { - if ($phid == $this->getActor()->getPHID()) { - $message_count = 1; - } else { - $message_count = 0; - } - $participants[$phid] = - id(new ConpherenceParticipant()) - ->setConpherencePHID($object->getPHID()) - ->setParticipantPHID($phid) - ->setSeenMessageCount($message_count) - ->save(); - $object->attachParticipants($participants); - } - break; - } - } - } - - protected function applyCustomInternalTransaction( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case ConpherenceTransaction::TYPE_PARTICIPANTS: - if (!$this->getIsNewObject()) {} - break; - } - - } - protected function applyBuiltinInternalTransaction( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { @@ -215,83 +131,23 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor { return parent::applyBuiltinInternalTransaction($object, $xaction); } - protected function applyCustomExternalTransaction( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case ConpherenceTransaction::TYPE_PARTICIPANTS: - if ($this->getIsNewObject()) { - continue; - } - $participants = $object->getParticipants(); - - $old_map = array_fuse($xaction->getOldValue()); - $new_map = array_fuse($xaction->getNewValue()); - - $remove = array_keys(array_diff_key($old_map, $new_map)); - foreach ($remove as $phid) { - $remove_participant = $participants[$phid]; - $remove_participant->delete(); - unset($participants[$phid]); - } - - $add = array_keys(array_diff_key($new_map, $old_map)); - foreach ($add as $phid) { - if ($phid == $this->getActor()->getPHID()) { - $message_count = $object->getMessageCount(); - } else { - $message_count = 0; - } - $participants[$phid] = - id(new ConpherenceParticipant()) - ->setConpherencePHID($object->getPHID()) - ->setParticipantPHID($phid) - ->setSeenMessageCount($message_count) - ->save(); - } - $object->attachParticipants($participants); - break; - } - } protected function applyFinalEffects( PhabricatorLiskDAO $object, array $xactions) { - if (!$xactions) { - return $xactions; + $acting_phid = $this->getActingAsPHID(); + $participants = $object->getParticipants(); + foreach ($participants as $participant) { + if ($participant->getParticipantPHID() == $acting_phid) { + $participant->markUpToDate($object); + } } - $message_count = 0; - foreach ($xactions as $xaction) { - switch ($xaction->getTransactionType()) { - case PhabricatorTransactions::TYPE_COMMENT: - $message_count++; - - // update everyone's participation status on a message -only- - $xaction_phid = $xaction->getPHID(); - $participants = $object->getParticipants(); - $user = $this->getActor(); - $time = time(); - foreach ($participants as $phid => $participant) { - if ($phid != $user->getPHID()) { - if ($participant->isUpToDate($object)) { - $participant->setSeenMessageCount( - $object->getMessageCount() - $message_count); - } - } else { - $participant->setSeenMessageCount($object->getMessageCount()); - } - $participant->save(); - } - - PhabricatorUserCache::clearCaches( - PhabricatorUserMessageCountCacheType::KEY_COUNT, - array_keys($participants)); - - break; - } + if ($participants) { + PhabricatorUserCache::clearCaches( + PhabricatorUserMessageCountCacheType::KEY_COUNT, + array_keys($participants)); } if ($xactions) { @@ -315,7 +171,7 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor { parent::requireCapabilities($object, $xaction); switch ($xaction->getTransactionType()) { - case ConpherenceTransaction::TYPE_PARTICIPANTS: + case ConpherenceThreadParticipantsTransaction::TRANSACTIONTYPE: $old_map = array_fuse($xaction->getOldValue()); $new_map = array_fuse($xaction->getNewValue()); @@ -339,8 +195,8 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor { $object, PhabricatorPolicyCapability::CAN_EDIT); } - break; + case ConpherenceThreadTitleTransaction::TRANSACTIONTYPE: case ConpherenceThreadTopicTransaction::TRANSACTIONTYPE: PhabricatorPolicyFilter::requireCapability( @@ -351,19 +207,6 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor { } } - protected function mergeTransactions( - PhabricatorApplicationTransaction $u, - PhabricatorApplicationTransaction $v) { - - $type = $u->getTransactionType(); - switch ($type) { - case ConpherenceTransaction::TYPE_PARTICIPANTS: - return $this->mergePHIDOrEdgeTransactions($u, $v); - } - - return parent::mergeTransactions($u, $v); - } - protected function shouldSendMail( PhabricatorLiskDAO $object, array $xactions) { @@ -466,43 +309,4 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor { return true; } - protected function validateTransaction( - PhabricatorLiskDAO $object, - $type, - array $xactions) { - - $errors = parent::validateTransaction($object, $type, $xactions); - - switch ($type) { - case ConpherenceTransaction::TYPE_PARTICIPANTS: - foreach ($xactions as $xaction) { - $new_phids = $this->getPHIDTransactionNewValue($xaction, array()); - $old_phids = nonempty($object->getParticipantPHIDs(), array()); - $phids = array_diff($new_phids, $old_phids); - - if (!$phids) { - continue; - } - - $users = id(new PhabricatorPeopleQuery()) - ->setViewer($this->requireActor()) - ->withPHIDs($phids) - ->execute(); - $users = mpull($users, null, 'getPHID'); - foreach ($phids as $phid) { - if (isset($users[$phid])) { - continue; - } - $errors[] = new PhabricatorApplicationTransactionValidationError( - $type, - pht('Invalid'), - pht('New room participant "%s" is not a valid user.', $phid), - $xaction); - } - } - break; - } - - return $errors; - } } diff --git a/src/applications/conpherence/mail/ConpherenceReplyHandler.php b/src/applications/conpherence/mail/ConpherenceReplyHandler.php index 946501c8b8..a462030fdd 100644 --- a/src/applications/conpherence/mail/ConpherenceReplyHandler.php +++ b/src/applications/conpherence/mail/ConpherenceReplyHandler.php @@ -55,7 +55,8 @@ final class ConpherenceReplyHandler extends PhabricatorMailReplyHandler { $xactions = array(); if ($this->getMailAddedParticipantPHIDs()) { $xactions[] = id(new ConpherenceTransaction()) - ->setTransactionType(ConpherenceTransaction::TYPE_PARTICIPANTS) + ->setTransactionType( + ConpherenceThreadParticipantsTransaction::TRANSACTIONTYPE) ->setNewValue(array('+' => $this->getMailAddedParticipantPHIDs())); } diff --git a/src/applications/conpherence/storage/ConpherenceTransaction.php b/src/applications/conpherence/storage/ConpherenceTransaction.php index 2fb3fca380..f23882d69b 100644 --- a/src/applications/conpherence/storage/ConpherenceTransaction.php +++ b/src/applications/conpherence/storage/ConpherenceTransaction.php @@ -3,8 +3,6 @@ final class ConpherenceTransaction extends PhabricatorModularTransaction { - const TYPE_PARTICIPANTS = 'participants'; - public function getApplicationName() { return 'conpherence'; } @@ -21,81 +19,4 @@ final class ConpherenceTransaction return 'ConpherenceThreadTransactionType'; } - public function getNoEffectDescription() { - switch ($this->getTransactionType()) { - case self::TYPE_PARTICIPANTS: - return pht( - 'You can not add a participant who has already been added.'); - break; - } - - return parent::getNoEffectDescription(); - } - - public function shouldHide() { - $old = $this->getOldValue(); - - switch ($this->getTransactionType()) { - case self::TYPE_PARTICIPANTS: - return ($old === null); - } - - return parent::shouldHide(); - } - - public function getTitle() { - $author_phid = $this->getAuthorPHID(); - - $old = $this->getOldValue(); - $new = $this->getNewValue(); - - switch ($this->getTransactionType()) { - case self::TYPE_PARTICIPANTS: - $add = array_diff($new, $old); - $rem = array_diff($old, $new); - - if ($add && $rem) { - $title = pht( - '%s edited participant(s), added %d: %s; removed %d: %s.', - $this->renderHandleLink($author_phid), - count($add), - $this->renderHandleList($add), - count($rem), - $this->renderHandleList($rem)); - } else if ($add) { - $title = pht( - '%s added %d participant(s): %s.', - $this->renderHandleLink($author_phid), - count($add), - $this->renderHandleList($add)); - } else { - $title = pht( - '%s removed %d participant(s): %s.', - $this->renderHandleLink($author_phid), - count($rem), - $this->renderHandleList($rem)); - } - return $title; - break; - } - - return parent::getTitle(); - } - - public function getRequiredHandlePHIDs() { - $phids = parent::getRequiredHandlePHIDs(); - - $old = $this->getOldValue(); - $new = $this->getNewValue(); - - $phids[] = $this->getAuthorPHID(); - switch ($this->getTransactionType()) { - case self::TYPE_PARTICIPANTS: - $phids = array_merge($phids, $this->getOldValue()); - $phids = array_merge($phids, $this->getNewValue()); - break; - } - - return $phids; - } } diff --git a/src/applications/conpherence/xaction/ConpherenceThreadParticipantsTransaction.php b/src/applications/conpherence/xaction/ConpherenceThreadParticipantsTransaction.php new file mode 100644 index 0000000000..681c6d639c --- /dev/null +++ b/src/applications/conpherence/xaction/ConpherenceThreadParticipantsTransaction.php @@ -0,0 +1,117 @@ +getParticipantPHIDs(); + } + + public function generateNewValue($object, $value) { + $old = $this->generateOldValue($object); + return $this->getPHIDList($old, $value); + } + + public function applyExternalEffects($object, $value) { + $participants = $object->getParticipants(); + + $old = array_keys($participants); + $new = $value; + + $add_map = array_fuse(array_diff($new, $old)); + $rem_map = array_fuse(array_diff($old, $new)); + + foreach ($rem_map as $phid) { + $remove_participant = $participants[$phid]; + $remove_participant->delete(); + unset($participants[$phid]); + } + + foreach ($add_map as $phid) { + if (isset($participants[$phid])) { + continue; + } + + $participants[$phid] = id(new ConpherenceParticipant()) + ->setConpherencePHID($object->getPHID()) + ->setParticipantPHID($phid) + ->setSeenMessageCount(0) + ->save(); + } + + $object->attachParticipants($participants); + } + + public function getTitle() { + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + $add = array_diff($new, $old); + $rem = array_diff($old, $new); + + $author_phid = $this->getAuthorPHID(); + + if ($add && $rem) { + return pht( + '%s edited participant(s), added %d: %s; removed %d: %s.', + $this->renderAuthor(), + count($add), + $this->renderHandleList($add), + count($rem), + $this->renderHandleList($rem)); + } else if ((in_array($author_phid, $add)) && (count($add) == 1)) { + return pht( + '%s joined the room.', + $this->renderAuthor()); + } else if ((in_array($author_phid, $rem)) && (count($rem) == 1)) { + return pht( + '%s left the room.', + $this->renderAuthor()); + } else if ($add) { + return pht( + '%s added %d participant(s): %s.', + $this->renderAuthor(), + count($add), + $this->renderHandleList($add)); + } else { + return pht( + '%s removed %d participant(s): %s.', + $this->renderAuthor(), + count($rem), + $this->renderHandleList($rem)); + } + } + + public function validateTransactions($object, array $xactions) { + $errors = array(); + + foreach ($xactions as $xaction) { + $old = $object->getParticipantPHIDs(); + + $new = $xaction->getNewValue(); + $new = $this->getPHIDList($old, $new); + + $add_map = array_fuse(array_diff($new, $old)); + $rem_map = array_fuse(array_diff($old, $new)); + + foreach ($add_map as $user_phid) { + $user = id(new PhabricatorPeopleQuery()) + ->setViewer($this->getActor()) + ->withPHIDs(array($user_phid)) + ->executeOne(); + if (!$user) { + $errors[] = $this->newInvalidError( + pht( + 'Participant PHID "%s" is not a valid user PHID.', + $user_phid)); + continue; + } + } + } + + return $errors; + } + +} diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php index eb6f88d65b..8362d1328d 100644 --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -1801,7 +1801,10 @@ abstract class PhabricatorApplicationTransactionEditor $old = array_fuse($xaction->getOldValue()); } - $new = $xaction->getNewValue(); + return $this->getPHIDList($old, $xaction->getNewValue()); + } + + public function getPHIDList(array $old, array $new) { $new_add = idx($new, '+', array()); unset($new['+']); $new_rem = idx($new, '-', array()); diff --git a/src/applications/transactions/storage/PhabricatorModularTransactionType.php b/src/applications/transactions/storage/PhabricatorModularTransactionType.php index a315a5b9cf..9d8510cdc9 100644 --- a/src/applications/transactions/storage/PhabricatorModularTransactionType.php +++ b/src/applications/transactions/storage/PhabricatorModularTransactionType.php @@ -309,4 +309,10 @@ abstract class PhabricatorModularTransactionType return $this->getStorage()->getIsCreateTransaction(); } + final protected function getPHIDList(array $old, array $new) { + $editor = $this->getEditor(); + + return $editor->getPHIDList($old, $new); + } + }