Page MenuHomePhabricator
Files F15445544

D8402.diff
No OneTemporary
Actions

D8402.diff
View Options

Index: src/applications/differential/constants/DifferentialReviewerStatus.php
===================================================================
--- src/applications/differential/constants/DifferentialReviewerStatus.php
+++ src/applications/differential/constants/DifferentialReviewerStatus.php
@@ -7,6 +7,8 @@
const STATUS_ACCEPTED = 'accepted';
const STATUS_REJECTED = 'rejected';
const STATUS_COMMENTED = 'commented';
+ const STATUS_ACCEPTED_OLDER = 'accepted-older';
+ const STATUS_REJECTED_OLDER = 'rejected-older';
/**
* Returns the relative strength of a status, used to pick a winner when a
@@ -27,8 +29,11 @@
self::STATUS_BLOCKING => 3,
- self::STATUS_ACCEPTED => 4,
- self::STATUS_REJECTED => 4,
+ self::STATUS_ACCEPTED_OLDER => 4,
+ self::STATUS_REJECTED_OLDER => 4,
+
+ self::STATUS_ACCEPTED => 5,
+ self::STATUS_REJECTED => 5,
);
return idx($map, $constant, 0);
Index: src/applications/differential/editor/DifferentialTransactionEditor.php
===================================================================
--- src/applications/differential/editor/DifferentialTransactionEditor.php
+++ src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -137,6 +137,9 @@
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
+ $status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW;
+ $status_revision = ArcanistDifferentialRevisionStatus::NEEDS_REVISION;
+
switch ($xaction->getTransactionType()) {
case PhabricatorTransactions::TYPE_VIEW_POLICY:
$object->setViewPolicy($xaction->getNewValue());
@@ -151,12 +154,10 @@
case PhabricatorTransactions::TYPE_EDGE:
return;
case DifferentialTransaction::TYPE_UPDATE:
+ $object->setStatus($status_review);
// TODO: Update the `diffPHID` once we add that.
return;
case DifferentialTransaction::TYPE_ACTION:
- $status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW;
- $status_revision = ArcanistDifferentialRevisionStatus::NEEDS_REVISION;
-
switch ($xaction->getNewValue()) {
case DifferentialAction::ACTION_RESIGN:
case DifferentialAction::ACTION_ACCEPT:
@@ -203,6 +204,38 @@
$results = parent::expandTransaction($object, $xaction);
switch ($xaction->getTransactionType()) {
+ case DifferentialTransaction::TYPE_UPDATE:
+ $new_accept = DifferentialReviewerStatus::STATUS_ACCEPTED;
+ $new_reject = DifferentialReviewerStatus::STATUS_REJECTED;
+ $old_accept = DifferentialReviewerStatus::STATUS_ACCEPTED_OLDER;
+ $old_reject = DifferentialReviewerStatus::STATUS_REJECTED_OLDER;
+
+ // When a revision is updated, change all "reject" to "rejected older
+ // revision". This means we won't immediately push the update back into
+ // "needs review", but outstanding rejects will still block it from
+ // moving to "accepted".
+ $edits = array();
+ foreach ($object->getReviewerStatus() as $reviewer) {
+ if ($reviewer->getStatus() == $new_reject) {
+ $edits[$reviewer->getReviewerPHID()] = array(
+ 'data' => array(
+ 'status' => $old_reject,
+ ),
+ );
+ }
+
+ // TODO: If sticky accept is off, do a similar update for accepts.
+ }
+
+ if ($edits) {
+ $results[] = id(new DifferentialTransaction())
+ ->setTransactionType($type_edge)
+ ->setMetadataValue('edge:type', $edge_reviewer)
+ ->setIgnoreOnNoEffect(true)
+ ->setNewValue(array('+' => $edits));
+ }
+ break;
+
case PhabricatorTransactions::TYPE_COMMENT:
// When a user leaves a comment, upgrade their reviewer status from
// "added" to "commented" if they're also a reviewer. We may further
@@ -433,10 +466,12 @@
//
// - at least one accepting reviewer who is a user; and
// - no rejects; and
+ // - no rejects of older diffs; and
// - no blocking reviewers.
$has_accepting_user = false;
$has_rejecting_reviewer = false;
+ $has_rejecting_older_reviewer = false;
$has_blocking_reviewer = false;
foreach ($new_revision->getReviewerStatus() as $reviewer) {
$reviewer_status = $reviewer->getStatus();
@@ -444,6 +479,9 @@
case DifferentialReviewerStatus::STATUS_REJECTED:
$has_rejecting_reviewer = true;
break;
+ case DifferentialReviewerStatus::STATUS_REJECTED_OLDER:
+ $has_rejecting_older_reviewer = true;
+ break;
case DifferentialReviewerStatus::STATUS_BLOCKING:
$has_blocking_reviewer = true;
break;
@@ -458,6 +496,7 @@
$new_status = null;
if ($has_accepting_user &&
!$has_rejecting_reviewer &&
+ !$has_rejecting_older_reviewer &&
!$has_blocking_reviewer) {
$new_status = $status_accepted;
} else if ($has_rejecting_reviewer) {
Index: src/applications/differential/view/DifferentialReviewersView.php
===================================================================
--- src/applications/differential/view/DifferentialReviewersView.php
+++ src/applications/differential/view/DifferentialReviewersView.php
@@ -58,6 +58,12 @@
}
break;
+ case DifferentialReviewerStatus::STATUS_ACCEPTED_OLDER:
+ $item->setIcon(
+ 'accept-dark',
+ pht('Accepted Prior Diff'));
+ break;
+
case DifferentialReviewerStatus::STATUS_REJECTED:
if ($is_current) {
$item->setIcon(
@@ -70,6 +76,12 @@
}
break;
+ case DifferentialReviewerStatus::STATUS_REJECTED_OLDER:
+ $item->setIcon(
+ 'reject-dark',
+ pht('Rejected Prior Diff'));
+ break;
+
case DifferentialReviewerStatus::STATUS_COMMENTED:
if ($is_current) {
$item->setIcon(

File Metadata

Mime Type
text/plain
Expires
Fri, Mar 28, 2:30 PM (1 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7221770
Default Alt Text
D8402.diff (6 KB)

Event Timeline

Phabricator · Built by Phacility