Improve behavior of "owner" transaction in "maniphest.edit" endpoint

Summary:
Fixes T10117.

  - I accidentally broke setting `null` to unassign tasks at some point when I added richer validation.
  - Raise a better error if the user passes junk.

Test Plan:
  - Unassigned a task via API and web UI.
  - Reassigned a task via API and web UI.
  - Tried to do an invalid assign via API, got a sensible error.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10117

Differential Revision: https://secure.phabricator.com/D14992

src/__phutil_library_map__.php

@@ -249,6 +249,7 @@ phutil_register_library_map(array(
     'ConduitStringParameterType' => 'applications/conduit/parametertype/ConduitStringParameterType.php',
     'ConduitTokenGarbageCollector' => 'applications/conduit/garbagecollector/ConduitTokenGarbageCollector.php',
     'ConduitUserListParameterType' => 'applications/conduit/parametertype/ConduitUserListParameterType.php',
+    'ConduitUserParameterType' => 'applications/conduit/parametertype/ConduitUserParameterType.php',
     'ConduitWildParameterType' => 'applications/conduit/parametertype/ConduitWildParameterType.php',
     'ConpherenceColumnViewController' => 'applications/conpherence/controller/ConpherenceColumnViewController.php',
     'ConpherenceConduitAPIMethod' => 'applications/conpherence/conduit/ConpherenceConduitAPIMethod.php',
@@ -4184,6 +4185,7 @@ phutil_register_library_map(array(
     'ConduitStringParameterType' => 'ConduitParameterType',
     'ConduitTokenGarbageCollector' => 'PhabricatorGarbageCollector',
     'ConduitUserListParameterType' => 'ConduitListParameterType',
+    'ConduitUserParameterType' => 'ConduitParameterType',
     'ConduitWildParameterType' => 'ConduitListParameterType',
     'ConpherenceColumnViewController' => 'ConpherenceController',
     'ConpherenceConduitAPIMethod' => 'ConduitAPIMethod',

src/applications/conduit/parametertype/ConduitUserParameterType.php

@@ -0,0 +1,47 @@
+<?php
+
+final class ConduitUserParameterType
+  extends ConduitParameterType {
+
+  protected function getParameterValue(array $request, $key) {
+    $value = parent::getParameterValue($request, $key);
+
+    if ($value === null) {
+      return null;
+    }
+
+    if (!is_string($value)) {
+      $this->raiseValidationException(
+        $request,
+        $key,
+        pht('Expected PHID or null, got something else.'));
+    }
+
+    $user_phids = id(new PhabricatorUserPHIDResolver())
+      ->setViewer($this->getViewer())
+      ->resolvePHIDs(array($value));
+
+    return nonempty(head($user_phids), null);
+  }
+
+  protected function getParameterTypeName() {
+    return 'phid|string|null';
+  }
+
+  protected function getParameterFormatDescriptions() {
+    return array(
+      pht('User PHID.'),
+      pht('Username.'),
+      pht('Literal null.'),
+    );
+  }
+
+  protected function getParameterExamples() {
+    return array(
+      '"PHID-USER-1111"',
+      '"alincoln"',
+      'null',
+    );
+  }
+
+}

src/applications/maniphest/editor/ManiphestTransactionEditor.php

@@ -829,6 +829,33 @@ final class ManiphestTransactionEditor
             last($with_effect));
         }
         break;
+      case ManiphestTransaction::TYPE_OWNER:
+        foreach ($xactions as $xaction) {
+          $old = $xaction->getOldValue();
+          $new = $xaction->getNewValue();
+          if (!strlen($new)) {
+            continue;
+          }
+
+          if ($new === $old) {
+            continue;
+          }
+
+          $assignee_list = id(new PhabricatorPeopleQuery())
+            ->setViewer($this->getActor())
+            ->withPHIDs(array($new))
+            ->execute();
+          if (!$assignee_list) {
+            $errors[] = new PhabricatorApplicationTransactionValidationError(
+              $type,
+              pht('Invalid'),
+              pht(
+                'User "%s" is not a valid user.',
+                $new),
+              $xaction);
+          }
+        }
+        break;
     }
 
     return $errors;

src/applications/transactions/editengine/PhabricatorEditEngine.php

@@ -1632,6 +1632,7 @@ abstract class PhabricatorEditEngine
     array $types,
     PhabricatorApplicationTransaction $template) {
 
+    $viewer = $request->getUser();
     $transactions_key = 'transactions';
 
     $xactions = $request->getValue($transactions_key);
@@ -1688,6 +1689,8 @@ abstract class PhabricatorEditEngine
       // use usernames in list<user> fields, for example.
       $parameter_type = $type->getConduitParameterType();
 
+      $parameter_type->setViewer($viewer);
+
       try {
         $xaction['value'] = $parameter_type->getValue($xaction, 'value');
       } catch (Exception $ex) {

src/applications/transactions/editfield/PhabricatorPHIDListEditField.php

@@ -100,6 +100,7 @@ abstract class PhabricatorPHIDListEditField
 
     $type = new PhabricatorDatasourceEditType();
     $type->setIsSingleValue($this->getIsSingleValue());
+    $type->setConduitParameterType($this->newConduitParameterType());
     return $type;
   }
 

src/applications/transactions/editfield/PhabricatorUsersEditField.php

@@ -12,7 +12,11 @@ final class PhabricatorUsersEditField
   }
 
   protected function newConduitParameterType() {
+    if ($this->getIsSingleValue()) {
+      return new ConduitUserParameterType();
+    } else {
       return new ConduitUserListParameterType();
     }
+  }
 
 }