Policy Transactions - add a details view for custom policy

Summary: 'cuz those can be complicated. Fixes T4738. I needed to do a fair amount of heavy lifting to get the policy stuff rendering correctly. For now, I made this end point very one purpose and tried to make that clear. Test Plan: looked at some custom policies. see screenshots. Reviewers: chad, epriestley Reviewed By: epriestley Subscribers: epriestley, Korvin Maniphest Tasks: T4738 Differential Revision: https://secure.phabricator.com/D8890
2024-11-22 14:52:41 +01:00 · 2014-04-29 09:42:54 -07:00 · 2014-04-29 09:42:54 -07:00 · 94a2cfbe44
commit 94a2cfbe44
parent 3bc2db199a
10 changed files with 238 additions and 23 deletions
--- 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',
--- 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',
--- 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
--- 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.)',
--- 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/(?<phid>[^/]+)/'
          => 'PhabricatorApplicationTransactionDetailController',
+        '(?P<value>old|new)/(?<phid>[^/]+)/'
+          => 'PhabricatorApplicationTransactionValueController',
      ),
    );
  }
--- 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;
+  }
+
 }
--- 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'))
--- a/src/applications/transactions/controller/PhabricatorApplicationTransactionValueController.php
+++ 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;
+  }
+
+}
--- 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);
--- a/webroot/rsrc/css/application/policy/policy-transaction-detail.css
+++ 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;
+}