diff --git a/resources/celerity/map.php b/resources/celerity/map.php index ec97b2b5cc..50c2597e36 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -87,6 +87,7 @@ return array( 'rsrc/css/application/phrequent/phrequent.css' => 'ffc185ad', 'rsrc/css/application/phriction/phriction-document-css.css' => '7d7f0071', 'rsrc/css/application/policy/policy-edit.css' => '05cca26a', + 'rsrc/css/application/policy/policy-transaction-detail.css' => '21b7daba', 'rsrc/css/application/policy/policy.css' => '957ea14c', 'rsrc/css/application/ponder/comments.css' => '6cdccea7', 'rsrc/css/application/ponder/feed.css' => 'e62615b6', @@ -771,6 +772,7 @@ return array( 'phui-workpanel-view-css' => '97b69459', 'policy-css' => '957ea14c', 'policy-edit-css' => '05cca26a', + 'policy-transaction-detail-css' => '21b7daba', 'ponder-comment-table-css' => '6cdccea7', 'ponder-feed-view-css' => 'e62615b6', 'ponder-post-css' => 'ebab8a70', diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index f5d4693452..f0b45d8c40 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1176,6 +1176,7 @@ phutil_register_library_map(array( 'PhabricatorApplicationTransactionTextDiffDetailView' => 'applications/transactions/view/PhabricatorApplicationTransactionTextDiffDetailView.php', 'PhabricatorApplicationTransactionValidationError' => 'applications/transactions/error/PhabricatorApplicationTransactionValidationError.php', 'PhabricatorApplicationTransactionValidationException' => 'applications/transactions/exception/PhabricatorApplicationTransactionValidationException.php', + 'PhabricatorApplicationTransactionValueController' => 'applications/transactions/controller/PhabricatorApplicationTransactionValueController.php', 'PhabricatorApplicationTransactionView' => 'applications/transactions/view/PhabricatorApplicationTransactionView.php', 'PhabricatorApplicationTransactions' => 'applications/transactions/application/PhabricatorApplicationTransactions.php', 'PhabricatorApplicationTypeahead' => 'applications/typeahead/application/PhabricatorApplicationTypeahead.php', @@ -3917,6 +3918,7 @@ phutil_register_library_map(array( 'PhabricatorApplicationTransactionTextDiffDetailView' => 'AphrontView', 'PhabricatorApplicationTransactionValidationError' => 'Phobject', 'PhabricatorApplicationTransactionValidationException' => 'Exception', + 'PhabricatorApplicationTransactionValueController' => 'PhabricatorApplicationTransactionController', 'PhabricatorApplicationTransactionView' => 'AphrontView', 'PhabricatorApplicationTransactions' => 'PhabricatorApplication', 'PhabricatorApplicationTypeahead' => 'PhabricatorApplication', diff --git a/src/applications/policy/rule/PhabricatorPolicyRule.php b/src/applications/policy/rule/PhabricatorPolicyRule.php index ea663499c8..6540d797df 100644 --- a/src/applications/policy/rule/PhabricatorPolicyRule.php +++ b/src/applications/policy/rule/PhabricatorPolicyRule.php @@ -34,6 +34,28 @@ abstract class PhabricatorPolicyRule { return $value; } + public function getRequiredHandlePHIDsForSummary($value) { + $phids = array(); + switch ($this->getValueControlType()) { + case self::CONTROL_TYPE_TOKENIZER: + $phids = $value; + break; + case self::CONTROL_TYPE_TEXT: + case self::CONTROL_TYPE_SELECT: + case self::CONTROL_TYPE_NONE: + default: + if (phid_get_type($value) != + PhabricatorPHIDConstants::PHID_TYPE_UNKNOWN) { + $phids = array($value); + } else { + $phids = array(); + } + break; + } + + return $phids; + } + /** * Return true if the given value creates a rule with a meaningful effect. * An example of a rule with no meaningful effect is a "users" rule with no diff --git a/src/applications/policy/storage/PhabricatorPolicy.php b/src/applications/policy/storage/PhabricatorPolicy.php index f52ab4ecbc..c32bb450a5 100644 --- a/src/applications/policy/storage/PhabricatorPolicy.php +++ b/src/applications/policy/storage/PhabricatorPolicy.php @@ -11,6 +11,7 @@ final class PhabricatorPolicy private $shortName; private $type; private $href; + private $workflow; private $icon; protected $rules = array(); @@ -132,6 +133,15 @@ final class PhabricatorPolicy return $this->href; } + public function setWorkflow($workflow) { + $this->workflow = $workflow; + return $this; + } + + public function getWorkflow() { + return $this->workflow; + } + public function getIcon() { switch ($this->getType()) { case PhabricatorPolicyType::TYPE_GLOBAL: @@ -234,11 +244,12 @@ final class PhabricatorPolicy } if ($this->getHref()) { - $desc = phutil_tag( + $desc = javelin_tag( 'a', array( 'href' => $this->getHref(), 'class' => 'policy-link', + 'sigil' => $this->getWorkflow() ? 'workflow' : null, ), array( $img, @@ -256,7 +267,7 @@ final class PhabricatorPolicy case PhabricatorPolicyType::TYPE_PROJECT: return pht('%s (Project)', $desc); case PhabricatorPolicyType::TYPE_CUSTOM: - return pht('Custom Policy'); + return $desc; case PhabricatorPolicyType::TYPE_MASKED: return pht( '%s (You do not have permission to view policy details.)', diff --git a/src/applications/transactions/application/PhabricatorApplicationTransactions.php b/src/applications/transactions/application/PhabricatorApplicationTransactions.php index e91cb3dc80..72e3311bfb 100644 --- a/src/applications/transactions/application/PhabricatorApplicationTransactions.php +++ b/src/applications/transactions/application/PhabricatorApplicationTransactions.php @@ -19,6 +19,8 @@ final class PhabricatorApplicationTransactions extends PhabricatorApplication { => 'PhabricatorApplicationTransactionCommentHistoryController', 'detail/(?[^/]+)/' => 'PhabricatorApplicationTransactionDetailController', + '(?Pold|new)/(?[^/]+)/' + => 'PhabricatorApplicationTransactionValueController', ), ); } diff --git a/src/applications/transactions/controller/PhabricatorApplicationTransactionController.php b/src/applications/transactions/controller/PhabricatorApplicationTransactionController.php index 7e797c7f06..0f37e4e181 100644 --- a/src/applications/transactions/controller/PhabricatorApplicationTransactionController.php +++ b/src/applications/transactions/controller/PhabricatorApplicationTransactionController.php @@ -3,4 +3,25 @@ abstract class PhabricatorApplicationTransactionController extends PhabricatorController { + protected function guessCancelURI( + PhabricatorUser $viewer, + PhabricatorApplicationTransaction $xaction) { + + // Take an educated guess at the URI where the transactions appear so we + // can send the cancel button somewhere sensible. This won't always get the + // best answer (for example, Diffusion's history is visible on a page other + // than the main object view page) but should always get a reasonable one. + + $cancel_uri = '/'; + $handle = id(new PhabricatorHandleQuery()) + ->setViewer($viewer) + ->withPHIDs(array($xaction->getObjectPHID())) + ->executeOne(); + if ($handle) { + $cancel_uri = $handle->getURI(); + } + + return $cancel_uri; + } + } diff --git a/src/applications/transactions/controller/PhabricatorApplicationTransactionDetailController.php b/src/applications/transactions/controller/PhabricatorApplicationTransactionDetailController.php index f345866d20..6b3a657470 100644 --- a/src/applications/transactions/controller/PhabricatorApplicationTransactionDetailController.php +++ b/src/applications/transactions/controller/PhabricatorApplicationTransactionDetailController.php @@ -23,20 +23,7 @@ final class PhabricatorApplicationTransactionDetailController $details = $xaction->renderChangeDetails($viewer); - // Take an educated guess at the URI where the transactions appear so we - // can send the cancel button somewhere sensible. This won't always get the - // best answer (for example, Diffusion's history is visible on a page other - // than the main object view page) but should always get a reasonable one. - - $cancel_uri = '/'; - $handle = id(new PhabricatorHandleQuery()) - ->setViewer($viewer) - ->withPHIDs(array($xaction->getObjectPHID())) - ->executeOne(); - if ($handle) { - $cancel_uri = $handle->getURI(); - } - + $cancel_uri = $this->guessCancelURI($viewer, $xaction); $dialog = id(new AphrontDialogView()) ->setUser($viewer) ->setTitle(pht('Change Details')) diff --git a/src/applications/transactions/controller/PhabricatorApplicationTransactionValueController.php b/src/applications/transactions/controller/PhabricatorApplicationTransactionValueController.php new file mode 100644 index 0000000000..6f40dc9c65 --- /dev/null +++ b/src/applications/transactions/controller/PhabricatorApplicationTransactionValueController.php @@ -0,0 +1,141 @@ +phid = $data['phid']; + $this->value = $data['value']; + } + + public function processRequest() { + $request = $this->getRequest(); + $viewer = $request->getUser(); + + $xaction = id(new PhabricatorObjectQuery()) + ->setViewer($viewer) + ->withPHIDs(array($this->phid)) + ->executeOne(); + if (!$xaction) { + return new Aphront404Response(); + } + + // For now, this pathway only supports policy transactions + // to show the details of custom policies. If / when this pathway + // supports more transaction types, rendering coding should be moved + // into PhabricatorTransactions e.g. feed rendering code. + switch ($xaction->getTransactionType()) { + case PhabricatorTransactions::TYPE_VIEW_POLICY: + case PhabricatorTransactions::TYPE_EDIT_POLICY: + case PhabricatorTransactions::TYPE_JOIN_POLICY: + break; + default: + return new Aphront404Response(); + break; + } + + if ($this->value == 'old') { + $value = $xaction->getOldValue(); + } else { + $value = $xaction->getNewValue(); + } + $policy = id(new PhabricatorPolicyQuery()) + ->setViewer($viewer) + ->withPHIDs(array($value)) + ->executeOne(); + if (!$policy) { + return new Aphront404Response(); + } + if ($policy->getType() != PhabricatorPolicyType::TYPE_CUSTOM) { + return new Aphront404Response(); + } + + $rule_objects = array(); + foreach ($policy->getCustomRuleClasses() as $class) { + $rule_objects[$class] = newv($class, array()); + } + $policy->attachRuleObjects($rule_objects); + $handle_phids = $this->extractPHIDs($policy, $rule_objects); + $handles = $this->loadHandles($handle_phids); + + $this->requireResource('policy-transaction-detail-css'); + $cancel_uri = $this->guessCancelURI($viewer, $xaction); + $dialog = id(new AphrontDialogView()) + ->setUser($viewer) + ->setTitle($policy->getFullName()) + ->setWidth(AphrontDialogView::WIDTH_FORM) + ->appendChild( + $this->renderPolicyDetails($policy, $rule_objects)) + ->addCancelButton($cancel_uri, pht('Close')); + + return id(new AphrontDialogResponse())->setDialog($dialog); + } + + private function extractPHIDs( + PhabricatorPolicy $policy, + array $rule_objects) { + + $phids = array(); + foreach ($policy->getRules() as $rule) { + $rule_object = $rule_objects[$rule['rule']]; + $phids[] = + $rule_object->getRequiredHandlePHIDsForSummary($rule['value']); + } + return array_filter(array_mergev($phids)); + } + + private function renderPolicyDetails( + PhabricatorPolicy $policy, + array $rule_objects) { + $details = array(); + $details[] = phutil_tag( + 'p', + array( + 'class' => 'policy-transaction-detail-intro' + ), + pht('These rules are processed in order:')); + + foreach ($policy->getRules() as $index => $rule) { + $rule_object = $rule_objects[$rule['rule']]; + if ($rule['action'] == 'allow') { + $icon = 'fa-check-circle green'; + } else { + $icon = 'fa-minus-circle red'; + } + $icon = id(new PHUIIconView()) + ->setIconFont($icon) + ->setText( + ucfirst($rule['action']).' '.$rule_object->getRuleDescription()); + $handle_phids = + $rule_object->getRequiredHandlePHIDsForSummary($rule['value']); + if ($handle_phids) { + $value = $this->renderHandlesForPHIDs($handle_phids, ','); + } else { + $value = $rule['value']; + } + $details[] = phutil_tag('div', + array( + 'class' => 'policy-transaction-detail-row' + ), + array( + $icon, + $value)); + } + + $details[] = phutil_tag( + 'p', + array( + 'class' => 'policy-transaction-detail-end' + ), + pht( + 'If no rules match, %s all other users.', + phutil_tag('b', + array(), + $policy->getDefaultAction()))); + return $details; + } + +} diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php index 4654df287a..0e8236091a 100644 --- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php +++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php @@ -311,11 +311,19 @@ abstract class PhabricatorApplicationTransaction } } - public function renderPolicyName($phid) { + private function renderPolicyName($phid, $state = 'old') { $policy = PhabricatorPolicy::newFromPolicyAndHandle( $phid, $this->getHandleIfExists($phid)); if ($this->renderingTarget == self::TARGET_HTML) { + switch ($policy->getType()) { + case PhabricatorPolicyType::TYPE_CUSTOM: + $policy->setHref('/transactions/'.$state.'/'.$this->getPHID().'/'); + $policy->setWorkflow(true); + break; + default: + break; + } $output = $policy->renderDescription(); } else { $output = hsprintf('%s', $policy->getFullName()); @@ -504,22 +512,22 @@ abstract class PhabricatorApplicationTransaction '%s changed the visibility of this %s from "%s" to "%s".', $this->renderHandleLink($author_phid), $this->getApplicationObjectTypeName(), - $this->renderPolicyName($old), - $this->renderPolicyName($new)); + $this->renderPolicyName($old, 'old'), + $this->renderPolicyName($new, 'new')); case PhabricatorTransactions::TYPE_EDIT_POLICY: return pht( '%s changed the edit policy of this %s from "%s" to "%s".', $this->renderHandleLink($author_phid), $this->getApplicationObjectTypeName(), - $this->renderPolicyName($old), - $this->renderPolicyName($new)); + $this->renderPolicyName($old, 'old'), + $this->renderPolicyName($new, 'new')); case PhabricatorTransactions::TYPE_JOIN_POLICY: return pht( '%s changed the join policy of this %s from "%s" to "%s".', $this->renderHandleLink($author_phid), $this->getApplicationObjectTypeName(), - $this->renderPolicyName($old), - $this->renderPolicyName($new)); + $this->renderPolicyName($old, 'old'), + $this->renderPolicyName($new, 'new')); case PhabricatorTransactions::TYPE_SUBSCRIBERS: $add = array_diff($new, $old); $rem = array_diff($old, $new); diff --git a/webroot/rsrc/css/application/policy/policy-transaction-detail.css b/webroot/rsrc/css/application/policy/policy-transaction-detail.css new file mode 100644 index 0000000000..1715ebe5e4 --- /dev/null +++ b/webroot/rsrc/css/application/policy/policy-transaction-detail.css @@ -0,0 +1,19 @@ +/** + * @provides policy-transaction-detail-css + */ + +.policy-transaction-detail-intro { + margin-bottom: 12px; +} + +.policy-transaction-detail-row { + margin: 4px 0px 4px 8px; +} + +.policy-transaction-detail-row .phui-icon-view { + padding-right: 4px; +} + +.policy-transaction-detail-end { + margin-top: 12px; +}