D21565.diff
No OneTemporary
Actions

Size

32 KB

Referenced Files

None

Subscribers

None

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: Sun, Mar 23, 6:58 AM (1 w, 3 d ago)
Storage Engine: blob
Storage Format: Encrypted (AES-256-CBC)
Storage Handle: 7225883
Default Alt Text: D21565.diff (32 KB)

D21565.diffNo OneTemporaryActions

D21565.diffView Options

File Metadata

Event Timeline

D21565.diff
No OneTemporary
Actions

D21565.diff
View Options