Move Owners to EditEngine

Summary: Ref T9132. Paste is in fairly good shape so Owners is up next. Reasoning: - One install wants API access for it. - It's a simple application for getting CustomFields working with EditEngine. This only does the EditEngine part, so CustomFields are no longer editable until I make that work. That will be up next, and I'll hold this until that's ready. Test Plan: - Created and edited packages via web UI. - Created and edited package editing forms via web UI. - Created and edited packages via Conduit. Reviewers: chad Reviewed By: chad Maniphest Tasks: T9132 Differential Revision: https://secure.phabricator.com/D14598
2025-01-31 00:48:21 +01:00 · 2015-11-29 09:33:36 -08:00 · 2015-11-29 09:33:36 -08:00 · a407b83dc2
commit a407b83dc2
parent b596d850ff
7 changed files with 159 additions and 249 deletions
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@ -1403,6 +1403,7 @@ phutil_register_library_map(array(
    'NuanceSourceViewController' => 'applications/nuance/controller/NuanceSourceViewController.php',
    'NuanceTransaction' => 'applications/nuance/storage/NuanceTransaction.php',
    'OwnersConduitAPIMethod' => 'applications/owners/conduit/OwnersConduitAPIMethod.php',
+    'OwnersEditConduitAPIMethod' => 'applications/owners/conduit/OwnersEditConduitAPIMethod.php',
    'OwnersPackageReplyHandler' => 'applications/owners/mail/OwnersPackageReplyHandler.php',
    'OwnersQueryConduitAPIMethod' => 'applications/owners/conduit/OwnersQueryConduitAPIMethod.php',
    'PHIDConduitAPIMethod' => 'applications/phid/conduit/PHIDConduitAPIMethod.php',
@ -2564,6 +2565,7 @@ phutil_register_library_map(array(
    'PhabricatorOwnersOwner' => 'applications/owners/storage/PhabricatorOwnersOwner.php',
    'PhabricatorOwnersPackage' => 'applications/owners/storage/PhabricatorOwnersPackage.php',
    'PhabricatorOwnersPackageDatasource' => 'applications/owners/typeahead/PhabricatorOwnersPackageDatasource.php',
+    'PhabricatorOwnersPackageEditEngine' => 'applications/owners/editor/PhabricatorOwnersPackageEditEngine.php',
    'PhabricatorOwnersPackageFunctionDatasource' => 'applications/owners/typeahead/PhabricatorOwnersPackageFunctionDatasource.php',
    'PhabricatorOwnersPackageOwnerDatasource' => 'applications/owners/typeahead/PhabricatorOwnersPackageOwnerDatasource.php',
    'PhabricatorOwnersPackagePHIDType' => 'applications/owners/phid/PhabricatorOwnersPackagePHIDType.php',
@ -5398,6 +5400,7 @@ phutil_register_library_map(array(
    'NuanceSourceViewController' => 'NuanceController',
    'NuanceTransaction' => 'PhabricatorApplicationTransaction',
    'OwnersConduitAPIMethod' => 'ConduitAPIMethod',
+    'OwnersEditConduitAPIMethod' => 'PhabricatorEditEngineAPIMethod',
    'OwnersPackageReplyHandler' => 'PhabricatorMailReplyHandler',
    'OwnersQueryConduitAPIMethod' => 'OwnersConduitAPIMethod',
    'PHIDConduitAPIMethod' => 'ConduitAPIMethod',
@ -6741,6 +6744,7 @@ phutil_register_library_map(array(
      'PhabricatorCustomFieldInterface',
    ),
    'PhabricatorOwnersPackageDatasource' => 'PhabricatorTypeaheadDatasource',
+    'PhabricatorOwnersPackageEditEngine' => 'PhabricatorEditEngine',
    'PhabricatorOwnersPackageFunctionDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
    'PhabricatorOwnersPackageOwnerDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
    'PhabricatorOwnersPackagePHIDType' => 'PhabricatorPHIDType',
--- a/src/applications/owners/application/PhabricatorOwnersApplication.php
+++ b/src/applications/owners/application/PhabricatorOwnersApplication.php
@ -43,10 +43,12 @@ final class PhabricatorOwnersApplication extends PhabricatorApplication {
    return array(
      '/owners/' => array(
        '(?:query/(?P<queryKey>[^/]+)/)?' => 'PhabricatorOwnersListController',
-        'edit/(?P<id>[1-9]\d*)/' => 'PhabricatorOwnersEditController',
        'new/' => 'PhabricatorOwnersEditController',
        'package/(?P<id>[1-9]\d*)/' => 'PhabricatorOwnersDetailController',
        'paths/(?P<id>[1-9]\d*)/' => 'PhabricatorOwnersPathsController',
+
+        $this->getEditRoutePattern('edit/')
+          => 'PhabricatorOwnersEditController',
      ),
    );
  }
--- a/src/applications/owners/conduit/OwnersEditConduitAPIMethod.php
+++ b/src/applications/owners/conduit/OwnersEditConduitAPIMethod.php
@ -0,0 +1,20 @@
+<?php
+
+final class OwnersEditConduitAPIMethod
+  extends PhabricatorEditEngineAPIMethod {
+
+  public function getAPIMethodName() {
+    return 'owners.edit';
+  }
+
+  public function newEditEngine() {
+    return new PhabricatorOwnersPackageEditEngine();
+  }
+
+  public function getMethodSummary() {
+    return pht(
+      'Apply transactions to create a new Owners package or edit an existing '.
+      'one.');
+  }
+
+}
--- a/src/applications/owners/controller/PhabricatorOwnersEditController.php
+++ b/src/applications/owners/controller/PhabricatorOwnersEditController.php
@ -4,201 +4,9 @@ final class PhabricatorOwnersEditController
  extends PhabricatorOwnersController {

  public function handleRequest(AphrontRequest $request) {
-    $viewer = $request->getUser();
-
-    $id = $request->getURIData('id');
-    if ($id) {
-      $package = id(new PhabricatorOwnersPackageQuery())
-        ->setViewer($viewer)
-        ->withIDs(array($id))
-        ->requireCapabilities(
-          array(
-            PhabricatorPolicyCapability::CAN_VIEW,
-            // TODO: Support this capability.
-            // PhabricatorPolicyCapability::CAN_EDIT,
-          ))
-        ->needOwners(true)
-        ->executeOne();
-      if (!$package) {
-        return new Aphront404Response();
-      }
-      $is_new = false;
-    } else {
-      $package = PhabricatorOwnersPackage::initializeNewPackage($viewer);
-      $is_new = true;
-    }
-
-    $e_name = true;
-
-    $v_name = $package->getName();
-    $v_owners = mpull($package->getOwners(), 'getUserPHID');
-    $v_auditing = $package->getAuditingEnabled();
-    $v_description = $package->getDescription();
-    $v_status = $package->getStatus();
-
-    $field_list = PhabricatorCustomField::getObjectFields(
-      $package,
-      PhabricatorCustomField::ROLE_EDIT);
-    $field_list->setViewer($viewer);
-    $field_list->readFieldsFromStorage($package);
-
-    $errors = array();
-    if ($request->isFormPost()) {
-      $xactions = array();
-
-      $v_name = $request->getStr('name');
-      $v_owners = $request->getArr('owners');
-      $v_auditing = ($request->getStr('auditing') == 'enabled');
-      $v_description = $request->getStr('description');
-      $v_status = $request->getStr('status');
-
-      $type_name = PhabricatorOwnersPackageTransaction::TYPE_NAME;
-      $type_owners = PhabricatorOwnersPackageTransaction::TYPE_OWNERS;
-      $type_auditing = PhabricatorOwnersPackageTransaction::TYPE_AUDITING;
-      $type_description = PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION;
-      $type_status = PhabricatorOwnersPackageTransaction::TYPE_STATUS;
-
-      $xactions[] = id(new PhabricatorOwnersPackageTransaction())
-        ->setTransactionType($type_name)
-        ->setNewValue($v_name);
-
-      $xactions[] = id(new PhabricatorOwnersPackageTransaction())
-        ->setTransactionType($type_owners)
-        ->setNewValue($v_owners);
-
-      $xactions[] = id(new PhabricatorOwnersPackageTransaction())
-        ->setTransactionType($type_auditing)
-        ->setNewValue($v_auditing);
-
-      $xactions[] = id(new PhabricatorOwnersPackageTransaction())
-        ->setTransactionType($type_description)
-        ->setNewValue($v_description);
-
-      if (!$is_new) {
-        $xactions[] = id(new PhabricatorOwnersPackageTransaction())
-          ->setTransactionType($type_status)
-          ->setNewValue($v_status);
-      }
-
-      $field_xactions = $field_list->buildFieldTransactionsFromRequest(
-        new PhabricatorOwnersPackageTransaction(),
-        $request);
-
-      $xactions = array_merge($xactions, $field_xactions);
-
-      $editor = id(new PhabricatorOwnersPackageTransactionEditor())
-        ->setActor($viewer)
-        ->setContentSourceFromRequest($request)
-        ->setContinueOnNoEffect(true);
-
-      try {
-        $editor->applyTransactions($package, $xactions);
-
-        $id = $package->getID();
-        if ($is_new) {
-          $next_uri = '/owners/paths/'.$id.'/';
-        } else {
-          $next_uri = '/owners/package/'.$id.'/';
-        }
-
-        return id(new AphrontRedirectResponse())->setURI($next_uri);
-      } catch (AphrontDuplicateKeyQueryException $ex) {
-        $e_name = pht('Duplicate');
-        $errors[] = pht('Package name must be unique.');
-      } catch (PhabricatorApplicationTransactionValidationException $ex) {
-        $validation_exception = $ex;
-
-        $e_name = $ex->getShortMessage($type_name);
-      }
-    }
-
-    if ($is_new) {
-      $cancel_uri = '/owners/';
-      $title = pht('New Package');
-      $button_text = pht('Continue');
-    } else {
-      $cancel_uri = '/owners/package/'.$package->getID().'/';
-      $title = pht('Edit Package');
-      $button_text = pht('Save Package');
-    }
-
-    $form = id(new AphrontFormView())
-      ->setUser($viewer)
-      ->appendChild(
-        id(new AphrontFormTextControl())
-          ->setLabel(pht('Name'))
-          ->setName('name')
-          ->setValue($v_name)
-          ->setError($e_name))
-      ->appendControl(
-        id(new AphrontFormTokenizerControl())
-          ->setDatasource(new PhabricatorProjectOrUserDatasource())
-          ->setLabel(pht('Owners'))
-          ->setName('owners')
-          ->setValue($v_owners));
-
-    if (!$is_new) {
-      $form->appendChild(
-        id(new AphrontFormSelectControl())
-          ->setLabel(pht('Status'))
-          ->setName('status')
-          ->setValue($v_status)
-          ->setOptions($package->getStatusNameMap()));
-    }
-
-    $form->appendChild(
-      id(new AphrontFormSelectControl())
-        ->setName('auditing')
-        ->setLabel(pht('Auditing'))
-        ->setCaption(
-          pht(
-            'With auditing enabled, all future commits that touch '.
-            'this package will be reviewed to make sure an owner '.
-            'of the package is involved and the commit message has '.
-            'a valid revision, reviewed by, and author.'))
-        ->setOptions(
-          array(
-            'disabled'  => pht('Disabled'),
-            'enabled'   => pht('Enabled'),
-          ))
-        ->setValue(($v_auditing ? 'enabled' : 'disabled')))
-      ->appendChild(
-        id(new PhabricatorRemarkupControl())
-          ->setUser($viewer)
-          ->setLabel(pht('Description'))
-          ->setName('description')
-          ->setValue($v_description));
-
-    $field_list->appendFieldsToForm($form);
-
-    $form->appendChild(
-      id(new AphrontFormSubmitControl())
-        ->addCancelButton($cancel_uri)
-        ->setValue($button_text));
-
-    $form_box = id(new PHUIObjectBoxView())
-      ->setHeaderText($title)
-      ->setFormErrors($errors)
-      ->setForm($form);
-
-    $crumbs = $this->buildApplicationCrumbs();
-    if ($package->getID()) {
-      $crumbs->addTextCrumb(
-        $package->getName(),
-        $this->getApplicationURI('package/'.$package->getID().'/'));
-      $crumbs->addTextCrumb(pht('Edit'));
-    } else {
-      $crumbs->addTextCrumb(pht('New Package'));
-    }
-
-    return $this->buildApplicationPage(
-      array(
-        $crumbs,
-        $form_box,
-      ),
-      array(
-        'title' => $title,
-      ));
+    return id(new PhabricatorOwnersPackageEditEngine())
+      ->setController($this)
+      ->buildResponse();
  }

 }
--- a/src/applications/owners/controller/PhabricatorOwnersListController.php
+++ b/src/applications/owners/controller/PhabricatorOwnersListController.php
@ -8,45 +8,17 @@ final class PhabricatorOwnersListController
  }

  public function handleRequest(AphrontRequest $request) {
-    $controller = id(new PhabricatorApplicationSearchController())
-      ->setQueryKey($request->getURIData('queryKey'))
-      ->setSearchEngine(new PhabricatorOwnersPackageSearchEngine())
-      ->setNavigation($this->buildSideNavView());
-
-    return $this->delegateToController($controller);
-  }
-
-  public function buildSideNavView($for_app = false) {
-    $viewer = $this->getViewer();
-
-    $nav = new AphrontSideNavFilterView();
-    $nav->setBaseURI(new PhutilURI($this->getApplicationURI()));
-
-    if ($for_app) {
-      $nav->addFilter('new/', pht('Create Package'));
-    }
-
-    id(new PhabricatorOwnersPackageSearchEngine())
-      ->setViewer($viewer)
-      ->addNavigationItems($nav->getMenu());
-
-    $nav->selectFilter(null);
-
-    return $nav;
-  }
-
-  public function buildApplicationMenu() {
-    return $this->buildSideNavView(true)->getMenu();
+    return id(new PhabricatorOwnersPackageSearchEngine())
+      ->setController($this)
+      ->buildResponse();
  }

  protected function buildApplicationCrumbs() {
    $crumbs = parent::buildApplicationCrumbs();

-    $crumbs->addAction(
-      id(new PHUIListItemView())
-        ->setName(pht('Create Package'))
-        ->setHref($this->getApplicationURI('new/'))
-        ->setIcon('fa-plus-square'));
+    id(new PhabricatorOwnersPackageEditEngine())
+      ->setViewer($this->getViewer())
+      ->addActionToCrumbs($crumbs);

    return $crumbs;
  }
--- a/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php
+++ b/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php
@ -0,0 +1,93 @@
+<?php
+
+final class PhabricatorOwnersPackageEditEngine
+  extends PhabricatorEditEngine {
+
+  const ENGINECONST = 'owners.package';
+
+  public function getEngineName() {
+    return pht('Owners Packages');
+  }
+
+  public function getEngineApplicationClass() {
+    return 'PhabricatorOwnersApplication';
+  }
+
+  protected function newEditableObject() {
+    return PhabricatorOwnersPackage::initializeNewPackage($this->getViewer());
+  }
+
+  protected function newObjectQuery() {
+    return id(new PhabricatorOwnersPackageQuery())
+      ->needOwners(true);
+  }
+
+  protected function getObjectCreateTitleText($object) {
+    return pht('Create New Package');
+  }
+
+  protected function getObjectEditTitleText($object) {
+    return pht('Edit Package %s', $object->getName());
+  }
+
+  protected function getObjectEditShortText($object) {
+    return pht('Package %d', $object->getID());
+  }
+
+  protected function getObjectCreateShortText() {
+    return pht('Create Package');
+  }
+
+  protected function getObjectViewURI($object) {
+    $id = $object->getID();
+    return "/owners/package/{$id}/";
+  }
+
+  protected function buildCustomEditFields($object) {
+    return array(
+      id(new PhabricatorTextEditField())
+        ->setKey('name')
+        ->setLabel(pht('Name'))
+        ->setDescription(pht('Name of the package.'))
+        ->setTransactionType(PhabricatorOwnersPackageTransaction::TYPE_NAME)
+        ->setIsRequired(true)
+        ->setValue($object->getName()),
+      id(new PhabricatorDatasourceEditField())
+        ->setKey('owners')
+        ->setLabel(pht('Owners'))
+        ->setDescription(pht('Users and projects which own the package.'))
+        ->setTransactionType(PhabricatorOwnersPackageTransaction::TYPE_OWNERS)
+        ->setDatasource(new PhabricatorProjectOrUserDatasource())
+        ->setValue($object->getOwnerPHIDs()),
+      id(new PhabricatorSelectEditField())
+        ->setKey('status')
+        ->setLabel(pht('Status'))
+        ->setDescription(pht('Archive or enable the package.'))
+        ->setTransactionType(PhabricatorOwnersPackageTransaction::TYPE_STATUS)
+        ->setValue($object->getStatus())
+        ->setOptions($object->getStatusNameMap()),
+      id(new PhabricatorSelectEditField())
+        ->setKey('auditing')
+        ->setLabel(pht('Auditing'))
+        ->setDescription(
+          pht(
+            'Automatically trigger audits for commits affecting files in '.
+            'this package.'))
+        ->setTransactionType(PhabricatorOwnersPackageTransaction::TYPE_AUDITING)
+        ->setValue($object->getAuditingEnabled())
+        ->setOptions(
+          array(
+            '' => pht('Disabled'),
+            '1' => pht('Enabled'),
+          )),
+      id(new PhabricatorRemarkupEditField())
+        ->setKey('description')
+        ->setLabel(pht('Description'))
+        ->setDescription(pht('Human-readable description of the package.'))
+        ->setTransactionType(
+          PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION)
+        ->setValue($object->getDescription()),
+    );
+  }
+
+}
--- a/src/applications/owners/storage/PhabricatorOwnersPackage.php
+++ b/src/applications/owners/storage/PhabricatorOwnersPackage.php
@ -27,25 +27,8 @@ final class PhabricatorOwnersPackage
      ->setAuditingEnabled(0)
      ->attachPaths(array())
      ->setStatus(self::STATUS_ACTIVE)
-      ->attachOwners(array());
-  }
-
-  public function getCapabilities() {
-    return array(
-      PhabricatorPolicyCapability::CAN_VIEW,
-    );
-  }
-
-  public function getPolicy($capability) {
-    return PhabricatorPolicies::POLICY_USER;
-  }
-
-  public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
-    return false;
-  }
-
-  public function describeAutomaticCapability($capability) {
-    return null;
+      ->attachOwners(array())
+      ->setDescription('');
  }

  public static function getStatusNameMap() {
@ -284,6 +267,34 @@ final class PhabricatorOwnersPackage
    return $this->assertAttached($this->owners);
  }

+  public function getOwnerPHIDs() {
+    return mpull($this->getOwners(), 'getUserPHID');
+  }
+
+
+/* -(  PhabricatorPolicyInterface  )----------------------------------------- */
+
+
+  public function getCapabilities() {
+    return array(
+      PhabricatorPolicyCapability::CAN_VIEW,
+      PhabricatorPolicyCapability::CAN_EDIT,
+    );
+  }
+
+  public function getPolicy($capability) {
+    // TODO: Implement proper policies.
+    return PhabricatorPolicies::POLICY_USER;
+  }
+
+  public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
+    return false;
+  }
+
+  public function describeAutomaticCapability($capability) {
+    return null;
+  }
+

 /* -(  PhabricatorApplicationTransactionInterface  )------------------------- */