mirror of
https://we.phorge.it/source/phorge.git
synced 2025-01-16 09:41:06 +01:00
Disable personal Herald rules from invalid owners
Summary: Since mailing list rules are now "global", don't run "personal" rules for disabled/invalid users. Test Plan: Added a personal rule that matches every revision for a test user. Created a revision, checked transcript, rule matched. Disabled user, updated revision, checked transcript, rule got auto-disabled. Reviewers: btrahan, jungejason, nh, xela Reviewed By: btrahan CC: aran, epriestley Differential Revision: https://secure.phabricator.com/D1517
This commit is contained in:
parent
5954ae84aa
commit
8b4c057c75
3 changed files with 36 additions and 5 deletions
|
@ -225,12 +225,10 @@ class HeraldEngine {
|
||||||
} else if (!$conditions) {
|
} else if (!$conditions) {
|
||||||
$reason = "Rule failed automatically because it has no conditions.";
|
$reason = "Rule failed automatically because it has no conditions.";
|
||||||
$result = false;
|
$result = false;
|
||||||
/* TOOD: Restore this in some form?
|
} else if ($rule->hasInvalidOwner()) {
|
||||||
} else if (!is_fb_employee($rule->getAuthorPHID())) {
|
$reason = "Rule failed automatically because its owner is invalid ".
|
||||||
$reason = "Rule failed automatically because its owner is not an ".
|
"or disabled.";
|
||||||
"active employee.";
|
|
||||||
$result = false;
|
$result = false;
|
||||||
*/
|
|
||||||
} else {
|
} else {
|
||||||
foreach ($conditions as $condition) {
|
foreach ($conditions as $condition) {
|
||||||
$match = $this->doesConditionMatch($rule, $condition, $object);
|
$match = $this->doesConditionMatch($rule, $condition, $object);
|
||||||
|
|
|
@ -31,6 +31,7 @@ class HeraldRule extends HeraldDAO {
|
||||||
protected $configVersion = 8;
|
protected $configVersion = 8;
|
||||||
|
|
||||||
private $ruleApplied = array(); // phids for which this rule has been applied
|
private $ruleApplied = array(); // phids for which this rule has been applied
|
||||||
|
private $invalidOwner = false;
|
||||||
|
|
||||||
public static function loadAllByContentTypeWithFullData(
|
public static function loadAllByContentTypeWithFullData(
|
||||||
$content_type,
|
$content_type,
|
||||||
|
@ -44,6 +45,8 @@ class HeraldRule extends HeraldDAO {
|
||||||
return array();
|
return array();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
self::flagDisabledUserRules($rules);
|
||||||
|
|
||||||
$rule_ids = mpull($rules, 'getID');
|
$rule_ids = mpull($rules, 'getID');
|
||||||
|
|
||||||
$conditions = id(new HeraldCondition())->loadAllWhere(
|
$conditions = id(new HeraldCondition())->loadAllWhere(
|
||||||
|
@ -75,6 +78,30 @@ class HeraldRule extends HeraldDAO {
|
||||||
return $rules;
|
return $rules;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private static function flagDisabledUserRules(array $rules) {
|
||||||
|
|
||||||
|
$users = array();
|
||||||
|
foreach ($rules as $rule) {
|
||||||
|
if ($rule->getRuleType() != HeraldRuleTypeConfig::RULE_TYPE_PERSONAL) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
$users[$rule->getAuthorPHID()] = true;
|
||||||
|
}
|
||||||
|
|
||||||
|
$handles = id(new PhabricatorObjectHandleData(array_keys($users)))
|
||||||
|
->loadHandles();
|
||||||
|
|
||||||
|
foreach ($rules as $key => $rule) {
|
||||||
|
if ($rule->getRuleType() != HeraldRuleTypeConfig::RULE_TYPE_PERSONAL) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
$handle = $handles[$rule->getAuthorPHID()];
|
||||||
|
if (!$handle->isComplete() || $handle->isDisabled()) {
|
||||||
|
$rule->invalidOwner = true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
public function getRuleApplied($phid) {
|
public function getRuleApplied($phid) {
|
||||||
if (idx($this->ruleApplied, $phid) === null) {
|
if (idx($this->ruleApplied, $phid) === null) {
|
||||||
throw new Exception("Call setRuleApplied() before getRuleApplied()!");
|
throw new Exception("Call setRuleApplied() before getRuleApplied()!");
|
||||||
|
@ -207,4 +234,8 @@ class HeraldRule extends HeraldDAO {
|
||||||
// $this->saveTransaction();
|
// $this->saveTransaction();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function hasInvalidOwner() {
|
||||||
|
return $this->invalidOwner;
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -6,10 +6,12 @@
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
phutil_require_module('phabricator', 'applications/herald/config/ruletype');
|
||||||
phutil_require_module('phabricator', 'applications/herald/storage/action');
|
phutil_require_module('phabricator', 'applications/herald/storage/action');
|
||||||
phutil_require_module('phabricator', 'applications/herald/storage/base');
|
phutil_require_module('phabricator', 'applications/herald/storage/base');
|
||||||
phutil_require_module('phabricator', 'applications/herald/storage/condition');
|
phutil_require_module('phabricator', 'applications/herald/storage/condition');
|
||||||
phutil_require_module('phabricator', 'applications/herald/storage/edithistory');
|
phutil_require_module('phabricator', 'applications/herald/storage/edithistory');
|
||||||
|
phutil_require_module('phabricator', 'applications/phid/handle/data');
|
||||||
phutil_require_module('phabricator', 'storage/queryfx');
|
phutil_require_module('phabricator', 'storage/queryfx');
|
||||||
|
|
||||||
phutil_require_module('phutil', 'utils');
|
phutil_require_module('phutil', 'utils');
|
||||||
|
|
Loading…
Reference in a new issue