Page MenuHomePhabricator
Files F14015249

D15916.diff
No OneTemporary
Actions

D15916.diff
View Options

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
@@ -1536,7 +1536,6 @@
$xactions = array();
if ($auto_subscribe) {
-
$xactions[] = $object->getApplicationTransactionTemplate()
->setAuthorPHID($owners_phid)
->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS)
@@ -1546,11 +1545,97 @@
));
}
- // TODO: Implement autoreview and autoblock, but these are more invovled.
+ $specs = array(
+ array($auto_review, false),
+ array($auto_block, true),
+ );
+
+ foreach ($specs as $spec) {
+ list($reviewers, $blocking) = $spec;
+ if (!$reviewers) {
+ continue;
+ }
+
+ $phids = mpull($reviewers, 'getPHID');
+ $xaction = $this->newAutoReviewTransaction($object, $phids, $blocking);
+ if ($xaction) {
+ $xactions[] = $xaction;
+ }
+ }
return $xactions;
}
+ private function newAutoReviewTransaction(
+ PhabricatorLiskDAO $object,
+ array $phids,
+ $is_blocking) {
+
+ // TODO: This is substantially similar to DifferentialReviewersHeraldAction
+ // and both are needlessly complex. This logic should live in the normal
+ // transaction application pipeline. See T10967.
+
+ $reviewers = $object->getReviewerStatus();
+ $reviewers = mpull($reviewers, null, 'getReviewerPHID');
+
+ if ($is_blocking) {
+ $new_status = DifferentialReviewerStatus::STATUS_BLOCKING;
+ } else {
+ $new_status = DifferentialReviewerStatus::STATUS_ADDED;
+ }
+
+ $new_strength = DifferentialReviewerStatus::getStatusStrength(
+ $new_status);
+
+ $current = array();
+ foreach ($phids as $phid) {
+ if (!isset($reviewers[$phid])) {
+ continue;
+ }
+
+ // If we're applying a stronger status (usually, upgrading a reviewer
+ // into a blocking reviewer), skip this check so we apply the change.
+ $old_strength = DifferentialReviewerStatus::getStatusStrength(
+ $reviewers[$phid]->getStatus());
+ if ($old_strength <= $new_strength) {
+ continue;
+ }
+
+ $current[] = $phid;
+ }
+
+ $phids = array_diff($phids, $current);
+
+ if (!$phids) {
+ return null;
+ }
+
+ $phids = array_fuse($phids);
+
+ $value = array();
+ foreach ($phids as $phid) {
+ $value[$phid] = array(
+ 'data' => array(
+ 'status' => $new_status,
+ ),
+ );
+ }
+
+ $edgetype_reviewer = DifferentialRevisionHasReviewerEdgeType::EDGECONST;
+
+ $owners_phid = id(new PhabricatorOwnersApplication())
+ ->getPHID();
+
+ return $object->getApplicationTransactionTemplate()
+ ->setAuthorPHID($owners_phid)
+ ->setTransactionType(PhabricatorTransactions::TYPE_EDGE)
+ ->setMetadataValue('edge:type', $edgetype_reviewer)
+ ->setNewValue(
+ array(
+ '+' => $value,
+ ));
+ }
+
protected function buildHeraldAdapter(
PhabricatorLiskDAO $object,
array $xactions) {
diff --git a/src/applications/owners/storage/PhabricatorOwnersPackage.php b/src/applications/owners/storage/PhabricatorOwnersPackage.php
--- a/src/applications/owners/storage/PhabricatorOwnersPackage.php
+++ b/src/applications/owners/storage/PhabricatorOwnersPackage.php
@@ -71,13 +71,12 @@
self::AUTOREVIEW_SUBSCRIBE => array(
'name' => pht('Subscribe to Changes'),
),
- // TODO: Implement these.
- // self::AUTOREVIEW_REVIEW => array(
- // 'name' => pht('Review Changes'),
- // ),
- // self::AUTOREVIEW_BLOCK => array(
- // 'name' => pht('Review Changes (Blocking)'),
- // ),
+ self::AUTOREVIEW_REVIEW => array(
+ 'name' => pht('Review Changes'),
+ ),
+ self::AUTOREVIEW_BLOCK => array(
+ 'name' => pht('Review Changes (Blocking)'),
+ ),
);
}
diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
--- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
@@ -1577,6 +1577,14 @@
$type = $xaction->getTransactionType();
if (isset($types[$type])) {
foreach ($types[$type] as $other_key) {
+ $other_xaction = $result[$other_key];
+
+ // Don't merge transactions with different authors. For example,
+ // don't merge Herald transactions and owners transactions.
+ if ($other_xaction->getAuthorPHID() != $xaction->getAuthorPHID()) {
+ continue;
+ }
+
$merged = $this->mergeTransactions($result[$other_key], $xaction);
if ($merged) {
$result[$other_key] = $merged;

File Metadata

Mime Type
text/plain
Expires
Mon, Nov 4, 4:17 PM (2 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6716209
Default Alt Text
D15916.diff (4 KB)

Event Timeline

Phabricator · Built by Phacility