diff --git a/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php b/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php index 0427f6587b..3c9199fc83 100644 --- a/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php +++ b/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php @@ -39,6 +39,7 @@ final class HeraldPreCommitContentAdapter extends HeraldPreCommitAdapter { self::FIELD_REPOSITORY_PROJECTS, self::FIELD_PUSHER, self::FIELD_PUSHER_PROJECTS, + self::FIELD_PUSHER_IS_COMMITTER, self::FIELD_DIFFERENTIAL_REVISION, self::FIELD_DIFFERENTIAL_ACCEPTED, self::FIELD_DIFFERENTIAL_REVIEWERS, @@ -116,6 +117,9 @@ final class HeraldPreCommitContentAdapter extends HeraldPreCommitAdapter { return $revision->getCCPHIDs(); case self::FIELD_IS_MERGE_COMMIT: return $this->getIsMergeCommit(); + case self::FIELD_PUSHER_IS_COMMITTER: + $pusher_phid = $this->getHookEngine()->getViewer()->getPHID(); + return ($this->getCommitterPHID() == $pusher_phid); } return parent::getHeraldField($field); @@ -202,18 +206,15 @@ final class HeraldPreCommitContentAdapter extends HeraldPreCommitAdapter { switch ($vcs) { case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL: - // Here, if there's no committer, we're going to return the author - // instead. + // If there's no committer information, we're going to return the + // author instead. However, if there's committer information and we + // can't resolve it, return `null`. $ref = $this->getCommitRef(); $committer = $ref->getCommitter(); if (!strlen($committer)) { return $this->getAuthorPHID(); } - $phid = $this->lookupUser($committer); - if (!$phid) { - return $this->getAuthorPHID(); - } - return $phid; + return $this->lookupUser($committer); case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: // In Subversion, the pusher is always the committer. return $this->getHookEngine()->getViewer()->getPHID(); diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php index 6f4a9cb145..c6551b3f15 100644 --- a/src/applications/herald/adapter/HeraldAdapter.php +++ b/src/applications/herald/adapter/HeraldAdapter.php @@ -41,6 +41,7 @@ abstract class HeraldAdapter { const FIELD_IS_NEW_OBJECT = 'new-object'; const FIELD_TASK_PRIORITY = 'taskpriority'; const FIELD_ARCANIST_PROJECT = 'arcanist-project'; + const FIELD_PUSHER_IS_COMMITTER = 'pusher-is-committer'; const CONDITION_CONTAINS = 'contains'; const CONDITION_NOT_CONTAINS = '!contains'; @@ -240,6 +241,7 @@ abstract class HeraldAdapter { self::FIELD_IS_NEW_OBJECT => pht('Is newly created?'), self::FIELD_TASK_PRIORITY => pht('Task priority'), self::FIELD_ARCANIST_PROJECT => pht('Arcanist Project'), + self::FIELD_PUSHER_IS_COMMITTER => pht('Pusher same as committer'), ); } @@ -287,8 +289,6 @@ abstract class HeraldAdapter { self::CONDITION_IS_NOT, self::CONDITION_REGEXP, ); - case self::FIELD_AUTHOR: - case self::FIELD_COMMITTER: case self::FIELD_REVIEWER: case self::FIELD_PUSHER: case self::FIELD_TASK_PRIORITY: @@ -299,6 +299,8 @@ abstract class HeraldAdapter { ); case self::FIELD_REPOSITORY: case self::FIELD_ASSIGNEE: + case self::FIELD_AUTHOR: + case self::FIELD_COMMITTER: return array( self::CONDITION_IS_ANY, self::CONDITION_IS_NOT_ANY, @@ -372,6 +374,7 @@ abstract class HeraldAdapter { case self::FIELD_IS_MERGE_COMMIT: case self::FIELD_DIFF_ENORMOUS: case self::FIELD_IS_NEW_OBJECT: + case self::FIELD_PUSHER_IS_COMMITTER: return array( self::CONDITION_IS_TRUE, self::CONDITION_IS_FALSE, diff --git a/src/applications/herald/storage/HeraldRule.php b/src/applications/herald/storage/HeraldRule.php index 749ebd943b..34ead08f62 100644 --- a/src/applications/herald/storage/HeraldRule.php +++ b/src/applications/herald/storage/HeraldRule.php @@ -17,7 +17,7 @@ final class HeraldRule extends HeraldDAO protected $isDisabled = 0; protected $triggerObjectPHID; - protected $configVersion = 32; + protected $configVersion = 33; // phids for which this rule has been applied private $ruleApplied = self::ATTACHABLE;