Page MenuHomePhabricator

D17757.diff
No OneTemporary

D17757.diff

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -1849,9 +1849,12 @@
'PhabricatorApplicationDatasource' => 'applications/meta/typeahead/PhabricatorApplicationDatasource.php',
'PhabricatorApplicationDetailViewController' => 'applications/meta/controller/PhabricatorApplicationDetailViewController.php',
'PhabricatorApplicationEditController' => 'applications/meta/controller/PhabricatorApplicationEditController.php',
+ 'PhabricatorApplicationEditEngine' => 'applications/meta/editor/PhabricatorApplicationEditEngine.php',
'PhabricatorApplicationEditHTTPParameterHelpView' => 'applications/transactions/view/PhabricatorApplicationEditHTTPParameterHelpView.php',
+ 'PhabricatorApplicationEditor' => 'applications/meta/editor/PhabricatorApplicationEditor.php',
'PhabricatorApplicationEmailCommandsController' => 'applications/meta/controller/PhabricatorApplicationEmailCommandsController.php',
'PhabricatorApplicationPanelController' => 'applications/meta/controller/PhabricatorApplicationPanelController.php',
+ 'PhabricatorApplicationPolicyChangeTransaction' => 'applications/meta/xactions/PhabricatorApplicationPolicyChangeTransaction.php',
'PhabricatorApplicationProfileMenuItem' => 'applications/search/menuitem/PhabricatorApplicationProfileMenuItem.php',
'PhabricatorApplicationQuery' => 'applications/meta/query/PhabricatorApplicationQuery.php',
'PhabricatorApplicationSchemaSpec' => 'applications/meta/storage/PhabricatorApplicationSchemaSpec.php',
@@ -6889,9 +6892,12 @@
'PhabricatorApplicationDatasource' => 'PhabricatorTypeaheadDatasource',
'PhabricatorApplicationDetailViewController' => 'PhabricatorApplicationsController',
'PhabricatorApplicationEditController' => 'PhabricatorApplicationsController',
+ 'PhabricatorApplicationEditEngine' => 'PhabricatorEditEngine',
'PhabricatorApplicationEditHTTPParameterHelpView' => 'AphrontView',
+ 'PhabricatorApplicationEditor' => 'PhabricatorApplicationTransactionEditor',
'PhabricatorApplicationEmailCommandsController' => 'PhabricatorApplicationsController',
'PhabricatorApplicationPanelController' => 'PhabricatorApplicationsController',
+ 'PhabricatorApplicationPolicyChangeTransaction' => 'PhabricatorApplicationTransactionType',
'PhabricatorApplicationProfileMenuItem' => 'PhabricatorProfileMenuItem',
'PhabricatorApplicationQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'PhabricatorApplicationSchemaSpec' => 'PhabricatorConfigSchemaSpec',
diff --git a/src/applications/base/PhabricatorApplication.php b/src/applications/base/PhabricatorApplication.php
--- a/src/applications/base/PhabricatorApplication.php
+++ b/src/applications/base/PhabricatorApplication.php
@@ -284,7 +284,6 @@
throw new PhutilMethodNotImplementedException();
}
-
/* -( Fact Integration )--------------------------------------------------- */
diff --git a/src/applications/meta/controller/PhabricatorApplicationDetailViewController.php b/src/applications/meta/controller/PhabricatorApplicationDetailViewController.php
--- a/src/applications/meta/controller/PhabricatorApplicationDetailViewController.php
+++ b/src/applications/meta/controller/PhabricatorApplicationDetailViewController.php
@@ -38,6 +38,11 @@
$header->setStatus('fa-ban', 'dark', pht('Uninstalled'));
}
+ $timeline = $this->buildTransactionTimeline(
+ $selected,
+ new PhabricatorApplicationApplicationTransactionQuery());
+ $timeline->setShouldTerminate(true);
+
$curtain = $this->buildCurtain($selected);
$details = $this->buildPropertySectionView($selected);
$policies = $this->buildPolicyView($selected);
@@ -61,6 +66,7 @@
->setMainColumn(array(
$policies,
$panels,
+ $timeline,
))
->addPropertySection(pht('Details'), $details);
diff --git a/src/applications/meta/controller/PhabricatorApplicationEditController.php b/src/applications/meta/controller/PhabricatorApplicationEditController.php
--- a/src/applications/meta/controller/PhabricatorApplicationEditController.php
+++ b/src/applications/meta/controller/PhabricatorApplicationEditController.php
@@ -30,8 +30,15 @@
->execute();
if ($request->isFormPost()) {
+ $xactions = array();
+
$result = array();
+ $template = $application->getApplicationTransactionTemplate();
foreach ($application->getCapabilities() as $capability) {
+ if (!$application->isCapabilityEditable($capability)) {
+ continue;
+ }
+
$old = $application->getPolicy($capability);
$new = $request->getStr('policy:'.$capability);
@@ -40,67 +47,36 @@
continue;
}
- if (empty($policies[$new])) {
- // Not a standard policy, check for a custom policy.
- $policy = id(new PhabricatorPolicyQuery())
- ->setViewer($user)
- ->withPHIDs(array($new))
- ->executeOne();
- if (!$policy) {
- // Not a custom policy either. Can't set the policy to something
- // invalid, so skip this.
- continue;
- }
- }
-
- if ($new == PhabricatorPolicies::POLICY_PUBLIC) {
- $capobj = PhabricatorPolicyCapability::getCapabilityByKey(
- $capability);
- if (!$capobj || !$capobj->shouldAllowPublicPolicySetting()) {
- // Can't set non-public policies to public.
- continue;
- }
- }
-
$result[$capability] = $new;
+
+ $xactions[] = id(clone $template)
+ ->setTransactionType(
+ PhabricatorApplicationPolicyChangeTransaction::TRANSACTIONTYPE)
+ ->setMetadataValue(
+ PhabricatorApplicationPolicyChangeTransaction::METADATA_ATTRIBUTE,
+ $capability)
+ ->setNewValue($new);
}
if ($result) {
- $key = 'phabricator.application-settings';
- $config_entry = PhabricatorConfigEntry::loadConfigEntry($key);
- $value = $config_entry->getValue();
-
- $phid = $application->getPHID();
- if (empty($value[$phid])) {
- $value[$application->getPHID()] = array();
- }
- if (empty($value[$phid]['policy'])) {
- $value[$phid]['policy'] = array();
+ $editor = id(new PhabricatorApplicationEditor())
+ ->setActor($user)
+ ->setContentSourceFromRequest($request)
+ ->setContinueOnNoEffect(true)
+ ->setContinueOnMissingFields(true);
+
+ try {
+ $editor->applyTransactions($application, $xactions);
+ return id(new AphrontRedirectResponse())->setURI($view_uri);
+ } catch (PhabricatorApplicationTransactionValidationException $ex) {
+ $validation_exception = $ex;
}
- $value[$phid]['policy'] = $result + $value[$phid]['policy'];
-
- // Don't allow users to make policy edits which would lock them out of
- // applications, since they would be unable to undo those actions.
- PhabricatorEnv::overrideConfig($key, $value);
- PhabricatorPolicyFilter::mustRetainCapability(
- $user,
- $application,
- PhabricatorPolicyCapability::CAN_VIEW);
-
- PhabricatorPolicyFilter::mustRetainCapability(
- $user,
- $application,
- PhabricatorPolicyCapability::CAN_EDIT);
-
- PhabricatorConfigEditor::storeNewValue(
- $user,
- $config_entry,
- $value,
- PhabricatorContentSource::newFromRequest($request));
+ return $this->newDialog()
+ ->setTitle('Validation Failed')
+ ->setValidationException($validation_exception)
+ ->addCancelButton($view_uri);
}
-
- return id(new AphrontRedirectResponse())->setURI($view_uri);
}
$descriptions = PhabricatorPolicyQuery::renderPolicyDescriptions(
diff --git a/src/applications/meta/editor/PhabricatorApplicationEditEngine.php b/src/applications/meta/editor/PhabricatorApplicationEditEngine.php
new file mode 100644
--- /dev/null
+++ b/src/applications/meta/editor/PhabricatorApplicationEditEngine.php
@@ -0,0 +1,64 @@
+<?php
+
+final class PhabricatorApplicationEditEngine
+ extends PhabricatorEditEngine {
+
+ const ENGINECONST = 'application.application';
+
+ public function getEngineApplicationClass() {
+ return 'PhabricatorApplicationsApplication';
+ }
+
+ public function getEngineName() {
+ return pht('Applications');
+ }
+
+ public function getSummaryHeader() {
+ return pht('Configure Application Forms');
+ }
+
+ public function getSummaryText() {
+ return pht('Configure creation and editing forms in Applications.');
+ }
+
+ public function isEngineConfigurable() {
+ return false;
+ }
+
+ protected function newEditableObject() {
+ throw new PhutilMethodNotImplementedException();
+ }
+
+ protected function newObjectQuery() {
+ return new PhabricatorApplicationQuery();
+ }
+
+ protected function getObjectCreateTitleText($object) {
+ return pht('Create New Application');
+ }
+
+ protected function getObjectEditTitleText($object) {
+ return pht('Edit Application: %s', $object->getName());
+ }
+
+ protected function getObjectEditShortText($object) {
+ return $object->getName();
+ }
+
+ protected function getObjectCreateShortText() {
+ return pht('Create Application');
+ }
+
+ protected function getObjectName() {
+ return pht('Application');
+ }
+
+ protected function getObjectViewURI($object) {
+ return $object->getViewURI();
+ }
+
+ protected function buildCustomEditFields($object) {
+ return array();
+ }
+
+}
diff --git a/src/applications/meta/editor/PhabricatorApplicationEditor.php b/src/applications/meta/editor/PhabricatorApplicationEditor.php
new file mode 100644
--- /dev/null
+++ b/src/applications/meta/editor/PhabricatorApplicationEditor.php
@@ -0,0 +1,46 @@
+<?php
+
+final class PhabricatorApplicationEditor
+ extends PhabricatorApplicationTransactionEditor {
+
+ public function getEditorApplicationClass() {
+ return 'PhabricatorApplicationsApplication';
+ }
+
+ public function getEditorObjectsDescription() {
+ return pht('Application');
+ }
+
+ protected function supportsSearch() {
+ return true;
+ }
+
+ public function getTransactionTypes() {
+ $types = parent::getTransactionTypes();
+ $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
+ $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY;
+
+ return $types;
+ }
+
+ protected function shouldSendMail(
+ PhabricatorLiskDAO $object,
+ array $xactions) {
+ return false;
+ }
+
+ protected function shouldPublishFeedStory(
+ PhabricatorLiskDAO $object,
+ array $xactions) {
+ return true;
+ }
+
+ protected function getMailTo(PhabricatorLiskDAO $object) {
+ return array();
+ }
+
+ protected function getMailCC(PhabricatorLiskDAO $object) {
+ return array();
+ }
+
+}
diff --git a/src/applications/meta/storage/PhabricatorApplicationApplicationTransaction.php b/src/applications/meta/storage/PhabricatorApplicationApplicationTransaction.php
--- a/src/applications/meta/storage/PhabricatorApplicationApplicationTransaction.php
+++ b/src/applications/meta/storage/PhabricatorApplicationApplicationTransaction.php
@@ -11,10 +11,6 @@
return PhabricatorApplicationApplicationPHIDType::TYPECONST;
}
- public function getApplicationTransactionCommentObject() {
- return new PhabricatorApplicationTransactionComment();
- }
-
public function getBaseTransactionClass() {
return 'PhabricatorApplicationTransactionType';
}
diff --git a/src/applications/meta/xactions/PhabricatorApplicationPolicyChangeTransaction.php b/src/applications/meta/xactions/PhabricatorApplicationPolicyChangeTransaction.php
new file mode 100644
--- /dev/null
+++ b/src/applications/meta/xactions/PhabricatorApplicationPolicyChangeTransaction.php
@@ -0,0 +1,197 @@
+<?php
+
+final class PhabricatorApplicationPolicyChangeTransaction
+ extends PhabricatorApplicationTransactionType {
+
+ const TRANSACTIONTYPE = 'application.policy';
+ const METADATA_ATTRIBUTE = 'capability.name';
+
+ private $policies;
+
+ public function generateOldValue($object) {
+ $application = $object;
+ $capability = $this->getCapabilityName();
+ return $application->getPolicy($capability);
+ }
+
+ public function applyInternalEffects($object, $value) {
+ $application = $object;
+ $user = $this->getActor();
+
+ $key = 'phabricator.application-settings';
+ $config_entry = PhabricatorConfigEntry::loadConfigEntry($key);
+ $current_value = $config_entry->getValue();
+
+ $phid = $application->getPHID();
+ if (empty($current_value[$phid])) {
+ $current_value[$application->getPHID()] = array();
+ }
+ if (empty($current_value[$phid]['policy'])) {
+ $current_value[$phid]['policy'] = array();
+ }
+
+ $new = array($this->getCapabilityName() => $value);
+ $current_value[$phid]['policy'] = $new + $current_value[$phid]['policy'];
+
+ $editor = $this->getEditor();
+ $content_source = $editor->getContentSource();
+ PhabricatorConfigEditor::storeNewValue(
+ $user,
+ $config_entry,
+ $current_value,
+ $content_source);
+ }
+
+ public function getTitle() {
+ $old = $this->renderPolicy($this->getOldValue());
+ $new = $this->renderPolicy($this->getNewValue());
+
+ return pht(
+ '%s changed the "%s" policy from "%s" to "%s".',
+ $this->renderAuthor(),
+ $this->renderCapability(),
+ $old,
+ $new);
+ }
+
+ public function getTitleForFeed() {
+ $old = $this->renderPolicy($this->getOldValue());
+ $new = $this->renderPolicy($this->getNewValue());
+
+ return pht(
+ '%s changed the "%s" policy for application %s from "%s" to "%s".',
+ $this->renderAuthor(),
+ $this->renderCapability(),
+ $this->renderObject(),
+ $old,
+ $new);
+ }
+
+ public function validateTransactions($object, array $xactions) {
+ $user = $this->getActor();
+ $application = $object;
+ $policies = id(new PhabricatorPolicyQuery())
+ ->setViewer($user)
+ ->setObject($application)
+ ->execute();
+
+ $errors = array();
+ foreach ($xactions as $xaction) {
+ $new = $xaction->getNewValue();
+ $capability = $xaction->getMetadataValue(self::METADATA_ATTRIBUTE);
+
+ if (empty($policies[$new])) {
+ // Not a standard policy, check for a custom policy.
+ $policy = id(new PhabricatorPolicyQuery())
+ ->setViewer($user)
+ ->withPHIDs(array($new))
+ ->executeOne();
+ if (!$policy) {
+ $errors[] = $this->newInvalidError(
+ pht('Policy does not exist.'));
+ continue;
+ }
+ } else {
+ $policy = idx($policies, $new);
+ }
+
+ if (!$policy->isValidPolicyForEdit()) {
+ $errors[] = $this->newInvalidError(
+ pht('Can\'t set the policy to a policy you can\'t view!'));
+ continue;
+ }
+
+ if ($new == PhabricatorPolicies::POLICY_PUBLIC) {
+ $capobj = PhabricatorPolicyCapability::getCapabilityByKey(
+ $capability);
+ if (!$capobj || !$capobj->shouldAllowPublicPolicySetting()) {
+ $errors[] = $this->newInvalidError(
+ pht('Can\'t set non-public policies to public.'));
+ continue;
+ }
+ }
+
+ if (!$application->isCapabilityEditable($capability)) {
+ $errors[] = $this->newInvalidError(
+ pht('Capability "%s" is not editable for this application.',
+ $capability));
+ continue;
+ }
+ }
+
+ // If we're changing these policies, the viewer needs to still be able to
+ // view or edit the application under the new policy.
+ $validate_map = array(
+ PhabricatorPolicyCapability::CAN_VIEW,
+ PhabricatorPolicyCapability::CAN_EDIT,
+ );
+ $validate_map = array_fill_keys($validate_map, array());
+
+ foreach ($xactions as $xaction) {
+ $capability = $xaction->getMetadataValue(self::METADATA_ATTRIBUTE);
+ if (!isset($validate_map[$capability])) {
+ continue;
+ }
+
+ $validate_map[$capability][] = $xaction;
+ }
+
+ foreach ($validate_map as $capability => $cap_xactions) {
+ if (!$cap_xactions) {
+ continue;
+ }
+
+ $editor = $this->getEditor();
+ $policy_errors = $editor->validatePolicyTransaction(
+ $object,
+ $cap_xactions,
+ self::TRANSACTIONTYPE,
+ $capability);
+
+ foreach ($policy_errors as $error) {
+ $errors[] = $error;
+ }
+ }
+
+ return $errors;
+ }
+
+ private function renderPolicy($name) {
+ $policies = $this->getAllPolicies();
+ if (empty($policies[$name])) {
+ // Not a standard policy, check for a custom policy.
+ $policy = id(new PhabricatorPolicyQuery())
+ ->setViewer($this->getViewer())
+ ->withPHIDs(array($name))
+ ->executeOne();
+ $policies[$name] = $policy;
+ }
+
+ $policy = idx($policies, $name);
+ return $this->renderValue($policy->getFullName());
+ }
+
+ private function getAllPolicies() {
+ if (!$this->policies) {
+ $viewer = $this->getViewer();
+ $application = $this->getObject();
+ $this->policies = id(new PhabricatorPolicyQuery())
+ ->setViewer($viewer)
+ ->setObject($application)
+ ->execute();
+ }
+
+ return $this->policies;
+ }
+
+ private function renderCapability() {
+ $application = $this->getObject();
+ $capability = $this->getCapabilityName();
+ return $application->getCapabilityLabel($capability);
+ }
+
+ private function getCapabilityName() {
+ return $this->getMetadataValue(self::METADATA_ATTRIBUTE);
+ }
+
+}
diff --git a/src/applications/policy/storage/PhabricatorPolicy.php b/src/applications/policy/storage/PhabricatorPolicy.php
--- a/src/applications/policy/storage/PhabricatorPolicy.php
+++ b/src/applications/policy/storage/PhabricatorPolicy.php
@@ -264,9 +264,11 @@
public function getFullName() {
switch ($this->getType()) {
case PhabricatorPolicyType::TYPE_PROJECT:
- return pht('Project: %s', $this->getName());
+ return pht('Members of Project: %s', $this->getName());
case PhabricatorPolicyType::TYPE_MASKED:
return pht('Other: %s', $this->getName());
+ case PhabricatorPolicyType::TYPE_USER:
+ return pht('Only User: %s', $this->getName());
default:
return $this->getName();
}
@@ -422,6 +424,10 @@
return ($this_strength > $other_strength);
}
+ public function isValidPolicyForEdit() {
+ return $this->getType() !== PhabricatorPolicyType::TYPE_MASKED;
+ }
+
public static function getSpecialRules(
PhabricatorPolicyInterface $object,
PhabricatorUser $viewer,
diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
--- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
@@ -334,6 +334,8 @@
$xtype = $this->getModularTransactionType($type);
if ($xtype) {
+ $xtype = clone $xtype;
+ $xtype->setStorage($xaction);
return $xtype->generateOldValue($object);
}
@@ -414,6 +416,8 @@
$xtype = $this->getModularTransactionType($type);
if ($xtype) {
+ $xtype = clone $xtype;
+ $xtype->setStorage($xaction);
return $xtype->generateNewValue($object, $xaction->getNewValue());
}
@@ -553,6 +557,8 @@
$xtype = $this->getModularTransactionType($type);
if ($xtype) {
+ $xtype = clone $xtype;
+ $xtype->setStorage($xaction);
return $xtype->applyInternalEffects($object, $xaction->getNewValue());
}
@@ -2163,7 +2169,7 @@
return array_mergev($errors);
}
- private function validatePolicyTransaction(
+ public function validatePolicyTransaction(
PhabricatorLiskDAO $object,
array $xactions,
$transaction_type,
@@ -2772,7 +2778,11 @@
}
if (!$has_support) {
- throw new Exception(pht('Capability not supported.'));
+ throw new Exception(
+ pht('The object being edited does not implement any standard '.
+ 'interfaces (like PhabricatorSubscribableInterface) which allow '.
+ 'CCs to be generated automatically. Override the "getMailCC()" '.
+ 'method and generate CCs explicitly.'));
}
return array_mergev($phids);
diff --git a/src/applications/transactions/storage/PhabricatorModularTransactionType.php b/src/applications/transactions/storage/PhabricatorModularTransactionType.php
--- a/src/applications/transactions/storage/PhabricatorModularTransactionType.php
+++ b/src/applications/transactions/storage/PhabricatorModularTransactionType.php
@@ -315,4 +315,8 @@
return $editor->getPHIDList($old, $new);
}
+ public function getMetadataValue($key, $default = null) {
+ return $this->getStorage()->getMetadataValue($key, $default);
+ }
+
}

File Metadata

Mime Type
text/plain
Expires
May 13 2024, 11:52 PM (4 w, 3 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6294201
Default Alt Text
D17757.diff (20 KB)

Event Timeline