diff --git a/src/applications/audit/editor/PhabricatorAuditEditor.php b/src/applications/audit/editor/PhabricatorAuditEditor.php --- a/src/applications/audit/editor/PhabricatorAuditEditor.php +++ b/src/applications/audit/editor/PhabricatorAuditEditor.php @@ -8,6 +8,7 @@ private $auditReasonMap = array(); private $affectedFiles; private $rawPatch; + private $auditorPHIDs = array(); private $didExpandInlineState; @@ -342,6 +343,9 @@ $object->writeImportStatusFlag($import_status_flag); } + // Collect auditor PHIDs for building mail. + $this->auditorPHIDs = mpull($object->getAudits(), 'getAuditorPHID'); + return $xactions; } @@ -689,10 +693,7 @@ $user_phids[$committer_phid][] = pht('Committer'); } - // we loaded this in applyFinalEffects - $audit_requests = $object->getAudits(); - $auditor_phids = mpull($audit_requests, 'getAuditorPHID'); - foreach ($auditor_phids as $auditor_phid) { + foreach ($this->auditorPHIDs as $auditor_phid) { $user_phids[$auditor_phid][] = pht('Auditor'); } @@ -983,4 +984,19 @@ return $this->shouldPublishRepositoryActivity($object, $xactions); } + protected function getCustomWorkerState() { + return array( + 'rawPatch' => $this->rawPatch, + 'affectedFiles' => $this->affectedFiles, + 'auditorPHIDs' => $this->auditorPHIDs, + ); + } + + protected function loadCustomWorkerState(array $state) { + $this->rawPatch = idx($state, 'rawPatch'); + $this->affectedFiles = idx($state, 'affectedFiles'); + $this->auditorPHIDs = idx($state, 'auditorPHIDs'); + return $this; + } + } diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php --- a/src/applications/differential/editor/DifferentialTransactionEditor.php +++ b/src/applications/differential/editor/DifferentialTransactionEditor.php @@ -1902,4 +1902,15 @@ return $section; } + protected function getCustomWorkerState() { + return array( + 'changedPriorToCommitURI' => $this->changedPriorToCommitURI, + ); + } + + protected function loadCustomWorkerState(array $state) { + $this->changedPriorToCommitURI = idx($state, 'changedPriorToCommitURI'); + return $this; + } + } diff --git a/src/applications/paste/editor/PhabricatorPasteEditor.php b/src/applications/paste/editor/PhabricatorPasteEditor.php --- a/src/applications/paste/editor/PhabricatorPasteEditor.php +++ b/src/applications/paste/editor/PhabricatorPasteEditor.php @@ -184,8 +184,4 @@ return false; } - protected function supportsWorkers() { - return true; - } - } diff --git a/src/applications/phortune/editor/PhortuneCartEditor.php b/src/applications/phortune/editor/PhortuneCartEditor.php --- a/src/applications/phortune/editor/PhortuneCartEditor.php +++ b/src/applications/phortune/editor/PhortuneCartEditor.php @@ -189,7 +189,7 @@ protected function getMailTo(PhabricatorLiskDAO $object) { $phids = array(); - // Relaod the cart to pull merchant and account information, in case we + // Reload the cart to pull merchant and account information, in case we // just created the object. $cart = id(new PhortuneCartQuery()) ->setViewer($this->requireActor()) @@ -220,4 +220,24 @@ ->setMailReceiver($object); } + protected function willPublish(PhabricatorLiskDAO $object, array $xactions) { + // We need the purchases in order to build mail. + return id(new PhortuneCartQuery()) + ->setViewer($this->getActor()) + ->withIDs(array($object->getID())) + ->needPurchases(true) + ->executeOne(); + } + + protected function getCustomWorkerState() { + return array( + 'invoiceIssues' => $this->invoiceIssues, + ); + } + + protected function loadCustomWorkerState(array $state) { + $this->invoiceIssues = idx($state, 'invoiceIssues'); + return $this; + } + } diff --git a/src/applications/phriction/editor/PhrictionTransactionEditor.php b/src/applications/phriction/editor/PhrictionTransactionEditor.php --- a/src/applications/phriction/editor/PhrictionTransactionEditor.php +++ b/src/applications/phriction/editor/PhrictionTransactionEditor.php @@ -13,6 +13,7 @@ private $skipAncestorCheck; private $contentVersion; private $processContentVersionError = true; + private $contentDiffURI; public function setDescription($description) { $this->description = $description; @@ -348,6 +349,20 @@ ->applyTransactions($this->moveAwayDocument, $move_away_xactions); } + // Compute the content diff URI for the publishing phase. + foreach ($xactions as $xaction) { + switch ($xaction->getTransactionType()) { + case PhrictionTransaction::TYPE_CONTENT: + $uri = id(new PhutilURI('/phriction/diff/'.$object->getID().'/')) + ->alter('l', $this->getOldContent()->getVersion()) + ->alter('r', $this->getNewContent()->getVersion()); + $this->contentDiffURI = (string)$uri; + break 2; + default: + break; + } + } + return $xactions; } @@ -403,24 +418,10 @@ $body->addTextSection( pht('DOCUMENT CONTENT'), $object->getContent()->getContent()); - } else { - - foreach ($xactions as $xaction) { - switch ($xaction->getTransactionType()) { - case PhrictionTransaction::TYPE_CONTENT: - $diff_uri = id(new PhutilURI( - '/phriction/diff/'.$object->getID().'/')) - ->alter('l', $this->getOldContent()->getVersion()) - ->alter('r', $this->getNewContent()->getVersion()); - $body->addLinkSection( - pht('DOCUMENT DIFF'), - PhabricatorEnv::getProductionURI($diff_uri)); - break 2; - default: - break; - } - } - + } else if ($this->contentDiffURI) { + $body->addLinkSection( + pht('DOCUMENT DIFF'), + PhabricatorEnv::getProductionURI($this->contentDiffURI)); } $body->addLinkSection( @@ -810,4 +811,15 @@ return $new_content; } + protected function getCustomWorkerState() { + return array( + 'contentDiffURI' => $this->contentDiffURI, + ); + } + + protected function loadCustomWorkerState(array $state) { + $this->contentDiffURI = idx($state, 'contentDiffURI'); + return $this; + } + } diff --git a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php --- a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php +++ b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php @@ -397,14 +397,13 @@ return parent::requireCapabilities($object, $xaction); } - protected function loadEdges( - PhabricatorLiskDAO $object, - array $xactions) { - + protected function willPublish(PhabricatorLiskDAO $object, array $xactions) { $member_phids = PhabricatorEdgeQuery::loadDestinationPHIDs( $object->getPHID(), PhabricatorProjectProjectHasMemberEdgeType::EDGECONST); $object->attachMemberPHIDs($member_phids); + + return $object; } protected function shouldSendMail( 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 @@ -1,6 +1,36 @@ heraldHeader = $herald_header; - if ($this->supportsWorkers()) { - PhabricatorWorker::scheduleTask( - 'PhabricatorApplicationTransactionPublishWorker', - array( - 'objectPHID' => $object->getPHID(), - 'actorPHID' => $this->getActingAsPHID(), - 'xactionPHIDs' => mpull($xactions, 'getPHID'), - 'state' => $this->getWorkerState(), - ), - array( - 'objectPHID' => $object->getPHID(), - 'priority' => PhabricatorWorker::PRIORITY_ALERTS, - )); - } else { - $this->publishTransactions($object, $xactions); + // We're going to compute some of the data we'll use to publish these + // transactions here, before queueing a worker. + // + // Primarily, this is more correct: we want to publish the object as it + // exists right now. The worker may not execute for some time, and we want + // to use the current To/CC list, not respect any changes which may occur + // between now and when the worker executes. + // + // As a secondary benefit, this tends to reduce the amount of state that + // Editors need to pass into workers. + $object = $this->willPublish($object, $xactions); + + if (!$this->getDisableEmail()) { + if ($this->shouldSendMail($object, $xactions)) { + $this->mailToPHIDs = $this->getMailTo($object); + $this->mailCCPHIDs = $this->getMailCC($object); + } } + if ($this->shouldPublishFeedStory($object, $xactions)) { + $this->feedRelatedPHIDs = $this->getFeedRelatedPHIDs($object, $xactions); + $this->feedNotifyPHIDs = $this->getFeedNotifyPHIDs($object, $xactions); + } + + PhabricatorWorker::scheduleTask( + 'PhabricatorApplicationTransactionPublishWorker', + array( + 'objectPHID' => $object->getPHID(), + 'actorPHID' => $this->getActingAsPHID(), + 'xactionPHIDs' => mpull($xactions, 'getPHID'), + 'state' => $this->getWorkerState(), + ), + array( + 'objectPHID' => $object->getPHID(), + 'priority' => PhabricatorWorker::PRIORITY_ALERTS, + )); + return $xactions; } @@ -936,7 +990,7 @@ // in various transaction phases). $this->loadSubscribers($object); // Hook for other edges that may need (re-)loading - $this->loadEdges($object, $xactions); + $object = $this->willPublish($object, $xactions); $this->loadHandles($xactions); @@ -1042,12 +1096,6 @@ } } - protected function loadEdges( - PhabricatorLiskDAO $object, - array $xactions) { - return; - } - private function validateEditParameters( PhabricatorLiskDAO $object, array $xactions) { @@ -2084,8 +2132,8 @@ } $email_force = array(); - $email_to = $this->getMailTo($object); - $email_cc = $this->getMailCC($object); + $email_to = $this->mailToPHIDs; + $email_cc = $this->mailCCPHIDs; $email_cc = array_merge($email_cc, $this->heraldEmailPHIDs); $email_force = $this->heraldForcedEmailPHIDs; @@ -2511,8 +2559,8 @@ return; } - $related_phids = $this->getFeedRelatedPHIDs($object, $xactions); - $subscribed_phids = $this->getFeedNotifyPHIDs($object, $xactions); + $related_phids = $this->feedRelatedPHIDs; + $subscribed_phids = $this->feedNotifyPHIDs; $story_type = $this->getFeedStoryType(); $story_data = $this->getFeedStoryData($object, $xactions); @@ -2800,11 +2848,17 @@ /** + * Load any object state which is required to publish transactions. + * + * This hook is invoked in the main process before we compute data related + * to publishing transactions (like email "To" and "CC" lists), and again in + * the worker before publishing occurs. + * + * @return object Publishable object. * @task workers */ - protected function supportsWorkers() { - // TODO: Remove this method once everything supports workers. - return false; + protected function willPublish(PhabricatorLiskDAO $object, array $xactions) { + return $object; } @@ -2825,11 +2879,10 @@ $state += array( 'excludeMailRecipientPHIDs' => $this->getExcludeMailRecipientPHIDs(), - ); - - return $state + array( 'custom' => $this->getCustomWorkerState(), ); + + return $state; } @@ -2900,6 +2953,10 @@ 'heraldEmailPHIDs', 'heraldForcedEmailPHIDs', 'heraldHeader', + 'mailToPHIDs', + 'mailCCPHIDs', + 'feedNotifyPHIDs', + 'feedRelatedPHIDs', ); }