From ead5f4fd9c03f3e2a9939a190739d3e2e65f45e2 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 22 Dec 2017 10:11:10 -0800 Subject: [PATCH] Add an "Accepting reviewers" Herald field for commits Summary: See PHI262. Fixes T12578. Although this is a bit niche and probably better accomplished through advisory/soft measures ("Add blocking reviewers") in most cases, it isn't difficult to implement and doesn't create any technical or product tension. If installs write a rule that blocks commits, that will probably also naturally lead them to an "add reviewers" rule anyway. Also, allow packages to be hit with the typeahead. They're valid reviewers but previously you couldn't write rules against them, for no actual reason. Test Plan: Used test console to run this against commits, got sensible results for the field value. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T12578 Differential Revision: https://secure.phabricator.com/D18839 --- src/__phutil_library_map__.php | 4 ++ ...tRevisionAcceptingReviewersHeraldField.php | 43 +++++++++++++++++++ ...sionCommitRevisionReviewersHeraldField.php | 2 +- ...tRevisionAcceptingReviewersHeraldField.php | 43 +++++++++++++++++++ ...mitContentRevisionReviewersHeraldField.php | 2 +- 5 files changed, 92 insertions(+), 2 deletions(-) create mode 100644 src/applications/diffusion/herald/DiffusionCommitRevisionAcceptingReviewersHeraldField.php create mode 100644 src/applications/diffusion/herald/DiffusionPreCommitContentRevisionAcceptingReviewersHeraldField.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index f031f7deca..849d4d4702 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -696,6 +696,7 @@ phutil_register_library_map(array( 'DiffusionCommitRevertsCommitEdgeType' => 'applications/diffusion/edge/DiffusionCommitRevertsCommitEdgeType.php', 'DiffusionCommitReviewerHeraldField' => 'applications/diffusion/herald/DiffusionCommitReviewerHeraldField.php', 'DiffusionCommitRevisionAcceptedHeraldField' => 'applications/diffusion/herald/DiffusionCommitRevisionAcceptedHeraldField.php', + 'DiffusionCommitRevisionAcceptingReviewersHeraldField' => 'applications/diffusion/herald/DiffusionCommitRevisionAcceptingReviewersHeraldField.php', 'DiffusionCommitRevisionHeraldField' => 'applications/diffusion/herald/DiffusionCommitRevisionHeraldField.php', 'DiffusionCommitRevisionReviewersHeraldField' => 'applications/diffusion/herald/DiffusionCommitRevisionReviewersHeraldField.php', 'DiffusionCommitRevisionSubscribersHeraldField' => 'applications/diffusion/herald/DiffusionCommitRevisionSubscribersHeraldField.php', @@ -809,6 +810,7 @@ phutil_register_library_map(array( 'DiffusionPreCommitContentRepositoryHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitContentRepositoryHeraldField.php', 'DiffusionPreCommitContentRepositoryProjectsHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitContentRepositoryProjectsHeraldField.php', 'DiffusionPreCommitContentRevisionAcceptedHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitContentRevisionAcceptedHeraldField.php', + 'DiffusionPreCommitContentRevisionAcceptingReviewersHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitContentRevisionAcceptingReviewersHeraldField.php', 'DiffusionPreCommitContentRevisionHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitContentRevisionHeraldField.php', 'DiffusionPreCommitContentRevisionReviewersHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitContentRevisionReviewersHeraldField.php', 'DiffusionPreCommitContentRevisionSubscribersHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitContentRevisionSubscribersHeraldField.php', @@ -5755,6 +5757,7 @@ phutil_register_library_map(array( 'DiffusionCommitRevertsCommitEdgeType' => 'PhabricatorEdgeType', 'DiffusionCommitReviewerHeraldField' => 'DiffusionCommitHeraldField', 'DiffusionCommitRevisionAcceptedHeraldField' => 'DiffusionCommitHeraldField', + 'DiffusionCommitRevisionAcceptingReviewersHeraldField' => 'DiffusionCommitHeraldField', 'DiffusionCommitRevisionHeraldField' => 'DiffusionCommitHeraldField', 'DiffusionCommitRevisionReviewersHeraldField' => 'DiffusionCommitHeraldField', 'DiffusionCommitRevisionSubscribersHeraldField' => 'DiffusionCommitHeraldField', @@ -5871,6 +5874,7 @@ phutil_register_library_map(array( 'DiffusionPreCommitContentRepositoryHeraldField' => 'DiffusionPreCommitContentHeraldField', 'DiffusionPreCommitContentRepositoryProjectsHeraldField' => 'DiffusionPreCommitContentHeraldField', 'DiffusionPreCommitContentRevisionAcceptedHeraldField' => 'DiffusionPreCommitContentHeraldField', + 'DiffusionPreCommitContentRevisionAcceptingReviewersHeraldField' => 'DiffusionPreCommitContentHeraldField', 'DiffusionPreCommitContentRevisionHeraldField' => 'DiffusionPreCommitContentHeraldField', 'DiffusionPreCommitContentRevisionReviewersHeraldField' => 'DiffusionPreCommitContentHeraldField', 'DiffusionPreCommitContentRevisionSubscribersHeraldField' => 'DiffusionPreCommitContentHeraldField', diff --git a/src/applications/diffusion/herald/DiffusionCommitRevisionAcceptingReviewersHeraldField.php b/src/applications/diffusion/herald/DiffusionCommitRevisionAcceptingReviewersHeraldField.php new file mode 100644 index 0000000000..6d266f1357 --- /dev/null +++ b/src/applications/diffusion/herald/DiffusionCommitRevisionAcceptingReviewersHeraldField.php @@ -0,0 +1,43 @@ +getAdapter()->loadDifferentialRevision(); + + if (!$revision) { + return array(); + } + + $diff_phid = $revision->getActiveDiffPHID(); + + $reviewer_phids = array(); + foreach ($revision->getReviewers() as $reviewer) { + if ($reviewer->isAccepted($diff_phid)) { + $reviewer_phids[] = $reviewer->getReviewerPHID(); + } + } + + return $reviewer_phids; + } + + protected function getHeraldFieldStandardType() { + return self::STANDARD_PHID_LIST; + } + + protected function getDatasource() { + return new DifferentialReviewerDatasource(); + } + +} diff --git a/src/applications/diffusion/herald/DiffusionCommitRevisionReviewersHeraldField.php b/src/applications/diffusion/herald/DiffusionCommitRevisionReviewersHeraldField.php index 50900ede78..a9a3f3e8ca 100644 --- a/src/applications/diffusion/herald/DiffusionCommitRevisionReviewersHeraldField.php +++ b/src/applications/diffusion/herald/DiffusionCommitRevisionReviewersHeraldField.php @@ -28,7 +28,7 @@ final class DiffusionCommitRevisionReviewersHeraldField } protected function getDatasource() { - return new PhabricatorProjectOrUserDatasource(); + return new DifferentialReviewerDatasource(); } } diff --git a/src/applications/diffusion/herald/DiffusionPreCommitContentRevisionAcceptingReviewersHeraldField.php b/src/applications/diffusion/herald/DiffusionPreCommitContentRevisionAcceptingReviewersHeraldField.php new file mode 100644 index 0000000000..b6d50d4a2a --- /dev/null +++ b/src/applications/diffusion/herald/DiffusionPreCommitContentRevisionAcceptingReviewersHeraldField.php @@ -0,0 +1,43 @@ +getAdapter()->getRevision(); + + if (!$revision) { + return array(); + } + + $diff_phid = $revision->getActiveDiffPHID(); + + $reviewer_phids = array(); + foreach ($revision->getReviewers() as $reviewer) { + if ($reviewer->isAccepted($diff_phid)) { + $reviewer_phids[] = $reviewer->getReviewerPHID(); + } + } + + return $reviewer_phids; + } + + protected function getHeraldFieldStandardType() { + return self::STANDARD_PHID_LIST; + } + + protected function getDatasource() { + return new DifferentialReviewerDatasource(); + } + +} diff --git a/src/applications/diffusion/herald/DiffusionPreCommitContentRevisionReviewersHeraldField.php b/src/applications/diffusion/herald/DiffusionPreCommitContentRevisionReviewersHeraldField.php index 936126ba89..aa2b2e9e09 100644 --- a/src/applications/diffusion/herald/DiffusionPreCommitContentRevisionReviewersHeraldField.php +++ b/src/applications/diffusion/herald/DiffusionPreCommitContentRevisionReviewersHeraldField.php @@ -28,7 +28,7 @@ final class DiffusionPreCommitContentRevisionReviewersHeraldField } protected function getDatasource() { - return new PhabricatorProjectOrUserDatasource(); + return new DifferentialReviewerDatasource(); } }