Move the Auth Provider edit flow toward a more modern layout

Summary:
Depends on D20095. Ref T13244. Currently, auth providers have a list item view and a single gigantic edit screen complete with a timeline, piles of instructions, supplemental information, etc.

As a step toward making this stuff easier to use and more modern, give them a separate view UI with normal actions, similar to basically every other type of object. Move the timeline and "Disable/Enable" to the view page (from the edit page and the list page, respectively).

Test Plan: Created, edited, and viewed auth providers.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13244

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

src/__phutil_library_map__.php

@@ -2335,6 +2335,7 @@ phutil_register_library_map(array(
     'PhabricatorAuthProviderConfigTransaction' => 'applications/auth/storage/PhabricatorAuthProviderConfigTransaction.php',
     'PhabricatorAuthProviderConfigTransactionQuery' => 'applications/auth/query/PhabricatorAuthProviderConfigTransactionQuery.php',
     'PhabricatorAuthProviderController' => 'applications/auth/controller/config/PhabricatorAuthProviderController.php',
+    'PhabricatorAuthProviderViewController' => 'applications/auth/controller/config/PhabricatorAuthProviderViewController.php',
     'PhabricatorAuthProvidersGuidanceContext' => 'applications/auth/guidance/PhabricatorAuthProvidersGuidanceContext.php',
     'PhabricatorAuthProvidersGuidanceEngineExtension' => 'applications/auth/guidance/PhabricatorAuthProvidersGuidanceEngineExtension.php',
     'PhabricatorAuthQueryPublicKeysConduitAPIMethod' => 'applications/auth/conduit/PhabricatorAuthQueryPublicKeysConduitAPIMethod.php',
@@ -8094,6 +8095,7 @@ phutil_register_library_map(array(
     'PhabricatorAuthProviderConfigTransaction' => 'PhabricatorApplicationTransaction',
     'PhabricatorAuthProviderConfigTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
     'PhabricatorAuthProviderController' => 'PhabricatorAuthController',
+    'PhabricatorAuthProviderViewController' => 'PhabricatorAuthProviderConfigController',
     'PhabricatorAuthProvidersGuidanceContext' => 'PhabricatorGuidanceContext',
     'PhabricatorAuthProvidersGuidanceEngineExtension' => 'PhabricatorGuidanceEngineExtension',
     'PhabricatorAuthQueryPublicKeysConduitAPIMethod' => 'PhabricatorAuthConduitAPIMethod',

src/applications/auth/application/PhabricatorAuthApplication.php

@@ -51,6 +51,7 @@ final class PhabricatorAuthApplication extends PhabricatorApplication {
           'edit/(?:(?P<id>\d+)/)?' => 'PhabricatorAuthEditController',
           '(?P<action>enable|disable)/(?P<id>\d+)/'
             => 'PhabricatorAuthDisableController',
+          'view/(?P<id>\d+)/' => 'PhabricatorAuthProviderViewController',
         ),
         'login/(?P<pkey>[^/]+)/(?:(?P<extra>[^/]+)/)?'
           => 'PhabricatorAuthLoginController',

src/applications/auth/controller/config/PhabricatorAuthDisableController.php

@@ -6,7 +6,8 @@ final class PhabricatorAuthDisableController
   public function handleRequest(AphrontRequest $request) {
     $this->requireApplicationCapability(
       AuthManageProvidersCapability::CAPABILITY);
-    $viewer = $request->getUser();
+
+    $viewer = $this->getViewer();
     $config_id = $request->getURIData('id');
     $action = $request->getURIData('action');
 
@@ -24,6 +25,7 @@ final class PhabricatorAuthDisableController
     }
 
     $is_enable = ($action === 'enable');
+    $done_uri = $config->getURI();
 
     if ($request->isDialogFormPost()) {
       $xactions = array();
@@ -39,8 +41,7 @@ final class PhabricatorAuthDisableController
         ->setContinueOnNoEffect(true)
         ->applyTransactions($config, $xactions);
 
-      return id(new AphrontRedirectResponse())->setURI(
+      return id(new AphrontRedirectResponse())->setURI($done_uri);
-        $this->getApplicationURI());
     }
 
     if ($is_enable) {
@@ -64,8 +65,9 @@ final class PhabricatorAuthDisableController
       // account and pop a warning like "YOU WILL NO LONGER BE ABLE TO LOGIN
       // YOU GOOF, YOU PROBABLY DO NOT MEAN TO DO THIS". None of this is
       // critical and we can wait to see how users manage to shoot themselves
-      // in the feet. Shortly, `bin/auth` will be able to recover from these
+      // in the feet.
-      // types of mistakes.
+
+      // `bin/auth` can recover from these types of mistakes.
 
       $title = pht('Disable Provider?');
       $body = pht(
@@ -77,14 +79,11 @@ final class PhabricatorAuthDisableController
       $button = pht('Disable Provider');
     }
 
-    $dialog = id(new AphrontDialogView())
+    return $this->newDialog()
-      ->setUser($viewer)
       ->setTitle($title)
       ->appendChild($body)
-      ->addCancelButton($this->getApplicationURI())
+      ->addCancelButton($done_uri)
       ->addSubmitButton($button);
-
-    return id(new AphrontDialogResponse())->setDialog($dialog);
   }
 
 }

src/applications/auth/controller/config/PhabricatorAuthEditController.php

@@ -156,12 +156,7 @@ final class PhabricatorAuthEditController
           ->setContinueOnNoEffect(true)
           ->applyTransactions($config, $xactions);
 
-        if ($provider->hasSetupStep() && $is_new) {
+        $next_uri = $config->getURI();
-          $id = $config->getID();
-          $next_uri = $this->getApplicationURI('config/edit/'.$id.'/');
-        } else {
-          $next_uri = $this->getApplicationURI();
-        }
 
         return id(new AphrontRedirectResponse())->setURI($next_uri);
       }
@@ -185,7 +180,7 @@ final class PhabricatorAuthEditController
       $crumb = pht('Edit Provider');
       $title = pht('Edit Auth Provider');
       $header_icon = 'fa-pencil';
-      $cancel_uri = $this->getApplicationURI();
+      $cancel_uri = $config->getURI();
     }
 
     $header = id(new PHUIHeaderView())
@@ -348,18 +343,6 @@ final class PhabricatorAuthEditController
     $crumbs->addTextCrumb($crumb);
     $crumbs->setBorder(true);
 
-    $timeline = null;
-    if (!$is_new) {
-      $timeline = $this->buildTransactionTimeline(
-        $config,
-        new PhabricatorAuthProviderConfigTransactionQuery());
-      $xactions = $timeline->getTransactions();
-      foreach ($xactions as $xaction) {
-        $xaction->setProvider($provider);
-      }
-      $timeline->setShouldTerminate(true);
-    }
-
     $form_box = id(new PHUIObjectBoxView())
       ->setHeaderText(pht('Provider'))
       ->setFormErrors($errors)
@@ -371,7 +354,6 @@ final class PhabricatorAuthEditController
       ->setFooter(array(
         $form_box,
         $footer,
-        $timeline,
       ));
 
     return $this->newPage()

src/applications/auth/controller/config/PhabricatorAuthListController.php

@@ -19,31 +19,18 @@ final class PhabricatorAuthListController
 
       $id = $config->getID();
 
-      $edit_uri = $this->getApplicationURI('config/edit/'.$id.'/');
+      $view_uri = $config->getURI();
-      $enable_uri = $this->getApplicationURI('config/enable/'.$id.'/');
-      $disable_uri = $this->getApplicationURI('config/disable/'.$id.'/');
 
       $provider = $config->getProvider();
-      if ($provider) {
+      $name = $provider->getProviderName();
-        $name = $provider->getProviderName();
-      } else {
-        $name = $config->getProviderType().' ('.$config->getProviderClass().')';
-      }
 
-      $item->setHeader($name);
+      $item
+        ->setHeader($name)
+        ->setHref($view_uri);
 
-      if ($provider) {
+      $domain = $provider->getProviderDomain();
-        $item->setHref($edit_uri);
+      if ($domain !== 'self') {
-      } else {
+        $item->addAttribute($domain);
-        $item->addAttribute(pht('Provider Implementation Missing!'));
-      }
-
-      $domain = null;
-      if ($provider) {
-        $domain = $provider->getProviderDomain();
-        if ($domain !== 'self') {
-          $item->addAttribute($domain);
-        }
       }
 
       if ($config->getShouldAllowRegistration()) {
@@ -54,21 +41,9 @@ final class PhabricatorAuthListController
 
       if ($config->getIsEnabled()) {
         $item->setStatusIcon('fa-check-circle green');
-        $item->addAction(
-          id(new PHUIListItemView())
-            ->setIcon('fa-times')
-            ->setHref($disable_uri)
-            ->setDisabled(!$can_manage)
-            ->addSigil('workflow'));
       } else {
         $item->setStatusIcon('fa-ban red');
         $item->addIcon('fa-ban grey', pht('Disabled'));
-        $item->addAction(
-          id(new PHUIListItemView())
-            ->setIcon('fa-plus')
-            ->setHref($enable_uri)
-            ->setDisabled(!$can_manage)
-            ->addSigil('workflow'));
       }
 
       $list->addItem($item);
@@ -123,10 +98,11 @@ final class PhabricatorAuthListController
 
     $view = id(new PHUITwoColumnView())
       ->setHeader($header)
-      ->setFooter(array(
+      ->setFooter(
-        $guidance,
+        array(
-        $list,
+          $guidance,
-      ));
+          $list,
+        ));
 
     $nav = $this->newNavigation()
       ->setCrumbs($crumbs)

src/applications/auth/controller/config/PhabricatorAuthProviderViewController.php

@@ -0,0 +1,119 @@
+<?php
+
+final class PhabricatorAuthProviderViewController
+  extends PhabricatorAuthProviderConfigController {
+
+  public function handleRequest(AphrontRequest $request) {
+    $this->requireApplicationCapability(
+      AuthManageProvidersCapability::CAPABILITY);
+
+    $viewer = $this->getViewer();
+    $id = $request->getURIData('id');
+
+    $config = id(new PhabricatorAuthProviderConfigQuery())
+      ->setViewer($viewer)
+      ->requireCapabilities(
+        array(
+          PhabricatorPolicyCapability::CAN_VIEW,
+          PhabricatorPolicyCapability::CAN_EDIT,
+        ))
+      ->withIDs(array($id))
+      ->executeOne();
+    if (!$config) {
+      return new Aphront404Response();
+    }
+
+    $header = $this->buildHeaderView($config);
+    $properties = $this->buildPropertiesView($config);
+    $curtain = $this->buildCurtain($config);
+
+    $timeline = $this->buildTransactionTimeline(
+      $config,
+      new PhabricatorAuthProviderConfigTransactionQuery());
+    $timeline->setShouldTerminate(true);
+
+    $view = id(new PHUITwoColumnView())
+      ->setHeader($header)
+      ->setCurtain($curtain)
+      ->addPropertySection(pht('Details'), $properties)
+      ->setMainColumn($timeline);
+
+    $crumbs = $this->buildApplicationCrumbs()
+      ->addTextCrumb($config->getObjectName())
+      ->setBorder(true);
+
+    return $this->newPage()
+      ->setTitle(pht('Auth Provider: %s', $config->getDisplayName()))
+      ->setCrumbs($crumbs)
+      ->appendChild($view);
+  }
+
+  private function buildHeaderView(PhabricatorAuthProviderConfig $config) {
+    $viewer = $this->getViewer();
+
+    $view = id(new PHUIHeaderView())
+      ->setViewer($viewer)
+      ->setHeader($config->getDisplayName());
+
+    if ($config->getIsEnabled()) {
+      $view->setStatus('fa-check', 'bluegrey', pht('Enabled'));
+    } else {
+      $view->setStatus('fa-ban', 'red', pht('Disabled'));
+    }
+
+    return $view;
+  }
+
+  private function buildCurtain(PhabricatorAuthProviderConfig $config) {
+    $viewer = $this->getViewer();
+    $id = $config->getID();
+
+    $can_edit = PhabricatorPolicyFilter::hasCapability(
+      $viewer,
+      $config,
+      PhabricatorPolicyCapability::CAN_EDIT);
+
+    $curtain = $this->newCurtainView($config);
+
+    $curtain->addAction(
+      id(new PhabricatorActionView())
+        ->setName(pht('Edit Auth Provider'))
+        ->setIcon('fa-pencil')
+        ->setHref($this->getApplicationURI("config/edit/{$id}/"))
+        ->setDisabled(!$can_edit)
+        ->setWorkflow(!$can_edit));
+
+    if ($config->getIsEnabled()) {
+      $disable_uri = $this->getApplicationURI('config/disable/'.$id.'/');
+      $disable_icon = 'fa-ban';
+      $disable_text = pht('Disable Provider');
+    } else {
+      $disable_uri = $this->getApplicationURI('config/enable/'.$id.'/');
+      $disable_icon = 'fa-check';
+      $disable_text = pht('Enable Provider');
+    }
+
+    $curtain->addAction(
+      id(new PhabricatorActionView())
+        ->setName($disable_text)
+        ->setIcon($disable_icon)
+        ->setHref($disable_uri)
+        ->setDisabled(!$can_edit)
+        ->setWorkflow(true));
+
+    return $curtain;
+  }
+
+  private function buildPropertiesView(PhabricatorAuthProviderConfig $config) {
+    $viewer = $this->getViewer();
+
+    $view = id(new PHUIPropertyListView())
+      ->setViewer($viewer);
+
+    $view->addProperty(
+      pht('Provider Type'),
+      $config->getProvider()->getProviderName());
+
+    return $view;
+  }
+}

src/applications/auth/query/PhabricatorAuthProviderConfigQuery.php

@@ -70,6 +70,19 @@ final class PhabricatorAuthProviderConfigQuery
     return $where;
   }
 
+  protected function willFilterPage(array $configs) {
+
+    foreach ($configs as $key => $config) {
+      $provider = $config->getProvider();
+      if (!$provider) {
+        unset($configs[$key]);
+        continue;
+      }
+    }
+
+    return $configs;
+  }
+
   public function getQueryApplicationClass() {
     return 'PhabricatorAuthApplication';
   }

src/applications/auth/storage/PhabricatorAuthProviderConfig.php

@@ -83,6 +83,18 @@ final class PhabricatorAuthProviderConfig
     return $this->provider;
   }
 
+  public function getURI() {
+    return '/auth/config/view/'.$this->getID().'/';
+  }
+
+  public function getObjectName() {
+    return pht('Auth Provider %d', $this->getID());
+  }
+
+  public function getDisplayName() {
+    return $this->getProvider()->getProviderName();
+  }
+
 
 /* -(  PhabricatorApplicationTransactionInterface  )------------------------- */