Skip to content

Commit d5ded80

Browse files
committed
Herald - fix change type bug
Summary: wasn't working due to some type issues. Fixes T4756. I also made it display nicer while I was debugging this. Test Plan: created a herald rule to block changes that added refs. git tag -a "test" -m "test test"; git push origin test got me blocked! Reviewers: epriestley Reviewed By: epriestley Subscribers: epriestley, Korvin Maniphest Tasks: T4756 Differential Revision: https://secure.phabricator.com/D8724
1 parent 582ec54 commit d5ded80

File tree

3 files changed

+27
-12
lines changed

3 files changed

+27
-12
lines changed

src/applications/herald/adapter/HeraldAdapter.php

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -544,9 +544,9 @@ public function doesConditionMatch(
544544
}
545545
return $result;
546546
case self::CONDITION_HAS_BIT:
547-
return (($condition_value & $field_value) === $condition_value);
547+
return (($condition_value & $field_value) === (int) $condition_value);
548548
case self::CONDITION_NOT_BIT:
549-
return (($condition_value & $field_value) !== $condition_value);
549+
return (($condition_value & $field_value) !== (int) $condition_value);
550550
default:
551551
throw new HeraldInvalidConditionException(
552552
"Unknown condition '{$condition_type}'.");
@@ -1039,6 +1039,16 @@ private function renderConditionValueAsText(
10391039
}
10401040
}
10411041
break;
1042+
case HeraldPreCommitRefAdapter::FIELD_REF_CHANGE:
1043+
$change_map =
1044+
PhabricatorRepositoryPushLog::getHeraldChangeflagConditionOptions();
1045+
foreach ($value as $index => $val) {
1046+
$name = idx($change_map, $val);
1047+
if ($name) {
1048+
$value[$index] = $name;
1049+
}
1050+
}
1051+
break;
10421052
default:
10431053
foreach ($value as $index => $val) {
10441054
$handle = idx($handles, $val);

src/applications/herald/controller/HeraldRuleController.php

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,8 @@ private function setupEditorBehavior(
463463
$rule->getRuleType());
464464
}
465465

466+
$changeflag_options =
467+
PhabricatorRepositoryPushLog::getHeraldChangeflagConditionOptions();
466468
Javelin::initBehavior(
467469
'herald-rule-editor',
468470
array(
@@ -490,16 +492,7 @@ private function setupEditorBehavior(
490492
'default' => PhabricatorRepositoryPushLog::REFTYPE_BRANCH,
491493
),
492494
HeraldPreCommitRefAdapter::VALUE_REF_CHANGE => array(
493-
'options' => array(
494-
PhabricatorRepositoryPushLog::CHANGEFLAG_ADD =>
495-
pht('change creates ref'),
496-
PhabricatorRepositoryPushLog::CHANGEFLAG_DELETE =>
497-
pht('change deletes ref'),
498-
PhabricatorRepositoryPushLog::CHANGEFLAG_REWRITE =>
499-
pht('change rewrites ref'),
500-
PhabricatorRepositoryPushLog::CHANGEFLAG_DANGEROUS =>
501-
pht('dangerous change'),
502-
),
495+
'options' => $changeflag_options,
503496
'default' => PhabricatorRepositoryPushLog::CHANGEFLAG_ADD,
504497
),
505498
),

src/applications/repository/storage/PhabricatorRepositoryPushLog.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,18 @@ public static function initializeNewLog(PhabricatorUser $viewer) {
5252
->setPusherPHID($viewer->getPHID());
5353
}
5454

55+
public static function getHeraldChangeflagConditionOptions() {
56+
return array(
57+
PhabricatorRepositoryPushLog::CHANGEFLAG_ADD =>
58+
pht('change creates ref'),
59+
PhabricatorRepositoryPushLog::CHANGEFLAG_DELETE =>
60+
pht('change deletes ref'),
61+
PhabricatorRepositoryPushLog::CHANGEFLAG_REWRITE =>
62+
pht('change rewrites ref'),
63+
PhabricatorRepositoryPushLog::CHANGEFLAG_DANGEROUS =>
64+
pht('dangerous change'));
65+
}
66+
5567
public function getConfiguration() {
5668
return array(
5769
self::CONFIG_AUX_PHID => true,

0 commit comments

Comments
 (0)