mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-22 23:02:42 +01:00
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
This commit is contained in:
parent
d1b4c9f50b
commit
f7f3dd5b20
16 changed files with 359 additions and 64 deletions
|
@ -458,6 +458,7 @@ phutil_register_library_map(array(
|
|||
'DifferentialGetWorkingCopy' => 'applications/differential/DifferentialGetWorkingCopy.php',
|
||||
'DifferentialGitSVNIDCommitMessageField' => 'applications/differential/field/DifferentialGitSVNIDCommitMessageField.php',
|
||||
'DifferentialHarbormasterField' => 'applications/differential/customfield/DifferentialHarbormasterField.php',
|
||||
'DifferentialHeraldStateReasons' => 'applications/differential/herald/DifferentialHeraldStateReasons.php',
|
||||
'DifferentialHiddenComment' => 'applications/differential/storage/DifferentialHiddenComment.php',
|
||||
'DifferentialHostField' => 'applications/differential/customfield/DifferentialHostField.php',
|
||||
'DifferentialHovercardEngineExtension' => 'applications/differential/engineextension/DifferentialHovercardEngineExtension.php',
|
||||
|
@ -1328,6 +1329,7 @@ phutil_register_library_map(array(
|
|||
'HeraldApplicationActionGroup' => 'applications/herald/action/HeraldApplicationActionGroup.php',
|
||||
'HeraldApplyTranscript' => 'applications/herald/storage/transcript/HeraldApplyTranscript.php',
|
||||
'HeraldBasicFieldGroup' => 'applications/herald/field/HeraldBasicFieldGroup.php',
|
||||
'HeraldBuildableState' => 'applications/herald/state/HeraldBuildableState.php',
|
||||
'HeraldCommitAdapter' => 'applications/diffusion/herald/HeraldCommitAdapter.php',
|
||||
'HeraldCondition' => 'applications/herald/storage/HeraldCondition.php',
|
||||
'HeraldConditionTranscript' => 'applications/herald/storage/transcript/HeraldConditionTranscript.php',
|
||||
|
@ -1351,6 +1353,7 @@ phutil_register_library_map(array(
|
|||
'HeraldGroup' => 'applications/herald/group/HeraldGroup.php',
|
||||
'HeraldInvalidActionException' => 'applications/herald/engine/exception/HeraldInvalidActionException.php',
|
||||
'HeraldInvalidConditionException' => 'applications/herald/engine/exception/HeraldInvalidConditionException.php',
|
||||
'HeraldMailableState' => 'applications/herald/state/HeraldMailableState.php',
|
||||
'HeraldManageGlobalRulesCapability' => 'applications/herald/capability/HeraldManageGlobalRulesCapability.php',
|
||||
'HeraldManiphestTaskAdapter' => 'applications/maniphest/herald/HeraldManiphestTaskAdapter.php',
|
||||
'HeraldNewController' => 'applications/herald/controller/HeraldNewController.php',
|
||||
|
@ -1388,6 +1391,8 @@ phutil_register_library_map(array(
|
|||
'HeraldSchemaSpec' => 'applications/herald/storage/HeraldSchemaSpec.php',
|
||||
'HeraldSelectFieldValue' => 'applications/herald/value/HeraldSelectFieldValue.php',
|
||||
'HeraldSpaceField' => 'applications/spaces/herald/HeraldSpaceField.php',
|
||||
'HeraldState' => 'applications/herald/state/HeraldState.php',
|
||||
'HeraldStateReasons' => 'applications/herald/state/HeraldStateReasons.php',
|
||||
'HeraldSubscribersField' => 'applications/subscriptions/herald/HeraldSubscribersField.php',
|
||||
'HeraldSupportActionGroup' => 'applications/herald/action/HeraldSupportActionGroup.php',
|
||||
'HeraldSupportFieldGroup' => 'applications/herald/field/HeraldSupportFieldGroup.php',
|
||||
|
@ -5467,6 +5472,7 @@ phutil_register_library_map(array(
|
|||
'DifferentialGetWorkingCopy' => 'Phobject',
|
||||
'DifferentialGitSVNIDCommitMessageField' => 'DifferentialCommitMessageField',
|
||||
'DifferentialHarbormasterField' => 'DifferentialCustomField',
|
||||
'DifferentialHeraldStateReasons' => 'HeraldStateReasons',
|
||||
'DifferentialHiddenComment' => 'DifferentialDAO',
|
||||
'DifferentialHostField' => 'DifferentialCustomField',
|
||||
'DifferentialHovercardEngineExtension' => 'PhabricatorHovercardEngineExtension',
|
||||
|
@ -6458,6 +6464,7 @@ phutil_register_library_map(array(
|
|||
'HeraldApplicationActionGroup' => 'HeraldActionGroup',
|
||||
'HeraldApplyTranscript' => 'Phobject',
|
||||
'HeraldBasicFieldGroup' => 'HeraldFieldGroup',
|
||||
'HeraldBuildableState' => 'HeraldState',
|
||||
'HeraldCommitAdapter' => array(
|
||||
'HeraldAdapter',
|
||||
'HarbormasterBuildableAdapterInterface',
|
||||
|
@ -6487,6 +6494,7 @@ phutil_register_library_map(array(
|
|||
'HeraldGroup' => 'Phobject',
|
||||
'HeraldInvalidActionException' => 'Exception',
|
||||
'HeraldInvalidConditionException' => 'Exception',
|
||||
'HeraldMailableState' => 'HeraldState',
|
||||
'HeraldManageGlobalRulesCapability' => 'PhabricatorPolicyCapability',
|
||||
'HeraldManiphestTaskAdapter' => 'HeraldAdapter',
|
||||
'HeraldNewController' => 'HeraldController',
|
||||
|
@ -6531,6 +6539,8 @@ phutil_register_library_map(array(
|
|||
'HeraldSchemaSpec' => 'PhabricatorConfigSchemaSpec',
|
||||
'HeraldSelectFieldValue' => 'HeraldFieldValue',
|
||||
'HeraldSpaceField' => 'HeraldField',
|
||||
'HeraldState' => 'Phobject',
|
||||
'HeraldStateReasons' => 'Phobject',
|
||||
'HeraldSubscribersField' => 'HeraldField',
|
||||
'HeraldSupportActionGroup' => 'HeraldActionGroup',
|
||||
'HeraldSupportFieldGroup' => 'HeraldFieldGroup',
|
||||
|
|
|
@ -1003,26 +1003,7 @@ final class DifferentialTransactionEditor
|
|||
protected function shouldApplyHeraldRules(
|
||||
PhabricatorLiskDAO $object,
|
||||
array $xactions) {
|
||||
|
||||
if ($this->getIsNewObject()) {
|
||||
return true;
|
||||
}
|
||||
|
||||
foreach ($xactions as $xaction) {
|
||||
switch ($xaction->getTransactionType()) {
|
||||
case DifferentialTransaction::TYPE_UPDATE:
|
||||
if (!$this->getIsCloseByCommit()) {
|
||||
return true;
|
||||
}
|
||||
break;
|
||||
case DifferentialRevisionCommandeerTransaction::TRANSACTIONTYPE:
|
||||
// When users commandeer revisions, we may need to trigger
|
||||
// signatures or author-based rules.
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
return parent::shouldApplyHeraldRules($object, $xactions);
|
||||
return true;
|
||||
}
|
||||
|
||||
protected function didApplyHeraldRules(
|
||||
|
@ -1211,6 +1192,33 @@ final class DifferentialTransactionEditor
|
|||
$revision,
|
||||
$revision->getActiveDiff());
|
||||
|
||||
// If the object is still a draft, prevent "Send me an email" and other
|
||||
// similar rules from acting yet.
|
||||
if (!$object->shouldBroadcast()) {
|
||||
$adapter->setForbiddenAction(
|
||||
HeraldMailableState::STATECONST,
|
||||
DifferentialHeraldStateReasons::REASON_DRAFT);
|
||||
}
|
||||
|
||||
// If this edit didn't actually change the diff (for example, a user
|
||||
// edited the title or changed subscribers), prevent "Run build plan"
|
||||
// and other similar rules from acting yet, since the build results will
|
||||
// not (or, at least, should not) change unless the actual source changes.
|
||||
$has_update = false;
|
||||
$type_update = DifferentialTransaction::TYPE_UPDATE;
|
||||
foreach ($xactions as $xaction) {
|
||||
if ($xaction->getTransactionType() == $type_update) {
|
||||
$has_update = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
if (!$has_update) {
|
||||
$adapter->setForbiddenAction(
|
||||
HeraldBuildableState::STATECONST,
|
||||
DifferentialHeraldStateReasons::REASON_UNCHANGED);
|
||||
}
|
||||
|
||||
return $adapter;
|
||||
}
|
||||
|
||||
|
|
|
@ -0,0 +1,22 @@
|
|||
<?php
|
||||
|
||||
final class DifferentialHeraldStateReasons
|
||||
extends HeraldStateReasons {
|
||||
|
||||
const REASON_DRAFT = 'differential.draft';
|
||||
const REASON_UNCHANGED = 'differential.unchanged';
|
||||
|
||||
public function explainReason($reason) {
|
||||
$reasons = array(
|
||||
self::REASON_DRAFT => pht(
|
||||
'This revision is still an unsubmitted draft, so mail will not '.
|
||||
'be sent yet.'),
|
||||
self::REASON_UNCHANGED => pht(
|
||||
'The update which triggered Herald did not update the diff for '.
|
||||
'this revision, so builds will not run.'),
|
||||
);
|
||||
|
||||
return idx($reasons, $reason);
|
||||
}
|
||||
|
||||
}
|
|
@ -7,6 +7,12 @@ final class HarbormasterRunBuildPlansHeraldAction
|
|||
|
||||
const ACTIONCONST = 'harbormaster.build';
|
||||
|
||||
public function getRequiredAdapterStates() {
|
||||
return array(
|
||||
HeraldBuildableState::STATECONST,
|
||||
);
|
||||
}
|
||||
|
||||
public function getActionGroupKey() {
|
||||
return HeraldSupportActionGroup::ACTIONGROUPKEY;
|
||||
}
|
||||
|
|
|
@ -17,6 +17,7 @@ abstract class HeraldAction extends Phobject {
|
|||
const DO_STANDARD_PERMISSION = 'do.standard.permission';
|
||||
const DO_STANDARD_INVALID_ACTION = 'do.standard.invalid-action';
|
||||
const DO_STANDARD_WRONG_RULE_TYPE = 'do.standard.wrong-rule-type';
|
||||
const DO_STANDARD_FORBIDDEN = 'do.standard.forbidden';
|
||||
|
||||
abstract public function getHeraldActionName();
|
||||
abstract public function supportsObject($object);
|
||||
|
@ -25,6 +26,10 @@ abstract class HeraldAction extends Phobject {
|
|||
|
||||
abstract public function renderActionDescription($value);
|
||||
|
||||
public function getRequiredAdapterStates() {
|
||||
return array();
|
||||
}
|
||||
|
||||
protected function renderActionEffectDescription($type, $data) {
|
||||
return null;
|
||||
}
|
||||
|
@ -336,6 +341,11 @@ abstract class HeraldAction extends Phobject {
|
|||
'color' => 'red',
|
||||
'name' => pht('Wrong Rule Type'),
|
||||
),
|
||||
self::DO_STANDARD_FORBIDDEN => array(
|
||||
'icon' => 'fa-ban',
|
||||
'color' => 'violet',
|
||||
'name' => pht('Forbidden'),
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
|
@ -381,6 +391,8 @@ abstract class HeraldAction extends Phobject {
|
|||
return pht(
|
||||
'This action does not support rules of type "%s".',
|
||||
$data);
|
||||
case self::DO_STANDARD_FORBIDDEN:
|
||||
return HeraldStateReasons::getExplanation($data);
|
||||
}
|
||||
|
||||
return null;
|
||||
|
|
|
@ -37,6 +37,7 @@ abstract class HeraldAdapter extends Phobject {
|
|||
private $fieldMap;
|
||||
private $actionMap;
|
||||
private $edgeCache = array();
|
||||
private $forbiddenActions = array();
|
||||
|
||||
public function getEmailPHIDs() {
|
||||
return array_values($this->emailPHIDs);
|
||||
|
@ -1116,4 +1117,38 @@ abstract class HeraldAdapter extends Phobject {
|
|||
return $this->edgeCache[$type];
|
||||
}
|
||||
|
||||
|
||||
/* -( Forbidden Actions )-------------------------------------------------- */
|
||||
|
||||
|
||||
final public function getForbiddenActions() {
|
||||
return array_keys($this->forbiddenActions);
|
||||
}
|
||||
|
||||
final public function setForbiddenAction($action, $reason) {
|
||||
$this->forbiddenActions[$action] = $reason;
|
||||
return $this;
|
||||
}
|
||||
|
||||
final public function getRequiredFieldStates($field_key) {
|
||||
return $this->requireFieldImplementation($field_key)
|
||||
->getRequiredAdapterStates();
|
||||
}
|
||||
|
||||
final public function getRequiredActionStates($action_key) {
|
||||
return $this->requireActionImplementation($action_key)
|
||||
->getRequiredAdapterStates();
|
||||
}
|
||||
|
||||
final public function getForbiddenReason($action) {
|
||||
if (!isset($this->forbiddenActions[$action])) {
|
||||
throw new Exception(
|
||||
pht(
|
||||
'Action "%s" is not forbidden!',
|
||||
$action));
|
||||
}
|
||||
|
||||
return $this->forbiddenActions[$action];
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -273,7 +273,11 @@ final class HeraldTranscriptController extends HeraldController {
|
|||
->setTarget(phutil_tag('strong', array(), pht('Conditions'))));
|
||||
|
||||
foreach ($cond_xscripts as $cond_xscript) {
|
||||
if ($cond_xscript->getResult()) {
|
||||
if ($cond_xscript->isForbidden()) {
|
||||
$icon = 'fa-ban';
|
||||
$color = 'indigo';
|
||||
$result = pht('Forbidden');
|
||||
} else if ($cond_xscript->getResult()) {
|
||||
$icon = 'fa-check';
|
||||
$color = 'green';
|
||||
$result = pht('Passed');
|
||||
|
@ -284,12 +288,17 @@ final class HeraldTranscriptController extends HeraldController {
|
|||
}
|
||||
|
||||
if ($cond_xscript->getNote()) {
|
||||
$note_text = $cond_xscript->getNote();
|
||||
if ($cond_xscript->isForbidden()) {
|
||||
$note_text = HeraldStateReasons::getExplanation($note_text);
|
||||
}
|
||||
|
||||
$note = phutil_tag(
|
||||
'div',
|
||||
array(
|
||||
'class' => 'herald-condition-note',
|
||||
),
|
||||
$cond_xscript->getNote());
|
||||
$note_text);
|
||||
} else {
|
||||
$note = null;
|
||||
}
|
||||
|
@ -310,7 +319,12 @@ final class HeraldTranscriptController extends HeraldController {
|
|||
$cond_list->addItem($cond_item);
|
||||
}
|
||||
|
||||
if ($rule_xscript->getResult()) {
|
||||
if ($rule_xscript->isForbidden()) {
|
||||
$last_icon = 'fa-ban';
|
||||
$last_color = 'indigo';
|
||||
$last_result = pht('Forbidden');
|
||||
$last_note = pht('Object state prevented rule evaluation.');
|
||||
} else if ($rule_xscript->getResult()) {
|
||||
$last_icon = 'fa-check-circle';
|
||||
$last_color = 'green';
|
||||
$last_result = pht('Passed');
|
||||
|
|
|
@ -12,6 +12,9 @@ final class HeraldEngine extends Phobject {
|
|||
protected $object;
|
||||
private $dryRun;
|
||||
|
||||
private $forbiddenFields = array();
|
||||
private $forbiddenActions = array();
|
||||
|
||||
public function setDryRun($dry_run) {
|
||||
$this->dryRun = $dry_run;
|
||||
return $this;
|
||||
|
@ -76,39 +79,42 @@ final class HeraldEngine extends Phobject {
|
|||
// This is not a dry run, and this rule is only supposed to be
|
||||
// applied a single time, and it's already been applied...
|
||||
// That means automatic failure.
|
||||
$xscript = id(new HeraldRuleTranscript())
|
||||
->setRuleID($rule->getID())
|
||||
$this->newRuleTranscript($rule)
|
||||
->setResult(false)
|
||||
->setRuleName($rule->getName())
|
||||
->setRuleOwner($rule->getAuthorPHID())
|
||||
->setReason(
|
||||
pht(
|
||||
'This rule is only supposed to be repeated a single time, '.
|
||||
'and it has already been applied.'));
|
||||
$this->transcript->addRuleTranscript($xscript);
|
||||
|
||||
$rule_matches = false;
|
||||
} else {
|
||||
$rule_matches = $this->doesRuleMatch($rule, $object);
|
||||
if ($this->isForbidden($rule, $object)) {
|
||||
$this->newRuleTranscript($rule)
|
||||
->setResult(HeraldRuleTranscript::RESULT_FORBIDDEN)
|
||||
->setReason(
|
||||
pht(
|
||||
'Object state is not compatible with rule.'));
|
||||
|
||||
$rule_matches = false;
|
||||
} else {
|
||||
$rule_matches = $this->doesRuleMatch($rule, $object);
|
||||
}
|
||||
}
|
||||
} catch (HeraldRecursiveConditionsException $ex) {
|
||||
$names = array();
|
||||
foreach ($this->stack as $rule_id => $ignored) {
|
||||
$names[] = '"'.$rules[$rule_id]->getName().'"';
|
||||
foreach ($this->stack as $rule_phid => $ignored) {
|
||||
$names[] = '"'.$rules[$rule_phid]->getName().'"';
|
||||
}
|
||||
$names = implode(', ', $names);
|
||||
foreach ($this->stack as $rule_id => $ignored) {
|
||||
$xscript = new HeraldRuleTranscript();
|
||||
$xscript->setRuleID($rule_id);
|
||||
$xscript->setResult(false);
|
||||
$xscript->setReason(
|
||||
pht(
|
||||
"Rules %s are recursively dependent upon one another! ".
|
||||
"Don't do this! You have formed an unresolvable cycle in the ".
|
||||
"dependency graph!",
|
||||
$names));
|
||||
$xscript->setRuleName($rules[$rule_id]->getName());
|
||||
$xscript->setRuleOwner($rules[$rule_id]->getAuthorPHID());
|
||||
$this->transcript->addRuleTranscript($xscript);
|
||||
foreach ($this->stack as $rule_phid => $ignored) {
|
||||
$this->newRuleTranscript($rules[$rule_phid])
|
||||
->setResult(false)
|
||||
->setReason(
|
||||
pht(
|
||||
"Rules %s are recursively dependent upon one another! ".
|
||||
"Don't do this! You have formed an unresolvable cycle in the ".
|
||||
"dependency graph!",
|
||||
$names));
|
||||
}
|
||||
$rule_matches = false;
|
||||
}
|
||||
|
@ -309,14 +315,9 @@ final class HeraldEngine extends Phobject {
|
|||
}
|
||||
}
|
||||
|
||||
$rule_transcript = new HeraldRuleTranscript();
|
||||
$rule_transcript->setRuleID($rule->getID());
|
||||
$rule_transcript->setResult($result);
|
||||
$rule_transcript->setReason($reason);
|
||||
$rule_transcript->setRuleName($rule->getName());
|
||||
$rule_transcript->setRuleOwner($rule->getAuthorPHID());
|
||||
|
||||
$this->transcript->addRuleTranscript($rule_transcript);
|
||||
$this->newRuleTranscript($rule)
|
||||
->setResult($result)
|
||||
->setReason($reason);
|
||||
|
||||
return $result;
|
||||
}
|
||||
|
@ -327,16 +328,7 @@ final class HeraldEngine extends Phobject {
|
|||
HeraldAdapter $object) {
|
||||
|
||||
$object_value = $this->getConditionObjectValue($condition, $object);
|
||||
$test_value = $condition->getValue();
|
||||
|
||||
$cond = $condition->getFieldCondition();
|
||||
|
||||
$transcript = new HeraldConditionTranscript();
|
||||
$transcript->setRuleID($rule->getID());
|
||||
$transcript->setConditionID($condition->getID());
|
||||
$transcript->setFieldName($condition->getFieldName());
|
||||
$transcript->setCondition($cond);
|
||||
$transcript->setTestValue($test_value);
|
||||
$transcript = $this->newConditionTranscript($rule, $condition);
|
||||
|
||||
try {
|
||||
$result = $object->doesConditionMatch(
|
||||
|
@ -351,8 +343,6 @@ final class HeraldEngine extends Phobject {
|
|||
|
||||
$transcript->setResult($result);
|
||||
|
||||
$this->transcript->addConditionTranscript($transcript);
|
||||
|
||||
return $result;
|
||||
}
|
||||
|
||||
|
@ -446,4 +436,136 @@ final class HeraldEngine extends Phobject {
|
|||
return false;
|
||||
}
|
||||
|
||||
private function newRuleTranscript(HeraldRule $rule) {
|
||||
$xscript = id(new HeraldRuleTranscript())
|
||||
->setRuleID($rule->getID())
|
||||
->setRuleName($rule->getName())
|
||||
->setRuleOwner($rule->getAuthorPHID());
|
||||
|
||||
$this->transcript->addRuleTranscript($xscript);
|
||||
|
||||
return $xscript;
|
||||
}
|
||||
|
||||
private function newConditionTranscript(
|
||||
HeraldRule $rule,
|
||||
HeraldCondition $condition) {
|
||||
|
||||
$xscript = id(new HeraldConditionTranscript())
|
||||
->setRuleID($rule->getID())
|
||||
->setConditionID($condition->getID())
|
||||
->setFieldName($condition->getFieldName())
|
||||
->setCondition($condition->getFieldCondition())
|
||||
->setTestValue($condition->getValue());
|
||||
|
||||
$this->transcript->addConditionTranscript($xscript);
|
||||
|
||||
return $xscript;
|
||||
}
|
||||
|
||||
private function newApplyTranscript(
|
||||
HeraldAdapter $adapter,
|
||||
HeraldRule $rule,
|
||||
HeraldActionRecord $action) {
|
||||
|
||||
$effect = id(new HeraldEffect())
|
||||
->setObjectPHID($adapter->getPHID())
|
||||
->setAction($action->getAction())
|
||||
->setTarget($action->getTarget())
|
||||
->setRule($rule);
|
||||
|
||||
$xscript = new HeraldApplyTranscript($effect, false);
|
||||
|
||||
$this->transcript->addApplyTranscript($xscript);
|
||||
|
||||
return $xscript;
|
||||
}
|
||||
|
||||
private function isForbidden(
|
||||
HeraldRule $rule,
|
||||
HeraldAdapter $adapter) {
|
||||
|
||||
$forbidden = $adapter->getForbiddenActions();
|
||||
if (!$forbidden) {
|
||||
return false;
|
||||
}
|
||||
|
||||
$forbidden = array_fuse($forbidden);
|
||||
|
||||
$is_forbidden = false;
|
||||
|
||||
foreach ($rule->getConditions() as $condition) {
|
||||
$field_key = $condition->getFieldName();
|
||||
|
||||
if (!isset($this->forbiddenFields[$field_key])) {
|
||||
$reason = null;
|
||||
|
||||
try {
|
||||
$states = $adapter->getRequiredFieldStates($field_key);
|
||||
} catch (Exception $ex) {
|
||||
$states = array();
|
||||
}
|
||||
|
||||
foreach ($states as $state) {
|
||||
if (!isset($forbidden[$state])) {
|
||||
continue;
|
||||
}
|
||||
$reason = $adapter->getForbiddenReason($state);
|
||||
break;
|
||||
}
|
||||
|
||||
$this->forbiddenFields[$field_key] = $reason;
|
||||
}
|
||||
|
||||
$forbidden_reason = $this->forbiddenFields[$field_key];
|
||||
if ($forbidden_reason !== null) {
|
||||
$this->newConditionTranscript($rule, $condition)
|
||||
->setResult(HeraldConditionTranscript::RESULT_FORBIDDEN)
|
||||
->setNote($forbidden_reason);
|
||||
|
||||
$is_forbidden = true;
|
||||
}
|
||||
}
|
||||
|
||||
foreach ($rule->getActions() as $action_record) {
|
||||
$action_key = $action_record->getAction();
|
||||
|
||||
if (!isset($this->forbiddenActions[$action_key])) {
|
||||
$reason = null;
|
||||
|
||||
try {
|
||||
$states = $adapter->getRequiredActionStates($action_key);
|
||||
} catch (Exception $ex) {
|
||||
$states = array();
|
||||
}
|
||||
|
||||
foreach ($states as $state) {
|
||||
if (!isset($forbidden[$state])) {
|
||||
continue;
|
||||
}
|
||||
$reason = $adapter->getForbiddenReason($state);
|
||||
break;
|
||||
}
|
||||
|
||||
$this->forbiddenActions[$action_key] = $reason;
|
||||
}
|
||||
|
||||
$forbidden_reason = $this->forbiddenActions[$action_key];
|
||||
if ($forbidden_reason !== null) {
|
||||
$this->newApplyTranscript($adapter, $rule, $action_record)
|
||||
->setAppliedReason(
|
||||
array(
|
||||
array(
|
||||
'type' => HeraldAction::DO_STANDARD_FORBIDDEN,
|
||||
'data' => $forbidden_reason,
|
||||
),
|
||||
));
|
||||
|
||||
$is_forbidden = true;
|
||||
}
|
||||
}
|
||||
|
||||
return $is_forbidden;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -20,6 +20,10 @@ abstract class HeraldField extends Phobject {
|
|||
return null;
|
||||
}
|
||||
|
||||
public function getRequiredAdapterStates() {
|
||||
return array();
|
||||
}
|
||||
|
||||
protected function getHeraldFieldStandardType() {
|
||||
throw new PhutilMethodNotImplementedException();
|
||||
}
|
||||
|
|
7
src/applications/herald/state/HeraldBuildableState.php
Normal file
7
src/applications/herald/state/HeraldBuildableState.php
Normal file
|
@ -0,0 +1,7 @@
|
|||
<?php
|
||||
|
||||
final class HeraldBuildableState extends HeraldState {
|
||||
|
||||
const STATECONST = 'buildable';
|
||||
|
||||
}
|
7
src/applications/herald/state/HeraldMailableState.php
Normal file
7
src/applications/herald/state/HeraldMailableState.php
Normal file
|
@ -0,0 +1,7 @@
|
|||
<?php
|
||||
|
||||
final class HeraldMailableState extends HeraldState {
|
||||
|
||||
const STATECONST = 'mailable';
|
||||
|
||||
}
|
3
src/applications/herald/state/HeraldState.php
Normal file
3
src/applications/herald/state/HeraldState.php
Normal file
|
@ -0,0 +1,3 @@
|
|||
<?php
|
||||
|
||||
abstract class HeraldState extends Phobject {}
|
26
src/applications/herald/state/HeraldStateReasons.php
Normal file
26
src/applications/herald/state/HeraldStateReasons.php
Normal file
|
@ -0,0 +1,26 @@
|
|||
<?php
|
||||
|
||||
abstract class HeraldStateReasons extends Phobject {
|
||||
|
||||
abstract public function explainReason($reason);
|
||||
|
||||
final public static function getAllReasons() {
|
||||
return id(new PhutilClassMapQuery())
|
||||
->setAncestorClass(__CLASS__)
|
||||
->execute();
|
||||
}
|
||||
|
||||
final public static function getExplanation($reason) {
|
||||
$reasons = self::getAllReasons();
|
||||
|
||||
foreach ($reasons as $reason_implementation) {
|
||||
$explanation = $reason_implementation->explainReason($reason);
|
||||
if ($explanation !== null) {
|
||||
return $explanation;
|
||||
}
|
||||
}
|
||||
|
||||
return pht('Unknown reason ("%s").', $reason);
|
||||
}
|
||||
|
||||
}
|
|
@ -10,6 +10,8 @@ final class HeraldConditionTranscript extends Phobject {
|
|||
protected $note;
|
||||
protected $result;
|
||||
|
||||
const RESULT_FORBIDDEN = 'forbidden';
|
||||
|
||||
public function setRuleID($rule_id) {
|
||||
$this->ruleID = $rule_id;
|
||||
return $this;
|
||||
|
@ -72,4 +74,9 @@ final class HeraldConditionTranscript extends Phobject {
|
|||
public function getResult() {
|
||||
return $this->result;
|
||||
}
|
||||
|
||||
public function isForbidden() {
|
||||
return ($this->getResult() === self::RESULT_FORBIDDEN);
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -9,6 +9,12 @@ final class HeraldRuleTranscript extends Phobject {
|
|||
protected $ruleName;
|
||||
protected $ruleOwner;
|
||||
|
||||
const RESULT_FORBIDDEN = 'forbidden';
|
||||
|
||||
public function isForbidden() {
|
||||
return ($this->getResult() === self::RESULT_FORBIDDEN);
|
||||
}
|
||||
|
||||
public function setResult($result) {
|
||||
$this->result = $result;
|
||||
return $this;
|
||||
|
|
|
@ -6,6 +6,12 @@ abstract class PhabricatorMetaMTAEmailHeraldAction
|
|||
const DO_SEND = 'do.send';
|
||||
const DO_FORCE = 'do.force';
|
||||
|
||||
public function getRequiredAdapterStates() {
|
||||
return array(
|
||||
HeraldMailableState::STATECONST,
|
||||
);
|
||||
}
|
||||
|
||||
public function supportsObject($object) {
|
||||
// NOTE: This implementation lacks generality, but there's no great way to
|
||||
// figure out if something generates email right now.
|
||||
|
|
Loading…
Reference in a new issue