mirror of
https://we.phorge.it/source/phorge.git
synced 2025-02-21 19:19:12 +01:00
Move "Delete User" action to user profiles
Summary: Ref T4065. The existence of two separate edit workflows for users is broadly confusing to administrators. I want to unify user administration and improve administration of system agent accounts. Particularly, I plan to: - Give administrators limited access to profile editing of system agents (e.g., change profile picture). - Give administrators limited access to Settings for system agents. - Broadly, move all the weird old special editing into standard editing. Test Plan: - Hit all the errors (delete self, no username, wrong username). - Deleted a user. - Visited page as a non-admin, got 403'd. - Viewed old edit UI. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T4065 Differential Revision: https://secure.phabricator.com/D8662
This commit is contained in:
parent
81fa847bc5
commit
a6a19ac721
5 changed files with 136 additions and 89 deletions
|
@ -1805,6 +1805,7 @@ phutil_register_library_map(array(
|
||||||
'PhabricatorPeopleApproveController' => 'applications/people/controller/PhabricatorPeopleApproveController.php',
|
'PhabricatorPeopleApproveController' => 'applications/people/controller/PhabricatorPeopleApproveController.php',
|
||||||
'PhabricatorPeopleCalendarController' => 'applications/people/controller/PhabricatorPeopleCalendarController.php',
|
'PhabricatorPeopleCalendarController' => 'applications/people/controller/PhabricatorPeopleCalendarController.php',
|
||||||
'PhabricatorPeopleController' => 'applications/people/controller/PhabricatorPeopleController.php',
|
'PhabricatorPeopleController' => 'applications/people/controller/PhabricatorPeopleController.php',
|
||||||
|
'PhabricatorPeopleDeleteController' => 'applications/people/controller/PhabricatorPeopleDeleteController.php',
|
||||||
'PhabricatorPeopleDisableController' => 'applications/people/controller/PhabricatorPeopleDisableController.php',
|
'PhabricatorPeopleDisableController' => 'applications/people/controller/PhabricatorPeopleDisableController.php',
|
||||||
'PhabricatorPeopleEditController' => 'applications/people/controller/PhabricatorPeopleEditController.php',
|
'PhabricatorPeopleEditController' => 'applications/people/controller/PhabricatorPeopleEditController.php',
|
||||||
'PhabricatorPeopleHovercardEventListener' => 'applications/people/event/PhabricatorPeopleHovercardEventListener.php',
|
'PhabricatorPeopleHovercardEventListener' => 'applications/people/event/PhabricatorPeopleHovercardEventListener.php',
|
||||||
|
@ -4608,6 +4609,7 @@ phutil_register_library_map(array(
|
||||||
'PhabricatorPeopleApproveController' => 'PhabricatorPeopleController',
|
'PhabricatorPeopleApproveController' => 'PhabricatorPeopleController',
|
||||||
'PhabricatorPeopleCalendarController' => 'PhabricatorPeopleController',
|
'PhabricatorPeopleCalendarController' => 'PhabricatorPeopleController',
|
||||||
'PhabricatorPeopleController' => 'PhabricatorController',
|
'PhabricatorPeopleController' => 'PhabricatorController',
|
||||||
|
'PhabricatorPeopleDeleteController' => 'PhabricatorPeopleController',
|
||||||
'PhabricatorPeopleDisableController' => 'PhabricatorPeopleController',
|
'PhabricatorPeopleDisableController' => 'PhabricatorPeopleController',
|
||||||
'PhabricatorPeopleEditController' => 'PhabricatorPeopleController',
|
'PhabricatorPeopleEditController' => 'PhabricatorPeopleController',
|
||||||
'PhabricatorPeopleHovercardEventListener' => 'PhabricatorEventListener',
|
'PhabricatorPeopleHovercardEventListener' => 'PhabricatorEventListener',
|
||||||
|
|
|
@ -43,6 +43,7 @@ final class PhabricatorApplicationPeople extends PhabricatorApplication {
|
||||||
'logs/' => 'PhabricatorPeopleLogsController',
|
'logs/' => 'PhabricatorPeopleLogsController',
|
||||||
'approve/(?P<id>[1-9]\d*)/' => 'PhabricatorPeopleApproveController',
|
'approve/(?P<id>[1-9]\d*)/' => 'PhabricatorPeopleApproveController',
|
||||||
'disable/(?P<id>[1-9]\d*)/' => 'PhabricatorPeopleDisableController',
|
'disable/(?P<id>[1-9]\d*)/' => 'PhabricatorPeopleDisableController',
|
||||||
|
'delete/(?P<id>[1-9]\d*)/' => 'PhabricatorPeopleDeleteController',
|
||||||
'edit/(?:(?P<id>[1-9]\d*)/(?:(?P<view>\w+)/)?)?'
|
'edit/(?:(?P<id>[1-9]\d*)/(?:(?P<view>\w+)/)?)?'
|
||||||
=> 'PhabricatorPeopleEditController',
|
=> 'PhabricatorPeopleEditController',
|
||||||
'ldap/' => 'PhabricatorPeopleLdapController',
|
'ldap/' => 'PhabricatorPeopleLdapController',
|
||||||
|
|
|
@ -0,0 +1,125 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
final class PhabricatorPeopleDeleteController
|
||||||
|
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->buildDeleteSelfResponse($profile_uri);
|
||||||
|
}
|
||||||
|
|
||||||
|
$errors = array();
|
||||||
|
|
||||||
|
$v_username = '';
|
||||||
|
$e_username = true;
|
||||||
|
if ($request->isFormPost()) {
|
||||||
|
$v_username = $request->getStr('username');
|
||||||
|
|
||||||
|
if (!$v_username) {
|
||||||
|
$errors[] = pht(
|
||||||
|
'You must type the username to confirm that you want to delete '.
|
||||||
|
'this user account.');
|
||||||
|
$e_username = pht('Required');
|
||||||
|
} else if ($v_username != $user->getUsername()) {
|
||||||
|
$errors[] = pht(
|
||||||
|
'You must type the username correctly to confirm that you want '.
|
||||||
|
'to delete this user account.');
|
||||||
|
$e_username = pht('Incorrect');
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!$errors) {
|
||||||
|
id(new PhabricatorUserEditor())
|
||||||
|
->setActor($admin)
|
||||||
|
->deleteUser($user);
|
||||||
|
|
||||||
|
$done_uri = $this->getApplicationURI();
|
||||||
|
|
||||||
|
return id(new AphrontRedirectResponse())->setURI($done_uri);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
$str1 = pht(
|
||||||
|
'Be careful when deleting users! This will permanently and '.
|
||||||
|
'irreversibly destroy this user account.');
|
||||||
|
|
||||||
|
$str2 = pht(
|
||||||
|
'If this user interacted with anything, it is generally better to '.
|
||||||
|
'disable them, not delete them. If you delete them, it will no longer '.
|
||||||
|
'be possible to (for example) search for objects they created, and you '.
|
||||||
|
'will lose other information about their history. Disabling them '.
|
||||||
|
'instead will prevent them from logging in but not destroy any of '.
|
||||||
|
'their data.');
|
||||||
|
|
||||||
|
$str3 = pht(
|
||||||
|
'It is generally safe to delete newly created users (and test users and '.
|
||||||
|
'so on), but less safe to delete established users. If possible, '.
|
||||||
|
'disable them instead.');
|
||||||
|
|
||||||
|
$form = id(new AphrontFormView())
|
||||||
|
->setUser($admin)
|
||||||
|
->appendRemarkupInstructions(
|
||||||
|
pht(
|
||||||
|
'To confirm that you want to permanently and irrevocably destroy '.
|
||||||
|
'this user account, type their username:'))
|
||||||
|
->appendChild(
|
||||||
|
id(new AphrontFormStaticControl())
|
||||||
|
->setLabel(pht('Username'))
|
||||||
|
->setValue($user->getUsername()))
|
||||||
|
->appendChild(
|
||||||
|
id(new AphrontFormTextControl())
|
||||||
|
->setLabel(pht('Confirm'))
|
||||||
|
->setValue($v_username)
|
||||||
|
->setName('username')
|
||||||
|
->setError($e_username));
|
||||||
|
|
||||||
|
if ($errors) {
|
||||||
|
$errors = id(new AphrontErrorView())->setErrors($errors);
|
||||||
|
}
|
||||||
|
|
||||||
|
return $this->newDialog()
|
||||||
|
->setWidth(AphrontDialogView::WIDTH_FORM)
|
||||||
|
->setTitle(pht('Really Delete User?'))
|
||||||
|
->setShortTitle(pht('Delete User'))
|
||||||
|
->appendChild($errors)
|
||||||
|
->appendParagraph($str1)
|
||||||
|
->appendParagraph($str2)
|
||||||
|
->appendParagraph($str3)
|
||||||
|
->appendChild($form->buildLayoutView())
|
||||||
|
->addSubmitButton(pht('Delete User'))
|
||||||
|
->addCancelButton($profile_uri);
|
||||||
|
}
|
||||||
|
|
||||||
|
private function buildDeleteSelfResponse($profile_uri) {
|
||||||
|
return $this->newDialog()
|
||||||
|
->setTitle(pht('You Shall Journey No Farther'))
|
||||||
|
->appendParagraph(
|
||||||
|
pht(
|
||||||
|
'As you stare into the gaping maw of the abyss, something '.
|
||||||
|
'holds you back.'))
|
||||||
|
->appendParagraph(
|
||||||
|
pht(
|
||||||
|
'You can not delete your own account.'))
|
||||||
|
->addCancelButton($profile_uri, pht('Turn Back'));
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
}
|
|
@ -44,7 +44,6 @@ final class PhabricatorPeopleEditController
|
||||||
if ($user->getIsSystemAgent()) {
|
if ($user->getIsSystemAgent()) {
|
||||||
$nav->addFilter('picture', pht('Set Account Picture'));
|
$nav->addFilter('picture', pht('Set Account Picture'));
|
||||||
}
|
}
|
||||||
$nav->addFilter('delete', pht('Delete User'));
|
|
||||||
|
|
||||||
if (!$user->getID()) {
|
if (!$user->getID()) {
|
||||||
$this->view = 'basic';
|
$this->view = 'basic';
|
||||||
|
@ -79,9 +78,6 @@ final class PhabricatorPeopleEditController
|
||||||
case 'picture':
|
case 'picture':
|
||||||
$response = $this->processSetAccountPicture($user);
|
$response = $this->processSetAccountPicture($user);
|
||||||
break;
|
break;
|
||||||
case 'delete':
|
|
||||||
$response = $this->processDeleteRequest($user);
|
|
||||||
break;
|
|
||||||
default:
|
default:
|
||||||
return new Aphront404Response();
|
return new Aphront404Response();
|
||||||
}
|
}
|
||||||
|
@ -573,91 +569,6 @@ final class PhabricatorPeopleEditController
|
||||||
return array($form_box);
|
return array($form_box);
|
||||||
}
|
}
|
||||||
|
|
||||||
private function processDeleteRequest(PhabricatorUser $user) {
|
|
||||||
$request = $this->getRequest();
|
|
||||||
$admin = $request->getUser();
|
|
||||||
|
|
||||||
$far1 = pht('As you stare into the gaping maw of the abyss, something '.
|
|
||||||
'hold you back.');
|
|
||||||
$far2 = pht('You can not delete your own account.');
|
|
||||||
|
|
||||||
if ($user->getPHID() == $admin->getPHID()) {
|
|
||||||
$error = new AphrontErrorView();
|
|
||||||
$error->setTitle(pht('You Shall Journey No Farther'));
|
|
||||||
$error->appendChild(hsprintf(
|
|
||||||
'<p>%s</p><p>%s</p>', $far1, $far2));
|
|
||||||
return $error;
|
|
||||||
}
|
|
||||||
|
|
||||||
$e_username = true;
|
|
||||||
$username = null;
|
|
||||||
|
|
||||||
$errors = array();
|
|
||||||
if ($request->isFormPost()) {
|
|
||||||
|
|
||||||
$username = $request->getStr('username');
|
|
||||||
if (!strlen($username)) {
|
|
||||||
$e_username = pht('Required');
|
|
||||||
$errors[] = pht('You must type the username to confirm deletion.');
|
|
||||||
} else if ($username != $user->getUsername()) {
|
|
||||||
$e_username = pht('Invalid');
|
|
||||||
$errors[] = pht('You must type the username correctly.');
|
|
||||||
}
|
|
||||||
|
|
||||||
if (!$errors) {
|
|
||||||
id(new PhabricatorUserEditor())
|
|
||||||
->setActor($admin)
|
|
||||||
->deleteUser($user);
|
|
||||||
|
|
||||||
return id(new AphrontRedirectResponse())->setURI('/people/');
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
$str1 = pht('Be careful when deleting users!');
|
|
||||||
$str2 = pht('If this user interacted with anything, it is generally '.
|
|
||||||
'better to disable them, not delete them. If you delete them, it will '.
|
|
||||||
'no longer be possible to search for their objects, for example, '.
|
|
||||||
'and you will lose other information about their history. Disabling '.
|
|
||||||
'them instead will prevent them from logging in but not destroy '.
|
|
||||||
'any of their data.');
|
|
||||||
$str3 = pht('It is generally safe to delete newly created users (and '.
|
|
||||||
'test users and so on), but less safe to delete established users. '.
|
|
||||||
'If possible, disable them instead.');
|
|
||||||
|
|
||||||
$form = new AphrontFormView();
|
|
||||||
$form
|
|
||||||
->setUser($admin)
|
|
||||||
->setAction($request->getRequestURI())
|
|
||||||
->appendChild(hsprintf(
|
|
||||||
'<p class="aphront-form-instructions">'.
|
|
||||||
'<strong>%s</strong> %s'.
|
|
||||||
'</p>'.
|
|
||||||
'<p class="aphront-form-instructions">'.
|
|
||||||
'%s'.
|
|
||||||
'</p>', $str1, $str2, $str3))
|
|
||||||
->appendChild(
|
|
||||||
id(new AphrontFormStaticControl())
|
|
||||||
->setLabel(pht('Username'))
|
|
||||||
->setValue($user->getUsername()))
|
|
||||||
->appendChild(
|
|
||||||
id(new AphrontFormTextControl())
|
|
||||||
->setLabel(pht('Confirm'))
|
|
||||||
->setValue($username)
|
|
||||||
->setName('username')
|
|
||||||
->setCaption(pht("Type the username again to confirm deletion."))
|
|
||||||
->setError($e_username))
|
|
||||||
->appendChild(
|
|
||||||
id(new AphrontFormSubmitControl())
|
|
||||||
->setValue(pht('Delete User')));
|
|
||||||
|
|
||||||
$form_box = id(new PHUIObjectBoxView())
|
|
||||||
->setHeaderText(pht('Delete User'))
|
|
||||||
->setFormErrors($errors)
|
|
||||||
->setForm($form);
|
|
||||||
|
|
||||||
return array($form_box);
|
|
||||||
}
|
|
||||||
|
|
||||||
private function getRoleInstructions() {
|
private function getRoleInstructions() {
|
||||||
$roles_link = phutil_tag(
|
$roles_link = phutil_tag(
|
||||||
'a',
|
'a',
|
||||||
|
|
|
@ -61,6 +61,14 @@ final class PhabricatorPeopleProfileController
|
||||||
->setWorkflow(!$can_edit));
|
->setWorkflow(!$can_edit));
|
||||||
|
|
||||||
if ($viewer->getIsAdmin()) {
|
if ($viewer->getIsAdmin()) {
|
||||||
|
$actions->addAction(
|
||||||
|
id(new PhabricatorActionView())
|
||||||
|
->setIcon('delete')
|
||||||
|
->setName(pht('Delete User'))
|
||||||
|
->setDisabled(($user->getPHID() == $viewer->getPHID()))
|
||||||
|
->setWorkflow(true)
|
||||||
|
->setHref($this->getApplicationURI('delete/'.$user->getID().'/')));
|
||||||
|
|
||||||
$actions->addAction(
|
$actions->addAction(
|
||||||
id(new PhabricatorActionView())
|
id(new PhabricatorActionView())
|
||||||
->setIcon('blame')
|
->setIcon('blame')
|
||||||
|
|
Loading…
Add table
Reference in a new issue