From f1eeaaf59f7ff0e1a17aa35cae2b9631ed414a3f Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 6 Apr 2017 14:56:21 -0700
Subject: [PATCH] Fix scope of "Accept" when you don't check all the "Force
Accept" boxes
Summary:
Ref T12272. I wrote this correctly, then broke it by adding the simplification which treats "accept the defaults" as "accept everything".
This simplification lets us render "epriestley accepted this revision." instead of "epriestley accepted this revision onbehalf of: long, list, of, every, default, reviewer, they, have, authority, over." so it's a good thing, but make it only affect the reviewers it's supposed to affect.
Test Plan:
- Did an accept with a force-accept available but unchecked.
- Before patch: incorrectly accepted all possible reviewers.
- After patch: accepted only checked reviewers.
- Also checked the force-accept box, accepted, got a proper force-accept.
Reviewers: chad, lvital
Reviewed By: lvital
Maniphest Tasks: T12272
Differential Revision: https://secure.phabricator.com/D17634
---
.../DifferentialRevisionReviewTransaction.php | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php b/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php
index 019c4c036f..d44de8706c 100644
--- a/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php
@@ -183,8 +183,19 @@ abstract class DifferentialRevisionReviewTransaction
// 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 we're applying an "accept the defaults" transaction, and this
+ // transaction type uses checkboxes, replace the value with the list of
+ // defaults.
+ if (!is_array($value)) {
+ list($options, $default) = $this->getActionOptions($viewer, $revision);
+ if ($options) {
+ $value = $default;
+ }
+ }
+
+ // If we have a specific list of reviewers to act on, usually because 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);
}