Add ApplicationTransactions/CustomField based user profile editor

Summary:
Adds a profile edit controller (with just one field and on links to it) that uses ApplicationTransactions and CustomField.

{F45617}

My plan is to move the other profile fields to this interface and get rid of Settings -> Profile. Basically, these will be "settings":

  - Sex
  - Language
  - Timezone

These will be "profile":

  - Real Name
  - Title
  - Blurb
  - Profile Image (but I'm going to put this on a separate UI)
  - Other custom fields

Test Plan: Edited my realname using the new interface.

Reviewers: chad, seporaitis

Reviewed By: chad

CC: aran

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

resources/sql/patches/20130606.userxactions.sql

@@ -0,0 +1,23 @@
+CREATE TABLE {$NAMESPACE}_user.user_transaction (
+  id INT UNSIGNED NOT NULL PRIMARY KEY AUTO_INCREMENT,
+  phid VARCHAR(64) NOT NULL COLLATE utf8_bin,
+  authorPHID VARCHAR(64) NOT NULL COLLATE utf8_bin,
+  objectPHID VARCHAR(64) NOT NULL COLLATE utf8_bin,
+  viewPolicy VARCHAR(64) NOT NULL COLLATE utf8_bin,
+  editPolicy VARCHAR(64) NOT NULL COLLATE utf8_bin,
+  commentPHID VARCHAR(64) COLLATE utf8_bin,
+  commentVersion INT UNSIGNED NOT NULL,
+  transactionType VARCHAR(32) NOT NULL COLLATE utf8_bin,
+  oldValue LONGTEXT NOT NULL COLLATE utf8_bin,
+  newValue LONGTEXT NOT NULL COLLATE utf8_bin,
+  metadata LONGTEXT NOT NULL COLLATE utf8_bin,
+  contentSource LONGTEXT NOT NULL COLLATE utf8_bin,
+  dateCreated INT UNSIGNED NOT NULL,
+  dateModified INT UNSIGNED NOT NULL,
+
+  UNIQUE KEY `key_phid` (phid),
+  KEY `key_object` (objectPHID)
+
+) ENGINE=InnoDB, COLLATE utf8_general_ci;
+
+

src/__phutil_library_map__.php

@@ -1279,6 +1279,7 @@ phutil_register_library_map(array(
     'PhabricatorPeopleListController' => 'applications/people/controller/PhabricatorPeopleListController.php',
     'PhabricatorPeopleLogsController' => 'applications/people/controller/PhabricatorPeopleLogsController.php',
     'PhabricatorPeopleProfileController' => 'applications/people/controller/PhabricatorPeopleProfileController.php',
+    'PhabricatorPeopleProfileEditController' => 'applications/people/controller/PhabricatorPeopleProfileEditController.php',
     'PhabricatorPeopleQuery' => 'applications/people/query/PhabricatorPeopleQuery.php',
     'PhabricatorPeopleSearchEngine' => 'applications/people/query/PhabricatorPeopleSearchEngine.php',
     'PhabricatorPeopleTestDataGenerator' => 'applications/people/lipsum/PhabricatorPeopleTestDataGenerator.php',
@@ -1471,6 +1472,7 @@ phutil_register_library_map(array(
     'PhabricatorSlugTestCase' => 'infrastructure/util/__tests__/PhabricatorSlugTestCase.php',
     'PhabricatorSortTableExample' => 'applications/uiexample/examples/PhabricatorSortTableExample.php',
     'PhabricatorSourceCodeView' => 'view/layout/PhabricatorSourceCodeView.php',
+    'PhabricatorStandardCustomField' => 'infrastructure/customfield/field/PhabricatorStandardCustomField.php',
     'PhabricatorStandardPageView' => 'view/page/PhabricatorStandardPageView.php',
     'PhabricatorStatusController' => 'applications/system/PhabricatorStatusController.php',
     'PhabricatorStorageFixtureScopeGuard' => 'infrastructure/testing/fixture/PhabricatorStorageFixtureScopeGuard.php',
@@ -1542,6 +1544,8 @@ phutil_register_library_map(array(
     'PhabricatorUITooltipExample' => 'applications/uiexample/examples/PhabricatorUITooltipExample.php',
     'PhabricatorUnitsTestCase' => 'view/__tests__/PhabricatorUnitsTestCase.php',
     'PhabricatorUser' => 'applications/people/storage/PhabricatorUser.php',
+    'PhabricatorUserCustomField' => 'applications/people/customfield/PhabricatorUserCustomField.php',
+    'PhabricatorUserCustomFieldInterface' => 'applications/people/customfield/PhabricatorUserCustomFieldInterface.php',
     'PhabricatorUserDAO' => 'applications/people/storage/PhabricatorUserDAO.php',
     'PhabricatorUserEditor' => 'applications/people/editor/PhabricatorUserEditor.php',
     'PhabricatorUserEmail' => 'applications/people/storage/PhabricatorUserEmail.php',
@@ -1550,6 +1554,8 @@ phutil_register_library_map(array(
     'PhabricatorUserOAuthInfo' => 'applications/people/storage/PhabricatorUserOAuthInfo.php',
     'PhabricatorUserPreferences' => 'applications/settings/storage/PhabricatorUserPreferences.php',
     'PhabricatorUserProfile' => 'applications/people/storage/PhabricatorUserProfile.php',
+    'PhabricatorUserProfileEditor' => 'applications/people/editor/PhabricatorUserProfileEditor.php',
+    'PhabricatorUserRealNameField' => 'applications/people/customfield/PhabricatorUserRealNameField.php',
     'PhabricatorUserSSHKey' => 'applications/settings/storage/PhabricatorUserSSHKey.php',
     'PhabricatorUserSearchIndexer' => 'applications/people/search/PhabricatorUserSearchIndexer.php',
     'PhabricatorUserStatus' => 'applications/people/storage/PhabricatorUserStatus.php',
@@ -2376,6 +2382,7 @@ phutil_register_library_map(array(
     array(
       0 => 'DivinerDAO',
       1 => 'PhabricatorPolicyInterface',
+      2 => 'PhabricatorMarkupInterface',
     ),
     'DivinerPublishCache' => 'DivinerDiskCache',
     'DivinerRemarkupRuleSymbol' => 'PhutilRemarkupRule',
@@ -3118,6 +3125,7 @@ phutil_register_library_map(array(
     ),
     'PhabricatorPeopleLogsController' => 'PhabricatorPeopleController',
     'PhabricatorPeopleProfileController' => 'PhabricatorPeopleController',
+    'PhabricatorPeopleProfileEditController' => 'PhabricatorPeopleController',
     'PhabricatorPeopleQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'PhabricatorPeopleSearchEngine' => 'PhabricatorApplicationSearchEngine',
     'PhabricatorPeopleTestDataGenerator' => 'PhabricatorTestDataGenerator',
@@ -3304,6 +3312,7 @@ phutil_register_library_map(array(
     'PhabricatorSlugTestCase' => 'PhabricatorTestCase',
     'PhabricatorSortTableExample' => 'PhabricatorUIExample',
     'PhabricatorSourceCodeView' => 'AphrontView',
+    'PhabricatorStandardCustomField' => 'PhabricatorCustomField',
     'PhabricatorStandardPageView' => 'PhabricatorBarePageView',
     'PhabricatorStatusController' => 'PhabricatorController',
     'PhabricatorStorageManagementDatabasesWorkflow' => 'PhabricatorStorageManagementWorkflow',
@@ -3374,6 +3383,12 @@ phutil_register_library_map(array(
       0 => 'PhabricatorUserDAO',
       1 => 'PhutilPerson',
       2 => 'PhabricatorPolicyInterface',
+      3 => 'PhabricatorCustomFieldInterface',
+    ),
+    'PhabricatorUserCustomField' =>
+    array(
+      0 => 'PhabricatorCustomField',
+      1 => 'PhabricatorUserCustomFieldInterface',
     ),
     'PhabricatorUserDAO' => 'PhabricatorLiskDAO',
     'PhabricatorUserEditor' => 'PhabricatorEditor',
@@ -3383,6 +3398,8 @@ phutil_register_library_map(array(
     'PhabricatorUserOAuthInfo' => 'PhabricatorUserDAO',
     'PhabricatorUserPreferences' => 'PhabricatorUserDAO',
     'PhabricatorUserProfile' => 'PhabricatorUserDAO',
+    'PhabricatorUserProfileEditor' => 'PhabricatorApplicationTransactionEditor',
+    'PhabricatorUserRealNameField' => 'PhabricatorUserCustomField',
     'PhabricatorUserSSHKey' => 'PhabricatorUserDAO',
     'PhabricatorUserSearchIndexer' => 'PhabricatorSearchDocumentIndexer',
     'PhabricatorUserStatus' => 'PhabricatorUserDAO',

src/applications/people/application/PhabricatorApplicationPeople.php

@@ -44,6 +44,8 @@ final class PhabricatorApplicationPeople extends PhabricatorApplication {
         'edit/(?:(?P<id>[1-9]\d*)/(?:(?P<view>\w+)/)?)?'
           => 'PhabricatorPeopleEditController',
         'ldap/' => 'PhabricatorPeopleLdapController',
+        'editprofile/(?P<id>[1-9]\d*)/' =>
+          'PhabricatorPeopleProfileEditController',
       ),
       '/p/(?P<username>[\w._-]+)/(?:(?P<page>\w+)/)?'
         => 'PhabricatorPeopleProfileController',

src/applications/people/controller/PhabricatorPeopleProfileEditController.php

@@ -0,0 +1,94 @@
+<?php
+
+final class PhabricatorPeopleProfileEditController
+  extends PhabricatorPeopleController {
+
+  private $id;
+
+  public function shouldRequireAdmin() {
+    return false;
+  }
+
+  public function willProcessRequest(array $data) {
+    $this->id = $data['id'];
+  }
+
+  public function processRequest() {
+    $request = $this->getRequest();
+    $viewer = $request->getUser();
+
+    $user = id(new PhabricatorPeopleQuery())
+      ->setViewer($viewer)
+      ->withIDs(array($this->id))
+      ->requireCapabilities(
+        array(
+          PhabricatorPolicyCapability::CAN_VIEW,
+          PhabricatorPolicyCapability::CAN_EDIT,
+        ))
+      ->executeOne();
+    if (!$user) {
+      return new Aphront404Response();
+    }
+
+    $profile_uri = '/p/'.$user->getUsername().'/';
+
+    $fields = PhabricatorCustomField::getObjectFields(
+      $user,
+      PhabricatorUserCustomFieldInterface::ROLE_EDIT);
+
+    if ($request->isFormPost()) {
+      $xactions = array();
+      foreach ($fields as $field) {
+        $field->setValueFromRequest($request);
+        $xactions[] = id(new PhabricatorUserTransaction())
+          ->setTransactionType(PhabricatorTransactions::TYPE_CUSTOMFIELD)
+          ->setMetadataValue('customfield:key', $field->getFieldKey())
+          ->setNewValue($field->getNewValueForApplicationTransactions());
+      }
+
+      $editor = id(new PhabricatorUserProfileEditor())
+        ->setActor($viewer)
+        ->setContentSource(
+          PhabricatorContentSource::newFromRequest($request))
+        ->setContinueOnNoEffect(true);
+
+      $editor->applyTransactions($user, $xactions);
+
+      return id(new AphrontRedirectResponse())->setURI($profile_uri);
+    }
+
+    $title = pht('Edit Profile');
+    $crumbs = $this->buildApplicationCrumbs();
+    $crumbs->addCrumb(
+      id(new PhabricatorCrumbView())
+        ->setName($user->getUsername())
+        ->setHref($profile_uri));
+    $crumbs->addCrumb(
+      id(new PhabricatorCrumbView())
+        ->setName($title));
+
+    $form = id(new AphrontFormView())
+      ->setUser($viewer);
+
+    foreach ($fields as $field) {
+      $form->appendChild($field->renderEditControl());
+    }
+
+    $form
+      ->appendChild(
+        id(new AphrontFormSubmitControl())
+          ->addCancelButton($profile_uri)
+          ->setValue(pht('Save Profile')));
+
+    return $this->buildApplicationPage(
+      array(
+        $crumbs,
+        $form,
+      ),
+      array(
+        'title' => $title,
+        'device' => true,
+        'dust' => true,
+      ));
+  }
+}

src/applications/people/customfield/PhabricatorUserCustomField.php

@@ -0,0 +1,32 @@
+<?php
+
+abstract class PhabricatorUserCustomField
+  extends PhabricatorCustomField
+  implements PhabricatorUserCustomFieldInterface {
+
+
+  public function shouldEnableForRole($role) {
+    switch ($role) {
+      case PhabricatorUserCustomFieldInterface::ROLE_EDIT:
+        return $this->shouldAppearOnProfileEdit();
+    }
+    return parent::shouldEnableForRole($role);
+  }
+
+  public function shouldAppearOnProfileEdit() {
+    return true;
+  }
+
+
+/* -(  PhabricatorCustomField  )--------------------------------------------- */
+
+
+  public function canDisableField() {
+    return false;
+  }
+
+  public function shouldAppearInApplicationTransactions() {
+    return true;
+  }
+
+}

src/applications/people/customfield/PhabricatorUserCustomFieldInterface.php

@@ -0,0 +1,9 @@
+<?php
+
+interface PhabricatorUserCustomFieldInterface {
+
+  const ROLE_EDIT = 'user.edit';
+
+  public function shouldAppearOnProfileEdit();
+
+}

src/applications/people/customfield/PhabricatorUserRealNameField.php

@@ -0,0 +1,48 @@
+<?php
+
+final class PhabricatorUserRealNameField
+  extends PhabricatorUserCustomField {
+
+  private $value;
+
+  public function getFieldKey() {
+    return 'user:realname';
+  }
+
+  public function getFieldName() {
+    return pht('Real Name');
+  }
+
+  public function getFieldDescription() {
+    return pht('Stores the real name of the user, like "Abraham Lincoln".');
+  }
+
+  protected function didSetObject(PhabricatorCustomFieldInterface $object) {
+    $this->value = $object->getRealName();
+  }
+
+  public function getOldValueForApplicationTransactions() {
+    return $this->getObject()->getRealName();
+  }
+
+  public function getNewValueForApplicationTransactions() {
+    return $this->value;
+  }
+
+  public function applyApplicationTransactionInternalEffects(
+    PhabricatorApplicationTransaction $xaction) {
+    $this->getObject()->setRealName($xaction->getNewValue());
+  }
+
+  public function setValueFromRequest(AphrontRequest $request) {
+    $this->value = $request->getStr($this->getFieldKey());
+  }
+
+  public function renderEditControl() {
+    return id(new AphrontFormTextControl())
+      ->setName($this->getFieldKey())
+      ->setValue($this->value)
+      ->setLabel($this->getFieldName());
+  }
+
+}

src/applications/people/editor/PhabricatorUserProfileEditor.php

@@ -0,0 +1,7 @@
+<?php
+
+final class PhabricatorUserProfileEditor
+  extends PhabricatorApplicationTransactionEditor {
+
+
+}

src/applications/people/storage/PhabricatorUser.php

@@ -2,7 +2,10 @@
 
 final class PhabricatorUser
   extends PhabricatorUserDAO
-  implements PhutilPerson, PhabricatorPolicyInterface {
+  implements
+    PhutilPerson,
+    PhabricatorPolicyInterface,
+    PhabricatorCustomFieldInterface {
 
   const SESSION_TABLE = 'phabricator_session';
   const NAMETOKEN_TABLE = 'user_nametoken';
@@ -32,6 +35,7 @@ final class PhabricatorUser
   private $status = null;
   private $preferences = null;
   private $omnipotent = false;
+  private $customFields = array();
 
   protected function readField($field) {
     switch ($field) {
@@ -756,4 +760,28 @@ EOBODY;
   }
 
 
+/* -(  PhabricatorCustomFieldInterface  )------------------------------------ */
+
+
+  public function getCustomFieldSpecificationForRole($role) {
+    return array();
+    // TODO: PhabricatorEnv::getEnvConfig('user.fields');
+  }
+
+  public function getCustomFieldBaseClass() {
+    return 'PhabricatorUserCustomFieldInterface';
+  }
+
+  public function getCustomFields($role) {
+    if (idx($this->customFields, $role) === null) {
+      PhabricatorCustomField::raiseUnattachedException($this, $role);
+    }
+    return $this->customFields[$role];
+  }
+
+  public function attachCustomFields($role, array $fields) {
+    $this->customFields[$role] = $fields;
+    return $this;
+  }
+
 }

src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php

@@ -146,7 +146,7 @@ abstract class PhabricatorApplicationTransactionEditor
         return $this->getEdgeTransactionNewValue($xaction);
       case PhabricatorTransactions::TYPE_CUSTOMFIELD:
         $field = $this->getCustomFieldForTransaction($object, $xaction);
-        return $field->getNewValueForApplicationTransactions();
+        return $field->getNewValueFromApplicationTransactions($xaction);
       default:
         return $this->getCustomTransactionNewValue($object, $xaction);
     }

src/infrastructure/customfield/field/PhabricatorCustomField.php

@@ -14,7 +14,7 @@ abstract class PhabricatorCustomField {
   private $object;
 
   const ROLE_APPLICATIONTRANSACTIONS  = 'ApplicationTransactions';
-  const ROLE_APPLICAITONSEARCH        = 'ApplicationSearch';
+  const ROLE_APPLICATIONSEARCH        = 'ApplicationSearch';
   const ROLE_STORAGE                  = 'storage';
   const ROLE_DEFAULT                  = 'default';
 
@@ -45,14 +45,6 @@ abstract class PhabricatorCustomField {
     } catch (PhabricatorCustomFieldNotAttachedException $ex) {
       $base_class = $object->getCustomFieldBaseClass();
 
-      if (!($base_class instanceof PhabricatorCustomField)) {
-        $obj_class = get_class($object);
-        throw new Exception(
-          "Object (of class '{$obj_class}') returned '{$base_class}' as its ".
-          "getCustomFieldBaseClass(), but this is not a recognized subclass ".
-          "of PhabricatorCustomField.");
-      }
-
       $spec = $object->getCustomFieldSpecificationForRole($role);
       if (!is_array($spec)) {
         $obj_class = get_class($object);
@@ -87,7 +79,6 @@ abstract class PhabricatorCustomField {
     PhabricatorCustomFieldInterface $object,
     $role,
     $field_key) {
-
     return idx(self::getObjectFields($object, $role), $field_key);
   }
 
@@ -96,12 +87,6 @@ abstract class PhabricatorCustomField {
    * @task apps
    */
   public static function buildFieldList($base_class, array $spec) {
-    $this_class = __CLASS__;
-    if (!($base_class instanceof $this_class)) {
-      throw new Exception(
-        "Base class ('{$base_class}') must extend '{$this_class}'.");
-    }
-
     $field_objects = id(new PhutilSymbolLoader())
       ->setAncestorClass($base_class)
       ->loadObjects();
@@ -124,12 +109,12 @@ abstract class PhabricatorCustomField {
     }
 
     foreach ($fields as $key => $field) {
-      if (!$field->isEnabled()) {
+      if (!$field->isFieldEnabled()) {
         unset($fields[$key]);
       }
     }
 
-    $fields = array_select_keys($fields, array_keys($spec));
+    $fields = array_select_keys($fields, array_keys($spec)) + $fields;
 
     foreach ($spec as $key => $config) {
       if (empty($fields[$key])) {
@@ -151,7 +136,7 @@ abstract class PhabricatorCustomField {
 
   /**
    * Return a key which uniquely identifies this field, like
-   * "mycompany.dinosaur.count". Normally you should provide some level of
+   * "mycompany:dinosaur:count". Normally you should provide some level of
    * namespacing to prevent collisions.
    *
    * @return string String which uniquely identifies this field.
@@ -278,6 +263,7 @@ abstract class PhabricatorCustomField {
    */
   final public function setObject(PhabricatorCustomFieldInterface $object) {
     $this->object = $object;
+    $this->didSetObject($object);
     return $this;
   }
 
@@ -293,6 +279,17 @@ abstract class PhabricatorCustomField {
   }
 
 
+  /**
+   * This is a hook, primarily for subclasses to load object data.
+   *
+   * @return PhabricatorCustomFieldInterface The object this field belongs to.
+   * @return void
+   */
+  protected function didSetObject(PhabricatorCustomFieldInterface $object) {
+    return;
+  }
+
+
   /**
    * @task context
    */
@@ -523,6 +520,14 @@ abstract class PhabricatorCustomField {
   }
 
 
+  /**
+   * @task appxaction
+   */
+  public function getNewValueForApplicationTransactions() {
+    return $this->getValueForStorage();
+  }
+
+
   /**
    * @task appxaction
    */
@@ -534,7 +539,7 @@ abstract class PhabricatorCustomField {
   /**
    * @task appxaction
    */
-  public function getNewValueForApplicationTransactions(
+  public function getNewValueFromApplicationTransactions(
     PhabricatorApplicationTransaction $xaction) {
     return $xaction->getNewValue();
   }
@@ -593,4 +598,32 @@ abstract class PhabricatorCustomField {
     return;
   }
 
+
+/* -(  Edit View  )---------------------------------------------------------- */
+
+
+  /**
+   * @task edit
+   */
+  public function shouldAppearOnEditView() {
+    return false;
+  }
+
+
+  /**
+   * @task edit
+   */
+  public function readValueFromRequest(AphrontRequest $request) {
+    throw new PhabricatorCustomFieldImplementationIncompleteException($this);
+  }
+
+
+  /**
+   * @task edit
+   */
+  public function renderEditControl() {
+    throw new PhabricatorCustomFieldImplementationIncompleteException($this);
+  }
+
+
 }

src/infrastructure/customfield/field/PhabricatorStandardCustomField.php

@@ -0,0 +1,72 @@
+<?php
+
+abstract class PhabricatorStandardCustomField
+  extends PhabricatorCustomField {
+
+  private $fieldKey;
+  private $fieldName;
+  private $fieldType;
+  private $fieldValue;
+  private $fieldDescription;
+
+  public function __construct($key) {
+    $this->fieldKey = $key;
+  }
+
+  public function setFieldName($name) {
+    $this->fieldName = $name;
+    return $this;
+  }
+
+  public function setFieldType($type) {
+    $this->fieldType = $type;
+    return $this;
+  }
+
+  public function getFieldValue() {
+    return $this->fieldValue;
+  }
+
+  public function setFieldValue($value) {
+    $this->fieldValue = $value;
+    return $this;
+  }
+
+  public function setFieldDescription($description) {
+    $this->fieldDescription = $description;
+    return $this;
+  }
+
+
+/* -(  PhabricatorCustomField  )--------------------------------------------- */
+
+
+  public function getFieldKey() {
+    return $this->fieldKey;
+  }
+
+  public function getFieldName() {
+    return coalesce($this->fieldName, parent::getFieldName());
+  }
+
+  public function getFieldDescription() {
+    return coalesce($this->fieldDescription, parent::getFieldDescription());
+  }
+
+  public function getStorageKey() {
+    return $this->getFieldKey();
+  }
+
+  public function getValueForStorage() {
+    return $this->getFieldValue();
+  }
+
+  public function setValueFromStorage($value) {
+    return $this->setFieldValue($value);
+  }
+
+  public function shouldAppearInApplicationTransactions() {
+    return true;
+  }
+
+}

src/infrastructure/customfield/interface/PhabricatorCustomFieldInterface.php

@@ -30,7 +30,7 @@ interface PhabricatorCustomFieldInterface {
     if (idx($this->customFields, $role) === null) {
       PhabricatorCustomField::raiseUnattachedException($this, $role);
     }
-    return $this->customFields;
+    return $this->customFields[$role];
   }
 
   public function attachCustomFields($role, array $fields) {

src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php

@@ -1350,6 +1350,10 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList {
         'type' => 'sql',
         'name' => $this->getPatchPath('20130602.namedqueries.sql'),
       ),
+      '20130606.userxactions.sql' => array(
+        'type' => 'sql',
+        'name' => $this->getPatchPath('20130606.userxactions.sql'),
+      ),
     );
   }
 }