mirror of
https://we.phorge.it/source/phorge.git
synced 2024-12-24 06:20:56 +01:00
Remove deprecated Maniphest "Can Edit <Specific Property>" capabilities
Summary: Depends on D19579. Fixes T10003. These have been deprecated with a setup warning about their impending removal for about two and a half years. Ref T13164. See PHI642. My overall goal here is to simplify how we handle transactions which have special policy behaviors. In particular, I'm hoping to replace `ApplicationTransactionEditor->requireCapabilities()` with a new, more clear policy check. A problem with `requireCapabilities()` is that it doesn't actually enforce any policies in almost all cases: the default is "nothing", not CAN_EDIT. So it ends up looking like it's the right place to specialize policy checks, but it usually isn't. For "Disable", I need to be able to weaken the check selectively (you can disable users if you have the permission, even if you can't edit them otherwise). We have a handful of other edits which work like this (notably, leaving and joining projects) but they're very rare. Test Plan: Grepped for all removed classes. Edited a Maniphest task. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13164, T10003 Differential Revision: https://secure.phabricator.com/D19581
This commit is contained in:
parent
f9673a72a8
commit
296bf046a8
10 changed files with 1 additions and 208 deletions
|
@ -1646,13 +1646,8 @@ phutil_register_library_map(array(
|
||||||
'ManiphestDAO' => 'applications/maniphest/storage/ManiphestDAO.php',
|
'ManiphestDAO' => 'applications/maniphest/storage/ManiphestDAO.php',
|
||||||
'ManiphestDefaultEditCapability' => 'applications/maniphest/capability/ManiphestDefaultEditCapability.php',
|
'ManiphestDefaultEditCapability' => 'applications/maniphest/capability/ManiphestDefaultEditCapability.php',
|
||||||
'ManiphestDefaultViewCapability' => 'applications/maniphest/capability/ManiphestDefaultViewCapability.php',
|
'ManiphestDefaultViewCapability' => 'applications/maniphest/capability/ManiphestDefaultViewCapability.php',
|
||||||
'ManiphestEditAssignCapability' => 'applications/maniphest/capability/ManiphestEditAssignCapability.php',
|
|
||||||
'ManiphestEditConduitAPIMethod' => 'applications/maniphest/conduit/ManiphestEditConduitAPIMethod.php',
|
'ManiphestEditConduitAPIMethod' => 'applications/maniphest/conduit/ManiphestEditConduitAPIMethod.php',
|
||||||
'ManiphestEditEngine' => 'applications/maniphest/editor/ManiphestEditEngine.php',
|
'ManiphestEditEngine' => 'applications/maniphest/editor/ManiphestEditEngine.php',
|
||||||
'ManiphestEditPoliciesCapability' => 'applications/maniphest/capability/ManiphestEditPoliciesCapability.php',
|
|
||||||
'ManiphestEditPriorityCapability' => 'applications/maniphest/capability/ManiphestEditPriorityCapability.php',
|
|
||||||
'ManiphestEditProjectsCapability' => 'applications/maniphest/capability/ManiphestEditProjectsCapability.php',
|
|
||||||
'ManiphestEditStatusCapability' => 'applications/maniphest/capability/ManiphestEditStatusCapability.php',
|
|
||||||
'ManiphestEmailCommand' => 'applications/maniphest/command/ManiphestEmailCommand.php',
|
'ManiphestEmailCommand' => 'applications/maniphest/command/ManiphestEmailCommand.php',
|
||||||
'ManiphestGetTaskTransactionsConduitAPIMethod' => 'applications/maniphest/conduit/ManiphestGetTaskTransactionsConduitAPIMethod.php',
|
'ManiphestGetTaskTransactionsConduitAPIMethod' => 'applications/maniphest/conduit/ManiphestGetTaskTransactionsConduitAPIMethod.php',
|
||||||
'ManiphestHovercardEngineExtension' => 'applications/maniphest/engineextension/ManiphestHovercardEngineExtension.php',
|
'ManiphestHovercardEngineExtension' => 'applications/maniphest/engineextension/ManiphestHovercardEngineExtension.php',
|
||||||
|
@ -7152,13 +7147,8 @@ phutil_register_library_map(array(
|
||||||
'ManiphestDAO' => 'PhabricatorLiskDAO',
|
'ManiphestDAO' => 'PhabricatorLiskDAO',
|
||||||
'ManiphestDefaultEditCapability' => 'PhabricatorPolicyCapability',
|
'ManiphestDefaultEditCapability' => 'PhabricatorPolicyCapability',
|
||||||
'ManiphestDefaultViewCapability' => 'PhabricatorPolicyCapability',
|
'ManiphestDefaultViewCapability' => 'PhabricatorPolicyCapability',
|
||||||
'ManiphestEditAssignCapability' => 'PhabricatorPolicyCapability',
|
|
||||||
'ManiphestEditConduitAPIMethod' => 'PhabricatorEditEngineAPIMethod',
|
'ManiphestEditConduitAPIMethod' => 'PhabricatorEditEngineAPIMethod',
|
||||||
'ManiphestEditEngine' => 'PhabricatorEditEngine',
|
'ManiphestEditEngine' => 'PhabricatorEditEngine',
|
||||||
'ManiphestEditPoliciesCapability' => 'PhabricatorPolicyCapability',
|
|
||||||
'ManiphestEditPriorityCapability' => 'PhabricatorPolicyCapability',
|
|
||||||
'ManiphestEditProjectsCapability' => 'PhabricatorPolicyCapability',
|
|
||||||
'ManiphestEditStatusCapability' => 'PhabricatorPolicyCapability',
|
|
||||||
'ManiphestEmailCommand' => 'MetaMTAEmailTransactionCommand',
|
'ManiphestEmailCommand' => 'MetaMTAEmailTransactionCommand',
|
||||||
'ManiphestGetTaskTransactionsConduitAPIMethod' => 'ManiphestConduitAPIMethod',
|
'ManiphestGetTaskTransactionsConduitAPIMethod' => 'ManiphestConduitAPIMethod',
|
||||||
'ManiphestHovercardEngineExtension' => 'PhabricatorHovercardEngineExtension',
|
'ManiphestHovercardEngineExtension' => 'PhabricatorHovercardEngineExtension',
|
||||||
|
|
|
@ -361,69 +361,4 @@ final class PhabricatorExtraConfigSetupCheck extends PhabricatorSetupCheck {
|
||||||
return $ancient_config;
|
return $ancient_config;
|
||||||
}
|
}
|
||||||
|
|
||||||
private function executeManiphestFieldChecks() {
|
|
||||||
$maniphest_appclass = 'PhabricatorManiphestApplication';
|
|
||||||
if (!PhabricatorApplication::isClassInstalled($maniphest_appclass)) {
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
$capabilities = array(
|
|
||||||
ManiphestEditAssignCapability::CAPABILITY,
|
|
||||||
ManiphestEditPoliciesCapability::CAPABILITY,
|
|
||||||
ManiphestEditPriorityCapability::CAPABILITY,
|
|
||||||
ManiphestEditProjectsCapability::CAPABILITY,
|
|
||||||
ManiphestEditStatusCapability::CAPABILITY,
|
|
||||||
);
|
|
||||||
|
|
||||||
// Check for any of these capabilities set to anything other than
|
|
||||||
// "All Users".
|
|
||||||
|
|
||||||
$any_set = false;
|
|
||||||
$app = new PhabricatorManiphestApplication();
|
|
||||||
foreach ($capabilities as $capability) {
|
|
||||||
$setting = $app->getPolicy($capability);
|
|
||||||
if ($setting != PhabricatorPolicies::POLICY_USER) {
|
|
||||||
$any_set = true;
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
if (!$any_set) {
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
$issue_summary = pht(
|
|
||||||
'Maniphest is currently configured with deprecated policy settings '.
|
|
||||||
'which will be removed in a future version of Phabricator.');
|
|
||||||
|
|
||||||
|
|
||||||
$message = pht(
|
|
||||||
'Some policy settings in Maniphest are now deprecated and will be '.
|
|
||||||
'removed in a future version of Phabricator. You are currently using '.
|
|
||||||
'at least one of these settings.'.
|
|
||||||
"\n\n".
|
|
||||||
'The deprecated settings are "Can Assign Tasks", '.
|
|
||||||
'"Can Edit Task Policies", "Can Prioritize Tasks", '.
|
|
||||||
'"Can Edit Task Projects", and "Can Edit Task Status". You can '.
|
|
||||||
'find these settings in Applications, or follow the link below.'.
|
|
||||||
"\n\n".
|
|
||||||
'You can find discussion of this change (including rationale and '.
|
|
||||||
'recommendations on how to configure similar features) in the upstream, '.
|
|
||||||
'at the link below.'.
|
|
||||||
"\n\n".
|
|
||||||
'To resolve this issue, set all of these policies to "All Users" after '.
|
|
||||||
'making any necessary form customization changes.');
|
|
||||||
|
|
||||||
$more_href = 'https://secure.phabricator.com/T10003';
|
|
||||||
$edit_href = '/applications/view/PhabricatorManiphestApplication/';
|
|
||||||
|
|
||||||
$issue = $this->newIssue('maniphest.T10003-per-field-policies')
|
|
||||||
->setShortName(pht('Deprecated Policies'))
|
|
||||||
->setName(pht('Deprecated Maniphest Field Policies'))
|
|
||||||
->setSummary($issue_summary)
|
|
||||||
->setMessage($message)
|
|
||||||
->addLink($more_href, pht('Learn More: Upstream Discussion'))
|
|
||||||
->addLink($edit_href, pht('Edit These Settings'));
|
|
||||||
}
|
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -85,11 +85,6 @@ final class PhabricatorManiphestApplication extends PhabricatorApplication {
|
||||||
'template' => ManiphestTaskPHIDType::TYPECONST,
|
'template' => ManiphestTaskPHIDType::TYPECONST,
|
||||||
'capability' => PhabricatorPolicyCapability::CAN_EDIT,
|
'capability' => PhabricatorPolicyCapability::CAN_EDIT,
|
||||||
),
|
),
|
||||||
ManiphestEditStatusCapability::CAPABILITY => array(),
|
|
||||||
ManiphestEditAssignCapability::CAPABILITY => array(),
|
|
||||||
ManiphestEditPoliciesCapability::CAPABILITY => array(),
|
|
||||||
ManiphestEditPriorityCapability::CAPABILITY => array(),
|
|
||||||
ManiphestEditProjectsCapability::CAPABILITY => array(),
|
|
||||||
ManiphestBulkEditCapability::CAPABILITY => array(),
|
ManiphestBulkEditCapability::CAPABILITY => array(),
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
|
@ -1,15 +0,0 @@
|
||||||
<?php
|
|
||||||
|
|
||||||
final class ManiphestEditAssignCapability extends PhabricatorPolicyCapability {
|
|
||||||
|
|
||||||
const CAPABILITY = 'maniphest.edit.assign';
|
|
||||||
|
|
||||||
public function getCapabilityName() {
|
|
||||||
return pht('Can Assign Tasks');
|
|
||||||
}
|
|
||||||
|
|
||||||
public function describeCapabilityRejection() {
|
|
||||||
return pht('You do not have permission to assign tasks.');
|
|
||||||
}
|
|
||||||
|
|
||||||
}
|
|
|
@ -1,16 +0,0 @@
|
||||||
<?php
|
|
||||||
|
|
||||||
final class ManiphestEditPoliciesCapability
|
|
||||||
extends PhabricatorPolicyCapability {
|
|
||||||
|
|
||||||
const CAPABILITY = 'maniphest.edit.policies';
|
|
||||||
|
|
||||||
public function getCapabilityName() {
|
|
||||||
return pht('Can Edit Task Policies');
|
|
||||||
}
|
|
||||||
|
|
||||||
public function describeCapabilityRejection() {
|
|
||||||
return pht('You do not have permission to edit task policies.');
|
|
||||||
}
|
|
||||||
|
|
||||||
}
|
|
|
@ -1,16 +0,0 @@
|
||||||
<?php
|
|
||||||
|
|
||||||
final class ManiphestEditPriorityCapability
|
|
||||||
extends PhabricatorPolicyCapability {
|
|
||||||
|
|
||||||
const CAPABILITY = 'maniphest.edit.priority';
|
|
||||||
|
|
||||||
public function getCapabilityName() {
|
|
||||||
return pht('Can Prioritize Tasks');
|
|
||||||
}
|
|
||||||
|
|
||||||
public function describeCapabilityRejection() {
|
|
||||||
return pht('You do not have permission to prioritize tasks.');
|
|
||||||
}
|
|
||||||
|
|
||||||
}
|
|
|
@ -1,16 +0,0 @@
|
||||||
<?php
|
|
||||||
|
|
||||||
final class ManiphestEditProjectsCapability
|
|
||||||
extends PhabricatorPolicyCapability {
|
|
||||||
|
|
||||||
const CAPABILITY = 'maniphest.edit.projects';
|
|
||||||
|
|
||||||
public function getCapabilityName() {
|
|
||||||
return pht('Can Edit Task Projects');
|
|
||||||
}
|
|
||||||
|
|
||||||
public function describeCapabilityRejection() {
|
|
||||||
return pht('You do not have permission to edit task projects.');
|
|
||||||
}
|
|
||||||
|
|
||||||
}
|
|
|
@ -1,15 +0,0 @@
|
||||||
<?php
|
|
||||||
|
|
||||||
final class ManiphestEditStatusCapability extends PhabricatorPolicyCapability {
|
|
||||||
|
|
||||||
const CAPABILITY = 'maniphest.edit.status';
|
|
||||||
|
|
||||||
public function getCapabilityName() {
|
|
||||||
return pht('Can Edit Task Status');
|
|
||||||
}
|
|
||||||
|
|
||||||
public function describeCapabilityRejection() {
|
|
||||||
return pht('You do not have permission to edit task status.');
|
|
||||||
}
|
|
||||||
|
|
||||||
}
|
|
|
@ -279,51 +279,6 @@ final class ManiphestTransactionEditor
|
||||||
->setTask($object);
|
->setTask($object);
|
||||||
}
|
}
|
||||||
|
|
||||||
protected function requireCapabilities(
|
|
||||||
PhabricatorLiskDAO $object,
|
|
||||||
PhabricatorApplicationTransaction $xaction) {
|
|
||||||
|
|
||||||
parent::requireCapabilities($object, $xaction);
|
|
||||||
|
|
||||||
$app_capability_map = array(
|
|
||||||
ManiphestTaskPriorityTransaction::TRANSACTIONTYPE =>
|
|
||||||
ManiphestEditPriorityCapability::CAPABILITY,
|
|
||||||
ManiphestTaskStatusTransaction::TRANSACTIONTYPE =>
|
|
||||||
ManiphestEditStatusCapability::CAPABILITY,
|
|
||||||
ManiphestTaskOwnerTransaction::TRANSACTIONTYPE =>
|
|
||||||
ManiphestEditAssignCapability::CAPABILITY,
|
|
||||||
PhabricatorTransactions::TYPE_EDIT_POLICY =>
|
|
||||||
ManiphestEditPoliciesCapability::CAPABILITY,
|
|
||||||
PhabricatorTransactions::TYPE_VIEW_POLICY =>
|
|
||||||
ManiphestEditPoliciesCapability::CAPABILITY,
|
|
||||||
);
|
|
||||||
|
|
||||||
|
|
||||||
$transaction_type = $xaction->getTransactionType();
|
|
||||||
|
|
||||||
$app_capability = null;
|
|
||||||
if ($transaction_type == PhabricatorTransactions::TYPE_EDGE) {
|
|
||||||
switch ($xaction->getMetadataValue('edge:type')) {
|
|
||||||
case PhabricatorProjectObjectHasProjectEdgeType::EDGECONST:
|
|
||||||
$app_capability = ManiphestEditProjectsCapability::CAPABILITY;
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
} else {
|
|
||||||
$app_capability = idx($app_capability_map, $transaction_type);
|
|
||||||
}
|
|
||||||
|
|
||||||
if ($app_capability) {
|
|
||||||
$app = id(new PhabricatorApplicationQuery())
|
|
||||||
->setViewer($this->getActor())
|
|
||||||
->withClasses(array('PhabricatorManiphestApplication'))
|
|
||||||
->executeOne();
|
|
||||||
PhabricatorPolicyFilter::requireCapability(
|
|
||||||
$this->getActor(),
|
|
||||||
$app,
|
|
||||||
$app_capability);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
protected function adjustObjectForPolicyChecks(
|
protected function adjustObjectForPolicyChecks(
|
||||||
PhabricatorLiskDAO $object,
|
PhabricatorLiskDAO $object,
|
||||||
array $xactions) {
|
array $xactions) {
|
||||||
|
|
|
@ -369,11 +369,7 @@ final class ManiphestTaskSearchEngine
|
||||||
$can_edit_priority = false;
|
$can_edit_priority = false;
|
||||||
$can_bulk_edit = false;
|
$can_bulk_edit = false;
|
||||||
} else {
|
} else {
|
||||||
$can_edit_priority = PhabricatorPolicyFilter::hasCapability(
|
$can_edit_priority = true;
|
||||||
$viewer,
|
|
||||||
$this->getApplication(),
|
|
||||||
ManiphestEditPriorityCapability::CAPABILITY);
|
|
||||||
|
|
||||||
$can_bulk_edit = PhabricatorPolicyFilter::hasCapability(
|
$can_bulk_edit = PhabricatorPolicyFilter::hasCapability(
|
||||||
$viewer,
|
$viewer,
|
||||||
$this->getApplication(),
|
$this->getApplication(),
|
||||||
|
|
Loading…
Reference in a new issue