1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-09-20 01:08:50 +02:00

Fix Herald exception when updating a diff that has carryover CCs

Summary:
This is pretty subtle and tricky, but some apply transcripts don't have a rule
ID because they're purely informational. We currently get an exception, which
prevnets diff updates.

jason/tuomas: don't update phabricator.fb.com until this lands :P

Test Plan:
Applied this patch live to secure.phabricator.com and was able to update D420.

Reviewed By: gc3
Reviewers: gc3, aran, jungejason, tuomaspelkonen
CC: aran, epriestley, gc3
Differential Revision: 421
This commit is contained in:
epriestley 2011-06-09 13:48:39 -07:00
parent 0ad2b526bc
commit 1e5fd3a386
2 changed files with 11 additions and 14 deletions

View file

@ -132,11 +132,14 @@ class HeraldEngine {
if (!$this->transcript->getDryRun()) {
// Mark all the rules that have had their effects applied as having been
// executed for the current object.
$ruleIDs = mpull($xscripts, 'getRuleID');
foreach ($ruleIDs as $ruleID) {
id(new HeraldRule())
->setID($ruleID)
->saveRuleApplied($object->getPHID());
$rule_ids = mpull($xscripts, 'getRuleID');
foreach ($rule_ids as $rule_id) {
if (!$rule_id) {
// Some apply transcripts are purely informational and not associated
// with a rule, e.g. carryover emails from earlier revisions.
continue;
}
HeraldRule::saveRuleApplied($rule_id, $object->getPHID());
}
}
}

View file

@ -85,18 +85,12 @@ class HeraldRule extends HeraldDAO {
$this->ruleApplied = array_fill_keys(ipull($applied, 'phid'), true);
}
public function saveRuleApplied($phid) {
if (!$this->getID()) {
throw new Exception("Save rule before saving children.");
}
public static function saveRuleApplied($rule_id, $phid) {
queryfx(
$this->establishConnection('w'),
id(new HeraldRule())->establishConnection('w'),
'INSERT IGNORE INTO %T (phid, ruleID) VALUES (%s, %d)',
self::TABLE_RULE_APPLIED, $phid, $this->getID()
self::TABLE_RULE_APPLIED, $phid, $rule_id
);
$this->setRuleApplied($phid);
}
public function loadConditions() {