mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-22 14:52:41 +01:00
Correct self-review check in Differential
Summary: This check is currently wrong -- the actor is only //coincidentally// the owner (and only most of the time). It also raises at parse time, preventing any user from parsing a message with their own name in the "Reviewers" field. Instead, check against the right owner PHID and raise it only if a revision is available. See https://github.com/facebook/arcanist/issues/54 and next diff. Test Plan: Tried to add myself as a reviewer to revisions I own via web and Conduit, got rejected. Parsed a message with myself in the "Reviewers:" field, it worked correctly. Reviewers: btrahan, vrana Reviewed By: btrahan CC: aran Differential Revision: https://secure.phabricator.com/D3820
This commit is contained in:
parent
731a6900bd
commit
846f01e221
2 changed files with 27 additions and 7 deletions
|
@ -841,6 +841,17 @@ abstract class DifferentialFieldSpecification {
|
|||
return $this->revision;
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* Determine if revision context is currently available.
|
||||
*
|
||||
* @task context
|
||||
*/
|
||||
final protected function hasRevision() {
|
||||
return (bool)$this->revision;
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* @task context
|
||||
*/
|
||||
|
|
|
@ -61,14 +61,23 @@ final class DifferentialReviewersFieldSpecification
|
|||
}
|
||||
|
||||
public function validateField() {
|
||||
$allow_self_accept = PhabricatorEnv::getEnvConfig(
|
||||
'differential.allow-self-accept', false);
|
||||
if (!$allow_self_accept
|
||||
&& in_array($this->getUser()->getPHID(), $this->reviewers)) {
|
||||
if (!$this->hasRevision()) {
|
||||
return;
|
||||
}
|
||||
|
||||
$self = PhabricatorEnv::getEnvConfig('differential.allow-self-accept');
|
||||
if ($self) {
|
||||
return;
|
||||
}
|
||||
|
||||
$author_phid = $this->getRevision()->getAuthorPHID();
|
||||
if (!in_array($author_phid, $this->reviewers)) {
|
||||
return;
|
||||
}
|
||||
|
||||
$this->error = 'Invalid';
|
||||
throw new DifferentialFieldValidationException(
|
||||
"You may not review your own revision!");
|
||||
}
|
||||
"The owner of a revision may not be a reviewer.");
|
||||
}
|
||||
|
||||
public function renderEditControl() {
|
||||
|
|
Loading…
Reference in a new issue