2015-07-24 17:19:47 +02:00
|
|
|
<?php
|
|
|
|
|
|
|
|
abstract class PhabricatorMetaMTAEmailHeraldAction
|
|
|
|
extends HeraldAction {
|
|
|
|
|
|
|
|
const DO_SEND = 'do.send';
|
|
|
|
const DO_FORCE = 'do.force';
|
|
|
|
|
Don't run Herald build and mail rules when they don't make sense
Summary:
Ref T2543. Fixes T10109.
Currently, Herald only runs in Differential when a change updates the diff. This is partly for historical reasons, and partly because we don't want to restart builds every time someone makes a comment. However, this behavior is inconsistent with other applications (which always trigger on any change), and occasionally confusing to users (in T10109, for example) or otherwise undesirable.
A similar issue is that T2543 has introduced a "Draft" state, where revisions don't send normal mail until builds finish. This interacts poorly with "Send me an email" rules (which shouldn't do anything here) and particularly with "Send me an email + only run these actions the first time the rule matches", since that might have an effect like "do nothing when the revision is created, then never anything again since you already did nothing once".
To navigate both of these issues, let objects tell Herald that certain actions (like mail or builds) are currently forbidden. If a rule uses a field or action which is currently forbidden, the whole rule automatically fails before it executes, but doesn't count toward "only the first time" as far as Herald's tracking of rule execution is concerned.
Then, forbid mail for draft revisions, and forbid builds for revisions which didn't just get updated. Forbidding mail fixes the issues with "Send me an email" that were created by the introduction of the draft state.
Finally, make Herald run on every revision update, not just substantive updates to the diff. This resolves T10109.
Test Plan:
Created revisions via the draft -> submit workflow, saw different transcripts. Here's a mail action being forbidden for a draft:
{F5237324}
Here's a build action being forbidden for a "mundane" update:
{F5237326}
Reviewers: amckinley
Reviewed By: amckinley
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T10109, T2543
Differential Revision: https://secure.phabricator.com/D18731
2017-10-24 23:04:06 +02:00
|
|
|
public function getRequiredAdapterStates() {
|
|
|
|
return array(
|
|
|
|
HeraldMailableState::STATECONST,
|
|
|
|
);
|
|
|
|
}
|
|
|
|
|
2015-07-24 17:19:47 +02:00
|
|
|
public function supportsObject($object) {
|
|
|
|
// NOTE: This implementation lacks generality, but there's no great way to
|
|
|
|
// figure out if something generates email right now.
|
|
|
|
|
|
|
|
if ($object instanceof DifferentialDiff) {
|
|
|
|
return false;
|
|
|
|
}
|
|
|
|
|
2015-08-15 19:54:33 +02:00
|
|
|
if ($object instanceof PhabricatorMetaMTAMail) {
|
|
|
|
return false;
|
|
|
|
}
|
|
|
|
|
2015-07-24 17:19:47 +02:00
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
|
|
|
public function getActionGroupKey() {
|
|
|
|
return HeraldNotifyActionGroup::ACTIONGROUPKEY;
|
|
|
|
}
|
|
|
|
|
|
|
|
protected function applyEmail(array $phids, $force) {
|
|
|
|
$adapter = $this->getAdapter();
|
|
|
|
|
2015-08-03 22:34:31 +02:00
|
|
|
$allowed_types = array(
|
|
|
|
PhabricatorPeopleUserPHIDType::TYPECONST,
|
|
|
|
PhabricatorProjectProjectPHIDType::TYPECONST,
|
|
|
|
);
|
|
|
|
|
|
|
|
// There's no stateful behavior for this action: we always just send an
|
|
|
|
// email.
|
|
|
|
$current = array();
|
|
|
|
|
|
|
|
$targets = $this->loadStandardTargets($phids, $allowed_types, $current);
|
|
|
|
if (!$targets) {
|
|
|
|
return;
|
|
|
|
}
|
|
|
|
|
|
|
|
$phids = array_fuse(array_keys($targets));
|
|
|
|
|
2015-07-24 17:19:47 +02:00
|
|
|
foreach ($phids as $phid) {
|
|
|
|
$adapter->addEmailPHID($phid, $force);
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($force) {
|
|
|
|
$this->logEffect(self::DO_FORCE, $phids);
|
|
|
|
} else {
|
|
|
|
$this->logEffect(self::DO_SEND, $phids);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
protected function getActionEffectMap() {
|
|
|
|
return array(
|
|
|
|
self::DO_SEND => array(
|
|
|
|
'icon' => 'fa-envelope',
|
|
|
|
'color' => 'green',
|
|
|
|
'name' => pht('Sent Mail'),
|
|
|
|
),
|
|
|
|
self::DO_FORCE => array(
|
|
|
|
'icon' => 'fa-envelope',
|
|
|
|
'color' => 'blue',
|
|
|
|
'name' => pht('Forced Mail'),
|
|
|
|
),
|
|
|
|
);
|
|
|
|
}
|
|
|
|
|
2015-08-03 15:01:24 +02:00
|
|
|
protected function renderActionEffectDescription($type, $data) {
|
2015-07-24 17:19:47 +02:00
|
|
|
switch ($type) {
|
|
|
|
case self::DO_SEND:
|
|
|
|
return pht(
|
|
|
|
'Queued email to be delivered to %s target(s): %s.',
|
2015-11-02 20:54:38 +01:00
|
|
|
phutil_count($data),
|
2015-07-24 17:19:47 +02:00
|
|
|
$this->renderHandleList($data));
|
|
|
|
case self::DO_FORCE:
|
|
|
|
return pht(
|
|
|
|
'Queued email to be delivered to %s target(s), ignoring their '.
|
|
|
|
'notification preferences: %s.',
|
2015-11-02 20:54:38 +01:00
|
|
|
phutil_count($data),
|
2015-07-24 17:19:47 +02:00
|
|
|
$this->renderHandleList($data));
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
}
|