diff --git a/resources/sql/autopatches/20140210.herald.rule-condition-mig.php b/resources/sql/autopatches/20140210.herald.rule-condition-mig.php new file mode 100644 index 0000000000..c653fa9d50 --- /dev/null +++ b/resources/sql/autopatches/20140210.herald.rule-condition-mig.php @@ -0,0 +1,31 @@ +establishConnection('w'); + +echo "Migrating Herald conditions of type Herald rule from IDs to PHIDs...\n"; +foreach (new LiskMigrationIterator($table) as $condition) { + if ($condition->getFieldName() != HeraldAdapter::FIELD_RULE) { + continue; + } + + $value = $condition->getValue(); + if (!is_numeric($value)) { + continue; + } + $id = $condition->getID(); + echo "Updating condition {$id}...\n"; + + $rule = id(new HeraldRuleQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withIDs(array($value)) + ->executeOne(); + + queryfx( + $conn_w, + 'UPDATE %T SET value = %s WHERE id = %d', + $table->getTableName(), + json_encode($rule->getPHID()), + $id); +} +echo "Done.\n"; diff --git a/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php b/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php index a7ea3844ff..0427f6587b 100644 --- a/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php +++ b/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php @@ -44,7 +44,6 @@ final class HeraldPreCommitContentAdapter extends HeraldPreCommitAdapter { self::FIELD_DIFFERENTIAL_REVIEWERS, self::FIELD_DIFFERENTIAL_CCS, self::FIELD_IS_MERGE_COMMIT, - self::FIELD_RULE, ), parent::getFields()); } diff --git a/src/applications/diffusion/herald/HeraldPreCommitRefAdapter.php b/src/applications/diffusion/herald/HeraldPreCommitRefAdapter.php index 75ae07a5e9..be7b42e648 100644 --- a/src/applications/diffusion/herald/HeraldPreCommitRefAdapter.php +++ b/src/applications/diffusion/herald/HeraldPreCommitRefAdapter.php @@ -41,7 +41,6 @@ final class HeraldPreCommitRefAdapter extends HeraldPreCommitAdapter { self::FIELD_REPOSITORY_PROJECTS, self::FIELD_PUSHER, self::FIELD_PUSHER_PROJECTS, - self::FIELD_RULE, ), parent::getFields()); } diff --git a/src/applications/herald/controller/HeraldRuleController.php b/src/applications/herald/controller/HeraldRuleController.php index 570fffe5de..f4a6d214cf 100644 --- a/src/applications/herald/controller/HeraldRuleController.php +++ b/src/applications/herald/controller/HeraldRuleController.php @@ -414,7 +414,7 @@ final class HeraldRuleController extends HeraldController { } $all_rules = $this->loadRulesThisRuleMayDependUpon($rule); - $all_rules = mpull($all_rules, 'getName', 'getID'); + $all_rules = mpull($all_rules, 'getName', 'getPHID'); asort($all_rules); $all_fields = $adapter->getFieldNameMap(); @@ -633,6 +633,15 @@ final class HeraldRuleController extends HeraldController { ->execute(); } + // mark disabled rules as disabled since they are not useful as such; + // don't filter though to keep edit cases sane / expected + foreach ($all_rules as $current_rule) { + if ($current_rule->getIsDisabled()) { + $current_rule->makeEphemeral(); + $current_rule->setName($rule->getName(). ' '.pht('(Disabled)')); + } + } + // A rule can not depend upon itself. unset($all_rules[$rule->getID()]); diff --git a/src/applications/herald/engine/HeraldEngine.php b/src/applications/herald/engine/HeraldEngine.php index f2915d703e..acc9556c97 100644 --- a/src/applications/herald/engine/HeraldEngine.php +++ b/src/applications/herald/engine/HeraldEngine.php @@ -20,8 +20,8 @@ final class HeraldEngine { return $this->dryRun; } - public function getRule($id) { - return idx($this->rules, $id); + public function getRule($phid) { + return idx($this->rules, $phid); } public function loadRulesForAdapter(HeraldAdapter $adapter) { @@ -49,7 +49,7 @@ final class HeraldEngine { assert_instances_of($rules, 'HeraldRule'); $t_start = microtime(true); - $rules = mpull($rules, null, 'getID'); + $rules = mpull($rules, null, 'getPHID'); $this->transcript = new HeraldTranscript(); $this->transcript->setObjectPHID((string)$object->getPHID()); @@ -59,7 +59,7 @@ final class HeraldEngine { $this->object = $object; $effects = array(); - foreach ($rules as $id => $rule) { + foreach ($rules as $phid => $rule) { $this->stack = array(); try { if (!$this->getDryRun() && @@ -70,7 +70,7 @@ final class HeraldEngine { // applied a single time, and it's already been applied... // That means automatic failure. $xscript = id(new HeraldRuleTranscript()) - ->setRuleID($id) + ->setRuleID($rule->getID()) ->setResult(false) ->setRuleName($rule->getName()) ->setRuleOwner($rule->getAuthorPHID()) @@ -102,7 +102,7 @@ final class HeraldEngine { } $rule_matches = false; } - $this->results[$id] = $rule_matches; + $this->results[$phid] = $rule_matches; if ($rule_matches) { foreach ($this->getRuleEffects($rule, $object) as $effect) { @@ -210,25 +210,25 @@ final class HeraldEngine { HeraldRule $rule, HeraldAdapter $object) { - $id = $rule->getID(); + $phid = $rule->getPHID(); - if (isset($this->results[$id])) { + if (isset($this->results[$phid])) { // If we've already evaluated this rule because another rule depends // on it, we don't need to reevaluate it. - return $this->results[$id]; + return $this->results[$phid]; } - if (isset($this->stack[$id])) { + if (isset($this->stack[$phid])) { // 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_id => $ignored) { - $this->results[$rule_id] = false; + foreach ($this->stack as $rule_phid => $ignored) { + $this->results[$rule_phid] = false; } throw new HeraldRecursiveConditionsException(); } - $this->stack[$id] = true; + $this->stack[$phid] = true; $all = $rule->getMustMatchAll();