Improve repository hinting and feedback

Summary: - Warn about "Read/Write" instead of disabling it, to prevent edits which mutate it after changing a hosted repository to an unhosted one. - Warn about authenticated connections with HTTPS auth disabled, and link to the relevant setting. - When "Autoclose" is disabled, show that "Autoclose Branches" won't have an effect. - For hosted repositories, show the HTTP and SSH clone URIs. - Make them easy to copy/paste. - Link to credential management. - Show if they're read-only. - This could be a bit nicer-looking than it is. Test Plan: Looked at repositories in a bunch of states and made various edits to them. Reviewers: btrahan, chad Reviewed By: btrahan CC: chad, aran Maniphest Tasks: T2230 Differential Revision: https://secure.phabricator.com/D7471
2024-11-26 08:42:41 +01:00 · 2013-11-01 17:35:43 -07:00 · 2013-11-01 17:35:43 -07:00 · a0e820ad9a
commit a0e820ad9a
parent d1649d176f
8 changed files with 232 additions and 40 deletions
--- a/src/__celerity_resource_map__.php
+++ b/src/__celerity_resource_map__.php
@ -1145,7 +1145,7 @@ celerity_register_resource_map(array(
  ),
  'diffusion-icons-css' =>
  array(
-    'uri' => '/res/b93e32c9/rsrc/css/application/diffusion/diffusion-icons.css',
+    'uri' => '/res/82e77537/rsrc/css/application/diffusion/diffusion-icons.css',
    'type' => 'css',
    'requires' =>
    array(
@ -2485,6 +2485,18 @@ celerity_register_resource_map(array(
    ),
    'disk' => '/rsrc/js/application/search/behavior-reorder-queries.js',
  ),
+  'javelin-behavior-select-on-click' =>
+  array(
+    'uri' => '/res/f021b754/rsrc/js/core/behavior-select-on-click.js',
+    'type' => 'js',
+    'requires' =>
+    array(
+      0 => 'javelin-behavior',
+      1 => 'javelin-stratcom',
+      2 => 'javelin-dom',
+    ),
+    'disk' => '/rsrc/js/core/behavior-select-on-click.js',
+  ),
  'javelin-behavior-slowvote-embed' =>
  array(
    'uri' => '/res/8e85e20d/rsrc/js/application/slowvote/behavior-slowvote-embed.js',
@ -4470,7 +4482,7 @@ celerity_register_resource_map(array(
      'uri' => '/res/pkg/5e9e5c4e/differential.pkg.js',
      'type' => 'js',
    ),
-    '270f4eb4' =>
+    '7aa115b4' =>
    array(
      'name' => 'diffusion.pkg.css',
      'symbols' =>
@ -4478,7 +4490,7 @@ celerity_register_resource_map(array(
        0 => 'diffusion-commit-view-css',
        1 => 'diffusion-icons-css',
      ),
-      'uri' => '/res/pkg/270f4eb4/diffusion.pkg.css',
+      'uri' => '/res/pkg/7aa115b4/diffusion.pkg.css',
      'type' => 'css',
    ),
    96909266 =>
@ -4570,8 +4582,8 @@ celerity_register_resource_map(array(
    'differential-revision-history-css' => '1084b12b',
    'differential-revision-list-css' => '1084b12b',
    'differential-table-of-contents-css' => '1084b12b',
-    'diffusion-commit-view-css' => '270f4eb4',
-    'diffusion-icons-css' => '270f4eb4',
+    'diffusion-commit-view-css' => '7aa115b4',
+    'diffusion-icons-css' => '7aa115b4',
    'global-drag-and-drop-css' => 'b05e33c6',
    'inline-comment-summary-css' => '1084b12b',
    'javelin-aphlict' => '2c1dba03',
--- a/src/applications/diffusion/config/PhabricatorDiffusionConfigOptions.php
+++ b/src/applications/diffusion/config/PhabricatorDiffusionConfigOptions.php
@ -101,7 +101,14 @@ final class PhabricatorDiffusionConfigOptions
            "long as Phabricator uses HTTPS, but it presents a much lower ".
            "barrier to attackers than SSH does.\n\n".
            "Consider using SSH for authenticated access to repositories ".
-            "instead of HTTP."))
+            "instead of HTTP.")),
+      $this->newOption('diffusion.ssh-user', 'string', null)
+        ->setSummary(pht('Login username for SSH connections to repositories.'))
+        ->setDescription(
+          pht(
+            'When constructing clone URIs to show to users, Diffusion will '.
+            'fill in this login username. If you have configured a VCS user '.
+            'like `git`, you should provide it here.')),
    );
  }

--- a/src/applications/diffusion/controller/DiffusionRepositoryController.php
+++ b/src/applications/diffusion/controller/DiffusionRepositoryController.php
@ -165,18 +165,64 @@ final class DiffusionRepositoryController extends DiffusionController {
      ->setUser($user);
    $view->addProperty(pht('Callsign'), $repository->getCallsign());

-    switch ($repository->getVersionControlSystem()) {
-      case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
-      case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
-        $view->addProperty(
-          pht('Clone URI'),
-          $repository->getPublicRemoteURI());
-        break;
-      case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
-        $view->addProperty(
-          pht('Repository Root'),
-          $repository->getPublicRemoteURI());
-        break;
+    if ($repository->isHosted()) {
+      $serve_off = PhabricatorRepository::SERVE_OFF;
+      $callsign = $repository->getCallsign();
+      $repo_path = '/diffusion/'.$callsign.'/';
+
+      $serve_ssh = $repository->getServeOverSSH();
+      if ($serve_ssh !== $serve_off) {
+        $uri = new PhutilURI(PhabricatorEnv::getProductionURI($repo_path));
+        $uri->setProtocol('ssh');
+
+        $ssh_user = PhabricatorEnv::getEnvConfig('diffusion.ssh-user');
+        if ($ssh_user) {
+          $uri->setUser($ssh_user);
+        }
+
+        // This isn't quite right, but for now it's probably better to drop
+        // the port than sometimes show ":8080", etc. Using most VCS commands
+        // against nonstandard ports is a huge pain so few installs are likely
+        // to configure SSH on a nonstandard port.
+        $uri->setPort(null);
+
+        $clone_uri = $this->renderCloneURI(
+          $uri,
+          $serve_ssh,
+          '/settings/panel/ssh/');
+
+        $view->addProperty(pht('Clone URI (SSH)'), $clone_uri);
+      }
+
+      $serve_http = $repository->getServeOverHTTP();
+      if ($serve_http !== $serve_off) {
+        $http_uri = PhabricatorEnv::getProductionURI($repo_path);
+
+        $clone_uri = $this->renderCloneURI(
+          $http_uri,
+          $serve_http,
+          PhabricatorEnv::getEnvConfig('diffusion.allow-http-auth')
+            ? '/settings/panel/vcspassword/'
+            : null);
+
+        $view->addProperty(pht('Clone URI (HTTP)'), $clone_uri);
+      }
+    } else {
+      switch ($repository->getVersionControlSystem()) {
+        case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
+        case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
+          $view->addProperty(
+            pht('Clone URI'),
+            $this->renderCloneURI(
+              $repository->getPublicRemoteURI()));
+          break;
+        case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
+          $view->addProperty(
+            pht('Repository Root'),
+            $this->renderCloneURI(
+              $repository->getPublicRemoteURI()));
+          break;
+      }
    }

    $description = $repository->getDetail('description');
@ -456,4 +502,53 @@ final class DiffusionRepositoryController extends DiffusionController {
    return $browse_panel;
  }

+  private function renderCloneURI(
+    $uri,
+    $serve_mode = null,
+    $manage_uri = null) {
+
+    require_celerity_resource('diffusion-icons-css');
+
+    Javelin::initBehavior('select-on-click');
+
+    $input = javelin_tag(
+      'input',
+      array(
+        'type' => 'text',
+        'value' => (string)$uri,
+        'class' => 'diffusion-clone-uri',
+        'sigil' => 'select-on-click',
+      ));
+
+    $extras = array();
+    if ($serve_mode) {
+      if ($serve_mode === PhabricatorRepository::SERVE_READONLY) {
+        $extras[] = pht('(Read Only)');
+      }
+    }
+
+    if ($manage_uri) {
+      if ($this->getRequest()->getUser()->isLoggedIn()) {
+        $extras[] = phutil_tag(
+          'a',
+          array(
+            'href' => $manage_uri,
+          ),
+          pht('Manage Credentials'));
+      }
+    }
+
+    if ($extras) {
+      $extras = phutil_implode_html(' ', $extras);
+      $extras = phutil_tag(
+        'div',
+        array(
+          'class' => 'diffusion-clone-extras',
+        ),
+        $extras);
+    }
+
+    return array($input, $extras);
+  }
+
 }
--- a/src/applications/diffusion/controller/DiffusionRepositoryEditHostingController.php
+++ b/src/applications/diffusion/controller/DiffusionRepositoryEditHostingController.php
@ -126,8 +126,12 @@ final class DiffusionRepositoryEditHostingController
    $request = $this->getRequest();
    $user = $request->getUser();

-    $v_http_mode = $repository->getServeOverHTTP();
-    $v_ssh_mode = $repository->getServeOverSSH();
+    $v_http_mode = $repository->getDetail(
+      'serve-over-http',
+      PhabricatorRepository::SERVE_OFF);
+    $v_ssh_mode = $repository->getDetail(
+      'serve-over-ssh',
+      PhabricatorRepository::SERVE_OFF);

    $edit_uri = $this->getRepositoryControllerURI($repository, 'edit/');
    $prev_uri = $this->getRepositoryControllerURI($repository, 'edit/hosting/');
@ -167,13 +171,19 @@ final class DiffusionRepositoryEditHostingController
    $title = pht('Edit Protocols (%s)', $repository->getName());


-    if ($repository->isHosted()) {
-      $rw_message = pht(
-        'Phabricator will serve a read-write copy of this repository');
-    } else {
-      $rw_message = pht(
-        'This repository is hosted elsewhere, so Phabricator can not perform '.
-        'writes.');
+    $rw_message = pht(
+      'Phabricator will serve a read-write copy of this repository.');
+
+    if (!$repository->isHosted()) {
+      $rw_message = array(
+        $rw_message,
+        phutil_tag('br'),
+        phutil_tag('br'),
+        pht(
+          '%s: This repository is hosted elsewhere, so Phabricator can not '.
+          'perform writes. This mode will act like "Read Only" for '.
+          'repositories hosted elsewhere.',
+          phutil_tag('strong', array(), 'WARNING')));
    }

    $ssh_control =
@ -185,19 +195,19 @@ final class DiffusionRepositoryEditHostingController
          PhabricatorRepository::SERVE_OFF,
          PhabricatorRepository::getProtocolAvailabilityName(
            PhabricatorRepository::SERVE_OFF),
-          pht('Phabricator will not serve this repository.'))
+          pht('Phabricator will not serve this repository over SSH.'))
        ->addButton(
          PhabricatorRepository::SERVE_READONLY,
          PhabricatorRepository::getProtocolAvailabilityName(
            PhabricatorRepository::SERVE_READONLY),
-          pht('Phabricator will serve a read-only copy of this repository.'))
+          pht(
+            'Phabricator will serve a read-only copy of this repository '.
+            'over SSH.'))
        ->addButton(
          PhabricatorRepository::SERVE_READWRITE,
          PhabricatorRepository::getProtocolAvailabilityName(
            PhabricatorRepository::SERVE_READWRITE),
-          $rw_message,
-          $repository->isHosted() ? null : 'disabled',
-          $repository->isHosted() ? null : true);
+          $rw_message);

    $http_control =
      id(new AphrontFormRadioButtonControl())
@ -208,19 +218,19 @@ final class DiffusionRepositoryEditHostingController
          PhabricatorRepository::SERVE_OFF,
          PhabricatorRepository::getProtocolAvailabilityName(
            PhabricatorRepository::SERVE_OFF),
-          pht('Phabricator will not serve this repository.'))
+          pht('Phabricator will not serve this repository over HTTP.'))
        ->addButton(
          PhabricatorRepository::SERVE_READONLY,
          PhabricatorRepository::getProtocolAvailabilityName(
            PhabricatorRepository::SERVE_READONLY),
-          pht('Phabricator will serve a read-only copy of this repository.'))
+          pht(
+            'Phabricator will serve a read-only copy of this repository '.
+            'over HTTP.'))
        ->addButton(
          PhabricatorRepository::SERVE_READWRITE,
          PhabricatorRepository::getProtocolAvailabilityName(
            PhabricatorRepository::SERVE_READWRITE),
-          $rw_message,
-          $repository->isHosted() ? null : 'disabled',
-          $repository->isHosted() ? null : true);
+          $rw_message);

    $form = id(new AphrontFormView())
      ->setUser($user)
@ -228,7 +238,19 @@ final class DiffusionRepositoryEditHostingController
        pht(
          'Phabricator can serve repositories over various protocols. You can '.
          'configure server protocols here.'))
-      ->appendChild($ssh_control)
+      ->appendChild($ssh_control);
+
+    if (!PhabricatorEnv::getEnvConfig('diffusion.allow-http-auth')) {
+      $form->appendRemarkupInstructions(
+        pht(
+          'NOTE: The configuration setting [[ %s | %s ]] is currently '.
+          'disabled. You must enable it to activate authenticated access '.
+          'to repositories over HTTP.',
+          '/config/edit/diffusion.allow-http-auth/',
+          'diffusion.allow-http-auth'));
+    }
+
+    $form
      ->appendChild($http_control)
      ->appendChild(
        id(new AphrontFormSubmitControl())
--- a/src/applications/diffusion/controller/DiffusionRepositoryEditMainController.php
+++ b/src/applications/diffusion/controller/DiffusionRepositoryEditMainController.php
@ -349,6 +349,11 @@ final class DiffusionRepositoryEditMainController
    $autoclose_only = nonempty(
      $repository->getHumanReadableDetail('close-commits-filter', array()),
      phutil_tag('em', array(), pht('Autoclose On All Branches')));
+
+    if ($repository->getDetail('disable-autoclose')) {
+      $autoclose_only = phutil_tag('em', array(), pht('Disabled'));
+    }
+
    $view->addProperty(pht('Autoclose Only'), $autoclose_only);

    return $view;
--- a/src/applications/repository/storage/PhabricatorRepository.php
+++ b/src/applications/repository/storage/PhabricatorRepository.php
@ -737,7 +737,8 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
  }

  public function getServeOverHTTP() {
-    return $this->getDetail('serve-over-http', self::SERVE_OFF);
+    $serve = $this->getDetail('serve-over-http', self::SERVE_OFF);
+    return $this->normalizeServeConfigSetting($serve);
  }

  public function setServeOverHTTP($mode) {
@ -745,7 +746,8 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
  }

  public function getServeOverSSH() {
-    return $this->getDetail('serve-over-ssh', self::SERVE_OFF);
+    $serve = $this->getDetail('serve-over-ssh', self::SERVE_OFF);
+    return $this->normalizeServeConfigSetting($serve);
  }

  public function setServeOverSSH($mode) {
@ -765,6 +767,22 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
    }
  }

+  private function normalizeServeConfigSetting($value) {
+    switch ($value) {
+      case self::SERVE_OFF:
+      case self::SERVE_READONLY:
+        return $value;
+      case self::SERVE_READWRITE:
+        if ($this->isHosted()) {
+          return self::SERVE_READWRITE;
+        } else {
+          return self::SERVE_READONLY;
+        }
+      default:
+        return self::SERVE_OFF;
+    }
+  }
+

  /**
   * Raise more useful errors when there are basic filesystem problems.
--- a/webroot/rsrc/css/application/diffusion/diffusion-icons.css
+++ b/webroot/rsrc/css/application/diffusion/diffusion-icons.css
@ -26,3 +26,17 @@
 .diffusion-path-icon-link {
  background-image: url(/rsrc/image/icon/fatcow/page_white_link.png);
 }
+
+input.diffusion-clone-uri {
+  display: block;
+  width: 100%;
+  border: 1px solid #efefef;
+  box-shadow: none;
+  height: 24px;
+}
+
+.diffusion-clone-extras {
+  font-size: 11px;
+  text-align: right;
+  color: {$lightgreytext};
+}
--- a/webroot/rsrc/js/core/behavior-select-on-click.js
+++ b/webroot/rsrc/js/core/behavior-select-on-click.js
@ -0,0 +1,19 @@
+/**
+ * @provides javelin-behavior-select-on-click
+ * @requires javelin-behavior
+ *           javelin-stratcom
+ *           javelin-dom
+ * @javelin
+ */
+
+JX.behavior('select-on-click', function(config) {
+  JX.Stratcom.listen(
+    'click',
+    'select-on-click',
+    function(e) {
+      e.kill();
+      var node = e.getNode('select-on-click');
+      JX.DOM.focus(node);
+      node.select();
+    });
+});