Put Disable/Admin flags on profiles

Summary: Ref T4065. Moves the "disable / enable" and "make / unmake administrator" actions to profiles.

Test Plan: Disabled and enabled users, and made and unmade administrators.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4065

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

src/__phutil_library_map__.php

@@ -1808,6 +1808,7 @@ phutil_register_library_map(array(
     'PhabricatorPeopleDeleteController' => 'applications/people/controller/PhabricatorPeopleDeleteController.php',
     'PhabricatorPeopleDisableController' => 'applications/people/controller/PhabricatorPeopleDisableController.php',
     'PhabricatorPeopleEditController' => 'applications/people/controller/PhabricatorPeopleEditController.php',
+    'PhabricatorPeopleEmpowerController' => 'applications/people/controller/PhabricatorPeopleEmpowerController.php',
     'PhabricatorPeopleHovercardEventListener' => 'applications/people/event/PhabricatorPeopleHovercardEventListener.php',
     'PhabricatorPeopleLdapController' => 'applications/people/controller/PhabricatorPeopleLdapController.php',
     'PhabricatorPeopleListController' => 'applications/people/controller/PhabricatorPeopleListController.php',
@@ -4613,6 +4614,7 @@ phutil_register_library_map(array(
     'PhabricatorPeopleDeleteController' => 'PhabricatorPeopleController',
     'PhabricatorPeopleDisableController' => 'PhabricatorPeopleController',
     'PhabricatorPeopleEditController' => 'PhabricatorPeopleController',
+    'PhabricatorPeopleEmpowerController' => 'PhabricatorPeopleController',
     'PhabricatorPeopleHovercardEventListener' => 'PhabricatorEventListener',
     'PhabricatorPeopleLdapController' => 'PhabricatorPeopleController',
     'PhabricatorPeopleListController' =>

src/applications/people/application/PhabricatorApplicationPeople.php

@@ -42,7 +42,11 @@ final class PhabricatorApplicationPeople extends PhabricatorApplication {
         '(query/(?P<key>[^/]+)/)?' => 'PhabricatorPeopleListController',
         'logs/' => 'PhabricatorPeopleLogsController',
         'approve/(?P<id>[1-9]\d*)/' => 'PhabricatorPeopleApproveController',
-        'disable/(?P<id>[1-9]\d*)/' => 'PhabricatorPeopleDisableController',
+        '(?P<via>disapprove)/(?P<id>[1-9]\d*)/'
+          => 'PhabricatorPeopleDisableController',
+        '(?P<via>disable)/(?P<id>[1-9]\d*)/'
+          => 'PhabricatorPeopleDisableController',
+        'empower/(?P<id>[1-9]\d*)/' => 'PhabricatorPeopleEmpowerController',
         'delete/(?P<id>[1-9]\d*)/' => 'PhabricatorPeopleDeleteController',
         'rename/(?P<id>[1-9]\d*)/' => 'PhabricatorPeopleRenameController',
         'edit/(?:(?P<id>[1-9]\d*)/(?:(?P<view>\w+)/)?)?'

src/applications/people/controller/PhabricatorPeopleDisableController.php

@@ -4,13 +4,14 @@ final class PhabricatorPeopleDisableController
   extends PhabricatorPeopleController {
 
   private $id;
+  private $via;
 
   public function willProcessRequest(array $data) {
-    $this->id = idx($data, 'id');
+    $this->id = $data['id'];
+    $this->via = $data['via'];
   }
 
   public function processRequest() {
-
     $request = $this->getRequest();
     $admin = $request->getUser();
 
@@ -22,27 +23,65 @@ final class PhabricatorPeopleDisableController
       return new Aphront404Response();
     }
 
+    // NOTE: We reach this controller via the administrative "Disable User"
+    // on profiles and also via the "X" action on the approval queue. We do
+    // things slightly differently depending on the context the actor is in.
+
+    $is_disapprove = ($this->via == 'disapprove');
+    if ($is_disapprove) {
       $done_uri = $this->getApplicationURI('query/approval/');
+      $should_disable = true;
+    } else {
+      $done_uri = '/p/'.$user->getUsername().'/';
+      $should_disable = !$user->getIsDisabled();
+    }
+
+    if ($admin->getPHID() == $user->getPHID()) {
+      return $this->newDialog()
+        ->setTitle(pht('Something Stays Your Hand'))
+        ->appendParagraph(
+          pht(
+            'Try as you might, you find you can not disable your '.
+            'own account.'))
+        ->addCancelButton($done_uri, pht('Curses!'));
+    }
 
     if ($request->isFormPost()) {
       id(new PhabricatorUserEditor())
         ->setActor($admin)
-        ->disableUser($user, true);
+        ->disableUser($user, $should_disable);
 
       return id(new AphrontRedirectResponse())->setURI($done_uri);
     }
 
-    $dialog = id(new AphrontDialogView())
-      ->setUser($admin)
-      ->setTitle(pht('Confirm Disable'))
-      ->appendChild(
-        pht(
+    if ($should_disable) {
+      $title = pht('Disable User?');
+      $short_title = pht('Disable User');
+
+      $body = pht(
         'Disable %s? They will no longer be able to access Phabricator or '.
         'receive email.',
-          phutil_tag('strong', array(), $user->getUsername())))
-      ->addCancelButton($done_uri)
-      ->addSubmitButton(pht('Disable Account'));
+        phutil_tag('strong', array(), $user->getUsername()));
 
-    return id(new AphrontDialogResponse())->setDialog($dialog);
+      $submit = pht('Disable User');
+    } else {
+      $title = pht('Enable User?');
+      $short_title = pht('Enable User');
+
+      $body = pht(
+        'Enable %s? They will be able to access Phabricator and receive '.
+        'email again.',
+        phutil_tag('strong', array(), $user->getUsername()));
+
+      $submit = pht('Enable User');
     }
+
+    return $this->newDialog()
+      ->setTitle($title)
+      ->setShortTitle($short_title)
+      ->appendParagraph($body)
+      ->addCancelButton($done_uri)
+      ->addSubmitButton($submit);
+  }
+
 }

src/applications/people/controller/PhabricatorPeopleEditController.php

@@ -35,7 +35,6 @@ final class PhabricatorPeopleEditController
     $nav->setBaseURI(new PhutilURI($base_uri));
     $nav->addLabel(pht('User Information'));
     $nav->addFilter('basic', pht('Basic Information'));
-    $nav->addFilter('role', pht('Edit Roles'));
     $nav->addFilter('cert', pht('Conduit Certificate'));
     $nav->addFilter('profile',
       pht('View Profile'), '/p/'.$user->getUsername().'/');
@@ -61,9 +60,6 @@ final class PhabricatorPeopleEditController
       case 'basic':
         $response = $this->processBasicRequest($user);
         break;
-      case 'role':
-        $response = $this->processRoleRequest($user);
-        break;
       case 'cert':
         $response = $this->processCertificateRequest($user);
         break;
@@ -331,103 +327,6 @@ final class PhabricatorPeopleEditController
     return array($form_box);
   }
 
-  private function processRoleRequest(PhabricatorUser $user) {
-    $request = $this->getRequest();
-    $admin = $request->getUser();
-
-    $is_self = ($user->getID() == $admin->getID());
-
-    $errors = array();
-
-    if ($request->isFormPost()) {
-
-      $log_template = PhabricatorUserLog::initializeNewLog(
-        $admin,
-        $user->getPHID(),
-        null);
-
-      $logs = array();
-
-      if ($is_self) {
-        $errors[] = pht("You can not edit your own role.");
-      } else {
-        $new_admin = (bool)$request->getBool('is_admin');
-        $old_admin = (bool)$user->getIsAdmin();
-        if ($new_admin != $old_admin) {
-          id(new PhabricatorUserEditor())
-            ->setActor($admin)
-            ->makeAdminUser($user, $new_admin);
-        }
-
-        $new_disabled = (bool)$request->getBool('is_disabled');
-        $old_disabled = (bool)$user->getIsDisabled();
-        if ($new_disabled != $old_disabled) {
-          id(new PhabricatorUserEditor())
-            ->setActor($admin)
-            ->disableUser($user, $new_disabled);
-        }
-      }
-
-      if (!$errors) {
-        return id(new AphrontRedirectResponse())
-          ->setURI($request->getRequestURI()->alter('saved', 'true'));
-      }
-    }
-
-    $form = id(new AphrontFormView())
-      ->setUser($admin)
-      ->setAction($request->getRequestURI()->alter('saved', null));
-
-    if ($is_self) {
-      $inst = pht('NOTE: You can not edit your own role.');
-      $form->appendChild(
-        phutil_tag('p', array('class' => 'aphront-form-instructions'), $inst));
-    }
-
-    $form
-      ->appendChild($this->getRoleInstructions())
-      ->appendChild(
-        id(new AphrontFormCheckboxControl())
-          ->addCheckbox(
-            'is_admin',
-            1,
-            pht('Administrator'),
-            $user->getIsAdmin())
-          ->setDisabled($is_self))
-      ->appendChild(
-        id(new AphrontFormCheckboxControl())
-          ->addCheckbox(
-            'is_disabled',
-            1,
-            pht('Disabled'),
-            $user->getIsDisabled())
-          ->setDisabled($is_self))
-      ->appendChild(
-        id(new AphrontFormCheckboxControl())
-          ->addCheckbox(
-            'is_agent',
-            1,
-            pht('System Agent (Bot/Script User)'),
-            $user->getIsSystemAgent())
-          ->setDisabled(true));
-
-    if (!$is_self) {
-      $form
-        ->appendChild(
-          id(new AphrontFormSubmitControl())
-            ->setValue(pht('Edit Role')));
-    }
-
-    $title = pht('Edit Role');
-
-    $form_box = id(new PHUIObjectBoxView())
-      ->setHeaderText($title)
-      ->setFormErrors($errors)
-      ->setForm($form);
-
-    return array($form_box);
-  }
-
   private function processCertificateRequest($user) {
     $request = $this->getRequest();
     $admin = $request->getUser();

src/applications/people/controller/PhabricatorPeopleEmpowerController.php

@@ -0,0 +1,71 @@
+<?php
+
+final class PhabricatorPeopleEmpowerController
+  extends PhabricatorPeopleController {
+
+  private $id;
+
+  public function willProcessRequest(array $data) {
+    $this->id = $data['id'];
+  }
+
+  public function processRequest() {
+    $request = $this->getRequest();
+    $admin = $request->getUser();
+
+    $user = id(new PhabricatorPeopleQuery())
+      ->setViewer($admin)
+      ->withIDs(array($this->id))
+      ->executeOne();
+    if (!$user) {
+      return new Aphront404Response();
+    }
+
+    $profile_uri = '/p/'.$user->getUsername();
+
+    if ($user->getPHID() == $admin->getPHID()) {
+      return $this->newDialog()
+        ->setTitle(pht('Your Way is Blocked'))
+        ->appendParagraph(
+          pht(
+            'After a time, your efforts fail. You can not adjust your own '.
+            'status as an administrator.'))
+        ->addCancelButton($profile_uri, pht('Accept Fate'));
+    }
+
+    if ($request->isFormPost()) {
+      id(new PhabricatorUserEditor())
+        ->setActor($admin)
+        ->makeAdminUser($user, !$user->getIsAdmin());
+
+      return id(new AphrontRedirectResponse())->setURI($profile_uri);
+    }
+
+    if ($user->getIsAdmin()) {
+      $title = pht('Remove as Administrator?');
+      $short = pht('Remove Administrator');
+      $body = pht(
+        'Remove %s as an administrator? They will no longer be able to '.
+        'perform administrative functions on this Phabricator install.',
+        phutil_tag('strong', array(), $user->getUsername()));
+      $submit = pht('Remove Administrator');
+    } else {
+      $title = pht('Make Administrator?');
+      $short = pht('Make Administrator');
+      $body = pht(
+        'Empower %s as an admistrator? They will be able to create users, '.
+        'approve users, make and remove administrators, delete accounts, and '.
+        'perform other administrative functions on this Phabricator install.',
+        phutil_tag('strong', array(), $user->getUsername()));
+      $submit = pht('Make Administrator');
+    }
+
+    return $this->newDialog()
+      ->setTitle($title)
+      ->setShortTitle($short)
+      ->appendParagraph($body)
+      ->addCancelButton($profile_uri)
+      ->addSubmitButton($submit);
+  }
+
+}

src/applications/people/controller/PhabricatorPeopleListController.php

@@ -92,7 +92,7 @@ final class PhabricatorPeopleListController extends PhabricatorPeopleController
               ->setIcon('disable')
               ->setName(pht('Disable'))
               ->setWorkflow(true)
-              ->setHref($this->getApplicationURI('disable/'.$user_id.'/')));
+              ->setHref($this->getApplicationURI('disapprove/'.$user_id.'/')));
           $item->addAction(
             id(new PHUIListItemView())
               ->setIcon('like')

src/applications/people/controller/PhabricatorPeopleProfileController.php

@@ -64,6 +64,22 @@ final class PhabricatorPeopleProfileController
         ->setWorkflow(!$can_edit));
 
     if ($viewer->getIsAdmin()) {
+      if ($user->getIsAdmin()) {
+        $empower_icon = 'lower-priority';
+        $empower_name = pht('Remove Administrator');
+      } else {
+        $empower_icon = 'raise-priority';
+        $empower_name = pht('Make Administrator');
+      }
+
+      $actions->addAction(
+        id(new PhabricatorActionView())
+          ->setIcon($empower_icon)
+          ->setName($empower_name)
+          ->setDisabled(($user->getPHID() == $viewer->getPHID()))
+          ->setWorkflow(true)
+          ->setHref($this->getApplicationURI('empower/'.$user->getID().'/')));
+
       $actions->addAction(
         id(new PhabricatorActionView())
           ->setIcon('tag')
@@ -71,6 +87,22 @@ final class PhabricatorPeopleProfileController
           ->setWorkflow(true)
           ->setHref($this->getApplicationURI('rename/'.$user->getID().'/')));
 
+      if ($user->getIsDisabled()) {
+        $disable_icon = 'enable';
+        $disable_name = pht('Enable User');
+      } else {
+        $disable_icon = 'disable';
+        $disable_name = pht('Disable User');
+      }
+
+      $actions->addAction(
+        id(new PhabricatorActionView())
+          ->setIcon($disable_icon)
+          ->setName($disable_name)
+          ->setDisabled(($user->getPHID() == $viewer->getPHID()))
+          ->setWorkflow(true)
+          ->setHref($this->getApplicationURI('disable/'.$user->getID().'/')));
+
       $actions->addAction(
         id(new PhabricatorActionView())
           ->setIcon('delete')