Add diffusion.uri.edit for creating and editing repository URIs

Summary: Ref T10748. Brings the rest of the transactions to EditEngine, supports creating via API. Test Plan: - Created a URI via API. - Created a URI via web. - Tried to apply sneaky transactions, got rejected with good error messages. <_< >_> Reviewers: chad Reviewed By: chad Maniphest Tasks: T10748 Differential Revision: https://secure.phabricator.com/D15821
2024-11-27 09:12:41 +01:00 · 2016-04-29 13:10:37 -07:00 · 2016-04-29 13:10:37 -07:00 · da599386f6
commit da599386f6
parent 128995f1ac
7 changed files with 188 additions and 6 deletions
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@ -816,6 +816,7 @@ phutil_register_library_map(array(
    'DiffusionTagListController' => 'applications/diffusion/controller/DiffusionTagListController.php',
    'DiffusionTagListView' => 'applications/diffusion/view/DiffusionTagListView.php',
    'DiffusionTagsQueryConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionTagsQueryConduitAPIMethod.php',
+    'DiffusionURIEditConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionURIEditConduitAPIMethod.php',
    'DiffusionURIEditEngine' => 'applications/diffusion/editor/DiffusionURIEditEngine.php',
    'DiffusionURIEditor' => 'applications/diffusion/editor/DiffusionURIEditor.php',
    'DiffusionURITestCase' => 'applications/diffusion/request/__tests__/DiffusionURITestCase.php',
@ -5044,6 +5045,7 @@ phutil_register_library_map(array(
    'DiffusionTagListController' => 'DiffusionController',
    'DiffusionTagListView' => 'DiffusionView',
    'DiffusionTagsQueryConduitAPIMethod' => 'DiffusionQueryConduitAPIMethod',
+    'DiffusionURIEditConduitAPIMethod' => 'PhabricatorEditEngineAPIMethod',
    'DiffusionURIEditEngine' => 'PhabricatorEditEngine',
    'DiffusionURIEditor' => 'PhabricatorApplicationTransactionEditor',
    'DiffusionURITestCase' => 'PhutilTestCase',
--- a/src/applications/diffusion/conduit/DiffusionURIEditConduitAPIMethod.php
+++ b/src/applications/diffusion/conduit/DiffusionURIEditConduitAPIMethod.php
@ -0,0 +1,20 @@
+<?php
+
+final class DiffusionURIEditConduitAPIMethod
+  extends PhabricatorEditEngineAPIMethod {
+
+  public function getAPIMethodName() {
+    return 'diffusion.uri.edit';
+  }
+
+  public function newEditEngine() {
+    return new DiffusionURIEditEngine();
+  }
+
+  public function getMethodSummary() {
+    return pht(
+      'Apply transactions to create a new repository URI or edit an existing '.
+      'one.');
+  }
+
+}
--- a/src/applications/diffusion/editor/DiffusionURIEditEngine.php
+++ b/src/applications/diffusion/editor/DiffusionURIEditEngine.php
@ -37,8 +37,15 @@ final class DiffusionURIEditEngine
  }

  protected function newEditableObject() {
+    $uri = PhabricatorRepositoryURI::initializeNewURI();
+
    $repository = $this->getRepository();
-    return PhabricatorRepositoryURI::initializeNewURI($repository);
+    if ($repository) {
+      $uri->setRepositoryPHID($repository->getPHID());
+      $uri->attachRepository($repository);
+    }
+
+    return $uri;
  }

  protected function newObjectQuery() {
@ -77,6 +84,23 @@ final class DiffusionURIEditEngine
    $viewer = $this->getViewer();

    return array(
+      id(new PhabricatorHandlesEditField())
+        ->setKey('repository')
+        ->setAliases(array('repositoryPHID'))
+        ->setLabel(pht('Repository'))
+        ->setIsRequired(true)
+        ->setIsConduitOnly(true)
+        ->setTransactionType(
+          PhabricatorRepositoryURITransaction::TYPE_REPOSITORY)
+        ->setDescription(pht('The repository this URI is associated with.'))
+        ->setConduitDescription(
+          pht(
+            'Create a URI in a given repository. This transaction type '.
+            'must be present when creating a new URI and must not be '.
+            'present when editing an existing URI.'))
+        ->setConduitTypeDescription(
+          pht('Repository PHID to create a new URI for.'))
+        ->setSingleValue($object->getRepositoryPHID()),
      id(new PhabricatorTextEditField())
        ->setKey('uri')
        ->setLabel(pht('URI'))
@ -104,6 +128,28 @@ final class DiffusionURIEditEngine
        ->setConduitTypeDescription(pht('New display behavior.'))
        ->setValue($object->getDisplayType())
        ->setOptions($object->getAvailableDisplayTypeOptions()),
+      id(new PhabricatorHandlesEditField())
+        ->setKey('credential')
+        ->setAliases(array('credentialPHID'))
+        ->setLabel(pht('Credential'))
+        ->setIsConduitOnly(true)
+        ->setTransactionType(
+          PhabricatorRepositoryURITransaction::TYPE_CREDENTIAL)
+        ->setDescription(
+          pht('The credential to use when interacting with this URI.'))
+        ->setConduitDescription(pht('Change the credential for this URI.'))
+        ->setConduitTypeDescription(pht('New credential PHID, or null.'))
+        ->setSingleValue($object->getCredentialPHID()),
+      id(new PhabricatorBoolEditField())
+        ->setKey('disable')
+        ->setLabel(pht('Disabled'))
+        ->setIsConduitOnly(true)
+        ->setTransactionType(PhabricatorRepositoryURITransaction::TYPE_DISABLE)
+        ->setDescription(pht('Active status of the URI.'))
+        ->setConduitDescription(pht('Disable or activate the URI.'))
+        ->setConduitTypeDescription(pht('True to disable the URI.'))
+        ->setOptions(pht('Enable'), pht('Disable'))
+        ->setValue($object->getIsDisabled()),
    );
  }

--- a/src/applications/diffusion/editor/DiffusionURIEditor.php
+++ b/src/applications/diffusion/editor/DiffusionURIEditor.php
@ -14,9 +14,12 @@ final class DiffusionURIEditor
  public function getTransactionTypes() {
    $types = parent::getTransactionTypes();

+    $types[] = PhabricatorRepositoryURITransaction::TYPE_REPOSITORY;
    $types[] = PhabricatorRepositoryURITransaction::TYPE_URI;
    $types[] = PhabricatorRepositoryURITransaction::TYPE_IO;
    $types[] = PhabricatorRepositoryURITransaction::TYPE_DISPLAY;
+    $types[] = PhabricatorRepositoryURITransaction::TYPE_CREDENTIAL;
+    $types[] = PhabricatorRepositoryURITransaction::TYPE_DISABLE;

    return $types;
  }
@ -32,6 +35,12 @@ final class DiffusionURIEditor
        return $object->getIOType();
      case PhabricatorRepositoryURITransaction::TYPE_DISPLAY:
        return $object->getDisplayType();
+      case PhabricatorRepositoryURITransaction::TYPE_REPOSITORY:
+        return $object->getRepositoryPHID();
+      case PhabricatorRepositoryURITransaction::TYPE_CREDENTIAL:
+        return $object->getCredentialPHID();
+      case PhabricatorRepositoryURITransaction::TYPE_DISABLE:
+        return (int)$object->getIsDisabled();
    }

    return parent::getCustomTransactionOldValue($object, $xaction);
@ -45,7 +54,11 @@ final class DiffusionURIEditor
      case PhabricatorRepositoryURITransaction::TYPE_URI:
      case PhabricatorRepositoryURITransaction::TYPE_IO:
      case PhabricatorRepositoryURITransaction::TYPE_DISPLAY:
+      case PhabricatorRepositoryURITransaction::TYPE_REPOSITORY:
+      case PhabricatorRepositoryURITransaction::TYPE_CREDENTIAL:
        return $xaction->getNewValue();
+      case PhabricatorRepositoryURITransaction::TYPE_DISABLE:
+        return (int)$xaction->getNewValue();
    }

    return parent::getCustomTransactionNewValue($object, $xaction);
@ -65,6 +78,15 @@ final class DiffusionURIEditor
      case PhabricatorRepositoryURITransaction::TYPE_DISPLAY:
        $object->setDisplayType($xaction->getNewValue());
        break;
+      case PhabricatorRepositoryURITransaction::TYPE_REPOSITORY:
+        $object->setRepositoryPHID($xaction->getNewValue());
+        break;
+      case PhabricatorRepositoryURITransaction::TYPE_CREDENTIAL:
+        $object->setCredentialPHID($xaction->getNewValue());
+        break;
+      case PhabricatorRepositoryURITransaction::TYPE_DISABLE:
+        $object->setIsDisabled($xaction->getNewValue());
+        break;
    }
  }

@ -76,6 +98,9 @@ final class DiffusionURIEditor
      case PhabricatorRepositoryURITransaction::TYPE_URI:
      case PhabricatorRepositoryURITransaction::TYPE_IO:
      case PhabricatorRepositoryURITransaction::TYPE_DISPLAY:
+      case PhabricatorRepositoryURITransaction::TYPE_REPOSITORY:
+      case PhabricatorRepositoryURITransaction::TYPE_CREDENTIAL:
+      case PhabricatorRepositoryURITransaction::TYPE_DISABLE:
        return;
    }

@ -90,6 +115,92 @@ final class DiffusionURIEditor
    $errors = parent::validateTransaction($object, $type, $xactions);

    switch ($type) {
+      case PhabricatorRepositoryURITransaction::TYPE_REPOSITORY:
+        $missing = $this->validateIsEmptyTextField(
+          $object->getRepositoryPHID(),
+          $xactions);
+        if ($missing) {
+          // NOTE: This isn't being marked as a missing field error because
+          // it's a fundamental, required property of the URI.
+          $errors[] = new PhabricatorApplicationTransactionValidationError(
+            $type,
+            pht('Required'),
+            pht(
+              'When creating a repository URI, you must specify which '.
+              'repository the URI will belong to.'),
+            nonempty(last($xactions), null));
+          break;
+        }
+
+        $viewer = $this->getActor();
+
+        foreach ($xactions as $xaction) {
+          $repository_phid = $xaction->getNewValue();
+
+          // If this isn't changing anything, let it through as-is.
+          if ($repository_phid == $object->getRepositoryPHID()) {
+            continue;
+          }
+
+          if (!$this->getIsNewObject()) {
+            $errors[] = new PhabricatorApplicationTransactionValidationError(
+              $type,
+              pht('Invalid'),
+              pht(
+                'The repository a URI is associated with is immutable, and '.
+                'can not be changed after the URI is created.'),
+              $xaction);
+            continue;
+          }
+
+          $repository = id(new PhabricatorRepositoryQuery())
+            ->setViewer($viewer)
+            ->withPHIDs(array($repository_phid))
+            ->requireCapabilities(
+              array(
+                PhabricatorPolicyCapability::CAN_VIEW,
+                PhabricatorPolicyCapability::CAN_EDIT,
+              ))
+            ->executeOne();
+          if (!$repository) {
+            $errors[] = new PhabricatorApplicationTransactionValidationError(
+              $type,
+              pht('Invalid'),
+              pht(
+                'To create a URI for a repository ("%s"), it must exist and '.
+                'you must have permission to edit it.',
+                $repository_phid),
+              $xaction);
+            continue;
+          }
+        }
+        break;
+      case PhabricatorRepositoryURITransaction::TYPE_CREDENTIAL:
+        $viewer = $this->getActor();
+        foreach ($xactions as $xaction) {
+          $credential_phid = $xaction->getNewValue();
+
+          if ($credential_phid == $object->getCredentialPHID()) {
+            continue;
+          }
+
+          $credential = id(new PassphraseCredentialQuery())
+            ->setViewer($viewer)
+            ->withPHIDs(array($credential_phid))
+            ->executeOne();
+          if (!$credential) {
+            $errors[] = new PhabricatorApplicationTransactionValidationError(
+              $type,
+              pht('Invalid'),
+              pht(
+                'You can only associate a credential ("%s") with a repository '.
+                'URI if it exists and you have permission to see it.',
+                $credential_phid),
+              $xaction);
+            continue;
+          }
+        }
+        break;
      case PhabricatorRepositoryURITransaction::TYPE_URI:
        $missing = $this->validateIsEmptyTextField(
          $object->getURI(),
@ -99,7 +210,7 @@ final class DiffusionURIEditor
          $error = new PhabricatorApplicationTransactionValidationError(
            $type,
            pht('Required'),
-            pht('Repository URI is required.'),
+            pht('A repository URI must have a nonempty URI.'),
            nonempty(last($xactions), null));

          $error->setIsMissingFieldError(true);
--- a/src/applications/repository/storage/PhabricatorRepository.php
+++ b/src/applications/repository/storage/PhabricatorRepository.php
@ -2179,7 +2179,9 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
    $uris = array();
    foreach ($protocol_map as $protocol => $proto_supported) {
      foreach ($identifier_map as $identifier => $id_supported) {
-        $uris[] = PhabricatorRepositoryURI::initializeNewURI($this)
+        $uris[] = PhabricatorRepositoryURI::initializeNewURI()
+          ->setRepositoryPHID($this->getPHID())
+          ->attachRepository($this)
          ->setBuiltinProtocol($protocol)
          ->setBuiltinIdentifier($identifier)
          ->setIsDisabled(!$proto_supported || !$id_supported);
--- a/src/applications/repository/storage/PhabricatorRepositoryURI.php
+++ b/src/applications/repository/storage/PhabricatorRepositoryURI.php
@ -62,10 +62,8 @@ final class PhabricatorRepositoryURI
    ) + parent::getConfiguration();
  }

-  public static function initializeNewURI(PhabricatorRepository $repository) {
+  public static function initializeNewURI() {
    return id(new self())
-      ->attachRepository($repository)
-      ->setRepositoryPHID($repository->getPHID())
      ->setIoType(self::IO_DEFAULT)
      ->setDisplayType(self::DISPLAY_DEFAULT)
      ->setIsDisabled(0);
--- a/src/applications/repository/storage/PhabricatorRepositoryURITransaction.php
+++ b/src/applications/repository/storage/PhabricatorRepositoryURITransaction.php
@ -3,9 +3,12 @@
 final class PhabricatorRepositoryURITransaction
  extends PhabricatorApplicationTransaction {

+  const TYPE_REPOSITORY = 'diffusion.uri.repository';
  const TYPE_URI = 'diffusion.uri.uri';
  const TYPE_IO = 'diffusion.uri.io';
  const TYPE_DISPLAY = 'diffusion.uri.display';
+  const TYPE_CREDENTIAL = 'diffusion.uri.credential';
+  const TYPE_DISABLE = 'diffusion.uri.disable';

  public function getApplicationName() {
    return 'repository';