@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.)
hq.recaptime.dev/wiki/Phorge
phorge
phabricator
Fix two transactional issues around revision status
Summary:
Two issues:
- Herald is currently overwriting accepts and rejects with "blocking reviewer". Just stop it from doing that.
- When you update an accepted revision, we put it back in "needs review", then return it to "accepted", generating an extra transaction. Instead, don't.
Test Plan:
- Updated a revision with an accepting, herald-based blocking project reviewer. Reviewer was still accepting.
- Updated an accepted revision, didn't get an extra transaction.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: aran, epriestley
Differential Revision: https://secure.phabricator.com/D8506
+24
-2
+24
-2
src/applications/differential/editor/DifferentialTransactionEditor.php
+24
-2
src/applications/differential/editor/DifferentialTransactionEditor.php
···
178
178
case PhabricatorTransactions::TYPE_EDGE:
179
179
return;
180
180
case DifferentialTransaction::TYPE_UPDATE:
181
-
if (!$this->getIsCloseByCommit()) {
181
+
if (!$this->getIsCloseByCommit() &&
182
+
(($object->getStatus() == $status_revision) ||
183
+
($object->getStatus() == $status_plan))) {
182
184
$object->setStatus($status_review);
183
185
}
184
186
// TODO: Update the `diffPHID` once we add that.
···
600
602
$new_status = $status_review;
601
603
}
602
604
603
-
if ($new_status !== null && $new_status != $old_status) {
605
+
if ($new_status !== null && ($new_status != $old_status)) {
604
606
$xaction = id(new DifferentialTransaction())
605
607
->setTransactionType(DifferentialTransaction::TYPE_STATUS)
606
608
->setOldValue($old_status)
···
1379
1381
array_keys($adapter->getBlockingReviewersAddedByHerald()),
1380
1382
);
1381
1383
1384
+
$old_reviewers = $object->getReviewerStatus();
1385
+
$old_reviewers = mpull($old_reviewers, null, 'getReviewerPHID');
1386
+
1382
1387
$value = array();
1383
1388
foreach ($reviewers as $status => $phids) {
1384
1389
foreach ($phids as $phid) {
···
1386
1391
// Don't try to add the revision's author as a reviewer, since this
1387
1392
// isn't valid and doesn't make sense.
1388
1393
continue;
1394
+
}
1395
+
1396
+
// If the target is already a reviewer, don't try to change anything
1397
+
// if their current status is at least as strong as the new status.
1398
+
// For example, don't downgrade an "Accepted" to a "Blocking Reviewer".
1399
+
$old_reviewer = idx($old_reviewers, $phid);
1400
+
if ($old_reviewer) {
1401
+
$old_status = $old_reviewer->getStatus();
1402
+
1403
+
$old_strength = DifferentialReviewerStatus::getStatusStrength(
1404
+
$old_status);
1405
+
$new_strength = DifferentialReviewerStatus::getStatusStrength(
1406
+
$status);
1407
+
1408
+
if ($new_strength <= $old_strength) {
1409
+
continue;
1410
+
}
1389
1411
}
1390
1412
1391
1413
$value['+'][$phid] = array(