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 @@ -815,6 +815,7 @@ 'HeraldRulePHIDType' => 'applications/herald/phid/HeraldRulePHIDType.php', 'HeraldRuleQuery' => 'applications/herald/query/HeraldRuleQuery.php', 'HeraldRuleSearchEngine' => 'applications/herald/query/HeraldRuleSearchEngine.php', + 'HeraldRuleTestCase' => 'applications/herald/storage/__tests__/HeraldRuleTestCase.php', 'HeraldRuleTransaction' => 'applications/herald/storage/HeraldRuleTransaction.php', 'HeraldRuleTransactionComment' => 'applications/herald/storage/HeraldRuleTransactionComment.php', 'HeraldRuleTranscript' => 'applications/herald/storage/transcript/HeraldRuleTranscript.php', @@ -3712,6 +3713,7 @@ 'HeraldRulePHIDType' => 'PhabricatorPHIDType', 'HeraldRuleQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'HeraldRuleSearchEngine' => 'PhabricatorApplicationSearchEngine', + 'HeraldRuleTestCase' => 'PhabricatorTestCase', 'HeraldRuleTransaction' => 'PhabricatorApplicationTransaction', 'HeraldRuleTransactionComment' => 'PhabricatorApplicationTransactionComment', 'HeraldRuleViewController' => 'HeraldController', 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 @@ -49,6 +49,8 @@ assert_instances_of($rules, 'HeraldRule'); $t_start = microtime(true); + // Rules execute in a well-defined order: sort them into execution order. + $rules = msort($rules, 'getRuleExecutionOrderSortKey'); $rules = mpull($rules, null, 'getPHID'); $this->transcript = new HeraldTranscript(); diff --git a/src/applications/herald/storage/HeraldRule.php b/src/applications/herald/storage/HeraldRule.php --- a/src/applications/herald/storage/HeraldRule.php +++ b/src/applications/herald/storage/HeraldRule.php @@ -228,6 +228,42 @@ return $this->assertAttached($this->triggerObject); } + /** + * Get a sortable key for rule execution order. + * + * Rules execute in a well-defined order: personal rules first, then object + * rules, then global rules. Within each rule type, rules execute from lowest + * ID to highest ID. + * + * This ordering allows more powerful rules (like global rules) to override + * weaker rules (like personal rules) when multiple rules exist which try to + * affect the same field. Executing from low IDs to high IDs makes + * interactions easier to understand when adding new rules, because the newest + * rules always happen last. + * + * @return string A sortable key for this rule. + */ + public function getRuleExecutionOrderSortKey() { + + $rule_type = $this->getRuleType(); + + switch ($rule_type) { + case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL: + $type_order = 1; + break; + case HeraldRuleTypeConfig::RULE_TYPE_OBJECT: + $type_order = 2; + break; + case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL: + $type_order = 3; + break; + default: + throw new Exception(pht('Unknown rule type "%s"!', $rule_type)); + } + + return sprintf('~%d%010d', $type_order, $this->getID()); + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */ diff --git a/src/applications/herald/storage/__tests__/HeraldRuleTestCase.php b/src/applications/herald/storage/__tests__/HeraldRuleTestCase.php new file mode 100644 --- /dev/null +++ b/src/applications/herald/storage/__tests__/HeraldRuleTestCase.php @@ -0,0 +1,41 @@ + HeraldRuleTypeConfig::RULE_TYPE_GLOBAL, + 2 => HeraldRuleTypeConfig::RULE_TYPE_GLOBAL, + 3 => HeraldRuleTypeConfig::RULE_TYPE_OBJECT, + 4 => HeraldRuleTypeConfig::RULE_TYPE_PERSONAL, + 5 => HeraldRuleTypeConfig::RULE_TYPE_GLOBAL, + 6 => HeraldRuleTypeConfig::RULE_TYPE_PERSONAL, + ); + + foreach ($rules as $id => $type) { + $rules[$id] = id(new HeraldRule()) + ->setID($id) + ->setRuleType($type); + } + + shuffle($rules); + $rules = msort($rules, 'getRuleExecutionOrderSortKey'); + $this->assertEqual( + array( + // Personal + 4, + 6, + + // Object + 3, + + // Global + 1, + 2, + 5 + ), + array_values(mpull($rules, 'getID'))); + } + + +}