Make Phortune account members editable and modernize the edge constant

Summary:
Ref T2787.

  - Account members can add and remove other members (major use case is corporate accounts).
  - Use a modern edge constant setup.

Test Plan: See screenshots.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T2787

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

src/__phutil_library_map__.php

@@ -2550,6 +2550,7 @@ phutil_register_library_map(array(
     'PhortuneAccount' => 'applications/phortune/storage/PhortuneAccount.php',
     'PhortuneAccountEditController' => 'applications/phortune/controller/PhortuneAccountEditController.php',
     'PhortuneAccountEditor' => 'applications/phortune/editor/PhortuneAccountEditor.php',
+    'PhortuneAccountHasMemberEdgeType' => 'applications/phortune/edge/PhortuneAccountHasMemberEdgeType.php',
     'PhortuneAccountListController' => 'applications/phortune/controller/PhortuneAccountListController.php',
     'PhortuneAccountPHIDType' => 'applications/phortune/phid/PhortuneAccountPHIDType.php',
     'PhortuneAccountQuery' => 'applications/phortune/query/PhortuneAccountQuery.php',
@@ -2581,6 +2582,7 @@ phutil_register_library_map(array(
     'PhortuneDAO' => 'applications/phortune/storage/PhortuneDAO.php',
     'PhortuneErrCode' => 'applications/phortune/constants/PhortuneErrCode.php',
     'PhortuneLandingController' => 'applications/phortune/controller/PhortuneLandingController.php',
+    'PhortuneMemberHasAccountEdgeType' => 'applications/phortune/edge/PhortuneMemberHasAccountEdgeType.php',
     'PhortuneMerchant' => 'applications/phortune/storage/PhortuneMerchant.php',
     'PhortuneMerchantCapability' => 'applications/phortune/capability/PhortuneMerchantCapability.php',
     'PhortuneMerchantController' => 'applications/phortune/controller/PhortuneMerchantController.php',
@@ -5608,6 +5610,7 @@ phutil_register_library_map(array(
     ),
     'PhortuneAccountEditController' => 'PhortuneController',
     'PhortuneAccountEditor' => 'PhabricatorApplicationTransactionEditor',
+    'PhortuneAccountHasMemberEdgeType' => 'PhabricatorEdgeType',
     'PhortuneAccountListController' => 'PhortuneController',
     'PhortuneAccountPHIDType' => 'PhabricatorPHIDType',
     'PhortuneAccountQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
@@ -5642,6 +5645,7 @@ phutil_register_library_map(array(
     'PhortuneDAO' => 'PhabricatorLiskDAO',
     'PhortuneErrCode' => 'PhortuneConstants',
     'PhortuneLandingController' => 'PhortuneController',
+    'PhortuneMemberHasAccountEdgeType' => 'PhabricatorEdgeType',
     'PhortuneMerchant' => array(
       'PhortuneDAO',
       'PhabricatorPolicyInterface',

src/applications/phortune/controller/PhortuneAccountEditController.php

@@ -28,34 +28,39 @@ final class PhortuneAccountEditController extends PhortuneController {
       $is_new = false;
     } else {
       $account = PhortuneAccount::initializeNewAccount($viewer);
+      $account->attachMemberPHIDs(array($viewer->getPHID()));
       $is_new = true;
     }
 
     $v_name = $account->getName();
     $e_name = true;
+
+    $v_members = $account->getMemberPHIDs();
+    $e_members = null;
+
     $validation_exception = null;
 
     if ($request->isFormPost()) {
       $v_name = $request->getStr('name');
+      $v_members = $request->getArr('memberPHIDs');
 
       $type_name = PhortuneAccountTransaction::TYPE_NAME;
+      $type_edge = PhabricatorTransactions::TYPE_EDGE;
 
       $xactions = array();
       $xactions[] = id(new PhortuneAccountTransaction())
         ->setTransactionType($type_name)
         ->setNewValue($v_name);
 
-      if ($is_new) {
+      $xactions[] = id(new PhortuneAccountTransaction())
-        $xactions[] = id(new PhortuneAccountTransaction())
+        ->setTransactionType($type_edge)
-          ->setTransactionType(PhabricatorTransactions::TYPE_EDGE)
+        ->setMetadataValue(
-          ->setMetadataValue(
+          'edge:type',
-            'edge:type',
+          PhortuneAccountHasMemberEdgeType::EDGECONST)
-            PhabricatorEdgeConfig::TYPE_ACCOUNT_HAS_MEMBER)
+        ->setNewValue(
-          ->setNewValue(
+          array(
-            array(
+            '=' => array_fuse($v_members),
-              '=' => array($viewer->getPHID() => $viewer->getPHID()),
+          ));
-            ));
-      }
 
       $editor = id(new PhortuneAccountEditor())
         ->setActor($viewer)
@@ -70,6 +75,7 @@ final class PhortuneAccountEditController extends PhortuneController {
       } catch (PhabricatorApplicationTransactionValidationException $ex) {
         $validation_exception = $ex;
         $e_name = $ex->getShortMessage($type_name);
+        $e_members = $ex->getShortMessage($type_edge);
       }
     }
 
@@ -91,6 +97,8 @@ final class PhortuneAccountEditController extends PhortuneController {
       $submit_button = pht('Save Changes');
     }
 
+    $member_handles = $this->loadViewerHandles($v_members);
+
     $form = id(new AphrontFormView())
       ->setUser($viewer)
       ->appendChild(
@@ -99,6 +107,13 @@ final class PhortuneAccountEditController extends PhortuneController {
           ->setLabel(pht('Name'))
           ->setValue($v_name)
           ->setError($e_name))
+      ->appendChild(
+        id(new AphrontFormTokenizerControl())
+          ->setDatasource(new PhabricatorPeopleDatasource())
+          ->setLabel(pht('Members'))
+          ->setName('memberPHIDs')
+          ->setValue($member_handles)
+          ->setError($e_members))
       ->appendChild(
         id(new AphrontFormSubmitControl())
           ->setValue($submit_button)

src/applications/phortune/controller/PhortuneAccountViewController.php

@@ -53,13 +53,7 @@ final class PhortuneAccountViewController extends PhortuneController {
           ->setIcon('fa-pencil')
           ->setHref($edit_uri)
           ->setDisabled(!$can_edit)
-          ->setWorkflow(!$can_edit))
+          ->setWorkflow(!$can_edit));
-      ->addAction(
-        id(new PhabricatorActionView())
-          ->setName(pht('Edit Members'))
-          ->setIcon('fa-users')
-          ->setHref('#')
-          ->setDisabled(true));
 
     $crumbs->setActionList($actions);
 
@@ -67,6 +61,12 @@ final class PhortuneAccountViewController extends PhortuneController {
       ->setObject($account)
       ->setUser($user);
 
+    $this->loadHandles($account->getMemberPHIDs());
+
+    $properties->addProperty(
+      pht('Members'),
+      $this->renderHandlesForPHIDs($account->getMemberPHIDs()));
+
     $properties->setActionList($actions);
 
     $payment_methods = $this->buildPaymentMethodsSection($account);

src/applications/phortune/edge/PhortuneAccountHasMemberEdgeType.php

@@ -0,0 +1,101 @@
+<?php
+
+final class PhortuneAccountHasMemberEdgeType extends PhabricatorEdgeType {
+
+  const EDGECONST = 27;
+
+  public function getInverseEdgeConstant() {
+    return PhortuneMemberHasAccountEdgeType::EDGECONST;
+  }
+
+  public function getTransactionAddString(
+    $actor,
+    $add_count,
+    $add_edges) {
+
+    return pht(
+      '%s added %s account member(s): %s.',
+      $actor,
+      $add_count,
+      $add_edges);
+  }
+
+  public function getTransactionRemoveString(
+    $actor,
+    $rem_count,
+    $rem_edges) {
+
+    return pht(
+      '%s removed %s account member(s): %s.',
+      $actor,
+      $rem_count,
+      $rem_edges);
+  }
+
+  public function getTransactionEditString(
+    $actor,
+    $total_count,
+    $add_count,
+    $add_edges,
+    $rem_count,
+    $rem_edges) {
+
+    return pht(
+      '%s edited %s account member(s), added %s: %s; removed %s: %s.',
+      $actor,
+      $total_count,
+      $add_count,
+      $add_edges,
+      $rem_count,
+      $rem_edges);
+  }
+
+  public function getFeedAddString(
+    $actor,
+    $object,
+    $add_count,
+    $add_edges) {
+
+    return pht(
+      '%s added %s account member(s) to %s: %s.',
+      $actor,
+      $add_count,
+      $object,
+      $add_edges);
+  }
+
+  public function getFeedRemoveString(
+    $actor,
+    $object,
+    $rem_count,
+    $rem_edges) {
+
+    return pht(
+      '%s removed %s account member(s) from %s: %s.',
+      $actor,
+      $rem_count,
+      $object,
+      $rem_edges);
+  }
+
+  public function getFeedEditString(
+    $actor,
+    $object,
+    $total_count,
+    $add_count,
+    $add_edges,
+    $rem_count,
+    $rem_edges) {
+
+    return pht(
+      '%s edited %s account member(s) for %s, added %s: %s; removed %s: %s.',
+      $actor,
+      $total_count,
+      $object,
+      $add_count,
+      $add_edges,
+      $rem_count,
+      $rem_edges);
+  }
+
+}

src/applications/phortune/edge/PhortuneMemberHasAccountEdgeType.php

@@ -0,0 +1,12 @@
+<?php
+
+final class PhortuneMemberHasAccountEdgeType
+  extends PhabricatorEdgeType {
+
+  const EDGECONST = 28;
+
+  public function getInverseEdgeConstant() {
+    return PhortuneAccountHasMemberEdgeType::EDGECONST;
+  }
+
+}

src/applications/phortune/editor/PhortuneAccountEditor.php

@@ -91,6 +91,29 @@ final class PhortuneAccountEditor
           $errors[] = $error;
         }
         break;
+      case PhabricatorTransactions::TYPE_EDGE:
+        foreach ($xactions as $xaction) {
+          switch ($xaction->getMetadataValue('edge:type')) {
+            case PhortuneAccountHasMemberEdgeType::EDGECONST:
+              // TODO: This is a bit cumbersome, but validation happens before
+              // transaction normalization. Maybe provide a cleaner attack on
+              // this eventually? There's no way to generate "+" or "-"
+              // transactions right now.
+              $new = $xaction->getNewValue();
+              $set = idx($new, '=', array());
+
+              if (empty($set[$this->requireActor()->getPHID()])) {
+                $error = new PhabricatorApplicationTransactionValidationError(
+                  $type,
+                  pht('Invalid'),
+                  pht('You can not remove yourself as an account member.'),
+                  $xaction);
+                $errors[] = $error;
+              }
+              break;
+          }
+        }
+        break;
     }
 
     return $errors;

src/applications/phortune/query/PhortuneAccountQuery.php

@@ -79,11 +79,12 @@ final class PhortuneAccountQuery
   protected function willFilterPage(array $accounts) {
     $query = id(new PhabricatorEdgeQuery())
       ->withSourcePHIDs(mpull($accounts, 'getPHID'))
-      ->withEdgeTypes(array(PhabricatorEdgeConfig::TYPE_ACCOUNT_HAS_MEMBER));
+      ->withEdgeTypes(array(PhortuneAccountHasMemberEdgeType::EDGECONST));
     $query->execute();
 
     foreach ($accounts as $account) {
       $member_phids = $query->getDestinationPHIDs(array($account->getPHID()));
+      $member_phids = array_reverse($member_phids);
       $account->attachMemberPHIDs($member_phids);
     }
 
@@ -127,7 +128,7 @@ final class PhortuneAccountQuery
         $conn,
         'LEFT JOIN %T m ON a.phid = m.src AND m.type = %d',
         PhabricatorEdgeConfig::TABLE_NAME_EDGE,
-        PhabricatorEdgeConfig::TYPE_ACCOUNT_HAS_MEMBER);
+        PhortuneAccountHasMemberEdgeType::EDGECONST);
     }
 
     return implode(' ', $joins);

src/applications/phortune/storage/PhortuneAccount.php

@@ -36,7 +36,7 @@ final class PhortuneAccount extends PhortuneDAO
       ->setTransactionType(PhabricatorTransactions::TYPE_EDGE)
       ->setMetadataValue(
         'edge:type',
-        PhabricatorEdgeConfig::TYPE_ACCOUNT_HAS_MEMBER)
+        PhortuneAccountHasMemberEdgeType::EDGECONST)
       ->setNewValue(
         array(
           '=' => array($actor->getPHID() => $actor->getPHID()),

src/applications/phortune/storage/PhortuneAccountTransaction.php

@@ -37,31 +37,6 @@ final class PhortuneAccountTransaction
             $new);
         }
         break;
-      case PhabricatorTransactions::TYPE_EDGE:
-        switch ($this->getMetadataValue('edge:type')) {
-          case PhabricatorEdgeConfig::TYPE_ACCOUNT_HAS_MEMBER:
-            $add = array_diff(array_keys($new), array_keys($old));
-            $rem = array_diff(array_keys($old), array_keys($new));
-            if ($add && $rem) {
-              return pht(
-                '%s changed account members, added %s; removed %s.',
-                $this->renderHandleLink($author_phid),
-                $this->renderHandleList($add),
-                $this->renderHandleList($rem));
-            } else if ($add) {
-              return pht(
-                '%s added account members: %s',
-                $this->renderHandleLink($author_phid),
-                $this->renderHandleList($add));
-            } else {
-              return pht(
-                '%s removed account members: %s',
-                $this->renderHandleLink($author_phid),
-                $this->renderHandleList($add));
-            }
-            break;
-        }
-        break;
     }
 
     return parent::getTitle();

src/infrastructure/edges/constants/PhabricatorEdgeConfig.php

@@ -36,9 +36,6 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants {
   const TYPE_OBJECT_HAS_FILE            = 25;
   const TYPE_FILE_HAS_OBJECT            = 26;
 
-  const TYPE_ACCOUNT_HAS_MEMBER         = 27;
-  const TYPE_MEMBER_HAS_ACCOUNT         = 28;
-
   const TYPE_PURCAHSE_HAS_CHARGE        = 29;
   const TYPE_CHARGE_HAS_PURCHASE        = 30;
 
@@ -103,6 +100,9 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants {
       array(9000),
       range(80000, 80005));
 
+    $exclude[] = 27; // Was TYPE_ACCOUNT_HAS_MEMBER
+    $exclude[] = 28; // Was TYPE_MEMBER_HAS_ACCOUNT
+
     $exclude[] = 43; // Was TYPE_OBJECT_HAS_COLUMN
     $exclude[] = 44; // Was TYPE_COLUMN_HAS_OBJECT
 
@@ -164,9 +164,6 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants {
       self::TYPE_OBJECT_HAS_FILE => self::TYPE_FILE_HAS_OBJECT,
       self::TYPE_FILE_HAS_OBJECT => self::TYPE_OBJECT_HAS_FILE,
 
-      self::TYPE_ACCOUNT_HAS_MEMBER => self::TYPE_MEMBER_HAS_ACCOUNT,
-      self::TYPE_MEMBER_HAS_ACCOUNT => self::TYPE_ACCOUNT_HAS_MEMBER,
-
       self::TYPE_DREV_HAS_COMMIT => self::TYPE_COMMIT_HAS_DREV,
       self::TYPE_COMMIT_HAS_DREV => self::TYPE_DREV_HAS_COMMIT,
 
@@ -284,10 +281,6 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants {
         return '%s edited unsubcriber(s), added %d: %s; removed %d: %s.';
       case self::TYPE_OBJECT_HAS_FILE:
         return '%s edited file(s), added %d: %s; removed %d: %s.';
-      case self::TYPE_ACCOUNT_HAS_MEMBER:
-        return '%s edited member(s), added %d: %s; removed %d: %s.';
-      case self::TYPE_MEMBER_HAS_ACCOUNT:
-        return '%s edited account(s), added %d: %s; removed %d: %s.';
       case self::TYPE_PURCAHSE_HAS_CHARGE:
         return '%s edited charge(s), added %d: %s; removed %d: %s.';
       case self::TYPE_CHARGE_HAS_PURCHASE:
@@ -354,10 +347,6 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants {
         return '%s added %d unsubcriber(s): %s.';
       case self::TYPE_OBJECT_HAS_FILE:
         return '%s added %d file(s): %s.';
-      case self::TYPE_ACCOUNT_HAS_MEMBER:
-        return '%s added %d member(s): %s.';
-      case self::TYPE_MEMBER_HAS_ACCOUNT:
-        return '%s added %d account(s): %s.';
       case self::TYPE_PURCAHSE_HAS_CHARGE:
         return '%s added %d charge(s): %s.';
       case self::TYPE_CHARGE_HAS_PURCHASE:
@@ -427,10 +416,6 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants {
         return '%s removed %d unsubcriber(s): %s.';
       case self::TYPE_OBJECT_HAS_FILE:
         return '%s removed %d file(s): %s.';
-      case self::TYPE_ACCOUNT_HAS_MEMBER:
-        return '%s removed %d member(s): %s.';
-      case self::TYPE_MEMBER_HAS_ACCOUNT:
-        return '%s removed %d account(s): %s.';
       case self::TYPE_PURCAHSE_HAS_CHARGE:
         return '%s removed %d charge(s): %s.';
       case self::TYPE_CHARGE_HAS_PURCHASE:
@@ -496,10 +481,6 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants {
         return '%s updated unsubcribers of %s.';
       case self::TYPE_OBJECT_HAS_FILE:
         return '%s updated files of %s.';
-      case self::TYPE_ACCOUNT_HAS_MEMBER:
-        return '%s updated members of %s.';
-      case self::TYPE_MEMBER_HAS_ACCOUNT:
-        return '%s updated accounts of %s.';
       case self::TYPE_PURCAHSE_HAS_CHARGE:
         return '%s updated charges of %s.';
       case self::TYPE_CHARGE_HAS_PURCHASE: