diff --git a/src/applications/maniphest/editor/ManiphestTransactionEditor.php b/src/applications/maniphest/editor/ManiphestTransactionEditor.php --- a/src/applications/maniphest/editor/ManiphestTransactionEditor.php +++ b/src/applications/maniphest/editor/ManiphestTransactionEditor.php @@ -326,7 +326,9 @@ return true; } - protected function supportsHerald() { + protected function shouldApplyHeraldRules( + PhabricatorLiskDAO $object, + array $xactions) { return true; } @@ -343,6 +345,9 @@ HeraldAdapter $adapter, HeraldTranscript $transcript) { + // TODO: Convert these to transactions. The way Maniphest deals with these + // transactions is currently unconventional and messy. + $save_again = false; $cc_phids = $adapter->getCcPHIDs(); if ($cc_phids) { @@ -352,12 +357,6 @@ $save_again = true; } - $assign_phid = $adapter->getAssignPHID(); - if ($assign_phid) { - $object->setOwnerPHID($assign_phid); - $save_again = true; - } - $project_phids = $adapter->getProjectPHIDs(); if ($project_phids) { $existing_projects = $object->getProjectPHIDs(); @@ -370,6 +369,17 @@ if ($save_again) { $object->save(); } + + $xactions = array(); + + $assign_phid = $adapter->getAssignPHID(); + if ($assign_phid) { + $xactions[] = id(new ManiphestTransaction()) + ->setTransactionType(ManiphestTransaction::TYPE_OWNER) + ->setNewValue($assign_phid); + } + + return $xactions; } protected function requireCapabilities( diff --git a/src/applications/metamta/contentsource/PhabricatorContentSource.php b/src/applications/metamta/contentsource/PhabricatorContentSource.php --- a/src/applications/metamta/contentsource/PhabricatorContentSource.php +++ b/src/applications/metamta/contentsource/PhabricatorContentSource.php @@ -10,6 +10,7 @@ const SOURCE_TABLET = 'tablet'; const SOURCE_FAX = 'fax'; const SOURCE_CONSOLE = 'console'; + const SOURCE_HERALD = 'herald'; const SOURCE_LEGACY = 'legacy'; private $source; @@ -70,7 +71,8 @@ self::SOURCE_FAX => pht('Fax'), self::SOURCE_CONSOLE => pht('Console'), self::SOURCE_LEGACY => pht('Legacy'), - self::SOURCE_UNKNOWN => pht('Other'), + self::SOURCE_HERALD => pht('Herald'), + self::SOURCE_UNKNOWN => pht('Old World'), ); } diff --git a/src/applications/metamta/contentsource/PhabricatorContentSourceView.php b/src/applications/metamta/contentsource/PhabricatorContentSourceView.php --- a/src/applications/metamta/contentsource/PhabricatorContentSourceView.php +++ b/src/applications/metamta/contentsource/PhabricatorContentSourceView.php @@ -12,15 +12,7 @@ public function render() { require_celerity_resource('phabricator-content-source-view-css'); - $map = array( - PhabricatorContentSource::SOURCE_WEB => pht('Web'), - PhabricatorContentSource::SOURCE_CONDUIT => pht('Conduit'), - PhabricatorContentSource::SOURCE_EMAIL => pht('Email'), - PhabricatorContentSource::SOURCE_MOBILE => pht('Mobile'), - PhabricatorContentSource::SOURCE_TABLET => pht('Tablet'), - PhabricatorContentSource::SOURCE_FAX => pht('Fax'), - PhabricatorContentSource::SOURCE_LEGACY => pht('Old World'), - ); + $map = PhabricatorContentSource::getSourceNameMap(); $source = $this->contentSource->getSource(); $type = idx($map, $source, null); diff --git a/src/applications/pholio/editor/PholioMockEditor.php b/src/applications/pholio/editor/PholioMockEditor.php --- a/src/applications/pholio/editor/PholioMockEditor.php +++ b/src/applications/pholio/editor/PholioMockEditor.php @@ -410,7 +410,9 @@ return true; } - protected function supportsHerald() { + protected function shouldApplyHeraldRules( + PhabricatorLiskDAO $object, + array $xactions) { return true; } @@ -427,6 +429,8 @@ HeraldAdapter $adapter, HeraldTranscript $transcript) { + // TODO: Convert this to be transaction-based. + $cc_phids = $adapter->getCcPHIDs(); if ($cc_phids) { id(new PhabricatorSubscriptionsEditor()) @@ -435,6 +439,8 @@ ->subscribeImplicit($cc_phids) ->save(); } + + return array(); } protected function sortTransactions(array $xactions) { diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -23,6 +23,20 @@ private $subscribers; private $isPreview; + private $isHeraldEditor; + private $actingAsPHID; + + public function setActingAsPHID($acting_as_phid) { + $this->actingAsPHID = $acting_as_phid; + return $this; + } + + public function getActingAsPHID() { + if ($this->actingAsPHID) { + return $this->actingAsPHID; + } + return $this->getActor()->getPHID(); + } /** * When the editor tries to apply transactions that have no effect, should @@ -105,6 +119,15 @@ return $this->isPreview; } + public function setIsHeraldEditor($is_herald_editor) { + $this->isHeraldEditor = $is_herald_editor; + return $this; + } + + public function getIsHeraldEditor() { + return $this->isHeraldEditor; + } + public function getTransactionTypes() { $types = array(); @@ -391,9 +414,14 @@ // TODO: This needs to be more sophisticated once we have meta-policies. $xaction->setViewPolicy(PhabricatorPolicies::POLICY_PUBLIC); - $xaction->setEditPolicy($actor->getPHID()); - $xaction->setAuthorPHID($actor->getPHID()); + if ($actor->isOmnipotent()) { + $xaction->setEditPolicy(PhabricatorPolicies::POLICY_NOONE); + } else { + $xaction->setEditPolicy($actor->getPHID()); + } + + $xaction->setAuthorPHID($this->getActingAsPHID()); $xaction->setContentSource($this->getContentSource()); $xaction->attachViewer($actor); $xaction->attachObject($object); @@ -575,10 +603,6 @@ $this->applyExternalEffects($object, $xaction); } - if ($this->supportsHerald()) { - $this->applyHeraldRules($object, $xactions); - } - $xactions = $this->applyFinalEffects($object, $xactions); if ($read_locking) { @@ -588,6 +612,63 @@ $object->saveTransaction(); + // Now that we've completely applied the core transaction set, try to apply + // Herald rules. Herald rules are allowed to either take direct actions on + // the database (like writing flags), or take indirect actions (like saving + // some targets for CC when we generate mail a little later), or return + // transactions which we'll apply normally using another Editor. + + // First, check if *this* is a sub-editor which is itself applying Herald + // rules: if it is, stop working and return so we don't descend into + // madness. + + // Otherwise, we're not a Herald editor, so process Herald rules (possibly + // using a Herald editor to apply resulting transactions) and then send out + // mail, notifications, and feed updates about everything. + + if ($this->getIsHeraldEditor()) { + // We are the Herald editor, so stop work here and return the updated + // transactions. + return $xactions; + } else if ($this->shouldApplyHeraldRules($object, $xactions)) { + // We are not the Herald editor, so try to apply Herald rules. + $herald_xactions = $this->applyHeraldRules($object, $xactions); + + if ($herald_xactions) { + // NOTE: We're acting as the omnipotent user because rules deal with + // their own policy issues. We use a synthetic author PHID (the + // Herald application) as the author of record, so that transactions + // will render in a reasonable way ("Herald assigned this task ..."). + $herald_actor = PhabricatorUser::getOmnipotentUser(); + $herald_phid = id(new PhabricatorApplicationHerald())->getPHID(); + + // TODO: It would be nice to give transactions a more specific source + // which points at the rule which generated them. You can figure this + // out from transcripts, but it would be cleaner if you didn't have to. + + $herald_source = PhabricatorContentSource::newForSource( + PhabricatorContentSource::SOURCE_HERALD, + array()); + + $herald_editor = newv(get_class($this), array()) + ->setContinueOnNoEffect(true) + ->setContinueOnMissingFields(true) + ->setParentMessageID($this->getParentMessageID()) + ->setIsHeraldEditor(true) + ->setActor($herald_actor) + ->setActingAsPHID($herald_phid) + ->setContentSource($herald_source); + + $herald_xactions = $herald_editor->applyTransactions( + $object, + $herald_xactions); + + // Merge the new transactions into the transaction list: we want to + // send email and publish feed stories about them, too. + $xactions = array_merge($xactions, $herald_xactions); + } + } + $this->loadHandles($xactions); $mail = null; @@ -1789,7 +1870,9 @@ /* -( Herald Integration )-------------------------------------------------- */ - protected function supportsHerald() { + protected function shouldApplyHeraldRules( + PhabricatorLiskDAO $object, + array $xactions) { return false; } @@ -1829,14 +1912,14 @@ $this->setHeraldAdapter($adapter); $this->setHeraldTranscript($xscript); - $this->didApplyHeraldRules($object, $adapter, $xscript); + return $this->didApplyHeraldRules($object, $adapter, $xscript); } protected function didApplyHeraldRules( PhabricatorLiskDAO $object, HeraldAdapter $adapter, HeraldTranscript $transcript) { - + return array(); }