From 2bbddb8c0f2943f57793ed7ce95ea303d89c238e Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Fri, 28 Oct 2016 13:21:38 -0700
Subject: [PATCH] Improve some setInitialValue() behavior for PhortuneMerchants

Summary:
This fixes the permissions issue with D16750, which is actually not really a permissions issue, exactly.

This is the only place anywhere that we use a tokenizer field //and// give it a default value which is not the same as the object value (when creating a merchant, we default it to the viewer).

In other cases (like Maniphest) we avoid this because you can edit the form to have defaults, which would collide with whatever default we provide. Some disucssion in T10222.

Since we aren't going to let you edit these forms for the forseeable future, this behavior is reasonable here though.

However, it triggered a sort-of-bug related to conflict detection for these fields (see T4768). These fields actually have two values: a hidden "initial" value, and a visible edited value.

When you submit the form, we compute your edit by comparing the edited value to the initial value, then applying adds/removes, instead of just saying "set value equal to new value". This prevents issues when two people edit at the same time and both make changes to the field.

In this case, the initial value was being set to the display value, so the field would say "Value: [(alincoln x)]" but internally have that as the intitial value, too. When you submitted, it would see "you didn't change anything", and thus not add any members.

So the viewer wouldn't actually be added as a member, then the policy check would correctly fail.

Note that there are still some policy issues here (you can remove yourself from a Merchant and lock yourself out) but they fall into the realm of stuff discussed in D16677.

Test Plan: Created a merchant account with D16750 applied.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D16764
---
 .../phortune/editor/PhortuneMerchantEditEngine.php       | 1 +
 .../transactions/editfield/PhabricatorEditField.php      | 9 ++++++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/applications/phortune/editor/PhortuneMerchantEditEngine.php b/src/applications/phortune/editor/PhortuneMerchantEditEngine.php
index 1c68fe67c9..8c2356e6fe 100644
--- a/src/applications/phortune/editor/PhortuneMerchantEditEngine.php
+++ b/src/applications/phortune/editor/PhortuneMerchantEditEngine.php
@@ -112,6 +112,7 @@ final class PhortuneMerchantEditEngine
         ->setDescription(pht('Initial merchant members.'))
         ->setConduitDescription(pht('Set merchant members.'))
         ->setConduitTypeDescription(pht('New list of members.'))
+        ->setInitialValue($object->getMemberPHIDs())
         ->setValue($member_phids),
 
     );
diff --git a/src/applications/transactions/editfield/PhabricatorEditField.php b/src/applications/transactions/editfield/PhabricatorEditField.php
index 6ff84da870..a0b63e2da1 100644
--- a/src/applications/transactions/editfield/PhabricatorEditField.php
+++ b/src/applications/transactions/editfield/PhabricatorEditField.php
@@ -401,8 +401,15 @@ abstract class PhabricatorEditField extends Phobject {
 
   public function setValue($value) {
     $this->hasValue = true;
-    $this->initialValue = $value;
     $this->value = $value;
+
+    // If we don't have an initial value set yet, use the value as the
+    // initial value.
+    $initial_value = $this->getInitialValue();
+    if ($initial_value === null) {
+      $this->initialValue = $value;
+    }
+
     return $this;
   }