Explicitly decline to add commit authors as auditors from Herald
Summary:
Fixes T12304. If you have a Herald rule which tries to add a commit author as an auditor, it fails validation when trying to apply.
Stop trying to apply these transactions, and explicitly tell the user why. Differential already uses a similar ruleset around reviewers, but Audit was using older code.
Test Plan:
- Wrote a Herald rule to add A, B and C as auditors.
- Committed as A.
- After change, saw B and C added with transacript guidance that A was the author.
{F3235660}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12304
Differential Revision: https://secure.phabricator.com/D17404
This commit is contained in:
@@ -3,6 +3,7 @@
|
|||||||
abstract class DiffusionAuditorsHeraldAction
|
abstract class DiffusionAuditorsHeraldAction
|
||||||
extends HeraldAction {
|
extends HeraldAction {
|
||||||
|
|
||||||
|
const DO_AUTHORS = 'do.authors';
|
||||||
const DO_ADD_AUDITORS = 'do.add-auditors';
|
const DO_ADD_AUDITORS = 'do.add-auditors';
|
||||||
|
|
||||||
public function getActionGroupKey() {
|
public function getActionGroupKey() {
|
||||||
@@ -19,6 +20,22 @@ abstract class DiffusionAuditorsHeraldAction
|
|||||||
|
|
||||||
$auditors = $object->getAudits();
|
$auditors = $object->getAudits();
|
||||||
|
|
||||||
|
// Don't try to add commit authors as auditors.
|
||||||
|
$authors = array();
|
||||||
|
foreach ($phids as $key => $phid) {
|
||||||
|
if ($phid == $object->getAuthorPHID()) {
|
||||||
|
$authors[] = $phid;
|
||||||
|
unset($phids[$key]);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if ($authors) {
|
||||||
|
$this->logEffect(self::DO_AUTHORS, $authors);
|
||||||
|
if (!$phids) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
$current = array();
|
$current = array();
|
||||||
foreach ($auditors as $auditor) {
|
foreach ($auditors as $auditor) {
|
||||||
if ($auditor->isInteresting()) {
|
if ($auditor->isInteresting()) {
|
||||||
@@ -53,6 +70,11 @@ abstract class DiffusionAuditorsHeraldAction
|
|||||||
|
|
||||||
protected function getActionEffectMap() {
|
protected function getActionEffectMap() {
|
||||||
return array(
|
return array(
|
||||||
|
self::DO_AUTHORS => array(
|
||||||
|
'icon' => 'fa-user',
|
||||||
|
'color' => 'grey',
|
||||||
|
'name' => pht('Commit Author'),
|
||||||
|
),
|
||||||
self::DO_ADD_AUDITORS => array(
|
self::DO_ADD_AUDITORS => array(
|
||||||
'icon' => 'fa-user',
|
'icon' => 'fa-user',
|
||||||
'color' => 'green',
|
'color' => 'green',
|
||||||
@@ -63,6 +85,10 @@ abstract class DiffusionAuditorsHeraldAction
|
|||||||
|
|
||||||
protected function renderActionEffectDescription($type, $data) {
|
protected function renderActionEffectDescription($type, $data) {
|
||||||
switch ($type) {
|
switch ($type) {
|
||||||
|
case self::DO_AUTHORS:
|
||||||
|
return pht(
|
||||||
|
'Declined to add commit author as auditor: %s.',
|
||||||
|
$this->renderHandleList($data));
|
||||||
case self::DO_ADD_AUDITORS:
|
case self::DO_ADD_AUDITORS:
|
||||||
return pht(
|
return pht(
|
||||||
'Added %s auditor(s): %s.',
|
'Added %s auditor(s): %s.',
|
||||||
|
|||||||
Reference in New Issue
Block a user