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 @@ -1847,9 +1847,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', @@ -3872,10 +3875,12 @@ 'PhabricatorSlowvoteApplication' => 'applications/slowvote/application/PhabricatorSlowvoteApplication.php', 'PhabricatorSlowvoteChoice' => 'applications/slowvote/storage/PhabricatorSlowvoteChoice.php', 'PhabricatorSlowvoteCloseController' => 'applications/slowvote/controller/PhabricatorSlowvoteCloseController.php', + 'PhabricatorSlowvoteCloseTransaction' => 'applications/slowvote/xactions/PhabricatorSlowvoteCloseTransaction.php', 'PhabricatorSlowvoteCommentController' => 'applications/slowvote/controller/PhabricatorSlowvoteCommentController.php', 'PhabricatorSlowvoteController' => 'applications/slowvote/controller/PhabricatorSlowvoteController.php', 'PhabricatorSlowvoteDAO' => 'applications/slowvote/storage/PhabricatorSlowvoteDAO.php', 'PhabricatorSlowvoteDefaultViewCapability' => 'applications/slowvote/capability/PhabricatorSlowvoteDefaultViewCapability.php', + 'PhabricatorSlowvoteDescriptionTransaction' => 'applications/slowvote/xactions/PhabricatorSlowvoteDescriptionTransaction.php', 'PhabricatorSlowvoteEditController' => 'applications/slowvote/controller/PhabricatorSlowvoteEditController.php', 'PhabricatorSlowvoteEditor' => 'applications/slowvote/editor/PhabricatorSlowvoteEditor.php', 'PhabricatorSlowvoteListController' => 'applications/slowvote/controller/PhabricatorSlowvoteListController.php', @@ -3885,12 +3890,16 @@ 'PhabricatorSlowvotePollController' => 'applications/slowvote/controller/PhabricatorSlowvotePollController.php', 'PhabricatorSlowvotePollPHIDType' => 'applications/slowvote/phid/PhabricatorSlowvotePollPHIDType.php', 'PhabricatorSlowvoteQuery' => 'applications/slowvote/query/PhabricatorSlowvoteQuery.php', + 'PhabricatorSlowvoteQuestionTransaction' => 'applications/slowvote/xactions/PhabricatorSlowvoteQuestionTransaction.php', 'PhabricatorSlowvoteReplyHandler' => 'applications/slowvote/mail/PhabricatorSlowvoteReplyHandler.php', + 'PhabricatorSlowvoteResponsesTransaction' => 'applications/slowvote/xactions/PhabricatorSlowvoteResponsesTransaction.php', 'PhabricatorSlowvoteSchemaSpec' => 'applications/slowvote/storage/PhabricatorSlowvoteSchemaSpec.php', 'PhabricatorSlowvoteSearchEngine' => 'applications/slowvote/query/PhabricatorSlowvoteSearchEngine.php', + 'PhabricatorSlowvoteShuffleTransaction' => 'applications/slowvote/xactions/PhabricatorSlowvoteShuffleTransaction.php', 'PhabricatorSlowvoteTransaction' => 'applications/slowvote/storage/PhabricatorSlowvoteTransaction.php', 'PhabricatorSlowvoteTransactionComment' => 'applications/slowvote/storage/PhabricatorSlowvoteTransactionComment.php', 'PhabricatorSlowvoteTransactionQuery' => 'applications/slowvote/query/PhabricatorSlowvoteTransactionQuery.php', + 'PhabricatorSlowvoteTransactionType' => 'applications/slowvote/xactions/PhabricatorSlowvoteTransactionType.php', 'PhabricatorSlowvoteVoteController' => 'applications/slowvote/controller/PhabricatorSlowvoteVoteController.php', 'PhabricatorSlug' => 'infrastructure/util/PhabricatorSlug.php', 'PhabricatorSlugTestCase' => 'infrastructure/util/__tests__/PhabricatorSlugTestCase.php', @@ -6848,9 +6857,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', @@ -9236,10 +9248,12 @@ 'PhabricatorSlowvoteApplication' => 'PhabricatorApplication', 'PhabricatorSlowvoteChoice' => 'PhabricatorSlowvoteDAO', 'PhabricatorSlowvoteCloseController' => 'PhabricatorSlowvoteController', + 'PhabricatorSlowvoteCloseTransaction' => 'PhabricatorSlowvoteTransactionType', 'PhabricatorSlowvoteCommentController' => 'PhabricatorSlowvoteController', 'PhabricatorSlowvoteController' => 'PhabricatorController', 'PhabricatorSlowvoteDAO' => 'PhabricatorLiskDAO', 'PhabricatorSlowvoteDefaultViewCapability' => 'PhabricatorPolicyCapability', + 'PhabricatorSlowvoteDescriptionTransaction' => 'PhabricatorSlowvoteTransactionType', 'PhabricatorSlowvoteEditController' => 'PhabricatorSlowvoteController', 'PhabricatorSlowvoteEditor' => 'PhabricatorApplicationTransactionEditor', 'PhabricatorSlowvoteListController' => 'PhabricatorSlowvoteController', @@ -9259,12 +9273,16 @@ 'PhabricatorSlowvotePollController' => 'PhabricatorSlowvoteController', 'PhabricatorSlowvotePollPHIDType' => 'PhabricatorPHIDType', 'PhabricatorSlowvoteQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', + 'PhabricatorSlowvoteQuestionTransaction' => 'PhabricatorSlowvoteTransactionType', 'PhabricatorSlowvoteReplyHandler' => 'PhabricatorApplicationTransactionReplyHandler', + 'PhabricatorSlowvoteResponsesTransaction' => 'PhabricatorSlowvoteTransactionType', 'PhabricatorSlowvoteSchemaSpec' => 'PhabricatorConfigSchemaSpec', 'PhabricatorSlowvoteSearchEngine' => 'PhabricatorApplicationSearchEngine', + 'PhabricatorSlowvoteShuffleTransaction' => 'PhabricatorSlowvoteTransactionType', 'PhabricatorSlowvoteTransaction' => 'PhabricatorApplicationTransaction', 'PhabricatorSlowvoteTransactionComment' => 'PhabricatorApplicationTransactionComment', 'PhabricatorSlowvoteTransactionQuery' => 'PhabricatorApplicationTransactionQuery', + 'PhabricatorSlowvoteTransactionType' => 'PhabricatorModularTransactionType', 'PhabricatorSlowvoteVoteController' => 'PhabricatorSlowvoteController', 'PhabricatorSlug' => 'Phobject', 'PhabricatorSlugTestCase' => 'PhabricatorTestCase', 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,77 +30,48 @@ ->execute(); if ($request->isFormPost()) { + $xactions = array(); + $result = array(); + $template = $application->getApplicationTransactionTemplate(); foreach ($application->getCapabilities() as $capability) { - $old = $application->getPolicy($capability); - $new = $request->getStr('policy:'.$capability); - - if ($old == $new) { - // No change to the setting. + if (!$application->isCapabilityEditable($capability)) { 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; - } - } + $old = $application->getPolicy($capability); + $new = $request->getStr('policy:'.$capability); $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 @@ +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 @@ +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(); + $old = $xaction->getOldValue(); + $capability = $xaction->getMetadataValue(self::METADATA_ATTRIBUTE); + + if ($old == $new) { + // No change to the setting. + 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) { + $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 @@ -322,6 +322,8 @@ $xtype = $this->getModularTransactionType($type); if ($xtype) { + $xtype = clone $xtype; + $xtype->setStorage($xaction); return $xtype->generateOldValue($object); } @@ -402,6 +404,8 @@ $xtype = $this->getModularTransactionType($type); if ($xtype) { + $xtype = clone $xtype; + $xtype->setStorage($xaction); return $xtype->generateNewValue($object, $xaction->getNewValue()); } @@ -541,6 +545,8 @@ $xtype = $this->getModularTransactionType($type); if ($xtype) { + $xtype = clone $xtype; + $xtype->setStorage($xaction); return $xtype->applyInternalEffects($object, $xaction->getNewValue()); } @@ -2151,7 +2157,7 @@ return array_mergev($errors); } - private function validatePolicyTransaction( + public function validatePolicyTransaction( PhabricatorLiskDAO $object, array $xactions, $transaction_type, @@ -2760,7 +2766,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); + } + }