Page MenuHomePhabricator
Files F14008243

D8496.id20157.diff
No OneTemporary
Actions

D8496.id20157.diff
View Options

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
@@ -177,4 +177,19 @@
return $this->renderObjectList($handles);
}
+ public function validateCommitMessageValue($value) {
+ $author_phid = $this->getObject()->getAuthorPHID();
+
+ $config_self_accept_key = 'differential.allow-self-accept';
+ $allow_self_accept = PhabricatorEnv::getEnvConfig($config_self_accept_key);
+
+ foreach ($value as $phid) {
+ if (($phid == $author_phid) && !$allow_self_accept) {
+ throw new DifferentialFieldValidationException(
+ pht('The author of a revision can not be a reviewer.'));
+ }
+ }
+ }
+
+
}
diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php
--- a/src/applications/differential/editor/DifferentialTransactionEditor.php
+++ b/src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -395,10 +395,15 @@
);
}
+ // NOTE: We're setting setIsCommandeerSideEffect() on this because
+ // normally you can't add a revision's author as a reviewer, but
+ // this action swaps them after validation executes.
+
$results[] = id(new DifferentialTransaction())
->setTransactionType($type_edge)
->setMetadataValue('edge:type', $edge_reviewer)
->setIgnoreOnNoEffect(true)
+ ->setIsCommandeerSideEffect(true)
->setNewValue($edits);
break;
@@ -625,8 +630,45 @@
$errors = parent::validateTransaction($object, $type, $xactions);
+ $config_self_accept_key = 'differential.allow-self-accept';
+ $allow_self_accept = PhabricatorEnv::getEnvConfig($config_self_accept_key);
+
foreach ($xactions as $xaction) {
switch ($type) {
+ case PhabricatorTransactions::TYPE_EDGE:
+ switch ($xaction->getMetadataValue('edge:type')) {
+ case PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER:
+
+ // Prevent the author from becoming a reviewer.
+
+ // NOTE: This is pretty gross, but this restriction is unusual.
+ // If we end up with too much more of this, we should try to clean
+ // this up -- maybe by moving validation to after transactions
+ // are adjusted (so we can just examine the final value) or adding
+ // a second phase there?
+
+ $author_phid = $object->getAuthorPHID();
+ $new = $xaction->getNewValue();
+
+ $add = idx($new, '+', array());
+ $eq = idx($new, '=', array());
+ $phids = array_keys($add + $eq);
+
+ foreach ($phids as $phid) {
+ if (($phid == $author_phid) &&
+ !$allow_self_accept &&
+ !$xaction->getIsCommandeerSideEffect()) {
+ $errors[] =
+ new PhabricatorApplicationTransactionValidationError(
+ $type,
+ pht('Invalid'),
+ pht('The author of a revision can not be a reviewer.'),
+ $xaction);
+ }
+ }
+ break;
+ }
+ break;
case DifferentialTransaction::TYPE_UPDATE:
$diff = $this->loadDiff($xaction->getNewValue());
if (!$diff) {
@@ -1340,6 +1382,12 @@
$value = array();
foreach ($reviewers as $status => $phids) {
foreach ($phids as $phid) {
+ if ($phid == $object->getAuthorPHID()) {
+ // Don't try to add the revision's author as a reviewer, since this
+ // isn't valid and doesn't make sense.
+ continue;
+ }
+
$value['+'][$phid] = array(
'data' => array(
'status' => $status,
diff --git a/src/applications/differential/storage/DifferentialTransaction.php b/src/applications/differential/storage/DifferentialTransaction.php
--- a/src/applications/differential/storage/DifferentialTransaction.php
+++ b/src/applications/differential/storage/DifferentialTransaction.php
@@ -2,6 +2,18 @@
final class DifferentialTransaction extends PhabricatorApplicationTransaction {
+ private $isCommandeerSideEffect;
+
+
+ public function setIsCommandeerSideEffect($is_side_effect) {
+ $this->isCommandeerSideEffect = $is_side_effect;
+ return $this;
+ }
+
+ public function getIsCommandeerSideEffect() {
+ return $this->isCommandeerSideEffect;
+ }
+
const TYPE_INLINE = 'differential:inline';
const TYPE_UPDATE = 'differential:update';
const TYPE_ACTION = 'differential:action';

File Metadata

Mime Type
text/plain
Expires
Wed, Oct 30, 6:23 PM (2 w, 5 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6742416
Default Alt Text
D8496.id20157.diff (4 KB)

Event Timeline

Phabricator · Built by Phacility