Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15415946
D15933.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
11 KB
Referenced Files
None
Subscribers
None
D15933.diff
View Options
diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -361,6 +361,7 @@
'DifferentialAuthorField' => 'applications/differential/customfield/DifferentialAuthorField.php',
'DifferentialBlameRevisionField' => 'applications/differential/customfield/DifferentialBlameRevisionField.php',
'DifferentialBlockHeraldAction' => 'applications/differential/herald/DifferentialBlockHeraldAction.php',
+ 'DifferentialBlockingReviewerDatasource' => 'applications/differential/typeahead/DifferentialBlockingReviewerDatasource.php',
'DifferentialBranchField' => 'applications/differential/customfield/DifferentialBranchField.php',
'DifferentialChangeDetailMailView' => 'applications/differential/mail/DifferentialChangeDetailMailView.php',
'DifferentialChangeHeraldFieldGroup' => 'applications/differential/herald/DifferentialChangeHeraldFieldGroup.php',
@@ -497,6 +498,7 @@
'DifferentialRevertPlanField' => 'applications/differential/customfield/DifferentialRevertPlanField.php',
'DifferentialReviewedByField' => 'applications/differential/customfield/DifferentialReviewedByField.php',
'DifferentialReviewer' => 'applications/differential/storage/DifferentialReviewer.php',
+ 'DifferentialReviewerDatasource' => 'applications/differential/typeahead/DifferentialReviewerDatasource.php',
'DifferentialReviewerForRevisionEdgeType' => 'applications/differential/edge/DifferentialReviewerForRevisionEdgeType.php',
'DifferentialReviewerStatus' => 'applications/differential/constants/DifferentialReviewerStatus.php',
'DifferentialReviewersAddBlockingReviewersHeraldAction' => 'applications/differential/herald/DifferentialReviewersAddBlockingReviewersHeraldAction.php',
@@ -4561,6 +4563,7 @@
'DifferentialAuthorField' => 'DifferentialCustomField',
'DifferentialBlameRevisionField' => 'DifferentialStoredCustomField',
'DifferentialBlockHeraldAction' => 'HeraldAction',
+ 'DifferentialBlockingReviewerDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
'DifferentialBranchField' => 'DifferentialCustomField',
'DifferentialChangeDetailMailView' => 'DifferentialMailView',
'DifferentialChangeHeraldFieldGroup' => 'HeraldFieldGroup',
@@ -4713,6 +4716,7 @@
'DifferentialRevertPlanField' => 'DifferentialStoredCustomField',
'DifferentialReviewedByField' => 'DifferentialCoreCustomField',
'DifferentialReviewer' => 'Phobject',
+ 'DifferentialReviewerDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
'DifferentialReviewerForRevisionEdgeType' => 'PhabricatorEdgeType',
'DifferentialReviewerStatus' => 'Phobject',
'DifferentialReviewersAddBlockingReviewersHeraldAction' => 'DifferentialReviewersHeraldAction',
diff --git a/src/applications/differential/customfield/DifferentialReviewersField.php b/src/applications/differential/customfield/DifferentialReviewersField.php
--- a/src/applications/differential/customfield/DifferentialReviewersField.php
+++ b/src/applications/differential/customfield/DifferentialReviewersField.php
@@ -36,43 +36,83 @@
}
public function readValueFromRequest(AphrontRequest $request) {
- // Compute a new set of reviewer objects. For reviewers who haven't been
- // added or removed, retain their existing status. Also, respect the new
- // order.
+ // Compute a new set of reviewer objects. We're going to respect the new
+ // reviewer order, add or remove any missing or new reviewers, and respect
+ // any blocking or unblocking changes. For reviewers who were there before
+ // and are still there, we're going to keep the current value because it
+ // may be something like "Accept", "Reject", etc.
$old_status = $this->getValue();
- $old_status = mpull($old_status, null, 'getReviewerPHID');
+ $old_status = mpull($old_status, 'getStatus', 'getReviewerPHID');
+
+ $datasource = id(new DifferentialBlockingReviewerDatasource())
+ ->setViewer($request->getViewer());
$new_phids = $request->getArr($this->getFieldKey());
- $new_phids = array_fuse($new_phids);
+ $new_phids = $datasource->evaluateTokens($new_phids);
+
+ $status_blocking = DifferentialReviewerStatus::STATUS_BLOCKING;
+
+ $specs = array();
+ foreach ($new_phids as $spec) {
+ if (!is_array($spec)) {
+ $spec = array(
+ 'type' => DifferentialReviewerStatus::STATUS_ADDED,
+ 'phid' => $spec,
+ );
+ }
+ $specs[$spec['phid']] = $spec;
+ }
$new_status = array();
- foreach ($new_phids as $new_phid) {
- if (empty($old_status[$new_phid])) {
- $new_status[$new_phid] = new DifferentialReviewer(
- $new_phid,
- array(
- 'status' => DifferentialReviewerStatus::STATUS_ADDED,
- ));
- } else {
- $new_status[$new_phid] = $old_status[$new_phid];
+ foreach ($specs as $phid => $spec) {
+ $new = $spec['type'];
+ $old = idx($old_status, $phid);
+
+ // If we have an old status and this didn't make the reviewer blocking
+ // or nonblocking, just retain the old status. This makes sure we don't
+ // throw away rejects, accepts, etc.
+ if ($old) {
+ $is_block = ($old !== $status_blocking && $new === $status_blocking);
+ $is_unblock = ($old === $status_blocking && $new !== $status_blocking);
+ if (!$is_block && !$is_unblock) {
+ $new_status[$phid] = $old;
+ continue;
+ }
}
+
+ $new_status[$phid] = $new;
+ }
+
+ foreach ($new_status as $phid => $status) {
+ $new_status[$phid] = new DifferentialReviewer(
+ $phid,
+ array(
+ 'status' => $status,
+ ));
}
$this->setValue($new_status);
}
public function renderEditControl(array $handles) {
- $phids = array();
- if ($this->getValue()) {
- $phids = mpull($this->getValue(), 'getReviewerPHID');
+ $status_blocking = DifferentialReviewerStatus::STATUS_BLOCKING;
+
+ $value = array();
+ foreach ($this->getValue() as $reviewer) {
+ $phid = $reviewer->getReviewerPHID();
+ if ($reviewer->getStatus() == $status_blocking) {
+ $value[] = 'blocking('.$phid.')';
+ } else {
+ $value[] = $phid;
+ }
}
return id(new AphrontFormTokenizerControl())
->setUser($this->getViewer())
->setName($this->getFieldKey())
- ->setDatasource(new DiffusionAuditorDatasource())
- ->setValue($phids)
+ ->setDatasource(new DifferentialReviewerDatasource())
+ ->setValue($value)
->setError($this->getFieldError())
->setLabel($this->getFieldName());
}
diff --git a/src/applications/differential/typeahead/DifferentialBlockingReviewerDatasource.php b/src/applications/differential/typeahead/DifferentialBlockingReviewerDatasource.php
new file mode 100644
--- /dev/null
+++ b/src/applications/differential/typeahead/DifferentialBlockingReviewerDatasource.php
@@ -0,0 +1,128 @@
+<?php
+
+final class DifferentialBlockingReviewerDatasource
+ extends PhabricatorTypeaheadCompositeDatasource {
+
+ public function getBrowseTitle() {
+ return pht('Browse Blocking Reviewers');
+ }
+
+ public function getPlaceholderText() {
+ return pht('Type a user, project, or package name...');
+ }
+
+ public function getDatasourceApplicationClass() {
+ return 'PhabricatorDifferentialApplication';
+ }
+
+ public function getComponentDatasources() {
+ return array(
+ new PhabricatorPeopleDatasource(),
+ new PhabricatorProjectDatasource(),
+ new PhabricatorOwnersPackageDatasource(),
+ );
+ }
+
+ public function getDatasourceFunctions() {
+ return array(
+ 'blocking' => array(
+ 'name' => pht('Blocking: ...'),
+ 'arguments' => pht('reviewer'),
+ 'summary' => pht('Select a blocking reviewer.'),
+ 'description' => pht(
+ "This function allows you to add a reviewer as a blocking ".
+ "reviewer. For example, this will add `%s` as a blocking reviewer: ".
+ "\n\n%s\n\n",
+ 'alincoln',
+ '> blocking(alincoln)'),
+ ),
+ );
+ }
+
+
+ protected function didLoadResults(array $results) {
+ foreach ($results as $result) {
+ $result
+ ->setColor('red')
+ ->setTokenType(PhabricatorTypeaheadTokenView::TYPE_FUNCTION)
+ ->setIcon('fa-asterisk')
+ ->setPHID('blocking('.$result->getPHID().')')
+ ->setDisplayName(pht('Blocking: %s', $result->getDisplayName()))
+ ->setName($result->getName().' blocking');
+ }
+
+ return $results;
+ }
+
+ protected function evaluateFunction($function, array $argv_list) {
+ $phids = array();
+ foreach ($argv_list as $argv) {
+ $phids[] = head($argv);
+ }
+
+ $phids = $this->resolvePHIDs($phids);
+
+ $results = array();
+ foreach ($phids as $phid) {
+ $results[] = array(
+ 'type' => DifferentialReviewerStatus::STATUS_BLOCKING,
+ 'phid' => $phid,
+ );
+ }
+
+ return $results;
+ }
+
+ public function renderFunctionTokens($function, array $argv_list) {
+ $phids = array();
+ foreach ($argv_list as $argv) {
+ $phids[] = head($argv);
+ }
+
+ $phids = $this->resolvePHIDs($phids);
+
+ $tokens = $this->renderTokens($phids);
+ foreach ($tokens as $token) {
+ $token->setColor(null);
+ if ($token->isInvalid()) {
+ $token
+ ->setValue(pht('Blocking: Invalid Reviewer'));
+ } else {
+ $token
+ ->setIcon('fa-asterisk')
+ ->setTokenType(PhabricatorTypeaheadTokenView::TYPE_FUNCTION)
+ ->setColor('red')
+ ->setKey('blocking('.$token->getKey().')')
+ ->setValue(pht('Blocking: %s', $token->getValue()));
+ }
+ }
+
+ return $tokens;
+ }
+
+ private function resolvePHIDs(array $phids) {
+ $usernames = array();
+ foreach ($phids as $key => $phid) {
+ if (phid_get_type($phid) != PhabricatorPeopleUserPHIDType::TYPECONST) {
+ $usernames[$key] = $phid;
+ }
+ }
+
+ if ($usernames) {
+ $users = id(new PhabricatorPeopleQuery())
+ ->setViewer($this->getViewer())
+ ->withUsernames($usernames)
+ ->execute();
+ $users = mpull($users, null, 'getUsername');
+ foreach ($usernames as $key => $username) {
+ $user = idx($users, $username);
+ if ($user) {
+ $phids[$key] = $user->getPHID();
+ }
+ }
+ }
+
+ return $phids;
+ }
+
+}
diff --git a/src/applications/differential/typeahead/DifferentialReviewerDatasource.php b/src/applications/differential/typeahead/DifferentialReviewerDatasource.php
new file mode 100644
--- /dev/null
+++ b/src/applications/differential/typeahead/DifferentialReviewerDatasource.php
@@ -0,0 +1,27 @@
+<?php
+
+final class DifferentialReviewerDatasource
+ extends PhabricatorTypeaheadCompositeDatasource {
+
+ public function getBrowseTitle() {
+ return pht('Browse Reviewers');
+ }
+
+ public function getPlaceholderText() {
+ return pht('Type a user, project, or package name...');
+ }
+
+ public function getDatasourceApplicationClass() {
+ return 'PhabricatorDifferentialApplication';
+ }
+
+ public function getComponentDatasources() {
+ return array(
+ new PhabricatorPeopleDatasource(),
+ new PhabricatorProjectDatasource(),
+ new PhabricatorOwnersPackageDatasource(),
+ new DifferentialBlockingReviewerDatasource(),
+ );
+ }
+
+}
diff --git a/src/applications/differential/view/DifferentialAddCommentView.php b/src/applications/differential/view/DifferentialAddCommentView.php
--- a/src/applications/differential/view/DifferentialAddCommentView.php
+++ b/src/applications/differential/view/DifferentialAddCommentView.php
@@ -69,7 +69,9 @@
);
$mailable_source = new PhabricatorMetaMTAMailableDatasource();
- $reviewer_source = new PhabricatorProjectOrUserDatasource();
+
+ // TODO: This should be a reviewers datasource, but it's a mess.
+ $reviewer_source = new PhabricatorMetaMTAMailableDatasource();
$form = new AphrontFormView();
$form
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Mar 21 2025, 8:30 AM (5 w, 19 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7248218
Default Alt Text
D15933.diff (11 KB)
Attached To
Mode
D15933: Allow blocking reviewers to be added via the web UI
Attached
Detach File
Event Timeline
Log In to Comment