Page MenuHomePhabricator
Files F17732064

D21565.diff
No OneTemporary
Actions

D21565.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -1621,6 +1621,7 @@
'HeraldRuleDisableTransaction' => 'applications/herald/xaction/HeraldRuleDisableTransaction.php',
'HeraldRuleEditTransaction' => 'applications/herald/xaction/HeraldRuleEditTransaction.php',
'HeraldRuleEditor' => 'applications/herald/editor/HeraldRuleEditor.php',
+ 'HeraldRuleEvaluationException' => 'applications/herald/engine/exception/HeraldRuleEvaluationException.php',
'HeraldRuleField' => 'applications/herald/field/rule/HeraldRuleField.php',
'HeraldRuleFieldGroup' => 'applications/herald/field/rule/HeraldRuleFieldGroup.php',
'HeraldRuleIndexEngineExtension' => 'applications/herald/engineextension/HeraldRuleIndexEngineExtension.php',
@@ -1631,6 +1632,7 @@
'HeraldRulePHIDType' => 'applications/herald/phid/HeraldRulePHIDType.php',
'HeraldRuleQuery' => 'applications/herald/query/HeraldRuleQuery.php',
'HeraldRuleReplyHandler' => 'applications/herald/mail/HeraldRuleReplyHandler.php',
+ 'HeraldRuleResult' => 'applications/herald/storage/transcript/HeraldRuleResult.php',
'HeraldRuleSearchEngine' => 'applications/herald/query/HeraldRuleSearchEngine.php',
'HeraldRuleSerializer' => 'applications/herald/editor/HeraldRuleSerializer.php',
'HeraldRuleTestCase' => 'applications/herald/storage/__tests__/HeraldRuleTestCase.php',
@@ -7861,6 +7863,7 @@
'HeraldRuleDisableTransaction' => 'HeraldRuleTransactionType',
'HeraldRuleEditTransaction' => 'HeraldRuleTransactionType',
'HeraldRuleEditor' => 'PhabricatorApplicationTransactionEditor',
+ 'HeraldRuleEvaluationException' => 'Exception',
'HeraldRuleField' => 'HeraldField',
'HeraldRuleFieldGroup' => 'HeraldFieldGroup',
'HeraldRuleIndexEngineExtension' => 'PhabricatorEdgeIndexEngineExtension',
@@ -7871,6 +7874,7 @@
'HeraldRulePHIDType' => 'PhabricatorPHIDType',
'HeraldRuleQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'HeraldRuleReplyHandler' => 'PhabricatorApplicationTransactionReplyHandler',
+ 'HeraldRuleResult' => 'HeraldTranscriptResult',
'HeraldRuleSearchEngine' => 'PhabricatorApplicationSearchEngine',
'HeraldRuleSerializer' => 'Phobject',
'HeraldRuleTestCase' => 'PhabricatorTestCase',
diff --git a/src/applications/herald/controller/HeraldTranscriptController.php b/src/applications/herald/controller/HeraldTranscriptController.php
--- a/src/applications/herald/controller/HeraldTranscriptController.php
+++ b/src/applications/herald/controller/HeraldTranscriptController.php
@@ -224,6 +224,7 @@
}
private function buildActionTranscriptPanel(HeraldTranscript $xscript) {
+ $viewer = $this->getViewer();
$action_xscript = mgroup($xscript->getApplyTranscripts(), 'getRuleID');
$adapter = $this->getAdapter();
@@ -253,7 +254,9 @@
->setHeader($rule_xscript->getRuleName())
->setHref($rule_uri);
- if (!$rule_xscript->getResult()) {
+ $rule_result = $rule_xscript->getRuleResult();
+
+ if (!$rule_result->getShouldApplyActions()) {
$rule_item->setDisabled(true);
}
@@ -275,7 +278,7 @@
$color = $result->getIconColor();
$name = $result->getName();
- $result_details = $result->newDetailsView();
+ $result_details = $result->newDetailsView($viewer);
if ($result_details !== null) {
$result_details = phutil_tag(
'div',
@@ -301,27 +304,27 @@
$cond_list->addItem($cond_item);
}
- 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');
- $last_note = pht('Rule passed.');
- } else {
- $last_icon = 'fa-times-circle';
- $last_color = 'red';
- $last_result = pht('Failed');
- $last_note = pht('Rule failed.');
+ $rule_result = $rule_xscript->getRuleResult();
+
+ $last_icon = $rule_result->getIconIcon();
+ $last_color = $rule_result->getIconColor();
+ $last_result = $rule_result->getName();
+ $last_note = $rule_result->getDescription();
+
+ $last_details = $rule_result->newDetailsView($viewer);
+ if ($last_details !== null) {
+ $last_details = phutil_tag(
+ 'div',
+ array(
+ 'class' => 'herald-condition-note',
+ ),
+ $last_details);
}
$cond_last = id(new PHUIStatusItemView())
->setIcon($last_icon, $last_color)
->setTarget(phutil_tag('strong', array(), $last_result))
- ->setNote($last_note);
+ ->setNote(array($last_note, $last_details));
$cond_list->addItem($cond_last);
$cond_box = id(new PHUIBoxView())
@@ -330,11 +333,10 @@
$rule_item->appendChild($cond_box);
- if (!$rule_xscript->getResult()) {
- // If the rule didn't pass, don't generate an action transcript since
- // actions didn't apply.
- continue;
- }
+ // Not all rules will have any action transcripts, but we show them
+ // in general because they may have relevant information even when
+ // rules did not take actions. In particular, state-based actions may
+ // forbid rules from matching.
$cond_box->addMargin(PHUI::MARGIN_MEDIUM_BOTTOM);
diff --git a/src/applications/herald/engine/HeraldEngine.php b/src/applications/herald/engine/HeraldEngine.php
--- a/src/applications/herald/engine/HeraldEngine.php
+++ b/src/applications/herald/engine/HeraldEngine.php
@@ -3,8 +3,6 @@
final class HeraldEngine extends Phobject {
protected $rules = array();
- protected $results = array();
- protected $stack = array();
protected $activeRule;
protected $transcript;
@@ -20,6 +18,9 @@
private $profilerStack = array();
private $profilerFrames = array();
+ private $ruleResults;
+ private $ruleStack;
+
public function setDryRun($dry_run) {
$this->dryRun = $dry_run;
return $this;
@@ -54,6 +55,74 @@
return $engine->getTranscript();
}
+/* -( Rule Stack )--------------------------------------------------------- */
+
+ private function resetRuleStack() {
+ $this->ruleStack = array();
+ return $this;
+ }
+
+ private function hasRuleOnStack(HeraldRule $rule) {
+ $phid = $rule->getPHID();
+ return isset($this->ruleStack[$phid]);
+ }
+
+ private function pushRuleStack(HeraldRule $rule) {
+ $phid = $rule->getPHID();
+ $this->ruleStack[$phid] = $rule;
+ return $this;
+ }
+
+ private function getRuleStack() {
+ return array_values($this->ruleStack);
+ }
+
+/* -( Rule Results )------------------------------------------------------- */
+
+ private function resetRuleResults() {
+ $this->ruleResults = array();
+ return $this;
+ }
+
+ private function setRuleResult(
+ HeraldRule $rule,
+ HeraldRuleResult $result) {
+
+ $phid = $rule->getPHID();
+
+ if ($this->hasRuleResult($rule)) {
+ throw new Exception(
+ pht(
+ 'Herald rule "%s" already has an evaluation result.',
+ $phid));
+ }
+
+ $this->ruleResults[$phid] = $result;
+
+ $this->newRuleTranscript($rule)
+ ->setRuleResult($result);
+
+ return $this;
+ }
+
+ private function hasRuleResult(HeraldRule $rule) {
+ $phid = $rule->getPHID();
+ return isset($this->ruleResults[$phid]);
+ }
+
+ private function getRuleResult(HeraldRule $rule) {
+ $phid = $rule->getPHID();
+
+ if (!$this->hasRuleResult($rule)) {
+ throw new Exception(
+ pht(
+ 'Herald rule "%s" does not have an evaluation result.',
+ $phid));
+ }
+
+ return $this->ruleResults[$phid];
+ }
+
public function applyRules(array $rules, HeraldAdapter $object) {
assert_instances_of($rules, 'HeraldRule');
$t_start = microtime(true);
@@ -66,62 +135,70 @@
$this->transcript->setObjectPHID((string)$object->getPHID());
$this->fieldCache = array();
$this->fieldExceptions = array();
- $this->results = array();
$this->rules = $rules;
$this->object = $object;
+ $this->resetRuleResults();
+
$effects = array();
foreach ($rules as $phid => $rule) {
- $this->stack = array();
-
- $is_first_only = $rule->isRepeatFirst();
+ $this->resetRuleStack();
$caught = null;
+ $result = null;
try {
+ $is_first_only = $rule->isRepeatFirst();
+
if (!$this->getDryRun() &&
$is_first_only &&
$rule->getRuleApplied($object->getPHID())) {
+
// This is not a dry run, and this rule is only supposed to be
- // applied a single time, and it's already been applied...
+ // applied a single time, and it has already been applied.
// That means automatic failure.
- $this->newRuleTranscript($rule)
- ->setResult(false)
- ->setReason(
- pht(
- 'This rule is only supposed to be repeated a single time, '.
- 'and it has already been applied.'));
-
- $rule_matches = false;
+
+ $result_code = HeraldRuleResult::RESULT_ALREADY_APPLIED;
+ $result = HeraldRuleResult::newFromResultCode($result_code);
+ } else if ($this->isForbidden($rule, $object)) {
+ $result_code = HeraldRuleResult::RESULT_OBJECT_STATE;
+ $result = HeraldRuleResult::newFromResultCode($result_code);
} else {
- 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);
- }
+ $result = $this->getRuleMatchResult($rule, $object);
}
} catch (HeraldRecursiveConditionsException $ex) {
- $names = array();
- foreach ($this->stack as $rule_phid => $ignored) {
- $names[] = '"'.$rules[$rule_phid]->getName().'"';
+ $cycle_phids = array();
+
+ $stack = $this->getRuleStack();
+ foreach ($stack as $stack_rule) {
+ $cycle_phids[] = $stack_rule->getPHID();
}
- $names = implode(', ', $names);
- 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));
+ // Add the rule which actually cycled to the list to make the
+ // result more clear when we show it to the user.
+ $cycle_phids[] = $phid;
+
+ foreach ($stack as $stack_rule) {
+ if ($this->hasRuleResult($stack_rule)) {
+ continue;
+ }
+
+ $result_code = HeraldRuleResult::RESULT_RECURSION;
+ $result_data = array(
+ 'cyclePHIDs' => $cycle_phids,
+ );
+
+ $result = HeraldRuleResult::newFromResultCode($result_code)
+ ->setResultData($result_data);
+ $this->setRuleResult($stack_rule, $result);
}
- $rule_matches = false;
+
+ $result = $this->getRuleResult($rule);
+ } catch (HeraldRuleEvaluationException $ex) {
+ // When we encounter an evaluation exception, the condition which
+ // failed to evaluate is responsible for logging the details of the
+ // error.
+
+ $result_code = HeraldRuleResult::RESULT_EVALUATION_EXCEPTION;
+ $result = HeraldRuleResult::newFromResultCode($result_code);
} catch (Exception $ex) {
$caught = $ex;
} catch (Throwable $ex) {
@@ -129,17 +206,26 @@
}
if ($caught) {
- $this->newRuleTranscript($rules[$phid])
- ->setResult(false)
- ->setReason(
- pht(
- 'Rule encountered an exception while evaluting.'));
- $rule_matches = false;
+ // These exceptions are unexpected, and did not arise during rule
+ // evaluation, so we're responsible for handling the details.
+
+ $result_code = HeraldRuleResult::RESULT_EXCEPTION;
+
+ $result_data = array(
+ 'exception.class' => get_class($caught),
+ 'exception.message' => $ex->getMessage(),
+ );
+
+ $result = HeraldRuleResult::newFromResultCode($result_code)
+ ->setResultData($result_data);
}
- $this->results[$phid] = $rule_matches;
+ if (!$this->hasRuleResult($rule)) {
+ $this->setRuleResult($rule, $result);
+ }
+ $result = $this->getRuleResult($rule);
- if ($rule_matches) {
+ if ($result->getShouldApplyActions()) {
foreach ($this->getRuleEffects($rule, $object) as $effect) {
$effects[] = $effect;
}
@@ -286,58 +372,46 @@
public function doesRuleMatch(
HeraldRule $rule,
HeraldAdapter $object) {
+ $result = $this->getRuleMatchResult($rule, $object);
+ return $result->getShouldApplyActions();
+ }
- $phid = $rule->getPHID();
+ private function getRuleMatchResult(
+ HeraldRule $rule,
+ HeraldAdapter $object) {
- if (isset($this->results[$phid])) {
+ if ($this->hasRuleResult($rule)) {
// If we've already evaluated this rule because another rule depends
// on it, we don't need to reevaluate it.
- return $this->results[$phid];
+ return $this->getRuleResult($rule);
}
- if (isset($this->stack[$phid])) {
+ if ($this->hasRuleOnStack($rule)) {
// We've recursed, fail all of the rules on the stack. This happens when
// there's a dependency cycle with "Rule conditions match for rule ..."
// conditions.
- foreach ($this->stack as $rule_phid => $ignored) {
- $this->results[$rule_phid] = false;
- }
throw new HeraldRecursiveConditionsException();
}
-
- $this->stack[$phid] = true;
+ $this->pushRuleStack($rule);
$all = $rule->getMustMatchAll();
$conditions = $rule->getConditions();
- $result = null;
+ $result_code = null;
+ $result_data = array();
$local_version = id(new HeraldRule())->getConfigVersion();
if ($rule->getConfigVersion() > $local_version) {
- $reason = pht(
- 'Rule could not be processed, it was created with a newer version '.
- 'of Herald.');
- $result = false;
+ $result_code = HeraldRuleResult::RESULT_VERSION;
} else if (!$conditions) {
- $reason = pht(
- 'Rule failed automatically because it has no conditions.');
- $result = false;
+ $result_code = HeraldRuleResult::RESULT_EMPTY;
} else if (!$rule->hasValidAuthor()) {
- $reason = pht(
- 'Rule failed automatically because its owner is invalid '.
- 'or disabled.');
- $result = false;
+ $result_code = HeraldRuleResult::RESULT_OWNER;
} else if (!$this->canAuthorViewObject($rule, $object)) {
- $reason = pht(
- 'Rule failed automatically because it is a personal rule and its '.
- 'owner can not see the object.');
- $result = false;
+ $result_code = HeraldRuleResult::RESULT_VIEW_POLICY;
} else if (!$this->canRuleApplyToObject($rule, $object)) {
- $reason = pht(
- 'Rule failed automatically because it is an object rule which is '.
- 'not relevant for this object.');
- $result = false;
+ $result_code = HeraldRuleResult::RESULT_OBJECT_RULE;
} else {
foreach ($conditions as $condition) {
$caught = null;
@@ -347,6 +421,10 @@
$rule,
$condition,
$object);
+ } catch (HeraldRuleEvaluationException $ex) {
+ throw $ex;
+ } catch (HeraldRecursiveConditionsException $ex) {
+ throw $ex;
} catch (Exception $ex) {
$caught = $ex;
} catch (Throwable $ex) {
@@ -354,60 +432,59 @@
}
if ($caught) {
- throw $ex;
+ throw new HeraldRuleEvaluationException();
}
if (!$all && $match) {
- $reason = pht('Any condition matched.');
- $result = true;
+ $result_code = HeraldRuleResult::RESULT_ANY_MATCHED;
break;
}
if ($all && !$match) {
- $reason = pht('Not all conditions matched.');
- $result = false;
+ $result_code = HeraldRuleResult::RESULT_ANY_FAILED;
break;
}
}
- if ($result === null) {
+ if ($result_code === null) {
if ($all) {
- $reason = pht('All conditions matched.');
- $result = true;
+ $result_code = HeraldRuleResult::RESULT_ALL_MATCHED;
} else {
- $reason = pht('No conditions matched.');
- $result = false;
+ $result_code = HeraldRuleResult::RESULT_ALL_FAILED;
}
}
}
// If this rule matched, and is set to run "if it did not match the last
- // time", and we matched the last time, we're going to return a match in
- // the transcript but set a flag so we don't actually apply any effects.
+ // time", and we matched the last time, we're going to return a special
+ // result code which records a match but doesn't actually apply effects.
// We need the rule to match so that storage gets updated properly. If we
// just pretend the rule didn't match it won't cause any effects (which
// is correct), but it also won't set the "it matched" flag in storage,
// so the next run after this one would incorrectly trigger again.
+ $result = HeraldRuleResult::newFromResultCode($result_code)
+ ->setResultData($result_data);
+
+ $should_apply = $result->getShouldApplyActions();
+
$is_dry_run = $this->getDryRun();
- if ($result && !$is_dry_run) {
+ if ($should_apply && !$is_dry_run) {
$is_on_change = $rule->isRepeatOnChange();
if ($is_on_change) {
$did_apply = $rule->getRuleApplied($object->getPHID());
if ($did_apply) {
- $reason = pht(
- 'This rule matched, but did not take any actions because it '.
- 'is configured to act only if it did not match the last time.');
+ // Replace the result with our modified result.
+ $result_code = HeraldRuleResult::RESULT_LAST_MATCHED;
+ $result = HeraldRuleResult::newFromResultCode($result_code);
$this->skipEffects[$rule->getID()] = true;
}
}
}
- $this->newRuleTranscript($rule)
- ->setResult($result)
- ->setReason($reason);
+ $this->setRuleResult($rule, $result);
return $result;
}
@@ -439,6 +516,9 @@
} else {
$result_code = HeraldConditionResult::RESULT_FAILED;
}
+ } catch (HeraldRecursiveConditionsException $ex) {
+ $result_code = HeraldConditionResult::RESULT_RECURSION;
+ $caught = $ex;
} catch (HeraldInvalidConditionException $ex) {
$result_code = HeraldConditionResult::RESULT_INVALID;
$caught = $ex;
diff --git a/src/applications/herald/engine/exception/HeraldRuleEvaluationException.php b/src/applications/herald/engine/exception/HeraldRuleEvaluationException.php
new file mode 100644
--- /dev/null
+++ b/src/applications/herald/engine/exception/HeraldRuleEvaluationException.php
@@ -0,0 +1,3 @@
+<?php
+
+final class HeraldRuleEvaluationException extends Exception {}
diff --git a/src/applications/herald/storage/transcript/HeraldConditionResult.php b/src/applications/herald/storage/transcript/HeraldConditionResult.php
--- a/src/applications/herald/storage/transcript/HeraldConditionResult.php
+++ b/src/applications/herald/storage/transcript/HeraldConditionResult.php
@@ -7,6 +7,7 @@
const RESULT_FAILED = 'failed';
const RESULT_OBJECT_STATE = 'object-state';
const RESULT_INVALID = 'invalid';
+ const RESULT_RECURSION = 'recursion';
const RESULT_EXCEPTION = 'exception';
const RESULT_UNKNOWN = 'unknown';
@@ -22,7 +23,7 @@
return ($this->getSpecificationProperty('match') === true);
}
- public function newDetailsView() {
+ public function newDetailsView(PhabricatorUser $viewer) {
switch ($this->getResultCode()) {
case self::RESULT_OBJECT_STATE:
$reason = $this->getDataProperty('reason');
@@ -54,8 +55,6 @@
phutil_tag('strong', array(), $error_class),
phutil_escape_html_newlines($error_message));
break;
- $details = 'exception';
- break;
default:
$details = null;
break;
@@ -90,6 +89,12 @@
'color.icon' => 'yellow',
'name' => pht('Invalid'),
),
+ self::RESULT_RECURSION => array(
+ 'match' => null,
+ 'icon' => 'fa-exclamation-triangle',
+ 'color.icon' => 'red',
+ 'name' => pht('Recursion'),
+ ),
self::RESULT_EXCEPTION => array(
'match' => null,
'icon' => 'fa-exclamation-triangle',
diff --git a/src/applications/herald/storage/transcript/HeraldRuleResult.php b/src/applications/herald/storage/transcript/HeraldRuleResult.php
new file mode 100644
--- /dev/null
+++ b/src/applications/herald/storage/transcript/HeraldRuleResult.php
@@ -0,0 +1,238 @@
+<?php
+
+final class HeraldRuleResult
+ extends HeraldTranscriptResult {
+
+ const RESULT_ANY_MATCHED = 'any-match';
+ const RESULT_ALL_MATCHED = 'all-match';
+ const RESULT_ANY_FAILED = 'any-failed';
+ const RESULT_ALL_FAILED = 'all-failed';
+ const RESULT_LAST_MATCHED = 'last-match';
+ const RESULT_VERSION = 'version';
+ const RESULT_EMPTY = 'empty';
+ const RESULT_OWNER = 'owner';
+ const RESULT_VIEW_POLICY = 'view-policy';
+ const RESULT_OBJECT_RULE = 'object-rule';
+ const RESULT_EXCEPTION = 'exception';
+ const RESULT_EVALUATION_EXCEPTION = 'evaluation-exception';
+ const RESULT_UNKNOWN = 'unknown';
+ const RESULT_ALREADY_APPLIED = 'already-applied';
+ const RESULT_OBJECT_STATE = 'object-state';
+ const RESULT_RECURSION = 'recursion';
+
+ public static function newFromResultCode($result_code) {
+ return id(new self())->setResultCode($result_code);
+ }
+
+ public static function newFromResultMap(array $map) {
+ return id(new self())->loadFromResultMap($map);
+ }
+
+ public function getShouldRecordMatch() {
+ return ($this->getSpecificationProperty('match') === true);
+ }
+
+ public function getShouldApplyActions() {
+ return ($this->getSpecificationProperty('apply') === true);
+ }
+
+ public function getDescription() {
+ return $this->getSpecificationProperty('description');
+ }
+
+ public function newDetailsView(PhabricatorUser $viewer) {
+ switch ($this->getResultCode()) {
+ case self::RESULT_EXCEPTION:
+ $error_class = $this->getDataProperty('exception.class');
+ $error_message = $this->getDataProperty('exception.message');
+
+ if (!strlen($error_class)) {
+ $error_class = pht('Unknown Error');
+ }
+
+ if (!strlen($error_message)) {
+ $error_message = pht(
+ 'An unknown error occurred while evaluating this condition. No '.
+ 'additional information is available.');
+ }
+
+ $details = $this->newErrorView($error_class, $error_message);
+ break;
+ case self::RESULT_RECURSION:
+ $rule_phids = $this->getDataProperty('cyclePHIDs', array());
+ $handles = $viewer->loadHandles($rule_phids);
+
+ $links = array();
+ foreach ($rule_phids as $rule_phid) {
+ $links[] = $handles[$rule_phid]->renderLink();
+ }
+
+ $links = phutil_implode_html(' > ', $links);
+
+ $details = array(
+ pht('This rule has a dependency cycle and can not be evaluated:'),
+ ' ',
+ $links,
+ );
+ break;
+ default:
+ $details = null;
+ break;
+ }
+
+ return $details;
+ }
+
+ protected function newResultSpecificationMap() {
+ return array(
+ self::RESULT_ANY_MATCHED => array(
+ 'match' => true,
+ 'apply' => true,
+ 'name' => pht('Matched'),
+ 'description' => pht('Any condition matched.'),
+ 'icon' => 'fa-check-circle',
+ 'color.icon' => 'green',
+ ),
+ self::RESULT_ALL_MATCHED => array(
+ 'match' => true,
+ 'apply' => true,
+ 'name' => pht('Matched'),
+ 'description' => pht('All conditions matched.'),
+ 'icon' => 'fa-check-circle',
+ 'color.icon' => 'green',
+ ),
+ self::RESULT_ANY_FAILED => array(
+ 'match' => false,
+ 'apply' => false,
+ 'name' => pht('Failed'),
+ 'description' => pht('Not all conditions matched.'),
+ 'icon' => 'fa-times-circle',
+ 'color.icon' => 'red',
+ ),
+ self::RESULT_ALL_FAILED => array(
+ 'match' => false,
+ 'apply' => false,
+ 'name' => pht('Failed'),
+ 'description' => pht('No conditions matched.'),
+ 'icon' => 'fa-times-circle',
+ 'color.icon' => 'red',
+ ),
+ self::RESULT_LAST_MATCHED => array(
+ 'match' => true,
+ 'apply' => false,
+ 'name' => pht('Failed'),
+ 'description' => pht(
+ 'This rule matched, but did not take any actions because it '.
+ 'is configured to act only if it did not match the last time.'),
+ 'icon' => 'fa-times-circle',
+ 'color.icon' => 'red',
+ ),
+ self::RESULT_VERSION => array(
+ 'match' => null,
+ 'apply' => false,
+ 'name' => pht('Version Issue'),
+ 'description' => pht(
+ 'Rule could not be processed because it was created with a newer '.
+ 'version of Herald.'),
+ 'icon' => 'fa-times-circle',
+ 'color.icon' => 'red',
+ ),
+ self::RESULT_EMPTY => array(
+ 'match' => null,
+ 'apply' => false,
+ 'name' => pht('Empty'),
+ 'description' => pht(
+ 'Rule failed automatically because it has no conditions.'),
+ 'icon' => 'fa-times-circle',
+ 'color.icon' => 'red',
+ ),
+ self::RESULT_OWNER => array(
+ 'match' => null,
+ 'apply' => false,
+ 'name' => pht('Rule Owner'),
+ 'description' => pht(
+ 'Rule failed automatically because it is a personal rule and '.
+ 'its owner is invalid or disabled.'),
+ 'icon' => 'fa-times-circle',
+ 'color.icon' => 'red',
+ ),
+ self::RESULT_VIEW_POLICY => array(
+ 'match' => null,
+ 'apply' => false,
+ 'name' => pht('View Policy'),
+ 'description' => pht(
+ 'Rule failed automatically because it is a personal rule and '.
+ 'its owner does not have permission to view the object.'),
+ 'icon' => 'fa-times-circle',
+ 'color.icon' => 'red',
+ ),
+ self::RESULT_OBJECT_RULE => array(
+ 'match' => null,
+ 'apply' => false,
+ 'name' => pht('Object Rule'),
+ 'description' => pht(
+ 'Rule failed automatically because it is an object rule which is '.
+ 'not relevant for this object.'),
+ 'icon' => 'fa-times-circle',
+ 'color.icon' => 'red',
+ ),
+ self::RESULT_EXCEPTION => array(
+ 'match' => null,
+ 'apply' => false,
+ 'name' => pht('Exception'),
+ 'description' => pht(
+ 'Rule failed because an exception occurred.'),
+ 'icon' => 'fa-times-circle',
+ 'color.icon' => 'red',
+ ),
+ self::RESULT_EVALUATION_EXCEPTION => array(
+ 'match' => null,
+ 'apply' => false,
+ 'name' => pht('Exception'),
+ 'description' => pht(
+ 'Rule failed because an exception occurred while evaluating it.'),
+ 'icon' => 'fa-times-circle',
+ 'color.icon' => 'red',
+ ),
+ self::RESULT_UNKNOWN => array(
+ 'match' => null,
+ 'apply' => false,
+ 'name' => pht('Unknown'),
+ 'description' => pht(
+ 'Rule evaluation result is unknown.'),
+ 'icon' => 'fa-times-circle',
+ 'color.icon' => 'red',
+ ),
+ self::RESULT_ALREADY_APPLIED => array(
+ 'match' => null,
+ 'apply' => false,
+ 'name' => pht('Already Applied'),
+ 'description' => pht(
+ 'This rule is only supposed to be repeated a single time, '.
+ 'and it has already been applied.'),
+ 'icon' => 'fa-times-circle',
+ 'color.icon' => 'red',
+ ),
+ self::RESULT_OBJECT_STATE => array(
+ 'match' => null,
+ 'apply' => false,
+ 'name' => pht('Forbidden'),
+ 'description' => pht(
+ 'Object state prevented rule evaluation.'),
+ 'icon' => 'fa-ban',
+ 'color.icon' => 'indigo',
+ ),
+ self::RESULT_RECURSION => array(
+ 'match' => null,
+ 'apply' => false,
+ 'name' => pht('Recursion'),
+ 'description' => pht(
+ 'This rule has a recursive dependency on itself and can not '.
+ 'be evaluated.'),
+ 'icon' => 'fa-times-circle',
+ 'color.icon' => 'red',
+ ),
+ );
+ }
+
+}
diff --git a/src/applications/herald/storage/transcript/HeraldRuleTranscript.php b/src/applications/herald/storage/transcript/HeraldRuleTranscript.php
--- a/src/applications/herald/storage/transcript/HeraldRuleTranscript.php
+++ b/src/applications/herald/storage/transcript/HeraldRuleTranscript.php
@@ -3,35 +3,18 @@
final class HeraldRuleTranscript extends Phobject {
protected $ruleID;
- protected $result;
- protected $reason;
-
+ protected $ruleResultMap;
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;
- }
-
- public function getResult() {
- return $this->result;
- }
-
- public function setReason($reason) {
- $this->reason = $reason;
- return $this;
- }
+ // See T13586. This no longer has readers, but was written by older versions
+ // of Herald. It contained a human readable English-language description of
+ // the outcome of rule evaluation and was superseded by "HeraldRuleResult".
+ protected $reason;
- public function getReason() {
- return $this->reason;
- }
+ // See T13586. Older transcripts store a boolean "true", a boolean "false",
+ // or the string "forbidden" here.
+ protected $result;
public function setRuleID($rule_id) {
$this->ruleID = $rule_id;
@@ -59,4 +42,40 @@
public function getRuleOwner() {
return $this->ruleOwner;
}
+
+ public function setRuleResult(HeraldRuleResult $result) {
+ $this->ruleResultMap = $result->newResultMap();
+ return $this;
+ }
+
+ public function getRuleResult() {
+ $map = $this->ruleResultMap;
+
+ if (is_array($map)) {
+ $result = HeraldRuleResult::newFromResultMap($map);
+ } else {
+ $legacy_result = $this->result;
+
+ $result_data = array();
+
+ if ($legacy_result === 'forbidden') {
+ $result_code = HeraldRuleResult::RESULT_OBJECT_STATE;
+ $result_data = array(
+ 'reason' => $this->reason,
+ );
+ } else if ($legacy_result === true) {
+ $result_code = HeraldRuleResult::RESULT_ANY_MATCHED;
+ } else if ($legacy_result === false) {
+ $result_code = HeraldRuleResult::RESULT_ANY_FAILED;
+ } else {
+ $result_code = HeraldRuleResult::RESULT_UNKNOWN;
+ }
+
+ $result = HeraldRuleResult::newFromResultCode($result_code)
+ ->setResultData($result_data);
+ }
+
+ return $result;
+ }
+
}
diff --git a/src/applications/herald/storage/transcript/HeraldTranscriptResult.php b/src/applications/herald/storage/transcript/HeraldTranscriptResult.php
--- a/src/applications/herald/storage/transcript/HeraldTranscriptResult.php
+++ b/src/applications/herald/storage/transcript/HeraldTranscriptResult.php
@@ -47,9 +47,11 @@
return $this->getSpecificationProperty('name');
}
- final protected function getDataProperty($key) {
+ abstract public function newDetailsView(PhabricatorUser $viewer);
+
+ final protected function getDataProperty($key, $default = null) {
$data = $this->getResultData();
- return idx($data, $key);
+ return idx($data, $key, $default);
}
final public function newResultMap() {
@@ -79,4 +81,11 @@
abstract protected function newResultSpecificationMap();
+ final protected function newErrorView($error_class, $error_message) {
+ return pht(
+ '%s: %s',
+ phutil_tag('strong', array(), $error_class),
+ phutil_escape_html_newlines($error_message));
+ }
+
}

File Metadata

Mime Type
text/plain
Expires
Mon, Jul 21, 3:52 AM (5 d, 23 h ago)
Storage Engine
amazon-s3
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
phabricator/secure/bb/iv/l5xedhiyk5dwq4jb
Default Alt Text
D21565.diff (32 KB)

Event Timeline

Phabricator · Built by Phacility