From 8a5def8e0ae1bb22064a6a457b37f24e4b7c503f Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 19 Jan 2018 07:44:44 -0800 Subject: [PATCH] Remove legacy transaction editor "getDisableEmail()" method Summary: Ref T13042. This is a very, very old policy-violating option from yesteryear which supported build systems publishing updates by adding comments to revisions, without sending email about it. Harbormaster has served this role for a long time and this is policy-violating in the general case (it allows attackers to act in secret). Test Plan: Grepped for affected symbols. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13042 Differential Revision: https://secure.phabricator.com/D18881 --- ...ferentialCreateCommentConduitAPIMethod.php | 4 ++- ...habricatorApplicationTransactionEditor.php | 31 +++---------------- 2 files changed, 8 insertions(+), 27 deletions(-) diff --git a/src/applications/differential/conduit/DifferentialCreateCommentConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialCreateCommentConduitAPIMethod.php index 21b055a1e4..657237c718 100644 --- a/src/applications/differential/conduit/DifferentialCreateCommentConduitAPIMethod.php +++ b/src/applications/differential/conduit/DifferentialCreateCommentConduitAPIMethod.php @@ -106,9 +106,11 @@ final class DifferentialCreateCommentConduitAPIMethod } } + // NOTE: The legacy "silent" flag is now ignored and has no effect. See + // T13042. + $editor = id(new DifferentialTransactionEditor()) ->setActor($viewer) - ->setDisableEmail($request->getValue('silent')) ->setContentSource($request->newContentSource()) ->setContinueOnNoEffect(true) ->setContinueOnMissingFields(true); diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php index c08451624d..b4ad4365a7 100644 --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -59,7 +59,6 @@ abstract class PhabricatorApplicationTransactionEditor private $isHeraldEditor; private $isInverseEdgeEditor; private $actingAsPHID; - private $disableEmail; private $heraldEmailPHIDs = array(); private $heraldForcedEmailPHIDs = array(); @@ -206,21 +205,6 @@ abstract class PhabricatorApplicationTransactionEditor return $this->isHeraldEditor; } - /** - * Prevent this editor from generating email when applying transactions. - * - * @param bool True to disable email. - * @return this - */ - public function setDisableEmail($disable_email) { - $this->disableEmail = $disable_email; - return $this; - } - - public function getDisableEmail() { - return $this->disableEmail; - } - public function setUnmentionablePHIDMap(array $map) { $this->unmentionablePHIDMap = $map; return $this; @@ -1152,11 +1136,9 @@ abstract class PhabricatorApplicationTransactionEditor // 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->shouldSendMail($object, $xactions)) { + $this->mailToPHIDs = $this->getMailTo($object); + $this->mailCCPHIDs = $this->getMailCC($object); } if ($this->shouldPublishFeedStory($object, $xactions)) { @@ -1204,10 +1186,8 @@ abstract class PhabricatorApplicationTransactionEditor $this->object = $object; $messages = array(); - if (!$this->getDisableEmail()) { - if ($this->shouldSendMail($object, $xactions)) { - $messages = $this->buildMail($object, $xactions); - } + if ($this->shouldSendMail($object, $xactions)) { + $messages = $this->buildMail($object, $xactions); } if ($this->supportsSearch()) { @@ -3504,7 +3484,6 @@ abstract class PhabricatorApplicationTransactionEditor private function getAutomaticStateProperties() { return array( 'parentMessageID', - 'disableEmail', 'isNewObject', 'heraldEmailPHIDs', 'heraldForcedEmailPHIDs',