diff --git a/src/applications/differential/editor/DifferentialDiffEditor.php b/src/applications/differential/editor/DifferentialDiffEditor.php --- a/src/applications/differential/editor/DifferentialDiffEditor.php +++ b/src/applications/differential/editor/DifferentialDiffEditor.php @@ -145,12 +145,7 @@ } if ($blocking_effect) { - $rule = idx($rules, $effect->getRuleID()); - if ($rule && strlen($rule->getName())) { - $rule_name = $rule->getName(); - } else { - $rule_name = pht('Unnamed Herald Rule'); - } + $rule = $blocking_effect->getRule(); $message = $effect->getTarget(); if (!strlen($message)) { @@ -164,8 +159,8 @@ "Creation of this diff was rejected by Herald rule %s.\n". " Rule: %s\n". "Reason: %s", - 'H'.$effect->getRuleID(), - $rule_name, + $rule->getMonogram(), + $rule->getName(), $message)); } break; diff --git a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php --- a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php +++ b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php @@ -337,22 +337,16 @@ } if ($blocking_effect) { + $rule = $blocking_effect->getRule(); + $this->rejectCode = PhabricatorRepositoryPushLog::REJECT_HERALD; - $this->rejectDetails = $blocking_effect->getRulePHID(); + $this->rejectDetails = $rule->getPHID(); $message = $blocking_effect->getTarget(); if (!strlen($message)) { $message = pht('(None.)'); } - $rules = mpull($rules, null, 'getID'); - $rule = idx($rules, $effect->getRuleID()); - if ($rule && strlen($rule->getName())) { - $rule_name = $rule->getName(); - } else { - $rule_name = pht('Unnamed Herald Rule'); - } - $blocked_ref_name = coalesce( $blocked_update->getRefName(), $blocked_update->getRefNewShort()); @@ -364,9 +358,9 @@ "Change: %s\n". " Rule: %s\n". "Reason: %s", - 'H'.$blocking_effect->getRuleID(), + $rule->getMonogram(), $blocked_name, - $rule_name, + $rule->getName(), $message)); } } diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php --- a/src/applications/herald/adapter/HeraldAdapter.php +++ b/src/applications/herald/adapter/HeraldAdapter.php @@ -999,11 +999,8 @@ public static function applyFlagEffect(HeraldEffect $effect, $phid) { $color = $effect->getTarget(); - // TODO: Silly that we need to load this again here. - $rule = id(new HeraldRule())->load($effect->getRuleID()); - $user = id(new PhabricatorUser())->loadOneWhere( - 'phid = %s', - $rule->getAuthorPHID()); + $rule = $effect->getRule(); + $user = $rule->getAuthor(); $flag = PhabricatorFlagQuery::loadUserFlag($user, $phid); if ($flag) { diff --git a/src/applications/herald/adapter/HeraldCommitAdapter.php b/src/applications/herald/adapter/HeraldCommitAdapter.php --- a/src/applications/herald/adapter/HeraldCommitAdapter.php +++ b/src/applications/herald/adapter/HeraldCommitAdapter.php @@ -501,7 +501,7 @@ if (empty($this->addCCPHIDs[$phid])) { $this->addCCPHIDs[$phid] = array(); } - $this->addCCPHIDs[$phid][] = $effect->getRuleID(); + $this->addCCPHIDs[$phid][] = $effect->getRule()->getID(); } $result[] = new HeraldApplyTranscript( $effect, @@ -513,7 +513,7 @@ if (empty($this->auditMap[$phid])) { $this->auditMap[$phid] = array(); } - $this->auditMap[$phid][] = $effect->getRuleID(); + $this->auditMap[$phid][] = $effect->getRule()->getID(); } $result[] = new HeraldApplyTranscript( $effect, diff --git a/src/applications/herald/engine/HeraldEffect.php b/src/applications/herald/engine/HeraldEffect.php --- a/src/applications/herald/engine/HeraldEffect.php +++ b/src/applications/herald/engine/HeraldEffect.php @@ -5,10 +5,7 @@ private $objectPHID; private $action; private $target; - - private $ruleID; - private $rulePHID; - + private $rule; private $reason; public function setObjectPHID($object_phid) { @@ -38,22 +35,13 @@ return $this->target; } - public function setRuleID($rule_id) { - $this->ruleID = $rule_id; - return $this; - } - - public function getRuleID() { - return $this->ruleID; - } - - public function setRulePHID($rule_phid) { - $this->rulePHID = $rule_phid; + public function setRule(HeraldRule $rule) { + $this->rule = $rule; return $this; } - public function getRulePHID() { - return $this->rulePHID; + public function getRule() { + return $this->rule; } public function setReason($reason) { 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 @@ -368,16 +368,14 @@ $effects = array(); foreach ($rule->getActions() as $action) { - $effect = new HeraldEffect(); - $effect->setObjectPHID($object->getPHID()); - $effect->setAction($action->getAction()); - $effect->setTarget($action->getTarget()); - - $effect->setRuleID($rule->getID()); - $effect->setRulePHID($rule->getPHID()); + $effect = id(new HeraldEffect()) + ->setObjectPHID($object->getPHID()) + ->setAction($action->getAction()) + ->setTarget($action->getTarget()) + ->setRule($rule); $name = $rule->getName(); - $id = $rule->getID(); + $id = $rule->getID(); $effect->setReason( pht( 'Conditions were met for %s', 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 @@ -265,6 +265,10 @@ return sprintf('~%d%010d', $type_order, $this->getID()); } + public function getMonogram() { + return 'H'.$this->getID(); + } + /* -( PhabricatorApplicationTransactionInterface )------------------------- */ diff --git a/src/applications/herald/storage/transcript/HeraldApplyTranscript.php b/src/applications/herald/storage/transcript/HeraldApplyTranscript.php --- a/src/applications/herald/storage/transcript/HeraldApplyTranscript.php +++ b/src/applications/herald/storage/transcript/HeraldApplyTranscript.php @@ -16,7 +16,7 @@ $this->setAction($effect->getAction()); $this->setTarget($effect->getTarget()); - $this->setRuleID($effect->getRuleID()); + $this->setRuleID($effect->getRule()->getID()); $this->setReason($effect->getReason()); $this->setApplied($applied); $this->setAppliedReason($reason);