Page MenuHomePhabricator
Files F14044510

D15934.diff
No OneTemporary
Actions

D15934.diff
View Options

diff --git a/src/applications/differential/customfield/DifferentialCustomField.php b/src/applications/differential/customfield/DifferentialCustomField.php
--- a/src/applications/differential/customfield/DifferentialCustomField.php
+++ b/src/applications/differential/customfield/DifferentialCustomField.php
@@ -35,27 +35,39 @@
protected function parseObjectList(
$value,
array $types,
- $allow_partial = false) {
+ $allow_partial = false,
+ array $suffixes = array()) {
return id(new PhabricatorObjectListQuery())
->setViewer($this->getViewer())
->setAllowedTypes($types)
->setObjectList($value)
->setAllowPartialResults($allow_partial)
+ ->setSuffixes($suffixes)
->execute();
}
- protected function renderObjectList(array $handles) {
+ protected function renderObjectList(
+ array $handles,
+ array $suffixes = array()) {
+
if (!$handles) {
return null;
}
$out = array();
foreach ($handles as $handle) {
+ $phid = $handle->getPHID();
+
if ($handle->getPolicyFiltered()) {
- $out[] = $handle->getPHID();
+ $token = $phid;
} else if ($handle->isComplete()) {
- $out[] = $handle->getCommandLineObjectName();
+ $token = $handle->getCommandLineObjectName();
}
+
+ $suffix = idx($suffixes, $phid);
+ $token = $token.$suffix;
+
+ $out[] = $token;
}
return implode(', ', $out);
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,38 +36,37 @@
}
public function readValueFromRequest(AphrontRequest $request) {
- // 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, 'getStatus', 'getReviewerPHID');
-
$datasource = id(new DifferentialBlockingReviewerDatasource())
->setViewer($request->getViewer());
$new_phids = $request->getArr($this->getFieldKey());
$new_phids = $datasource->evaluateTokens($new_phids);
- $status_blocking = DifferentialReviewerStatus::STATUS_BLOCKING;
-
- $specs = array();
+ $reviewers = array();
foreach ($new_phids as $spec) {
if (!is_array($spec)) {
- $spec = array(
- 'type' => DifferentialReviewerStatus::STATUS_ADDED,
- 'phid' => $spec,
- );
+ $reviewers[$spec] = DifferentialReviewerStatus::STATUS_ADDED;
+ } else {
+ $reviewers[$spec['phid']] = $spec['type'];
}
- $specs[$spec['phid']] = $spec;
}
- $new_status = array();
- foreach ($specs as $phid => $spec) {
- $new = $spec['type'];
- $old = idx($old_status, $phid);
+ $this->updateReviewers($this->getValue(), $reviewers);
+ }
+
+ private function updateReviewers(array $old_reviewers, array $new_map) {
+ // Compute a new set of reviewer objects. We're going to respect the new
+ // reviewer order, add or remove any new or missing reviewers, and respect
+ // any blocking or unblocking changes. For reviewers who were there before
+ // and are still there, we're going to keep the old value because it
+ // may be something like "Accept", "Reject", etc.
+
+ $old_map = mpull($old_reviewers, 'getStatus', 'getReviewerPHID');
+ $status_blocking = DifferentialReviewerStatus::STATUS_BLOCKING;
+
+ $new_reviewers = array();
+ foreach ($new_map as $phid => $new) {
+ $old = idx($old_map, $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
@@ -76,23 +75,23 @@
$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;
+ $new_reviewers[$phid] = $old;
continue;
}
}
- $new_status[$phid] = $new;
+ $new_reviewers[$phid] = $new;
}
- foreach ($new_status as $phid => $status) {
- $new_status[$phid] = new DifferentialReviewer(
+ foreach ($new_reviewers as $phid => $status) {
+ $new_reviewers[$phid] = new DifferentialReviewer(
$phid,
array(
'status' => $status,
));
}
- $this->setValue($new_status);
+ $this->setValue($new_reviewers);
}
public function renderEditControl(array $handles) {
@@ -187,7 +186,9 @@
PhabricatorPeopleUserPHIDType::TYPECONST,
PhabricatorProjectProjectPHIDType::TYPECONST,
PhabricatorOwnersPackagePHIDType::TYPECONST,
- ));
+ ),
+ false,
+ array('!'));
}
public function getRequiredHandlePHIDsForCommitMessage() {
@@ -195,29 +196,40 @@
}
public function readValueFromCommitMessage($value) {
- $current_reviewers = $this->getObject()->getReviewerStatus();
- $current_reviewers = mpull($current_reviewers, null, 'getReviewerPHID');
-
$reviewers = array();
- foreach ($value as $phid) {
- $reviewer = idx($current_reviewers, $phid);
- if ($reviewer) {
- $reviewers[] = $reviewer;
+ foreach ($value as $spec) {
+ $phid = $spec['phid'];
+
+ $is_blocking = isset($spec['suffixes']['!']);
+ if ($is_blocking) {
+ $status = DifferentialReviewerStatus::STATUS_BLOCKING;
} else {
- $data = array(
- 'status' => DifferentialReviewerStatus::STATUS_ADDED,
- );
- $reviewers[] = new DifferentialReviewer($phid, $data);
+ $status = DifferentialReviewerStatus::STATUS_ADDED;
}
+
+ $reviewers[$phid] = $status;
}
- $this->setValue($reviewers);
+ $this->updateReviewers(
+ $this->getObject()->getReviewerStatus(),
+ $reviewers);
return $this;
}
public function renderCommitMessageValue(array $handles) {
- return $this->renderObjectList($handles);
+ $suffixes = array();
+
+ $status_blocking = DifferentialReviewerStatus::STATUS_BLOCKING;
+
+ foreach ($this->getValue() as $reviewer) {
+ if ($reviewer->getStatus() == $status_blocking) {
+ $phid = $reviewer->getReviewerPHID();
+ $suffixes[$phid] = '!';
+ }
+ }
+
+ return $this->renderObjectList($handles, $suffixes);
}
public function validateCommitMessageValue($value) {
@@ -226,7 +238,9 @@
$config_self_accept_key = 'differential.allow-self-accept';
$allow_self_accept = PhabricatorEnv::getEnvConfig($config_self_accept_key);
- foreach ($value as $phid) {
+ foreach ($value as $spec) {
+ $phid = $spec['phid'];
+
if (($phid == $author_phid) && !$allow_self_accept) {
throw new DifferentialFieldValidationException(
pht('The author of a revision can not be a reviewer.'));
@@ -265,4 +279,12 @@
return $warnings;
}
+ public function getProTips() {
+ return array(
+ pht(
+ 'You can mark a reviewer as blocking by adding an exclamation '.
+ 'mark ("!") after their name.'),
+ );
+ }
+
}
diff --git a/src/applications/phid/query/PhabricatorObjectListQuery.php b/src/applications/phid/query/PhabricatorObjectListQuery.php
--- a/src/applications/phid/query/PhabricatorObjectListQuery.php
+++ b/src/applications/phid/query/PhabricatorObjectListQuery.php
@@ -6,6 +6,7 @@
private $objectList;
private $allowedTypes = array();
private $allowPartialResults;
+ private $suffixes = array();
public function setAllowPartialResults($allow_partial_results) {
$this->allowPartialResults = $allow_partial_results;
@@ -16,6 +17,15 @@
return $this->allowPartialResults;
}
+ public function setSuffixes(array $suffixes) {
+ $this->suffixes = $suffixes;
+ return $this;
+ }
+
+ public function getSuffixes() {
+ return $this->suffixes;
+ }
+
public function setAllowedTypes(array $allowed_types) {
$this->allowedTypes = $allowed_types;
return $this;
@@ -87,6 +97,37 @@
}
}
+ // If we're parsing with suffixes, strip them off any tokens and keep
+ // track of them for later.
+ $suffixes = $this->getSuffixes();
+ if ($suffixes) {
+ $suffixes = array_fuse($suffixes);
+ $suffix_map = array();
+ $stripped_map = array();
+ foreach ($name_map as $key => $name) {
+ $found_suffixes = array();
+ do {
+ $has_any_suffix = false;
+ foreach ($suffixes as $suffix) {
+ if (!$this->hasSuffix($name, $suffix)) {
+ continue;
+ }
+
+ $key = $this->stripSuffix($key, $suffix);
+ $name = $this->stripSuffix($name, $suffix);
+
+ $found_suffixes[] = $suffix;
+ $has_any_suffix = true;
+ break;
+ }
+ } while ($has_any_suffix);
+
+ $stripped_map[$key] = $name;
+ $suffix_map[$key] = array_fuse($found_suffixes);
+ }
+ $name_map = $stripped_map;
+ }
+
$objects = $this->loadObjects(array_keys($name_map));
$types = array();
@@ -140,7 +181,18 @@
}
}
- return array_values(array_unique(mpull($objects, 'getPHID')));
+ $result = array_unique(mpull($objects, 'getPHID'));
+
+ if ($suffixes) {
+ foreach ($result as $key => $phid) {
+ $result[$key] = array(
+ 'phid' => $phid,
+ 'suffixes' => idx($suffix_map, $key, array()),
+ );
+ }
+ }
+
+ return array_values($result);
}
private function loadObjects($names) {
@@ -186,5 +238,16 @@
return $results;
}
+ private function hasSuffix($key, $suffix) {
+ return (substr($key, -strlen($suffix)) === $suffix);
+ }
+
+ private function stripSuffix($key, $suffix) {
+ if ($this->hasSuffix($key, $suffix)) {
+ return substr($key, 0, -strlen($suffix));
+ }
+
+ return $key;
+ }
}
diff --git a/src/applications/phid/query/__tests__/PhabricatorObjectListQueryTestCase.php b/src/applications/phid/query/__tests__/PhabricatorObjectListQueryTestCase.php
--- a/src/applications/phid/query/__tests__/PhabricatorObjectListQueryTestCase.php
+++ b/src/applications/phid/query/__tests__/PhabricatorObjectListQueryTestCase.php
@@ -28,6 +28,15 @@
$result = $this->parseObjectList('');
$this->assertEqual(array(), $result);
+ $result = $this->parseObjectList("{$name}!", array(), false, array('!'));
+ $this->assertEqual(
+ array(
+ array(
+ 'phid' => $phid,
+ 'suffixes' => array('!' => '!'),
+ ),
+ ),
+ $result);
$package = PhabricatorOwnersPackage::initializeNewPackage($user)
->setName(pht('Query Test Package'))
@@ -42,6 +51,23 @@
$result = $this->parseObjectList("{$package_mono} Any Text, {$name}");
$this->assertEqual(array($package_phid, $phid), $result);
+ $result = $this->parseObjectList(
+ "{$package_mono} Any Text!, {$name}",
+ array(),
+ false,
+ array('!'));
+ $this->assertEqual(
+ array(
+ array(
+ 'phid' => $package_phid,
+ 'suffixes' => array('!' => '!'),
+ ),
+ array(
+ 'phid' => $phid,
+ 'suffixes' => array(),
+ ),
+ ),
+ $result);
// Expect failure when loading a user if objects must be of type "DUCK".
$caught = null;
@@ -81,7 +107,8 @@
private function parseObjectList(
$list,
array $types = array(),
- $allow_partial = false) {
+ $allow_partial = false,
+ $suffixes = array()) {
$query = id(new PhabricatorObjectListQuery())
->setViewer(PhabricatorUser::getOmnipotentUser())
@@ -95,6 +122,10 @@
$query->setAllowPartialResults(true);
}
+ if ($suffixes) {
+ $query->setSuffixes($suffixes);
+ }
+
return $query->execute();
}

File Metadata

Mime Type
text/plain
Expires
Wed, Nov 13, 5:16 PM (5 d, 11 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6762287
Default Alt Text
D15934.diff (12 KB)

Event Timeline

Phabricator · Built by Phacility