From 4a45620b0458155d7853881efc5191aafc5e9e7a Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 3 Jun 2015 20:01:23 -0700 Subject: [PATCH] Reload revisions before publishing mail about them Fixes this, which only triggers on some kinds of mail: ``` Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000] [2015-06-04 02:56:38] EXCEPTION: (PhutilProxyException) Error while executing Task ID 902251. {>} (PhabricatorDataNotAttachedException) Attempting to access attached data on DifferentialRevision (via getActiveDiff()), but the data is not actually attached. Before accessing attachable data on an object, you must load and attach it. Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000] Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000] Data is normally attached by calling the corresponding needX() method on the Query class when the object is loaded. You can also call the corresponding attachX() method explicitly. at [/src/infrastructure/storage/lisk/PhabricatorLiskDAO.php:166] Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000] arcanist(head=master, ref.master=8c589f1f759f), phabricator(head=master, ref.master=6dede2e2c513), phutil(head=master, ref.master=afc05a9a7f00) Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000] #0 <#2> PhabricatorLiskDAO::assertAttached(string) called at [/src/applications/differential/storage/DifferentialRevision.php:158] Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000] #1 <#2> DifferentialRevision::getActiveDiff() called at [/src/applications/differential/customfield/DifferentialBranchField.php:72] Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000] #2 <#2> DifferentialBranchField::updateTransactionMailBody(PhabricatorMetaMTAMailBody, DifferentialTransactionEditor, array) called at [/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:2481] Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000] #3 <#2> PhabricatorApplicationTransactionEditor::addCustomFieldsToMailBody(PhabricatorMetaMTAMailBody, DifferentialRevision, array) called at [/src/applications/differential/editor/DifferentialTransactionEditor.php:1208] Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000] #4 <#2> DifferentialTransactionEditor::buildMailBody(DifferentialRevision, array) called at [/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:2178] Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000] #5 <#2> PhabricatorApplicationTransactionEditor::sendMailToTarget(DifferentialRevision, array, PhabricatorMailTarget) called at [/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:2152] Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000] #6 <#2> PhabricatorApplicationTransactionEditor::sendMail(DifferentialRevision, array) called at [/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:998] Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000] #7 <#2> PhabricatorApplicationTransactionEditor::publishTransactions(DifferentialRevision, array) called at [/src/applications/transactions/worker/PhabricatorApplicationTransactionPublishWorker.php:21] Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000] #8 <#2> PhabricatorApplicationTransactionPublishWorker::doWork() called at [/src/infrastructure/daemon/workers/PhabricatorWorker.php:91] Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000] #9 <#2> PhabricatorWorker::executeTask() called at [/src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php:162] Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000] #10 <#2> PhabricatorWorkerActiveTask::executeTask() called at [/src/infrastructure/daemon/workers/PhabricatorTaskmasterDaemon.php:20] Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000] #11 PhabricatorTaskmasterDaemon::run() called at [/src/daemon/PhutilDaemon.php:185] Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000] #12 PhutilDaemon::execute() called at [/scripts/daemon/exec/exec_daemon.php:125] ``` Auditors: btrahan --- .../editor/DifferentialTransactionEditor.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php index 09a7e676b2..952c45e014 100644 --- a/src/applications/differential/editor/DifferentialTransactionEditor.php +++ b/src/applications/differential/editor/DifferentialTransactionEditor.php @@ -1902,6 +1902,16 @@ final class DifferentialTransactionEditor return $section; } + protected function willPublish(PhabricatorLiskDAO $object, array $xactions) { + // Reload to pick up the active diff and reviewer status. + return id(new DifferentialRevisionQuery()) + ->setViewer($this->getActor()) + ->needReviewerStatus(true) + ->needActiveDiffs(true) + ->withIDs(array($object->getID())) + ->executeOne(); + } + protected function getCustomWorkerState() { return array( 'changedPriorToCommitURI' => $this->changedPriorToCommitURI,