1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-30 01:10:58 +01:00

Add an "Author's projects" Herald field to Differential

Summary:
Ref T1279. This allows installs to implement two different flavors of project review. They can either implement this rule:

  When:
    [ ... ] [ ... ]
  Take Action:
    [ Add blockign reviewers ] [ Security ]

...which means "every revision matching X needs to be signed off by someone else on the Security team, //even if the author is on that team//". The alternative is to implement this rule:

  When:
    [ Author's projects ] [ do not include ] [ Security ]
    [ ... ] [ ... ]
  Take Action:
    [ Add blocking reviewers ] [ Security ]

...which means that people on the Security team don't need a separate signoff from someone else on the team.

I think this weaker version maps to some of what, e.g., Google does (you need to be reviewed by someone with "readability" in a language, but if you have it that's good enough), but I could imagine cases like "Security" wanting to prevent self-review from satisfying the requirement.

@zeeg, not sure which of these use cases is relevant here, but either one should work after this.

Test Plan: Created rules with this field, verified it populated properly in the transcript.

Reviewers: btrahan

Reviewed By: btrahan

CC: zeeg, aran

Maniphest Tasks: T1279

Differential Revision: https://secure.phabricator.com/D7238
This commit is contained in:
epriestley 2013-10-05 12:04:31 -07:00
parent c80a4f51c1
commit b7297a3278
3 changed files with 20 additions and 2 deletions

View file

@ -23,6 +23,7 @@ abstract class HeraldAdapter {
const FIELD_AFFECTED_PACKAGE_OWNER = 'affected-package-owner';
const FIELD_CONTENT_SOURCE = 'contentsource';
const FIELD_ALWAYS = 'always';
const FIELD_AUTHOR_PROJECTS = 'authorprojects';
const CONDITION_CONTAINS = 'contains';
const CONDITION_NOT_CONTAINS = '!contains';
@ -149,6 +150,7 @@ abstract class HeraldAdapter {
pht("Any affected package's owner"),
self::FIELD_CONTENT_SOURCE => pht('Content Source'),
self::FIELD_ALWAYS => pht('Always'),
self::FIELD_AUTHOR_PROJECTS => pht("Author's projects"),
);
}
@ -166,7 +168,7 @@ abstract class HeraldAdapter {
self::CONDITION_IS_NOT_ANY => pht('is not any of'),
self::CONDITION_INCLUDE_ALL => pht('include all of'),
self::CONDITION_INCLUDE_ANY => pht('include any of'),
self::CONDITION_INCLUDE_NONE => pht('include none of'),
self::CONDITION_INCLUDE_NONE => pht('do not include'),
self::CONDITION_IS_ME => pht('is myself'),
self::CONDITION_IS_NOT_ME => pht('is not myself'),
self::CONDITION_REGEXP => pht('matches regexp'),
@ -201,6 +203,7 @@ abstract class HeraldAdapter {
case self::FIELD_TAGS:
case self::FIELD_REVIEWERS:
case self::FIELD_CC:
case self::FIELD_AUTHOR_PROJECTS:
return array(
self::CONDITION_INCLUDE_ALL,
self::CONDITION_INCLUDE_ANY,
@ -588,6 +591,8 @@ abstract class HeraldAdapter {
return self::VALUE_TAG;
case self::FIELD_AFFECTED_PACKAGE:
return self::VALUE_OWNERS_PACKAGE;
case self::FIELD_AUTHOR_PROJECTS:
return self::VALUE_PROJECT;
default:
return self::VALUE_USER;
}

View file

@ -43,6 +43,7 @@ final class HeraldDifferentialRevisionAdapter extends HeraldAdapter {
self::FIELD_TITLE,
self::FIELD_BODY,
self::FIELD_AUTHOR,
self::FIELD_AUTHOR_PROJECTS,
self::FIELD_REVIEWERS,
self::FIELD_CC,
self::FIELD_REPOSITORY,
@ -287,6 +288,18 @@ final class HeraldDifferentialRevisionAdapter extends HeraldAdapter {
case self::FIELD_AUTHOR:
return $this->revision->getAuthorPHID();
break;
case self::FIELD_AUTHOR_PROJECTS:
$author_phid = $this->revision->getAuthorPHID();
if (!$author_phid) {
return array();
}
$projects = id(new PhabricatorProjectQuery())
->setViewer(PhabricatorUser::getOmnipotentUser())
->withMemberPHIDs(array($author_phid))
->execute();
return mpull($projects, 'getPHID');
case self::FIELD_DIFF_FILE:
return $this->loadAffectedPaths();
case self::FIELD_CC:

View file

@ -13,7 +13,7 @@ final class HeraldRule extends HeraldDAO
protected $repetitionPolicy;
protected $ruleType;
protected $configVersion = 12;
protected $configVersion = 13;
private $ruleApplied = self::ATTACHABLE; // phids for which this rule has been applied
private $validAuthor = self::ATTACHABLE;