Page MenuHomePhabricator

D8890.diff
No OneTemporary

D8890.diff

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -87,6 +87,7 @@
'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 @@
'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
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -1176,6 +1176,7 @@
'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 @@
'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
--- a/src/applications/policy/rule/PhabricatorPolicyRule.php
+++ b/src/applications/policy/rule/PhabricatorPolicyRule.php
@@ -34,6 +34,28 @@
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
--- a/src/applications/policy/storage/PhabricatorPolicy.php
+++ b/src/applications/policy/storage/PhabricatorPolicy.php
@@ -11,6 +11,7 @@
private $shortName;
private $type;
private $href;
+ private $workflow;
private $icon;
protected $rules = array();
@@ -132,6 +133,15 @@
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 @@
}
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 @@
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
--- a/src/applications/transactions/application/PhabricatorApplicationTransactions.php
+++ b/src/applications/transactions/application/PhabricatorApplicationTransactions.php
@@ -19,6 +19,8 @@
=> 'PhabricatorApplicationTransactionCommentHistoryController',
'detail/(?<phid>[^/]+)/'
=> 'PhabricatorApplicationTransactionDetailController',
+ '(?P<value>old|new)/(?<phid>[^/]+)/'
+ => 'PhabricatorApplicationTransactionValueController',
),
);
}
diff --git a/src/applications/transactions/controller/PhabricatorApplicationTransactionController.php b/src/applications/transactions/controller/PhabricatorApplicationTransactionController.php
--- 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
--- a/src/applications/transactions/controller/PhabricatorApplicationTransactionDetailController.php
+++ b/src/applications/transactions/controller/PhabricatorApplicationTransactionDetailController.php
@@ -23,20 +23,7 @@
$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
--- /dev/null
+++ b/src/applications/transactions/controller/PhabricatorApplicationTransactionValueController.php
@@ -0,0 +1,141 @@
+<?php
+
+final class PhabricatorApplicationTransactionValueController
+ extends PhabricatorApplicationTransactionController {
+
+ private $value;
+ private $phid;
+
+ public function willProcessRequest(array $data) {
+ $this->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
--- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
+++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
@@ -311,11 +311,19 @@
}
}
- 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 @@
'%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
--- /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;
+}

File Metadata

Mime Type
text/plain
Expires
Sat, Nov 23, 2:05 AM (37 m, 46 s)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6776779
Default Alt Text
D8890.diff (15 KB)

Event Timeline