Support more transactions types in RepositoryEditEngine

Summary: Ref T10748. This supports more transaction types in the modern editor and improves validation so Conduit benefits. You can technically create repositories via `diffusion.repository.edit` now, although they aren't very useful. Test Plan: - Used `diffusion.repository.edit` to create and edit repositories. - Used `/editpro/` to edit repositories. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10748 Differential Revision: https://secure.phabricator.com/D15740
2024-09-20 09:18:48 +02:00 · 2016-04-16 18:17:13 -07:00 · 2016-04-16 18:17:13 -07:00 · fbfe730452
commit fbfe730452
parent 92c50de8aa
7 changed files with 284 additions and 39 deletions
--- a/src/applications/diffusion/controller/DiffusionRepositoryCreateController.php
+++ b/src/applications/diffusion/controller/DiffusionRepositoryCreateController.php
@ -162,9 +162,15 @@ final class DiffusionRepositoryCreateController

          $activate = $form->getPage('done')
            ->getControl('activate')->getValue();
+          if ($activate == 'start') {
+            $initial_status = PhabricatorRepository::STATUS_ACTIVE;
+          } else {
+            $initial_status = PhabricatorRepository::STATUS_INACTIVE;
+          }
+
          $xactions[] = id(clone $template)
            ->setTransactionType($type_activate)
-            ->setNewValue(($activate == 'start'));
+            ->setNewValue($initial_status);

          if ($service) {
            $xactions[] = id(clone $template)
--- a/src/applications/diffusion/controller/DiffusionRepositoryEditActivateController.php
+++ b/src/applications/diffusion/controller/DiffusionRepositoryEditActivateController.php
@ -16,12 +16,19 @@ final class DiffusionRepositoryEditActivateController
    $edit_uri = $this->getRepositoryControllerURI($repository, 'edit/');

    if ($request->isFormPost()) {
+      if (!$repository->isTracked()) {
+        $new_status = PhabricatorRepository::STATUS_ACTIVE;
+      } else {
+        $new_status = PhabricatorRepository::STATUS_INACTIVE;
+      }
+
      $xaction = id(new PhabricatorRepositoryTransaction())
        ->setTransactionType(PhabricatorRepositoryTransaction::TYPE_ACTIVATE)
-        ->setNewValue(!$repository->isTracked());
+        ->setNewValue($new_status);

      $editor = id(new PhabricatorRepositoryEditor())
        ->setContinueOnNoEffect(true)
+        ->setContinueOnMissingFields(true)
        ->setContentSourceFromRequest($request)
        ->setActor($viewer)
        ->applyTransactions($repository, array($xaction));
--- a/src/applications/diffusion/editor/DiffusionRepositoryEditEngine.php
+++ b/src/applications/diffusion/editor/DiffusionRepositoryEditEngine.php
@ -68,7 +68,28 @@ final class DiffusionRepositoryEditEngine
  }

  protected function buildCustomEditFields($object) {
+    $viewer = $this->getViewer();
+
+    $policies = id(new PhabricatorPolicyQuery())
+      ->setViewer($viewer)
+      ->setObject($object)
+      ->execute();
+
    return array(
+      id(new PhabricatorSelectEditField())
+        ->setKey('vcs')
+        ->setLabel(pht('Version Control System'))
+        ->setTransactionType(PhabricatorRepositoryTransaction::TYPE_VCS)
+        ->setIsConduitOnly(true)
+        ->setIsCopyable(true)
+        ->setOptions(PhabricatorRepositoryType::getAllRepositoryTypes())
+        ->setDescription(pht('Underlying repository version control system.'))
+        ->setConduitDescription(
+          pht(
+            'Choose which version control system to use when creating a '.
+            'repository.'))
+        ->setConduitTypeDescription(pht('Version control system selection.'))
+        ->setValue($object->getVersionControlSystem()),
      id(new PhabricatorTextEditField())
        ->setKey('name')
        ->setLabel(pht('Name'))
@ -78,6 +99,73 @@ final class DiffusionRepositoryEditEngine
        ->setConduitDescription(pht('Rename the repository.'))
        ->setConduitTypeDescription(pht('New repository name.'))
        ->setValue($object->getName()),
+      id(new PhabricatorTextEditField())
+        ->setKey('callsign')
+        ->setLabel(pht('Callsign'))
+        ->setTransactionType(PhabricatorRepositoryTransaction::TYPE_CALLSIGN)
+        ->setDescription(pht('The repository callsign.'))
+        ->setConduitDescription(pht('Change the repository callsign.'))
+        ->setConduitTypeDescription(pht('New repository callsign.'))
+        ->setValue($object->getCallsign()),
+      id(new PhabricatorTextEditField())
+        ->setKey('shortName')
+        ->setLabel(pht('Short Name'))
+        ->setTransactionType(PhabricatorRepositoryTransaction::TYPE_SLUG)
+        ->setDescription(pht('Short, unique repository name.'))
+        ->setConduitDescription(pht('Change the repository short name.'))
+        ->setConduitTypeDescription(pht('New short name for the repository.'))
+        ->setValue($object->getRepositorySlug()),
+      id(new PhabricatorRemarkupEditField())
+        ->setKey('description')
+        ->setLabel(pht('Description'))
+        ->setTransactionType(PhabricatorRepositoryTransaction::TYPE_DESCRIPTION)
+        ->setDescription(pht('Repository description.'))
+        ->setConduitDescription(pht('Change the repository description.'))
+        ->setConduitTypeDescription(pht('New repository description.'))
+        ->setValue($object->getDetail('description')),
+      id(new PhabricatorTextEditField())
+        ->setKey('encoding')
+        ->setLabel(pht('Text Encoding'))
+        ->setIsCopyable(true)
+        ->setTransactionType(PhabricatorRepositoryTransaction::TYPE_ENCODING)
+        ->setDescription(pht('Default text encoding.'))
+        ->setConduitDescription(pht('Change the default text encoding.'))
+        ->setConduitTypeDescription(pht('New text encoding.'))
+        ->setValue($object->getDetail('encoding')),
+      id(new PhabricatorSelectEditField())
+        ->setKey('status')
+        ->setLabel(pht('Status'))
+        ->setTransactionType(PhabricatorRepositoryTransaction::TYPE_ACTIVATE)
+        ->setIsConduitOnly(true)
+        ->setOptions(PhabricatorRepository::getStatusNameMap())
+        ->setDescription(pht('Active or inactive status.'))
+        ->setConduitDescription(pht('Active or deactivate the repository.'))
+        ->setConduitTypeDescription(pht('New repository status.'))
+        ->setValue($object->getStatus()),
+      id(new PhabricatorTextEditField())
+        ->setKey('defaultBranch')
+        ->setLabel(pht('Default Branch'))
+        ->setTransactionType(
+          PhabricatorRepositoryTransaction::TYPE_DEFAULT_BRANCH)
+        ->setIsCopyable(true)
+        ->setDescription(pht('Default branch name.'))
+        ->setConduitDescription(pht('Set the default branch name.'))
+        ->setConduitTypeDescription(pht('New default branch name.'))
+        ->setValue($object->getDetail('default-branch')),
+      id(new PhabricatorPolicyEditField())
+        ->setKey('policy.push')
+        ->setLabel(pht('Push Policy'))
+        ->setAliases(array('push'))
+        ->setIsCopyable(true)
+        ->setCapability(DiffusionPushCapability::CAPABILITY)
+        ->setPolicies($policies)
+        ->setTransactionType(PhabricatorRepositoryTransaction::TYPE_PUSH_POLICY)
+        ->setDescription(
+          pht('Controls who can push changes to the repository.'))
+        ->setConduitDescription(
+          pht('Change the push policy of the repository.'))
+        ->setConduitTypeDescription(pht('New policy PHID or constant.'))
+        ->setValue($object->getPolicy(DiffusionPushCapability::CAPABILITY)),
    );
  }

--- a/src/applications/repository/constants/PhabricatorRepositoryType.php
+++ b/src/applications/repository/constants/PhabricatorRepositoryType.php
@ -7,10 +7,10 @@ final class PhabricatorRepositoryType extends Phobject {
  const REPOSITORY_TYPE_MERCURIAL   = 'hg';

  public static function getAllRepositoryTypes() {
-    static $map = array(
-      self::REPOSITORY_TYPE_GIT       => 'Git',
-      self::REPOSITORY_TYPE_SVN       => 'Subversion',
-      self::REPOSITORY_TYPE_MERCURIAL => 'Mercurial',
+    $map = array(
+      self::REPOSITORY_TYPE_GIT       => pht('Git'),
+      self::REPOSITORY_TYPE_MERCURIAL => pht('Mercurial'),
+      self::REPOSITORY_TYPE_SVN       => pht('Subversion'),
    );
    return $map;
  }
--- a/src/applications/repository/editor/PhabricatorRepositoryEditor.php
+++ b/src/applications/repository/editor/PhabricatorRepositoryEditor.php
@ -248,25 +248,7 @@ final class PhabricatorRepositoryEditor
        $object->setCallsign($xaction->getNewValue());
        return;
      case PhabricatorRepositoryTransaction::TYPE_ENCODING:
-        // Make sure the encoding is valid by converting to UTF-8. This tests
-        // that the user has mbstring installed, and also that they didn't type
-        // a garbage encoding name. Note that we're converting from UTF-8 to
-        // the target encoding, because mbstring is fine with converting from
-        // a nonsense encoding.
-        $encoding = $xaction->getNewValue();
-        if (strlen($encoding)) {
-          try {
-            phutil_utf8_convert('.', $encoding, 'UTF-8');
-          } catch (Exception $ex) {
-            throw new PhutilProxyException(
-              pht(
-                "Error setting repository encoding '%s': %s'",
-                $encoding,
-                $ex->getMessage()),
-              $ex);
-          }
-        }
-        $object->setDetail('encoding', $encoding);
+        $object->setDetail('encoding', $xaction->getNewValue());
        break;
    }
  }
@ -461,6 +443,117 @@ final class PhabricatorRepositoryEditor
        }
        break;

+      case PhabricatorRepositoryTransaction::TYPE_VCS:
+        $vcs_map = PhabricatorRepositoryType::getAllRepositoryTypes();
+        $current_vcs = $object->getVersionControlSystem();
+
+        if (!$this->getIsNewObject()) {
+          foreach ($xactions as $xaction) {
+            if ($xaction->getNewValue() == $current_vcs) {
+              continue;
+            }
+
+            $errors[] = new PhabricatorApplicationTransactionValidationError(
+              $type,
+              pht('Immutable'),
+              pht(
+                'You can not change the version control system an existing '.
+                'repository uses. It can only be set when a repository is '.
+                'first created.'),
+              $xaction);
+          }
+        } else {
+          $value = $object->getVersionControlSystem();
+          foreach ($xactions as $xaction) {
+            $value = $xaction->getNewValue();
+
+            if (empty($vcs_map[$value])) {
+              $errors[] = new PhabricatorApplicationTransactionValidationError(
+                $type,
+                pht('Invalid'),
+                pht(
+                  'Specified version control system must be a VCS '.
+                  'recognized by Phabricator: %s.',
+                  implode(', ', array_keys($vcs_map))),
+                $xaction);
+            }
+          }
+
+          if (!strlen($value)) {
+            $error = new PhabricatorApplicationTransactionValidationError(
+              $type,
+              pht('Required'),
+              pht(
+                'When creating a repository, you must specify a valid '.
+                'underlying version control system: %s.',
+                implode(', ', array_keys($vcs_map))),
+              nonempty(last($xactions), null));
+            $error->setIsMissingFieldError(true);
+            $errors[] = $error;
+          }
+        }
+        break;
+
+      case PhabricatorRepositoryTransaction::TYPE_NAME:
+        $missing = $this->validateIsEmptyTextField(
+          $object->getName(),
+          $xactions);
+
+        if ($missing) {
+          $error = new PhabricatorApplicationTransactionValidationError(
+            $type,
+            pht('Required'),
+            pht('Repository name is required.'),
+            nonempty(last($xactions), null));
+
+          $error->setIsMissingFieldError(true);
+          $errors[] = $error;
+        }
+        break;
+
+      case PhabricatorRepositoryTransaction::TYPE_ACTIVATE:
+        $status_map = PhabricatorRepository::getStatusMap();
+        foreach ($xactions as $xaction) {
+          $status = $xaction->getNewValue();
+          if (empty($status_map[$status])) {
+            $errors[] = new PhabricatorApplicationTransactionValidationError(
+              $type,
+              pht('Invalid'),
+              pht(
+                'Repository status "%s" is not valid.',
+                $status),
+              $xaction);
+          }
+        }
+        break;
+
+      case PhabricatorRepositoryTransaction::TYPE_ENCODING:
+        foreach ($xactions as $xaction) {
+          // Make sure the encoding is valid by converting to UTF-8. This tests
+          // that the user has mbstring installed, and also that they didn't
+          // type a garbage encoding name. Note that we're converting from
+          // UTF-8 to the target encoding, because mbstring is fine with
+          // converting from a nonsense encoding.
+          $encoding = $xaction->getNewValue();
+          if (!strlen($encoding)) {
+            continue;
+          }
+
+          try {
+            phutil_utf8_convert('.', $encoding, 'UTF-8');
+          } catch (Exception $ex) {
+            $errors[] = new PhabricatorApplicationTransactionValidationError(
+              $type,
+              pht('Invalid'),
+              pht(
+                'Repository encoding "%s" is not valid: %s',
+                $encoding,
+                $ex->getMessage()),
+              $xaction);
+          }
+        }
+        break;
+
      case PhabricatorRepositoryTransaction::TYPE_SLUG:
        foreach ($xactions as $xaction) {
          $old = $xaction->getOldValue();
--- a/src/applications/repository/storage/PhabricatorRepository.php
+++ b/src/applications/repository/storage/PhabricatorRepository.php
@ -46,6 +46,9 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
  const BECAUSE_BRANCH_NOT_AUTOCLOSE = 'auto/noclose';
  const BECAUSE_AUTOCLOSE_FORCED = 'auto/forced';

+  const STATUS_ACTIVE = 'active';
+  const STATUS_INACTIVE = 'inactive';
+
  protected $name;
  protected $callsign;
  protected $repositorySlug;
@ -132,6 +135,31 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
      PhabricatorRepositoryRepositoryPHIDType::TYPECONST);
  }

+  public static function getStatusMap() {
+    return array(
+      self::STATUS_ACTIVE => array(
+        'name' => pht('Active'),
+        'isTracked' => 1,
+      ),
+      self::STATUS_INACTIVE => array(
+        'name' => pht('Inactive'),
+        'isTracked' => 0,
+      ),
+    );
+  }
+
+  public static function getStatusNameMap() {
+    return ipull(self::getStatusMap(), 'name');
+  }
+
+  public function getStatus() {
+    if ($this->isTracked()) {
+      return self::STATUS_ACTIVE;
+    } else {
+      return self::STATUS_INACTIVE;
+    }
+  }
+
  public function toDictionary() {
    return array(
      'id'          => $this->getID(),
@ -146,7 +174,7 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
      'isActive'    => $this->isTracked(),
      'isHosted'    => $this->isHosted(),
      'isImporting' => $this->isImporting(),
-      'encoding'    => $this->getDetail('encoding'),
+      'encoding'    => $this->getDefaultTextEncoding(),
      'staging' => array(
        'supported' => $this->supportsStaging(),
        'prefix' => 'phabricator',
@ -155,6 +183,10 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
    );
  }

+  public function getDefaultTextEncoding() {
+    return $this->getDetail('encoding', 'UTF-8');
+  }
+
  public function getMonogram() {
    $callsign = $this->getCallsign();
    if (strlen($callsign)) {
@ -994,7 +1026,20 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
  }

  public function isTracked() {
-    return $this->getDetail('tracking-enabled', false);
+    $status = $this->getDetail('tracking-enabled');
+    $map = self::getStatusMap();
+    $spec = idx($map, $status);
+
+    if (!$spec) {
+      if ($status) {
+        $status = self::STATUS_ACTIVE;
+      } else {
+        $status = self::STATUS_INACTIVE;
+      }
+      $spec = idx($map, $status);
+    }
+
+    return (bool)idx($spec, 'isTracked', false);
  }

  public function getDefaultBranch() {
--- a/src/applications/repository/storage/PhabricatorRepositoryTransaction.php
+++ b/src/applications/repository/storage/PhabricatorRepositoryTransaction.php
@ -4,24 +4,17 @@ final class PhabricatorRepositoryTransaction
  extends PhabricatorApplicationTransaction {

  const TYPE_VCS = 'repo:vcs';
-  const TYPE_ACTIVATE     = 'repo:activate';
-  const TYPE_NAME         = 'repo:name';
-  const TYPE_DESCRIPTION  = 'repo:description';
-  const TYPE_ENCODING     = 'repo:encoding';
+  const TYPE_ACTIVATE = 'repo:activate';
+  const TYPE_NAME = 'repo:name';
+  const TYPE_DESCRIPTION = 'repo:description';
+  const TYPE_ENCODING = 'repo:encoding';
  const TYPE_DEFAULT_BRANCH = 'repo:default-branch';
  const TYPE_TRACK_ONLY = 'repo:track-only';
  const TYPE_AUTOCLOSE_ONLY = 'repo:autoclose-only';
  const TYPE_SVN_SUBPATH = 'repo:svn-subpath';
-  const TYPE_UUID = 'repo:uuid';
  const TYPE_NOTIFY = 'repo:notify';
  const TYPE_AUTOCLOSE = 'repo:autoclose';
-  const TYPE_REMOTE_URI = 'repo:remote-uri';
-  const TYPE_LOCAL_PATH = 'repo:local-path';
-  const TYPE_HOSTING = 'repo:hosting';
-  const TYPE_PROTOCOL_HTTP = 'repo:serve-http';
-  const TYPE_PROTOCOL_SSH = 'repo:serve-ssh';
  const TYPE_PUSH_POLICY = 'repo:push-policy';
-  const TYPE_CREDENTIAL = 'repo:credential';
  const TYPE_DANGEROUS = 'repo:dangerous';
  const TYPE_SLUG = 'repo:slug';
  const TYPE_SERVICE = 'repo:service';
@ -37,6 +30,13 @@ final class PhabricatorRepositoryTransaction
  const TYPE_SSH_KEYFILE = 'repo:ssh-keyfile';
  const TYPE_HTTP_LOGIN = 'repo:http-login';
  const TYPE_HTTP_PASS = 'repo:http-pass';
+  const TYPE_CREDENTIAL = 'repo:credential';
+  const TYPE_PROTOCOL_HTTP = 'repo:serve-http';
+  const TYPE_PROTOCOL_SSH = 'repo:serve-ssh';
+  const TYPE_HOSTING = 'repo:hosting';
+  const TYPE_LOCAL_PATH = 'repo:local-path';
+  const TYPE_REMOTE_URI = 'repo:remote-uri';
+  const TYPE_UUID = 'repo:uuid';

  public function getApplicationName() {
    return 'repository';
@ -134,7 +134,13 @@ final class PhabricatorRepositoryTransaction
          '%s created this repository.',
          $this->renderHandleLink($author_phid));
      case self::TYPE_ACTIVATE:
-        if ($new) {
+        // TODO: Old versions of this transaction use a boolean value, but
+        // should be migrated.
+        $is_deactivate =
+          (!$new) ||
+          ($new == PhabricatorRepository::STATUS_INACTIVE);
+
+        if (!$is_deactivate) {
          return pht(
            '%s activated this repository.',
            $this->renderHandleLink($author_phid));