1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-10 00:42:41 +01:00

Add capabilities for editing task triage details (priority, assignee, etc)

Summary:
This is primarily a client request, and a little bit use-case specific, but policies seem to be holding up well and I'm getting more comfortable about maintaining this. Much if it can run through ApplicationTransactions.

Allow the ability to edit status, policies, priorities, assignees and projects of a task to be restricted to some subset of users. Also allow bulk edit to be locked. This affects the editor itself and the edit, view and list interfaces.

Test Plan: As a restricted user, created, edited and commented on tasks. Tried to drag them around.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7357
This commit is contained in:
epriestley 2013-10-21 16:59:06 -07:00
parent 00bf47f973
commit 7fedfacbca
14 changed files with 333 additions and 76 deletions

View file

@ -705,8 +705,14 @@ phutil_register_library_map(array(
'LiskMigrationIterator' => 'infrastructure/storage/lisk/LiskMigrationIterator.php',
'LiskRawMigrationIterator' => 'infrastructure/storage/lisk/LiskRawMigrationIterator.php',
'ManiphestBatchEditController' => 'applications/maniphest/controller/ManiphestBatchEditController.php',
'ManiphestCapabilityBulkEdit' => 'applications/maniphest/capability/ManiphestCapabilityBulkEdit.php',
'ManiphestCapabilityDefaultEdit' => 'applications/maniphest/capability/ManiphestCapabilityDefaultEdit.php',
'ManiphestCapabilityDefaultView' => 'applications/maniphest/capability/ManiphestCapabilityDefaultView.php',
'ManiphestCapabilityEditAssign' => 'applications/maniphest/capability/ManiphestCapabilityEditAssign.php',
'ManiphestCapabilityEditPolicies' => 'applications/maniphest/capability/ManiphestCapabilityEditPolicies.php',
'ManiphestCapabilityEditPriority' => 'applications/maniphest/capability/ManiphestCapabilityEditPriority.php',
'ManiphestCapabilityEditProjects' => 'applications/maniphest/capability/ManiphestCapabilityEditProjects.php',
'ManiphestCapabilityEditStatus' => 'applications/maniphest/capability/ManiphestCapabilityEditStatus.php',
'ManiphestConfiguredCustomField' => 'applications/maniphest/field/ManiphestConfiguredCustomField.php',
'ManiphestConstants' => 'applications/maniphest/constants/ManiphestConstants.php',
'ManiphestController' => 'applications/maniphest/controller/ManiphestController.php',
@ -2837,8 +2843,14 @@ phutil_register_library_map(array(
'LiskMigrationIterator' => 'PhutilBufferedIterator',
'LiskRawMigrationIterator' => 'PhutilBufferedIterator',
'ManiphestBatchEditController' => 'ManiphestController',
'ManiphestCapabilityBulkEdit' => 'PhabricatorPolicyCapability',
'ManiphestCapabilityDefaultEdit' => 'PhabricatorPolicyCapability',
'ManiphestCapabilityDefaultView' => 'PhabricatorPolicyCapability',
'ManiphestCapabilityEditAssign' => 'PhabricatorPolicyCapability',
'ManiphestCapabilityEditPolicies' => 'PhabricatorPolicyCapability',
'ManiphestCapabilityEditPriority' => 'PhabricatorPolicyCapability',
'ManiphestCapabilityEditProjects' => 'PhabricatorPolicyCapability',
'ManiphestCapabilityEditStatus' => 'PhabricatorPolicyCapability',
'ManiphestConfiguredCustomField' =>
array(
0 => 'ManiphestCustomField',

View file

@ -67,7 +67,7 @@ final class PhabricatorApplicationManiphest extends PhabricatorApplication {
'export/(?P<key>[^/]+)/' => 'ManiphestExportController',
'subpriority/' => 'ManiphestSubpriorityController',
'subscribe/(?P<action>add|rem)/(?P<id>[1-9]\d*)/'
=> 'ManiphestSubscribeController',
=> 'ManiphestSubscribeController',
),
);
}
@ -100,6 +100,18 @@ final class PhabricatorApplicationManiphest extends PhabricatorApplication {
'caption' => pht(
'Default edit policy for newly created tasks.'),
),
ManiphestCapabilityEditStatus::CAPABILITY => array(
),
ManiphestCapabilityEditAssign::CAPABILITY => array(
),
ManiphestCapabilityEditPolicies::CAPABILITY => array(
),
ManiphestCapabilityEditPriority::CAPABILITY => array(
),
ManiphestCapabilityEditProjects::CAPABILITY => array(
),
ManiphestCapabilityBulkEdit::CAPABILITY => array(
),
);
}

View file

@ -0,0 +1,20 @@
<?php
final class ManiphestCapabilityBulkEdit
extends PhabricatorPolicyCapability {
const CAPABILITY = 'maniphest.edit.bulk';
public function getCapabilityKey() {
return self::CAPABILITY;
}
public function getCapabilityName() {
return pht('Can Bulk Edit Tasks');
}
public function describeCapabilityRejection() {
return pht('You do not have permission to bulk edit tasks.');
}
}

View file

@ -0,0 +1,20 @@
<?php
final class ManiphestCapabilityEditAssign
extends PhabricatorPolicyCapability {
const CAPABILITY = 'maniphest.edit.assign';
public function getCapabilityKey() {
return self::CAPABILITY;
}
public function getCapabilityName() {
return pht('Can Assign Tasks');
}
public function describeCapabilityRejection() {
return pht('You do not have permission to assign tasks.');
}
}

View file

@ -0,0 +1,20 @@
<?php
final class ManiphestCapabilityEditPolicies
extends PhabricatorPolicyCapability {
const CAPABILITY = 'maniphest.edit.policies';
public function getCapabilityKey() {
return self::CAPABILITY;
}
public function getCapabilityName() {
return pht('Can Edit Task Policies');
}
public function describeCapabilityRejection() {
return pht('You do not have permission to edit task policies.');
}
}

View file

@ -0,0 +1,20 @@
<?php
final class ManiphestCapabilityEditPriority
extends PhabricatorPolicyCapability {
const CAPABILITY = 'maniphest.edit.priority';
public function getCapabilityKey() {
return self::CAPABILITY;
}
public function getCapabilityName() {
return pht('Can Prioritize Tasks');
}
public function describeCapabilityRejection() {
return pht('You do not have permission to prioritize tasks.');
}
}

View file

@ -0,0 +1,20 @@
<?php
final class ManiphestCapabilityEditProjects
extends PhabricatorPolicyCapability {
const CAPABILITY = 'maniphest.edit.projects';
public function getCapabilityKey() {
return self::CAPABILITY;
}
public function getCapabilityName() {
return pht('Can Edit Task Projects');
}
public function describeCapabilityRejection() {
return pht('You do not have permission to edit task projects.');
}
}

View file

@ -0,0 +1,20 @@
<?php
final class ManiphestCapabilityEditStatus
extends PhabricatorPolicyCapability {
const CAPABILITY = 'maniphest.edit.status';
public function getCapabilityKey() {
return self::CAPABILITY;
}
public function getCapabilityName() {
return pht('Can Edit Task Status');
}
public function describeCapabilityRejection() {
return pht('You do not have permission to edit task status.');
}
}

View file

@ -7,6 +7,9 @@ final class ManiphestBatchEditController extends ManiphestController {
public function processRequest() {
$this->requireApplicationCapability(
ManiphestCapabilityBulkEdit::CAPABILITY);
$request = $this->getRequest();
$user = $request->getUser();

View file

@ -165,6 +165,27 @@ final class ManiphestTaskDetailController extends ManiphestController {
ManiphestTransaction::TYPE_PROJECTS => pht('Associate Projects'),
);
// Remove actions the user doesn't have permission to take.
$requires = array(
ManiphestTransaction::TYPE_OWNER =>
ManiphestCapabilityEditAssign::CAPABILITY,
ManiphestTransaction::TYPE_PRIORITY =>
ManiphestCapabilityEditPriority::CAPABILITY,
ManiphestTransaction::TYPE_PROJECTS =>
ManiphestCapabilityEditProjects::CAPABILITY,
ManiphestTransaction::TYPE_STATUS =>
ManiphestCapabilityEditStatus::CAPABILITY,
);
foreach ($transaction_types as $type => $name) {
if (isset($requires[$type])) {
if (!$this->hasApplicationCapability($requires[$type])) {
unset($transaction_types[$type]);
}
}
}
if ($task->getStatus() == ManiphestTaskStatus::STATUS_OPEN) {
$resolution_types = array_select_keys(
$resolution_types,

View file

@ -1,8 +1,5 @@
<?php
/**
* @group maniphest
*/
final class ManiphestTaskEditController extends ManiphestController {
private $id;
@ -16,6 +13,17 @@ final class ManiphestTaskEditController extends ManiphestController {
$request = $this->getRequest();
$user = $request->getUser();
$can_edit_assign = $this->hasApplicationCapability(
ManiphestCapabilityEditAssign::CAPABILITY);
$can_edit_policies = $this->hasApplicationCapability(
ManiphestCapabilityEditPolicies::CAPABILITY);
$can_edit_priority = $this->hasApplicationCapability(
ManiphestCapabilityEditPriority::CAPABILITY);
$can_edit_projects = $this->hasApplicationCapability(
ManiphestCapabilityEditProjects::CAPABILITY);
$can_edit_status = $this->hasApplicationCapability(
ManiphestCapabilityEditStatus::CAPABILITY);
$files = array();
$parent_task = null;
$template_id = null;
@ -40,20 +48,24 @@ final class ManiphestTaskEditController extends ManiphestController {
if (!$request->isFormPost()) {
$task->setTitle($request->getStr('title'));
$default_projects = $request->getStr('projects');
if ($default_projects) {
$task->setProjectPHIDs(explode(';', $default_projects));
if ($can_edit_projects) {
$default_projects = $request->getStr('projects');
if ($default_projects) {
$task->setProjectPHIDs(explode(';', $default_projects));
}
}
$task->setDescription($request->getStr('description'));
$assign = $request->getStr('assign');
if (strlen($assign)) {
$assign_user = id(new PhabricatorUser())->loadOneWhere(
'username = %s',
$assign);
if ($assign_user) {
$task->setOwnerPHID($assign_user->getPHID());
if ($can_edit_assign) {
$assign = $request->getStr('assign');
if (strlen($assign)) {
$assign_user = id(new PhabricatorUser())->loadOneWhere(
'username = %s',
$assign);
if ($assign_user) {
$task->setOwnerPHID($assign_user->getPHID());
}
}
}
}
@ -122,7 +134,9 @@ final class ManiphestTaskEditController extends ManiphestController {
$changes[ManiphestTransaction::TYPE_TITLE] = $new_title;
$changes[ManiphestTransaction::TYPE_DESCRIPTION] = $new_desc;
$changes[ManiphestTransaction::TYPE_STATUS] = $new_status;
if ($can_edit_status) {
$changes[ManiphestTransaction::TYPE_STATUS] = $new_status;
}
$owner_tokenizer = $request->getArr('assigned_to');
$owner_phid = reset($owner_tokenizer);
@ -173,17 +187,27 @@ final class ManiphestTaskEditController extends ManiphestController {
$task->setProjectPHIDs($request->getArr('projects'));
} else {
$changes[ManiphestTransaction::TYPE_PRIORITY] =
$request->getInt('priority');
$changes[ManiphestTransaction::TYPE_OWNER] = $owner_phid;
$changes[ManiphestTransaction::TYPE_CCS] = $request->getArr('cc');
$changes[ManiphestTransaction::TYPE_PROJECTS] =
$request->getArr('projects');
if ($can_edit_priority) {
$changes[ManiphestTransaction::TYPE_PRIORITY] =
$request->getInt('priority');
}
if ($can_edit_assign) {
$changes[ManiphestTransaction::TYPE_OWNER] = $owner_phid;
}
$changes[PhabricatorTransactions::TYPE_VIEW_POLICY] =
$request->getStr('viewPolicy');
$changes[PhabricatorTransactions::TYPE_EDIT_POLICY] =
$request->getStr('editPolicy');
$changes[ManiphestTransaction::TYPE_CCS] = $request->getArr('cc');
if ($can_edit_projects) {
$changes[ManiphestTransaction::TYPE_PROJECTS] =
$request->getArr('projects');
}
if ($can_edit_policies) {
$changes[PhabricatorTransactions::TYPE_VIEW_POLICY] =
$request->getStr('viewPolicy');
$changes[PhabricatorTransactions::TYPE_EDIT_POLICY] =
$request->getStr('editPolicy');
}
if ($files) {
$file_map = mpull($files, 'getPHID');
@ -418,7 +442,7 @@ final class ManiphestTaskEditController extends ManiphestController {
->setHeight(AphrontFormTextAreaControl::HEIGHT_VERY_SHORT)
->setValue($task->getTitle()));
if ($task->getID()) {
if ($task->getID() && $can_edit_status) {
// Only show this in "edit" mode, not "create" mode, since creating a
// non-open task is kind of silly and it would just clutter up the
// "create" interface.
@ -436,58 +460,73 @@ final class ManiphestTaskEditController extends ManiphestController {
->setObject($task)
->execute();
$form
->appendChild(
if ($can_edit_assign) {
$form->appendChild(
id(new AphrontFormTokenizerControl())
->setLabel(pht('Assigned To'))
->setName('assigned_to')
->setValue($assigned_value)
->setUser($user)
->setDatasource('/typeahead/common/users/')
->setLimit(1))
->setLimit(1));
}
$form
->appendChild(
id(new AphrontFormTokenizerControl())
->setLabel(pht('CC'))
->setName('cc')
->setValue($cc_value)
->setUser($user)
->setDatasource('/typeahead/common/mailable/'))
->appendChild(
id(new AphrontFormSelectControl())
->setLabel(pht('Priority'))
->setName('priority')
->setOptions($priority_map)
->setValue($task->getPriority()))
->appendChild(
id(new AphrontFormPolicyControl())
->setUser($user)
->setCapability(PhabricatorPolicyCapability::CAN_VIEW)
->setPolicyObject($task)
->setPolicies($policies)
->setName('viewPolicy'))
->appendChild(
id(new AphrontFormPolicyControl())
->setUser($user)
->setCapability(PhabricatorPolicyCapability::CAN_EDIT)
->setPolicyObject($task)
->setPolicies($policies)
->setName('editPolicy'))
->appendChild(
id(new AphrontFormTokenizerControl())
->setLabel(pht('Projects'))
->setName('projects')
->setValue($projects_value)
->setID($project_tokenizer_id)
->setCaption(
javelin_tag(
'a',
array(
'href' => '/project/create/',
'mustcapture' => true,
'sigil' => 'project-create',
),
pht('Create New Project')))
->setDatasource('/typeahead/common/projects/'));
->setDatasource('/typeahead/common/mailable/'));
if ($can_edit_priority) {
$form
->appendChild(
id(new AphrontFormSelectControl())
->setLabel(pht('Priority'))
->setName('priority')
->setOptions($priority_map)
->setValue($task->getPriority()));
}
if ($can_edit_policies) {
$form
->appendChild(
id(new AphrontFormPolicyControl())
->setUser($user)
->setCapability(PhabricatorPolicyCapability::CAN_VIEW)
->setPolicyObject($task)
->setPolicies($policies)
->setName('viewPolicy'))
->appendChild(
id(new AphrontFormPolicyControl())
->setUser($user)
->setCapability(PhabricatorPolicyCapability::CAN_EDIT)
->setPolicyObject($task)
->setPolicies($policies)
->setName('editPolicy'));
}
if ($can_edit_projects) {
$form
->appendChild(
id(new AphrontFormTokenizerControl())
->setLabel(pht('Projects'))
->setName('projects')
->setValue($projects_value)
->setID($project_tokenizer_id)
->setCaption(
javelin_tag(
'a',
array(
'href' => '/project/create/',
'mustcapture' => true,
'sigil' => 'project-create',
),
pht('Create New Project')))
->setDatasource('/typeahead/common/projects/'));
}
foreach ($aux_fields as $aux_field) {
$aux_control = $aux_field->renderEditControl();

View file

@ -46,7 +46,11 @@ final class ManiphestTaskListController
$group_parameter,
$handles);
$can_edit_priority = $this->hasApplicationCapability(
ManiphestCapabilityEditPriority::CAPABILITY);
$can_drag = ($order_parameter == 'priority') &&
($can_edit_priority) &&
($group_parameter == 'none' || $group_parameter == 'priority');
if (!$viewer->isLoggedIn()) {
@ -91,11 +95,13 @@ final class ManiphestTaskListController
));
}
Javelin::initBehavior(
'maniphest-subpriority-editor',
array(
'uri' => '/maniphest/subpriority/',
));
if ($can_drag) {
Javelin::initBehavior(
'maniphest-subpriority-editor',
array(
'uri' => '/maniphest/subpriority/',
));
}
return phutil_tag(
'div',
@ -191,6 +197,11 @@ final class ManiphestTaskListController
private function renderBatchEditor(PhabricatorSavedQuery $saved_query) {
$user = $this->getRequest()->getUser();
$batch_capability = ManiphestCapabilityBulkEdit::CAPABILITY;
if (!$this->hasApplicationCapability($batch_capability)) {
return null;
}
if (!$user->isLoggedIn()) {
// Don't show the batch editor or excel export for logged-out users.
// Technically we //could// let them export, but ehh.

View file

@ -81,9 +81,9 @@ final class ManiphestTransactionEditor
case ManiphestTransaction::TYPE_EDGE:
return $xaction->getNewValue();
}
}
protected function transactionHasEffect(
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
@ -249,6 +249,43 @@ final class ManiphestTransactionEditor
}
}
protected function requireCapabilities(
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
parent::requireCapabilities($object, $xaction);
$app_capability_map = array(
ManiphestTransaction::TYPE_PRIORITY =>
ManiphestCapabilityEditPriority::CAPABILITY,
ManiphestTransaction::TYPE_STATUS =>
ManiphestCapabilityEditStatus::CAPABILITY,
ManiphestTransaction::TYPE_PROJECTS =>
ManiphestCapabilityEditProjects::CAPABILITY,
ManiphestTransaction::TYPE_OWNER =>
ManiphestCapabilityEditAssign::CAPABILITY,
PhabricatorTransactions::TYPE_EDIT_POLICY =>
ManiphestCapabilityEditPolicies::CAPABILITY,
PhabricatorTransactions::TYPE_VIEW_POLICY =>
ManiphestCapabilityEditPolicies::CAPABILITY,
);
$transaction_type = $xaction->getTransactionType();
$app_capability = idx($app_capability_map, $transaction_type);
if ($app_capability) {
$app = id(new PhabricatorApplicationQuery())
->setViewer($this->getActor())
->withClasses(array('PhabricatorApplicationManiphest'))
->executeOne();
PhabricatorPolicyFilter::requireCapability(
$this->getActor(),
$app,
$app_capability);
}
}
public static function getNextSubpriority($pri, $sub) {
// TODO: T603 Figure out what the policies here should be once this gets

View file

@ -460,6 +460,12 @@ abstract class PhabricatorApplicationTransactionEditor
return array();
}
// Now that we've merged, filtered, and combined transactions, check for
// required capabilities.
foreach ($xactions as $xaction) {
$this->requireCapabilities($object, $xaction);
}
$xactions = $this->sortTransactions($xactions);
if ($is_preview) {
@ -696,10 +702,6 @@ abstract class PhabricatorApplicationTransactionEditor
$actor,
$object,
PhabricatorPolicyCapability::CAN_VIEW);
foreach ($xactions as $xaction) {
$this->requireCapabilities($object, $xaction);
}
}
protected function requireCapabilities(