Page MenuHomestyx hydra
Files F233547

No OneTemporary
Actions

View Options

diff --git a/src/applications/differential/xaction/DifferentialRevisionResignTransaction.php b/src/applications/differential/xaction/DifferentialRevisionResignTransaction.php
index 5b75d7753f..c7805ad35f 100644
--- a/src/applications/differential/xaction/DifferentialRevisionResignTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionResignTransaction.php
@@ -1,86 +1,95 @@
<?php
final class DifferentialRevisionResignTransaction
extends DifferentialRevisionReviewTransaction {
const TRANSACTIONTYPE = 'differential.revision.resign';
const ACTIONKEY = 'resign';
protected function getRevisionActionLabel() {
return pht('Resign as Reviewer');
}
protected function getRevisionActionDescription() {
return pht('You will resign as a reviewer for this change.');
}
public function getIcon() {
return 'fa-flag';
}
public function getColor() {
return 'orange';
}
protected function getRevisionActionOrder() {
return 700;
}
public function getCommandKeyword() {
return 'resign';
}
public function getActionName() {
return pht('Resigned');
}
public function getCommandAliases() {
return array();
}
public function getCommandSummary() {
return pht('Resign from a revision.');
}
public function generateOldValue($object) {
$actor = $this->getActor();
- return !$this->isViewerAnyReviewer($object, $actor);
+ $resigned = DifferentialReviewerStatus::STATUS_RESIGNED;
+
+ return ($this->getViewerReviewerStatus($object, $actor) == $resigned);
}
public function applyExternalEffects($object, $value) {
$status = DifferentialReviewerStatus::STATUS_RESIGNED;
$actor = $this->getActor();
$this->applyReviewerEffect($object, $actor, $value, $status);
}
protected function validateAction($object, PhabricatorUser $viewer) {
if ($object->isClosed()) {
throw new Exception(
pht(
'You can not resign from this revision because it has already '.
'been closed. You can only resign from open revisions.'));
}
- if (!$this->isViewerAnyReviewer($object, $viewer)) {
+ $resigned = DifferentialReviewerStatus::STATUS_RESIGNED;
+ if ($this->getViewerReviewerStatus($object, $viewer) == $resigned) {
+ throw new Exception(
+ pht(
+ 'You can not resign from this revision because you have already '.
+ 'resigned.'));
+ }
+
+ if (!$this->isViewerAnyAuthority($object, $viewer)) {
throw new Exception(
pht(
'You can not resign from this revision because you are not a '.
- 'reviewer. You can only resign from revisions where you are a '.
- 'reviewer.'));
+ 'reviewer, and do not have authority over any reviewer.'));
}
}
public function getTitle() {
return pht(
'%s resigned from this revision.',
$this->renderAuthor());
}
public function getTitleForFeed() {
return pht(
'%s resigned from %s.',
$this->renderAuthor(),
$this->renderObject());
}
}
diff --git a/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php b/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php
index 86aba03e25..3db289470a 100644
--- a/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php
@@ -1,222 +1,236 @@
<?php
abstract class DifferentialRevisionReviewTransaction
extends DifferentialRevisionActionTransaction {
protected function getRevisionActionGroupKey() {
return DifferentialRevisionEditEngine::ACTIONGROUP_REVIEW;
}
public function generateNewValue($object, $value) {
if (!is_array($value)) {
return true;
}
// If the list of options is the same as the default list, just treat this
// as a "take the default action" transaction.
$viewer = $this->getActor();
list($options, $default) = $this->getActionOptions($viewer, $object);
sort($default);
sort($value);
if ($default === $value) {
return true;
}
return $value;
}
protected function isViewerAnyReviewer(
DifferentialRevision $revision,
PhabricatorUser $viewer) {
return ($this->getViewerReviewerStatus($revision, $viewer) !== null);
}
+ protected function isViewerAnyAuthority(
+ DifferentialRevision $revision,
+ PhabricatorUser $viewer) {
+
+ $reviewers = $revision->getReviewers();
+ foreach ($revision->getReviewers() as $reviewer) {
+ if ($reviewer->hasAuthority($viewer)) {
+ return true;
+ }
+ }
+
+ return false;
+ }
+
protected function isViewerFullyAccepted(
DifferentialRevision $revision,
PhabricatorUser $viewer) {
return $this->isViewerReviewerStatusFullyAmong(
$revision,
$viewer,
array(
DifferentialReviewerStatus::STATUS_ACCEPTED,
),
true);
}
protected function isViewerFullyRejected(
DifferentialRevision $revision,
PhabricatorUser $viewer) {
return $this->isViewerReviewerStatusFullyAmong(
$revision,
$viewer,
array(
DifferentialReviewerStatus::STATUS_REJECTED,
),
true);
}
protected function getViewerReviewerStatus(
DifferentialRevision $revision,
PhabricatorUser $viewer) {
if (!$viewer->getPHID()) {
return null;
}
foreach ($revision->getReviewers() as $reviewer) {
if ($reviewer->getReviewerPHID() != $viewer->getPHID()) {
continue;
}
return $reviewer->getReviewerStatus();
}
return null;
}
protected function isViewerReviewerStatusFullyAmong(
DifferentialRevision $revision,
PhabricatorUser $viewer,
array $status_list,
$require_current) {
// If the user themselves is not a reviewer, the reviews they have
// authority over can not all be in any set of states since their own
// personal review has no state.
$status = $this->getViewerReviewerStatus($revision, $viewer);
if ($status === null) {
return false;
}
$active_phid = $this->getActiveDiffPHID($revision);
// Otherwise, check that all reviews they have authority over are in
// the desired set of states.
$status_map = array_fuse($status_list);
foreach ($revision->getReviewers() as $reviewer) {
if (!$reviewer->hasAuthority($viewer)) {
continue;
}
$status = $reviewer->getReviewerStatus();
if (!isset($status_map[$status])) {
return false;
}
if ($require_current) {
if ($reviewer->getLastActionDiffPHID() != $active_phid) {
return false;
}
}
}
return true;
}
protected function applyReviewerEffect(
DifferentialRevision $revision,
PhabricatorUser $viewer,
$value,
$status) {
$map = array();
// When you accept or reject, you may accept or reject on behalf of all
// reviewers you have authority for. When you resign, you only affect
// yourself.
$with_authority = ($status != DifferentialReviewerStatus::STATUS_RESIGNED);
if ($with_authority) {
foreach ($revision->getReviewers() as $reviewer) {
if ($reviewer->hasAuthority($viewer)) {
$map[$reviewer->getReviewerPHID()] = $status;
}
}
}
// In all cases, you affect yourself.
$map[$viewer->getPHID()] = $status;
// If the user has submitted a specific list of reviewers to act as (by
// unchecking some checkboxes under "Accept"), only affect those reviewers.
if (is_array($value)) {
$map = array_select_keys($map, $value);
}
// Convert reviewer statuses into edge data.
foreach ($map as $reviewer_phid => $reviewer_status) {
$map[$reviewer_phid] = array(
'data' => array(
'status' => $reviewer_status,
),
);
}
// This is currently double-writing: to the old (edge) store and the new
// (reviewer) store. Do the old edge write first.
$src_phid = $revision->getPHID();
$edge_type = DifferentialRevisionHasReviewerEdgeType::EDGECONST;
$editor = new PhabricatorEdgeEditor();
foreach ($map as $dst_phid => $edge_data) {
if ($status == DifferentialReviewerStatus::STATUS_RESIGNED) {
// TODO: For now, we just remove these reviewers. In the future, we will
// store resignations explicitly.
$editor->removeEdge($src_phid, $edge_type, $dst_phid);
} else {
$editor->addEdge($src_phid, $edge_type, $dst_phid, $edge_data);
}
}
$editor->save();
// Now, do the new write.
if ($map) {
$diff = $this->getEditor()->getActiveDiff($revision);
if ($diff) {
$diff_phid = $diff->getPHID();
} else {
$diff_phid = null;
}
$table = new DifferentialReviewer();
$reviewers = $table->loadAllWhere(
'revisionPHID = %s AND reviewerPHID IN (%Ls)',
$src_phid,
array_keys($map));
$reviewers = mpull($reviewers, null, 'getReviewerPHID');
foreach ($map as $dst_phid => $edge_data) {
$reviewer = idx($reviewers, $dst_phid);
if (!$reviewer) {
$reviewer = id(new DifferentialReviewer())
->setRevisionPHID($src_phid)
->setReviewerPHID($dst_phid);
}
$old_status = $reviewer->getReviewerStatus();
$reviewer->setReviewerStatus($status);
if ($diff_phid) {
$reviewer->setLastActionDiffPHID($diff_phid);
}
if ($old_status !== $status) {
$reviewer->setLastActorPHID($this->getActingAsPHID());
}
try {
$reviewer->save();
} catch (AphrontDuplicateKeyQueryException $ex) {
// At least for now, just ignore it if we lost a race.
}
}
}
}
}
diff --git a/src/applications/differential/xaction/DifferentialRevisionTransactionType.php b/src/applications/differential/xaction/DifferentialRevisionTransactionType.php
index 6c36d0aaa8..79435bee78 100644
--- a/src/applications/differential/xaction/DifferentialRevisionTransactionType.php
+++ b/src/applications/differential/xaction/DifferentialRevisionTransactionType.php
@@ -1,69 +1,72 @@
<?php
abstract class DifferentialRevisionTransactionType
extends PhabricatorModularTransactionType {
protected function validateCommitMessageCorpusTransactions(
$object,
array $xactions,
$field_name) {
$errors = array();
foreach ($xactions as $xaction) {
$error = $this->validateMessageCorpus($xaction, $field_name);
if ($error) {
$errors[] = $error;
}
}
return $errors;
}
private function validateMessageCorpus($xaction, $field_name) {
$value = $xaction->getNewValue();
if (!strlen($value)) {
return null;
}
// Put a placeholder title on the message, because the first line of a
// message is now always parsed as a title.
$value = "<placeholder>\n".$value;
$viewer = $this->getActor();
$parser = DifferentialCommitMessageParser::newStandardParser($viewer);
// Set custom title and summary keys so we can detect the presence of
// "Summary:" in, e.g., a test plan.
$parser->setTitleKey('__title__');
$parser->setSummaryKey('__summary__');
$result = $parser->parseCorpus($value);
unset($result['__title__']);
unset($result['__summary__']);
if (!$result) {
return null;
}
return $this->newInvalidError(
pht(
'The value you have entered in "%s" can not be parsed '.
'unambiguously when rendered in a commit message. Edit the '.
'message so that keywords like "Summary:" and "Test Plan:" do '.
'not appear at the beginning of lines. Parsed keys: %s.',
$field_name,
implode(', ', array_keys($result))),
$xaction);
}
protected function getActiveDiffPHID(DifferentialRevision $revision) {
try {
$diff = $revision->getActiveDiff();
+ if (!$diff) {
+ return null;
+ }
return $diff->getPHID();
} catch (Exception $ex) {
return null;
}
}
}

File Metadata

Mime Type
text/x-diff
Expires
Mon, Jul 28, 8:27 AM (1 w, 23 h ago)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
186536
Default Alt Text
(12 KB)

Event Timeline

styx copyright 2020 sirocyl / ellie (kymkdd) / ncommander / styx contributors
Use of other names and brands is not intended to reflect a claim of, or right to, use the name or brand.
styx is an operating system using the Linux kernel and components, and "styx Linux" as a phrase does not reflect the inclusion of the Linux brand or trademark in the styx project.
Linux® is the registered trademark of Linus Torvalds in the U.S. and other countries.
Terms and Conditions (TODO: fix this!)