2014-02-19 01:32:55 +01:00
|
|
|
<?php
|
|
|
|
|
|
|
|
final class DifferentialTransactionEditor
|
|
|
|
extends PhabricatorApplicationTransactionEditor {
|
|
|
|
|
|
|
|
public function getTransactionTypes() {
|
|
|
|
$types = parent::getTransactionTypes();
|
|
|
|
|
2014-02-25 00:57:26 +01:00
|
|
|
$types[] = PhabricatorTransactions::TYPE_COMMENT;
|
2014-02-21 20:54:32 +01:00
|
|
|
$types[] = PhabricatorTransactions::TYPE_EDGE;
|
2014-02-19 01:32:55 +01:00
|
|
|
$types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
|
|
|
|
$types[] = PhabricatorTransactions::TYPE_EDIT_POLICY;
|
|
|
|
|
2014-02-25 00:57:26 +01:00
|
|
|
$types[] = DifferentialTransaction::TYPE_ACTION;
|
2014-02-25 00:57:35 +01:00
|
|
|
$types[] = DifferentialTransaction::TYPE_INLINE;
|
Update overall revision status after reviewers change
Summary:
Ref T2222. This doesn't feel super clean, but doesn't feel too bad either.
Basically, Differential transactions can have secondary state-based effects (changing the overall revision status) when reviewers resign, are removed, accept, or reject revisions.
To deal with this in ApplicationTransactions, I did this:
- `applyFinalEffects()` can now alter the transaction set (notably, add new ones). This mostly matters for email, notifications and feed.
- In Differential, check for an overall revision state transition in `applyFinalEffects()` (e.g., your reject moving the revision to a rejected state).
- I'm only writing the transaction if the transition is implied and indirect.
- For example, if you "Plan Changes", that action changes the state on its own so there's no implicit state change transaction added.
The transactions themselves are kind of fluff, but it seems useful to keep a record of when state changes occurred in the transaction log. If people complain we can hide/remove them.
Test Plan: {F118143}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2222
Differential Revision: https://secure.phabricator.com/D8339
2014-02-25 21:36:49 +01:00
|
|
|
$types[] = DifferentialTransaction::TYPE_STATUS;
|
2014-02-25 00:57:26 +01:00
|
|
|
|
2014-02-21 20:53:48 +01:00
|
|
|
/*
|
|
|
|
|
2014-02-19 01:32:55 +01:00
|
|
|
$types[] = DifferentialTransaction::TYPE_UPDATE;
|
|
|
|
*/
|
|
|
|
|
|
|
|
return $types;
|
|
|
|
}
|
|
|
|
|
|
|
|
protected function getCustomTransactionOldValue(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
PhabricatorApplicationTransaction $xaction) {
|
|
|
|
|
|
|
|
switch ($xaction->getTransactionType()) {
|
2014-02-21 20:53:48 +01:00
|
|
|
case PhabricatorTransactions::TYPE_VIEW_POLICY:
|
|
|
|
return $object->getViewPolicy();
|
|
|
|
case PhabricatorTransactions::TYPE_EDIT_POLICY:
|
|
|
|
return $object->getEditPolicy();
|
2014-02-25 00:57:26 +01:00
|
|
|
case DifferentialTransaction::TYPE_ACTION:
|
|
|
|
return null;
|
2014-02-25 00:57:35 +01:00
|
|
|
case DifferentialTransaction::TYPE_INLINE:
|
|
|
|
return null;
|
2014-02-19 01:32:55 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
return parent::getCustomTransactionOldValue($object, $xaction);
|
|
|
|
}
|
|
|
|
|
|
|
|
protected function getCustomTransactionNewValue(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
PhabricatorApplicationTransaction $xaction) {
|
|
|
|
|
|
|
|
switch ($xaction->getTransactionType()) {
|
2014-02-21 20:53:48 +01:00
|
|
|
case PhabricatorTransactions::TYPE_VIEW_POLICY:
|
|
|
|
case PhabricatorTransactions::TYPE_EDIT_POLICY:
|
2014-02-25 00:57:26 +01:00
|
|
|
case DifferentialTransaction::TYPE_ACTION:
|
2014-02-21 20:53:48 +01:00
|
|
|
return $xaction->getNewValue();
|
2014-02-25 00:57:35 +01:00
|
|
|
case DifferentialTransaction::TYPE_INLINE:
|
|
|
|
return null;
|
2014-02-19 01:32:55 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
return parent::getCustomTransactionNewValue($object, $xaction);
|
|
|
|
}
|
|
|
|
|
2014-02-25 00:57:26 +01:00
|
|
|
protected function transactionHasEffect(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
PhabricatorApplicationTransaction $xaction) {
|
|
|
|
|
|
|
|
switch ($xaction->getTransactionType()) {
|
2014-02-25 00:57:35 +01:00
|
|
|
case DifferentialTransaction::TYPE_INLINE:
|
|
|
|
return $xaction->hasComment();
|
2014-02-25 00:57:49 +01:00
|
|
|
case DifferentialTransaction::TYPE_ACTION:
|
|
|
|
$status_closed = ArcanistDifferentialRevisionStatus::CLOSED;
|
|
|
|
$status_abandoned = ArcanistDifferentialRevisionStatus::ABANDONED;
|
|
|
|
$status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW;
|
|
|
|
$status_revision = ArcanistDifferentialRevisionStatus::NEEDS_REVISION;
|
|
|
|
|
2014-02-25 21:36:39 +01:00
|
|
|
$action_type = $xaction->getNewValue();
|
|
|
|
switch ($action_type) {
|
|
|
|
case DifferentialAction::ACTION_ACCEPT:
|
|
|
|
case DifferentialAction::ACTION_REJECT:
|
|
|
|
if ($action_type == DifferentialAction::ACTION_ACCEPT) {
|
|
|
|
$new_status = DifferentialReviewerStatus::STATUS_ACCEPTED;
|
|
|
|
} else {
|
|
|
|
$new_status = DifferentialReviewerStatus::STATUS_REJECTED;
|
|
|
|
}
|
|
|
|
|
|
|
|
$actor = $this->getActor();
|
|
|
|
$actor_phid = $actor->getPHID();
|
|
|
|
|
|
|
|
// These transactions can cause effects in to ways: by altering the
|
|
|
|
// status of an existing reviewer; or by adding the actor as a new
|
|
|
|
// reviewer.
|
|
|
|
|
|
|
|
$will_add_reviewer = true;
|
|
|
|
foreach ($object->getReviewerStatus() as $reviewer) {
|
|
|
|
if ($reviewer->hasAuthority($actor)) {
|
|
|
|
if ($reviewer->getStatus() != $new_status) {
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
if ($reviewer->getReviewerPHID() == $actor_phid) {
|
|
|
|
$will_add_reviwer = false;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
return $will_add_reviewer;
|
2014-02-25 00:57:49 +01:00
|
|
|
case DifferentialAction::ACTION_CLOSE:
|
|
|
|
return ($object->getStatus() != $status_closed);
|
|
|
|
case DifferentialAction::ACTION_ABANDON:
|
|
|
|
return ($object->getStatus() != $status_abandoned);
|
|
|
|
case DifferentialAction::ACTION_RECLAIM:
|
|
|
|
return ($object->getStatus() == $status_abandoned);
|
|
|
|
case DifferentialAction::ACTION_REOPEN:
|
|
|
|
return ($object->getStatus() == $status_closed);
|
|
|
|
case DifferentialAction::ACTION_RETHINK:
|
|
|
|
return ($object->getStatus() != $status_revision);
|
|
|
|
case DifferentialAction::ACTION_REQUEST:
|
|
|
|
return ($object->getStatus() != $status_review);
|
2014-02-25 21:36:02 +01:00
|
|
|
case DifferentialAction::ACTION_RESIGN:
|
|
|
|
$actor_phid = $this->getActor()->getPHID();
|
|
|
|
foreach ($object->getReviewerStatus() as $reviewer) {
|
|
|
|
if ($reviewer->getReviewerPHID() == $actor_phid) {
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
return false;
|
2014-02-25 21:36:24 +01:00
|
|
|
case DifferentialAction::ACTION_CLAIM:
|
|
|
|
$actor_phid = $this->getActor()->getPHID();
|
|
|
|
return ($actor_phid != $object->getAuthorPHID());
|
2014-02-25 00:57:49 +01:00
|
|
|
}
|
2014-02-25 00:57:26 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
return parent::transactionHasEffect($object, $xaction);
|
|
|
|
}
|
|
|
|
|
|
|
|
|
2014-02-19 01:32:55 +01:00
|
|
|
protected function applyCustomInternalTransaction(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
PhabricatorApplicationTransaction $xaction) {
|
|
|
|
|
|
|
|
switch ($xaction->getTransactionType()) {
|
2014-02-21 20:53:48 +01:00
|
|
|
case PhabricatorTransactions::TYPE_VIEW_POLICY:
|
|
|
|
$object->setViewPolicy($xaction->getNewValue());
|
|
|
|
return;
|
|
|
|
case PhabricatorTransactions::TYPE_EDIT_POLICY:
|
|
|
|
$object->setEditPolicy($xaction->getNewValue());
|
|
|
|
return;
|
2014-02-21 20:54:08 +01:00
|
|
|
case PhabricatorTransactions::TYPE_SUBSCRIBERS:
|
2014-02-25 00:57:26 +01:00
|
|
|
case PhabricatorTransactions::TYPE_COMMENT:
|
2014-02-25 00:57:35 +01:00
|
|
|
case DifferentialTransaction::TYPE_INLINE:
|
|
|
|
return;
|
2014-02-21 20:54:32 +01:00
|
|
|
case PhabricatorTransactions::TYPE_EDGE:
|
2014-02-21 20:54:08 +01:00
|
|
|
return;
|
2014-02-25 00:57:26 +01:00
|
|
|
case DifferentialTransaction::TYPE_ACTION:
|
2014-02-25 00:57:49 +01:00
|
|
|
$status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW;
|
|
|
|
$status_revision = ArcanistDifferentialRevisionStatus::NEEDS_REVISION;
|
|
|
|
|
|
|
|
switch ($xaction->getNewValue()) {
|
2014-02-25 21:36:02 +01:00
|
|
|
case DifferentialAction::ACTION_RESIGN:
|
2014-02-25 21:36:39 +01:00
|
|
|
case DifferentialAction::ACTION_ACCEPT:
|
|
|
|
case DifferentialAction::ACTION_REJECT:
|
|
|
|
// These have no direct effects, and affect review status only
|
|
|
|
// indirectly by altering reviewers with TYPE_EDGE transactions.
|
|
|
|
return;
|
2014-02-25 00:57:49 +01:00
|
|
|
case DifferentialAction::ACTION_ABANDON:
|
|
|
|
$object->setStatus(ArcanistDifferentialRevisionStatus::ABANDONED);
|
2014-02-25 21:36:39 +01:00
|
|
|
return;
|
2014-02-25 00:57:49 +01:00
|
|
|
case DifferentialAction::ACTION_RETHINK:
|
|
|
|
$object->setStatus($status_revision);
|
2014-02-25 21:36:39 +01:00
|
|
|
return;
|
2014-02-25 00:57:49 +01:00
|
|
|
case DifferentialAction::ACTION_RECLAIM:
|
|
|
|
$object->setStatus($status_review);
|
2014-02-25 21:36:39 +01:00
|
|
|
return;
|
2014-02-25 00:57:49 +01:00
|
|
|
case DifferentialAction::ACTION_REOPEN:
|
|
|
|
$object->setStatus($status_review);
|
2014-02-25 21:36:39 +01:00
|
|
|
return;
|
2014-02-25 00:57:49 +01:00
|
|
|
case DifferentialAction::ACTION_REQUEST:
|
|
|
|
$object->setStatus($status_review);
|
2014-02-25 21:36:39 +01:00
|
|
|
return;
|
2014-02-25 00:57:49 +01:00
|
|
|
case DifferentialAction::ACTION_CLOSE:
|
|
|
|
$object->setStatus(ArcanistDifferentialRevisionStatus::CLOSED);
|
2014-02-25 21:36:39 +01:00
|
|
|
return;
|
2014-02-25 21:36:24 +01:00
|
|
|
case DifferentialAction::ACTION_CLAIM:
|
|
|
|
$object->setAuthorPHID($this->getActor()->getPHID());
|
2014-02-25 21:36:39 +01:00
|
|
|
return;
|
2014-02-25 00:57:49 +01:00
|
|
|
}
|
2014-02-25 21:36:39 +01:00
|
|
|
break;
|
2014-02-19 01:32:55 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
return parent::applyCustomInternalTransaction($object, $xaction);
|
|
|
|
}
|
|
|
|
|
2014-02-25 21:36:02 +01:00
|
|
|
protected function expandTransaction(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
PhabricatorApplicationTransaction $xaction) {
|
|
|
|
|
2014-02-25 21:37:11 +01:00
|
|
|
$actor = $this->getActor();
|
|
|
|
$actor_phid = $actor->getPHID();
|
|
|
|
$type_edge = PhabricatorTransactions::TYPE_EDGE;
|
|
|
|
$edge_reviewer = PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER;
|
|
|
|
|
2014-02-25 21:36:02 +01:00
|
|
|
$results = parent::expandTransaction($object, $xaction);
|
|
|
|
switch ($xaction->getTransactionType()) {
|
2014-02-25 21:37:11 +01:00
|
|
|
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
|
|
|
|
// upgrade this based on other actions in the transaction group.
|
|
|
|
|
|
|
|
$status_added = DifferentialReviewerStatus::STATUS_ADDED;
|
|
|
|
$status_commented = DifferentialReviewerStatus::STATUS_COMMENTED;
|
|
|
|
|
|
|
|
$data = array(
|
|
|
|
'status' => $status_commented,
|
|
|
|
);
|
|
|
|
|
|
|
|
$edits = array();
|
|
|
|
foreach ($object->getReviewerStatus() as $reviewer) {
|
|
|
|
if ($reviewer->getReviewerPHID() == $actor_phid) {
|
|
|
|
if ($reviewer->getStatus() == $status_added) {
|
|
|
|
$edits[$actor_phid] = array(
|
|
|
|
'data' => $data,
|
|
|
|
);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
2014-02-25 21:36:39 +01:00
|
|
|
|
2014-02-25 21:37:11 +01:00
|
|
|
if ($edits) {
|
|
|
|
$results[] = id(new DifferentialTransaction())
|
|
|
|
->setTransactionType($type_edge)
|
|
|
|
->setMetadataValue('edge:type', $edge_reviewer)
|
|
|
|
->setIgnoreOnNoEffect(true)
|
|
|
|
->setNewValue(array('+' => $edits));
|
|
|
|
}
|
|
|
|
break;
|
2014-02-25 21:36:24 +01:00
|
|
|
|
2014-02-25 21:37:11 +01:00
|
|
|
case DifferentialTransaction::TYPE_ACTION:
|
2014-02-25 21:36:39 +01:00
|
|
|
$action_type = $xaction->getNewValue();
|
|
|
|
|
|
|
|
switch ($action_type) {
|
|
|
|
case DifferentialAction::ACTION_ACCEPT:
|
|
|
|
case DifferentialAction::ACTION_REJECT:
|
|
|
|
if ($action_type == DifferentialAction::ACTION_ACCEPT) {
|
|
|
|
$data = array(
|
|
|
|
'status' => DifferentialReviewerStatus::STATUS_ACCEPTED,
|
|
|
|
);
|
|
|
|
} else {
|
|
|
|
$data = array(
|
|
|
|
'status' => DifferentialReviewerStatus::STATUS_REJECTED,
|
|
|
|
);
|
|
|
|
}
|
|
|
|
|
|
|
|
$edits = array();
|
|
|
|
|
|
|
|
foreach ($object->getReviewerStatus() as $reviewer) {
|
|
|
|
if ($reviewer->hasAuthority($actor)) {
|
|
|
|
$edits[$reviewer->getReviewerPHID()] = array(
|
|
|
|
'data' => $data,
|
|
|
|
);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
// Also either update or add the actor themselves as a reviewer.
|
|
|
|
$edits[$actor_phid] = array(
|
|
|
|
'data' => $data,
|
|
|
|
);
|
|
|
|
|
|
|
|
$results[] = id(new DifferentialTransaction())
|
|
|
|
->setTransactionType($type_edge)
|
|
|
|
->setMetadataValue('edge:type', $edge_reviewer)
|
|
|
|
->setIgnoreOnNoEffect(true)
|
|
|
|
->setNewValue(array('+' => $edits));
|
|
|
|
break;
|
2014-02-25 21:36:24 +01:00
|
|
|
|
|
|
|
case DifferentialAction::ACTION_CLAIM:
|
|
|
|
// If the user is commandeering, add the previous owner as a
|
|
|
|
// reviewer and remove the actor.
|
|
|
|
|
|
|
|
$edits = array(
|
|
|
|
'-' => array(
|
|
|
|
$actor_phid => $actor_phid,
|
|
|
|
),
|
|
|
|
);
|
|
|
|
|
|
|
|
$owner_phid = $object->getAuthorPHID();
|
|
|
|
if ($owner_phid) {
|
|
|
|
$reviewer = new DifferentialReviewer(
|
|
|
|
$owner_phid,
|
|
|
|
array(
|
|
|
|
'status' => DifferentialReviewerStatus::STATUS_ADDED,
|
|
|
|
));
|
|
|
|
|
|
|
|
$edits['+'] = array(
|
|
|
|
$owner_phid => array(
|
|
|
|
'data' => $reviewer->getEdgeData(),
|
|
|
|
),
|
|
|
|
);
|
|
|
|
}
|
|
|
|
|
|
|
|
$results[] = id(new DifferentialTransaction())
|
|
|
|
->setTransactionType($type_edge)
|
|
|
|
->setMetadataValue('edge:type', $edge_reviewer)
|
|
|
|
->setIgnoreOnNoEffect(true)
|
|
|
|
->setNewValue($edits);
|
|
|
|
|
|
|
|
break;
|
2014-02-25 21:36:02 +01:00
|
|
|
case DifferentialAction::ACTION_RESIGN:
|
|
|
|
// If the user is resigning, add a separate reviewer edit
|
|
|
|
// transaction which removes them as a reviewer.
|
|
|
|
|
|
|
|
$results[] = id(new DifferentialTransaction())
|
|
|
|
->setTransactionType($type_edge)
|
|
|
|
->setMetadataValue('edge:type', $edge_reviewer)
|
|
|
|
->setIgnoreOnNoEffect(true)
|
|
|
|
->setNewValue(
|
|
|
|
array(
|
|
|
|
'-' => array(
|
|
|
|
$actor_phid => $actor_phid,
|
|
|
|
),
|
|
|
|
));
|
|
|
|
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
|
|
|
|
return $results;
|
|
|
|
}
|
|
|
|
|
2014-02-19 01:32:55 +01:00
|
|
|
protected function applyCustomExternalTransaction(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
PhabricatorApplicationTransaction $xaction) {
|
|
|
|
|
|
|
|
switch ($xaction->getTransactionType()) {
|
2014-02-21 20:53:48 +01:00
|
|
|
case PhabricatorTransactions::TYPE_VIEW_POLICY:
|
|
|
|
case PhabricatorTransactions::TYPE_EDIT_POLICY:
|
|
|
|
return;
|
2014-02-21 20:54:08 +01:00
|
|
|
case PhabricatorTransactions::TYPE_SUBSCRIBERS:
|
2014-02-21 20:54:32 +01:00
|
|
|
case PhabricatorTransactions::TYPE_EDGE:
|
2014-02-25 00:57:26 +01:00
|
|
|
case PhabricatorTransactions::TYPE_COMMENT:
|
|
|
|
case DifferentialTransaction::TYPE_ACTION:
|
2014-02-25 00:57:35 +01:00
|
|
|
case DifferentialTransaction::TYPE_INLINE:
|
2014-02-21 20:54:08 +01:00
|
|
|
return;
|
2014-02-19 01:32:55 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
return parent::applyCustomExternalTransaction($object, $xaction);
|
|
|
|
}
|
|
|
|
|
2014-02-25 21:37:11 +01:00
|
|
|
protected function mergeEdgeData($type, array $u, array $v) {
|
|
|
|
$result = parent::mergeEdgeData($type, $u, $v);
|
|
|
|
|
|
|
|
switch ($type) {
|
|
|
|
case PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER:
|
|
|
|
// When the same reviewer has their status updated by multiple
|
|
|
|
// transactions, we want the strongest status to win. An example of
|
|
|
|
// this is when a user adds a comment and also accepts a revision which
|
|
|
|
// they are a reviewer on. The comment creates a "commented" status,
|
|
|
|
// while the accept creates an "accepted" status. Since accept is
|
|
|
|
// stronger, it should win and persist.
|
|
|
|
|
|
|
|
$u_status = idx($u, 'status');
|
|
|
|
$v_status = idx($v, 'status');
|
|
|
|
$u_str = DifferentialReviewerStatus::getStatusStrength($u_status);
|
|
|
|
$v_str = DifferentialReviewerStatus::getStatusStrength($v_status);
|
|
|
|
if ($u_str > $v_str) {
|
|
|
|
$result['status'] = $u_status;
|
|
|
|
} else {
|
|
|
|
$result['status'] = $v_status;
|
|
|
|
}
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
|
|
|
|
return $result;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
|
Update overall revision status after reviewers change
Summary:
Ref T2222. This doesn't feel super clean, but doesn't feel too bad either.
Basically, Differential transactions can have secondary state-based effects (changing the overall revision status) when reviewers resign, are removed, accept, or reject revisions.
To deal with this in ApplicationTransactions, I did this:
- `applyFinalEffects()` can now alter the transaction set (notably, add new ones). This mostly matters for email, notifications and feed.
- In Differential, check for an overall revision state transition in `applyFinalEffects()` (e.g., your reject moving the revision to a rejected state).
- I'm only writing the transaction if the transition is implied and indirect.
- For example, if you "Plan Changes", that action changes the state on its own so there's no implicit state change transaction added.
The transactions themselves are kind of fluff, but it seems useful to keep a record of when state changes occurred in the transaction log. If people complain we can hide/remove them.
Test Plan: {F118143}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2222
Differential Revision: https://secure.phabricator.com/D8339
2014-02-25 21:36:49 +01:00
|
|
|
protected function applyFinalEffects(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
array $xactions) {
|
|
|
|
|
|
|
|
$status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED;
|
|
|
|
$status_revision = ArcanistDifferentialRevisionStatus::NEEDS_REVISION;
|
|
|
|
$status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW;
|
|
|
|
|
|
|
|
$old_status = $object->getStatus();
|
|
|
|
switch ($old_status) {
|
|
|
|
case $status_accepted:
|
|
|
|
case $status_revision:
|
|
|
|
case $status_review:
|
|
|
|
// Load the most up-to-date version of the revision and its reviewers,
|
|
|
|
// so we don't need to try to deduce the state of reviewers by examining
|
|
|
|
// all the changes made by the transactions.
|
|
|
|
$new_revision = id(new DifferentialRevisionQuery())
|
|
|
|
->setViewer($this->getActor())
|
|
|
|
->needReviewerStatus(true)
|
|
|
|
->withIDs(array($object->getID()))
|
|
|
|
->executeOne();
|
|
|
|
if (!$new_revision) {
|
|
|
|
throw new Exception(
|
|
|
|
pht('Failed to load revision from transaction finalization.'));
|
|
|
|
}
|
|
|
|
|
|
|
|
// Try to move a revision to "accepted". We look for:
|
|
|
|
//
|
|
|
|
// - at least one accepting reviewer who is a user; and
|
|
|
|
// - no rejects; and
|
|
|
|
// - no blocking reviewers.
|
|
|
|
|
|
|
|
$has_accepting_user = false;
|
|
|
|
$has_rejecting_reviewer = false;
|
|
|
|
$has_blocking_reviewer = false;
|
|
|
|
foreach ($new_revision->getReviewerStatus() as $reviewer) {
|
|
|
|
$reviewer_status = $reviewer->getStatus();
|
|
|
|
switch ($reviewer_status) {
|
|
|
|
case DifferentialReviewerStatus::STATUS_REJECTED:
|
|
|
|
$has_rejecting_reviewer = true;
|
|
|
|
break;
|
|
|
|
case DifferentialReviewerStatus::STATUS_BLOCKING:
|
|
|
|
$has_blocking_reviewer = true;
|
|
|
|
break;
|
|
|
|
case DifferentialReviewerStatus::STATUS_ACCEPTED:
|
|
|
|
if ($reviewer->isUser()) {
|
|
|
|
$has_accepting_user = true;
|
|
|
|
}
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
$new_status = null;
|
|
|
|
if ($has_accepting_user &&
|
|
|
|
!$has_rejecting_reviewer &&
|
|
|
|
!$has_blocking_reviewer) {
|
|
|
|
$new_status = $status_accepted;
|
|
|
|
} else if ($has_rejecting_reviewer) {
|
|
|
|
// This isn't accepted, and there's at least one rejecting reviewer,
|
|
|
|
// so the revision needs changes. This usually happens after a
|
|
|
|
// "reject".
|
|
|
|
$new_status = $status_revision;
|
|
|
|
} else if ($old_status == $status_accepted) {
|
|
|
|
// This revision was accepted, but it no longer satisfies the
|
|
|
|
// conditions for acceptance. This usually happens after an accepting
|
|
|
|
// reviewer resigns or is removed.
|
|
|
|
$new_status = $status_review;
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($new_status !== null && $new_status != $old_status) {
|
|
|
|
$xaction = id(new DifferentialTransaction())
|
|
|
|
->setTransactionType(DifferentialTransaction::TYPE_STATUS)
|
|
|
|
->setOldValue($old_status)
|
|
|
|
->setNewValue($new_status);
|
|
|
|
|
|
|
|
$xaction = $this->populateTransaction($object, $xaction)->save();
|
|
|
|
|
|
|
|
$xactions[] = $xaction;
|
|
|
|
|
|
|
|
$object->setStatus($new_status)->save();
|
|
|
|
}
|
|
|
|
break;
|
|
|
|
default:
|
|
|
|
// Revisions can't transition out of other statuses (like closed or
|
|
|
|
// abandoned) as a side effect of reviewer status changes.
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
|
|
|
|
return $xactions;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
2014-02-19 01:32:55 +01:00
|
|
|
protected function validateTransaction(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
$type,
|
|
|
|
array $xactions) {
|
|
|
|
|
|
|
|
$errors = parent::validateTransaction($object, $type, $xactions);
|
|
|
|
|
2014-02-25 00:57:49 +01:00
|
|
|
foreach ($xactions as $xaction) {
|
|
|
|
switch ($type) {
|
|
|
|
case DifferentialTransaction::TYPE_ACTION:
|
|
|
|
$error = $this->validateDifferentialAction(
|
|
|
|
$object,
|
|
|
|
$type,
|
|
|
|
$xaction,
|
|
|
|
$xaction->getNewValue());
|
|
|
|
if ($error) {
|
|
|
|
$errors[] = new PhabricatorApplicationTransactionValidationError(
|
|
|
|
$type,
|
|
|
|
pht('Invalid'),
|
|
|
|
$error,
|
|
|
|
$xaction);
|
|
|
|
}
|
|
|
|
break;
|
|
|
|
}
|
2014-02-19 01:32:55 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
return $errors;
|
|
|
|
}
|
|
|
|
|
2014-02-25 00:57:49 +01:00
|
|
|
private function validateDifferentialAction(
|
|
|
|
DifferentialRevision $revision,
|
|
|
|
$type,
|
|
|
|
DifferentialTransaction $xaction,
|
|
|
|
$action) {
|
|
|
|
|
|
|
|
$author_phid = $revision->getAuthorPHID();
|
|
|
|
$actor_phid = $this->getActor()->getPHID();
|
|
|
|
$actor_is_author = ($author_phid == $actor_phid);
|
|
|
|
|
|
|
|
$config_close_key = 'differential.always-allow-close';
|
|
|
|
$always_allow_close = PhabricatorEnv::getEnvConfig($config_close_key);
|
|
|
|
|
|
|
|
$config_reopen_key = 'differential.allow-reopen';
|
|
|
|
$allow_reopen = PhabricatorEnv::getEnvConfig($config_reopen_key);
|
|
|
|
|
2014-02-25 21:36:39 +01:00
|
|
|
$config_self_accept_key = 'differential.allow-self-accept';
|
|
|
|
$allow_self_accept = PhabricatorEnv::getEnvConfig($config_self_accept_key);
|
|
|
|
|
2014-02-25 00:57:49 +01:00
|
|
|
$revision_status = $revision->getStatus();
|
|
|
|
|
|
|
|
$status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED;
|
|
|
|
$status_abandoned = ArcanistDifferentialRevisionStatus::ABANDONED;
|
|
|
|
$status_closed = ArcanistDifferentialRevisionStatus::CLOSED;
|
|
|
|
|
|
|
|
switch ($action) {
|
2014-02-25 21:36:39 +01:00
|
|
|
case DifferentialAction::ACTION_ACCEPT:
|
|
|
|
if ($actor_is_author && !$allow_self_accept) {
|
|
|
|
return pht(
|
|
|
|
'You can not accept this revision because you are the owner.');
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($revision_status == $status_abandoned) {
|
|
|
|
return pht(
|
|
|
|
'You can not accept this revision because it has been '.
|
|
|
|
'abandoned.');
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($revision_status == $status_closed) {
|
|
|
|
return pht(
|
|
|
|
'You can not accept this revision because it has already been '.
|
|
|
|
'closed.');
|
|
|
|
}
|
|
|
|
break;
|
|
|
|
|
|
|
|
case DifferentialAction::ACTION_REJECT:
|
|
|
|
if ($actor_is_author) {
|
|
|
|
return pht(
|
|
|
|
'You can not request changes to your own revision.');
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($revision_status == $status_abandoned) {
|
|
|
|
return pht(
|
|
|
|
'You can not request changes to this revision because it has been '.
|
|
|
|
'abandoned.');
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($revision_status == $status_closed) {
|
|
|
|
return pht(
|
|
|
|
'You can not request changes to this revision because it has '.
|
|
|
|
'already been closed.');
|
|
|
|
}
|
|
|
|
break;
|
|
|
|
|
2014-02-25 21:36:02 +01:00
|
|
|
case DifferentialAction::ACTION_RESIGN:
|
|
|
|
// You can always resign from a revision if you're a reviewer. If you
|
|
|
|
// aren't, this is a no-op rather than invalid.
|
|
|
|
break;
|
|
|
|
|
2014-02-25 21:36:24 +01:00
|
|
|
case DifferentialAction::ACTION_CLAIM:
|
|
|
|
// You can claim a revision if you're not the owner. If you are, this
|
|
|
|
// is a no-op rather than invalid.
|
|
|
|
|
|
|
|
if ($revision_status == $status_closed) {
|
|
|
|
return pht(
|
|
|
|
"You can not commandeer this revision because it has already been ".
|
|
|
|
"closed.");
|
|
|
|
}
|
|
|
|
break;
|
|
|
|
|
2014-02-25 00:57:49 +01:00
|
|
|
case DifferentialAction::ACTION_ABANDON:
|
|
|
|
if (!$actor_is_author) {
|
|
|
|
return pht(
|
|
|
|
"You can not abandon this revision because you do not own it. ".
|
|
|
|
"You can only abandon revisions you own.");
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($revision_status == $status_closed) {
|
|
|
|
return pht(
|
|
|
|
"You can not abandon this revision because it has already been ".
|
|
|
|
"closed.");
|
|
|
|
}
|
|
|
|
|
|
|
|
// NOTE: Abandons of already-abandoned revisions are treated as no-op
|
|
|
|
// instead of invalid. Other abandons are OK.
|
|
|
|
|
|
|
|
break;
|
|
|
|
|
|
|
|
case DifferentialAction::ACTION_RECLAIM:
|
|
|
|
if (!$actor_is_author) {
|
|
|
|
return pht(
|
|
|
|
"You can not reclaim this revision because you do not own ".
|
|
|
|
"it. You can only reclaim revisions you own.");
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($revision_status == $status_closed) {
|
|
|
|
return pht(
|
|
|
|
"You can not reclaim this revision because it has already been ".
|
|
|
|
"closed.");
|
|
|
|
}
|
|
|
|
|
|
|
|
// NOTE: Reclaims of other non-abandoned revisions are treated as no-op
|
|
|
|
// instead of invalid.
|
|
|
|
|
|
|
|
break;
|
|
|
|
|
|
|
|
case DifferentialAction::ACTION_REOPEN:
|
|
|
|
if (!$allow_reopen) {
|
|
|
|
return pht(
|
|
|
|
'The reopen action is not enabled on this Phabricator install. '.
|
|
|
|
'Adjust your configuration to enable it.');
|
|
|
|
}
|
|
|
|
|
|
|
|
// NOTE: If the revision is not closed, this is caught as a no-op
|
|
|
|
// instead of an invalid transaction.
|
|
|
|
|
|
|
|
break;
|
|
|
|
|
|
|
|
case DifferentialAction::ACTION_RETHINK:
|
|
|
|
if (!$actor_is_author) {
|
|
|
|
return pht(
|
|
|
|
"You can not plan changes to this revision because you do not ".
|
|
|
|
"own it. To plan chagnes to a revision, you must be its owner.");
|
|
|
|
}
|
|
|
|
|
|
|
|
switch ($revision_status) {
|
|
|
|
case ArcanistDifferentialRevisionStatus::ACCEPTED:
|
|
|
|
case ArcanistDifferentialRevisionStatus::NEEDS_REVISION:
|
|
|
|
case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW:
|
|
|
|
// These are OK.
|
|
|
|
break;
|
|
|
|
case ArcanistDifferentialRevisionStatus::ABANDONED:
|
|
|
|
return pht(
|
|
|
|
"You can not plan changes to this revision because it has ".
|
|
|
|
"been abandoned.");
|
|
|
|
case ArcanistDifferentialRevisionStatus::CLOSED:
|
|
|
|
return pht(
|
|
|
|
"You can not plan changes to this revision because it has ".
|
|
|
|
"already been closed.");
|
|
|
|
default:
|
|
|
|
throw new Exception(
|
|
|
|
pht(
|
|
|
|
'Encountered unexpected revision status ("%s") when '.
|
|
|
|
'validating "%s" action.',
|
|
|
|
$revision_status,
|
|
|
|
$action));
|
|
|
|
}
|
|
|
|
break;
|
|
|
|
|
|
|
|
case DifferentialAction::ACTION_REQUEST:
|
|
|
|
if (!$actor_is_author) {
|
|
|
|
return pht(
|
|
|
|
"You can not request review of this revision because you do ".
|
|
|
|
"not own it. To request review of a revision, you must be its ".
|
|
|
|
"owner.");
|
|
|
|
}
|
|
|
|
|
|
|
|
switch ($revision_status) {
|
|
|
|
case ArcanistDifferentialRevisionStatus::ACCEPTED:
|
|
|
|
case ArcanistDifferentialRevisionStatus::NEEDS_REVISION:
|
|
|
|
// These are OK.
|
|
|
|
break;
|
|
|
|
case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW:
|
|
|
|
// This will be caught as "no effect" later on.
|
|
|
|
break;
|
|
|
|
case ArcanistDifferentialRevisionStatus::ABANDONED:
|
|
|
|
return pht(
|
|
|
|
"You can not request review of this revision because it has ".
|
|
|
|
"been abandoned. Instead, reclaim it.");
|
|
|
|
case ArcanistDifferentialRevisionStatus::CLOSED:
|
|
|
|
return pht(
|
|
|
|
"You can not request review of this revision because it has ".
|
|
|
|
"already been closed.");
|
|
|
|
default:
|
|
|
|
throw new Exception(
|
|
|
|
pht(
|
|
|
|
'Encountered unexpected revision status ("%s") when '.
|
|
|
|
'validating "%s" action.',
|
|
|
|
$revision_status,
|
|
|
|
$action));
|
|
|
|
}
|
|
|
|
break;
|
|
|
|
|
|
|
|
case DifferentialAction::ACTION_CLOSE:
|
|
|
|
|
|
|
|
// TODO: Permit the daemons to take this action in all cases.
|
|
|
|
|
|
|
|
if (!$actor_is_author && !$always_allow_close) {
|
|
|
|
return pht(
|
|
|
|
"You can not close this revision because you do not own it. To ".
|
|
|
|
"close a revision, you must be its owner.");
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($revision_status != $status_accepted) {
|
|
|
|
return pht(
|
|
|
|
"You can not close this revision because it has not been ".
|
|
|
|
"accepted. You can only close accepted revisions.");
|
|
|
|
}
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
|
|
|
|
return null;
|
|
|
|
}
|
|
|
|
|
2014-02-25 00:57:35 +01:00
|
|
|
protected function sortTransactions(array $xactions) {
|
|
|
|
$head = array();
|
|
|
|
$tail = array();
|
|
|
|
|
|
|
|
// Move bare comments to the end, so the actions precede them.
|
|
|
|
foreach ($xactions as $xaction) {
|
|
|
|
$type = $xaction->getTransactionType();
|
|
|
|
if ($type == DifferentialTransaction::TYPE_INLINE) {
|
|
|
|
$tail[] = $xaction;
|
|
|
|
} else {
|
|
|
|
$head[] = $xaction;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
return array_values(array_merge($head, $tail));
|
|
|
|
}
|
|
|
|
|
2014-02-19 01:32:55 +01:00
|
|
|
protected function requireCapabilities(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
PhabricatorApplicationTransaction $xaction) {
|
|
|
|
|
|
|
|
switch ($xaction->getTransactionType()) {
|
|
|
|
}
|
|
|
|
|
|
|
|
return parent::requireCapabilities($object, $xaction);
|
|
|
|
}
|
|
|
|
|
2014-02-25 00:57:16 +01:00
|
|
|
protected function shouldSendMail(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
array $xactions) {
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
|
|
|
protected function getMailTo(PhabricatorLiskDAO $object) {
|
|
|
|
$phids = array();
|
|
|
|
$phids[] = $object->getAuthorPHID();
|
|
|
|
foreach ($object->getReviewerStatus() as $reviewer) {
|
|
|
|
$phids[] = $reviewer->getReviewerPHID();
|
|
|
|
}
|
|
|
|
return $phids;
|
|
|
|
}
|
|
|
|
|
|
|
|
protected function getMailSubjectPrefix() {
|
|
|
|
return PhabricatorEnv::getEnvConfig('metamta.differential.subject-prefix');
|
|
|
|
}
|
|
|
|
|
|
|
|
protected function buildReplyHandler(PhabricatorLiskDAO $object) {
|
|
|
|
return id(new DifferentialReplyHandler())
|
|
|
|
->setMailReceiver($object);
|
|
|
|
}
|
|
|
|
|
|
|
|
protected function buildMailTemplate(PhabricatorLiskDAO $object) {
|
|
|
|
$id = $object->getID();
|
|
|
|
$title = $object->getTitle();
|
|
|
|
|
|
|
|
$original_title = $object->getOriginalTitle();
|
|
|
|
|
|
|
|
$subject = "D{$id}: {$title}";
|
|
|
|
$thread_topic = "D{$id}: {$original_title}";
|
|
|
|
|
|
|
|
return id(new PhabricatorMetaMTAMail())
|
|
|
|
->setSubject($subject)
|
|
|
|
->addHeader('Thread-Topic', $thread_topic);
|
|
|
|
}
|
|
|
|
|
|
|
|
protected function buildMailBody(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
array $xactions) {
|
|
|
|
|
|
|
|
$body = parent::buildMailBody($object, $xactions);
|
|
|
|
|
2014-02-26 00:29:10 +01:00
|
|
|
$type_inline = DifferentialTransaction::TYPE_INLINE;
|
|
|
|
|
|
|
|
$inlines = array();
|
|
|
|
foreach ($xactions as $xaction) {
|
|
|
|
if ($xaction->getTransactionType() == $type_inline) {
|
|
|
|
$inlines[] = $xaction;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($inlines) {
|
|
|
|
$body->addTextSection(
|
|
|
|
pht('INLINE COMMENTS'),
|
|
|
|
$this->renderInlineCommentsForMail($object, $inlines));
|
|
|
|
}
|
|
|
|
|
2014-02-25 00:57:16 +01:00
|
|
|
$body->addTextSection(
|
|
|
|
pht('REVISION DETAIL'),
|
|
|
|
PhabricatorEnv::getProductionURI('/D'.$object->getID()));
|
|
|
|
|
2014-02-26 00:29:10 +01:00
|
|
|
|
2014-02-25 00:57:16 +01:00
|
|
|
return $body;
|
|
|
|
}
|
|
|
|
|
2014-02-19 01:32:55 +01:00
|
|
|
protected function supportsSearch() {
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
|
|
|
protected function extractFilePHIDsFromCustomTransaction(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
PhabricatorApplicationTransaction $xaction) {
|
|
|
|
|
|
|
|
switch ($xaction->getTransactionType()) {
|
|
|
|
}
|
|
|
|
|
|
|
|
return parent::extractFilePHIDsFromCustomTransaction($object, $xaction);
|
|
|
|
}
|
|
|
|
|
2014-02-26 00:29:10 +01:00
|
|
|
private function renderInlineCommentsForMail(
|
|
|
|
PhabricatorLiskDAO $object,
|
|
|
|
array $inlines) {
|
|
|
|
|
|
|
|
$context_key = 'metamta.differential.unified-comment-context';
|
|
|
|
$show_context = PhabricatorEnv::getEnvConfig($context_key);
|
|
|
|
|
|
|
|
$changeset_ids = array();
|
|
|
|
foreach ($inlines as $inline) {
|
|
|
|
$id = $inline->getComment()->getChangesetID();
|
|
|
|
$changeset_ids[$id] = $id;
|
|
|
|
}
|
|
|
|
|
|
|
|
// TODO: We should write a proper Query class for this eventually.
|
|
|
|
$changesets = id(new DifferentialChangeset())->loadAllWhere(
|
|
|
|
'id IN (%Ld)',
|
|
|
|
$changeset_ids);
|
|
|
|
if ($show_context) {
|
|
|
|
$hunk_parser = new DifferentialHunkParser();
|
|
|
|
foreach ($changesets as $changeset) {
|
|
|
|
$changeset->attachHunks($changeset->loadHunks());
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
$inline_groups = DifferentialTransactionComment::sortAndGroupInlines(
|
|
|
|
$inlines,
|
|
|
|
$changesets);
|
|
|
|
|
|
|
|
$result = array();
|
|
|
|
foreach ($inline_groups as $changeset_id => $group) {
|
|
|
|
$changeset = idx($changesets, $changeset_id);
|
|
|
|
if (!$changeset) {
|
|
|
|
continue;
|
|
|
|
}
|
|
|
|
|
|
|
|
foreach ($group as $inline) {
|
|
|
|
$comment = $inline->getComment();
|
|
|
|
$file = $changeset->getFilename();
|
|
|
|
$start = $comment->getLineNumber();
|
|
|
|
$len = $comment->getLineLength();
|
|
|
|
if ($len) {
|
|
|
|
$range = $start.'-'.($start + $len);
|
|
|
|
} else {
|
|
|
|
$range = $start;
|
|
|
|
}
|
|
|
|
|
|
|
|
$inline_content = $comment->getContent();
|
|
|
|
|
|
|
|
if (!$show_context) {
|
|
|
|
$result[] = "{$file}:{$range} {$inline_content}";
|
|
|
|
} else {
|
|
|
|
$result[] = "================";
|
|
|
|
$result[] = "Comment at: " . $file . ":" . $range;
|
|
|
|
$result[] = $hunk_parser->makeContextDiff(
|
|
|
|
$changeset->getHunks(),
|
|
|
|
$comment->getIsNewFile(),
|
|
|
|
$comment->getLineNumber(),
|
|
|
|
$comment->getLineLength(),
|
|
|
|
1);
|
|
|
|
$result[] = "----------------";
|
|
|
|
|
|
|
|
$result[] = $inline_content;
|
|
|
|
$result[] = null;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
return implode("\n", $result);
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
|
2014-02-19 01:32:55 +01:00
|
|
|
}
|