Revan
Revan

Reputation: 2322

Phabricator: Adjust Herald rules to modify task policies based on project membership

I try to adapt the Herald rules of Phabricator, so the view and edit policy is set based on the project membership. Currently, it is not possible to adjust the view and edit policy on tasks based on projects in an automatic way.

I found this modification by the Haskell project but this does not seem to work anymore.

Can anybody tell me how to fix them?

<?php
/**
 * Extends Herald with a custom 'Set "Visible To" policy' action for Maniphest
 * tasks.
 */
final class SetTaskViewPolicyHeraldAction extends HeraldAction {

  public function appliesToAdapter(HeraldAdapter $adapter) {
    return $adapter instanceof HeraldManiphestTaskAdapter;
  }

  public function appliesToRuleType($type) {
    return $type == HeraldRuleTypeConfig::RULE_TYPE_GLOBAL;
  }

  public function getActionKey() {
    return 'custom:view-policy';
  }

  public function getActionName() {
    return 'Set view policy to project';
  }

  public function getActionType() {
    return HeraldAdapter::VALUE_PROJECT;
  }

  public function applyEffect(
    HeraldAdapter $adapter,
    $object,
    HeraldEffect $effect) {

    // First off, ensure there's only one set project
    if (count($effect->getTarget()) != 1) {
      throw new HeraldInvalidConditionException(
        'Expected only one project to be set for visibility policy');
    }

    $project = $effect->getTarget();
    $project_phid = $project[0];

    // Set new value by queueing a transaction, and returning the transcript.
    $adapter->queueTransaction(
      id(new ManiphestTransaction())
      ->setTransactionType(PhabricatorTransactions::TYPE_VIEW_POLICY)
      ->setNewValue($project_phid));

    return new HeraldApplyTranscript(
      $effect,
      true,
      pht('Set view policy of task'));
  }

}

I only added the source code for the old version of SetTaskViewPolicyHeraldAction.php, at this link there is also the file SetTaskEditPolicyHeraldAction.php, which is also related to my question.

Upvotes: 3

Views: 459

Answers (1)

Revan
Revan

Reputation: 2322

Found a solution finally, or more precisely written an extension.

<?php
final class HeraldManiphestMoveSpaceAction extends HeraldAction {
  // ACTIONCONST: internal ID, unique (assumably for mapping objects)
  const ACTIONCONST = 'space.move';
  // supposably key for mapping history entries
  const DO_MOVE_SPACE = 'do.move.space';
  // entry in Herald action selection drop down menu when configuring a rule
  public function getHeraldActionName() {
    return pht('Move to space');
  }
  // section in Herald action selection drop down menu
  public function getActionGroupKey() {
    return HeraldSupportActionGroup::ACTIONGROUPKEY;
  }
  // source for input field
  protected function getDatasource() {
    return new PhabricatorSpacesNamespaceDatasource();
  }
  // which UI element to show when configuring the action
  public function getHeraldActionStandardType() {
    return self::STANDARD_PHID_LIST;
  }
  // allowed applicable objects
  public function supportsObject($object) {
    return ($object instanceof PhabricatorProjectInterface);
  }
  // permitted user roles (globally or locally)
  public function supportsRuleType($rule_type) {
    return ($rule_type == HeraldRuleTypeConfig::RULE_TYPE_GLOBAL);
  }
  // appearance in transcript
  protected function getActionEffectMap() {
    return array(
      self::DO_MOVE_SPACE => array(
        'icon' => 'fa-diamond',
        'color' => 'green',
        'name' => pht('Moved to space'),
      ),
    );
  }
  // description of action that will be taken (present tense)
  public function renderActionDescription($value) {
    return pht('Move to space: %s.', $this->renderHandleList($value));
  }
  // description of action that has been taken (past tense, for history view etc.)
  protected function renderActionEffectDescription($type, $data) {
        switch ($type) {
      case self::DO_MOVE_SPACE:
        return pht(
          'Moved to %s space: %s.',
          phutil_count($data),
          $this->renderHandleList($data));
    }
  }
  // executed by Herald rules on objects that match condition (calls function applySpace)
  public function applyEffect($object, HeraldEffect $effect) {
    $current_space = array($object->getSpacePHID());
    // allowed objects for transaction
    $allowed_types = array(
      PhabricatorSpacesNamespacePHIDType::TYPECONST,
    );
    // loadStandardTargets() figures out the to-set spaces from the Phabricator IDs ($phids)
    // and excludes $current_space from this list, potentially resulting in an empty list (NULL).
    // Misconfigured Herald action may result in an empty $phids.
    $new_phids = $effect->getTarget();
    $new_spaces = $this->loadStandardTargets($new_phids, $allowed_types, $current_space);
    // if no spaces need to be set (either because of bad rule (see above comment), or space already manually set), avoid doing work
    if(!$new_spaces) {
      return;
    } else {
      // One object can only be at one space at a time. This silently fixes if one misconfigured Herald rule tries to move one object into different spaces. 
      $phid = head_key($new_spaces);
      $adapter = $this->getAdapter();
      $xaction = $adapter->newTransaction()
        ->setTransactionType(PhabricatorTransactions::TYPE_SPACE)
        ->setNewValue($phid);
      $adapter->queueTransaction($xaction);
      $this->logEffect(self::DO_MOVE_SPACE, array($phid));
    }
  }
}

A few things should be noted here (also documented in the Github repo of this extension). I use this extension mainly to shift tasks automatically in corresponding spaces (based on project management), but it should also work on other objects (all that can have a space) - but I have not tested it.

A problem is, that Herald does not understand (yet) that objects can only have one space - if multiple Herald rules apply, they will all be executed and the one that was last created will decide about the object's space. As a workaround, there is also a patch in the Github repo which creates a Herald rule "is exactly".

diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php
index 78ce86294..600033247 100644
--- a/src/applications/herald/adapter/HeraldAdapter.php
+++ b/src/applications/herald/adapter/HeraldAdapter.php
@@ -10,6 +10,7 @@ abstract class HeraldAdapter extends Phobject {
   const CONDITION_IS_NOT_ANY      = '!isany';
   const CONDITION_INCLUDE_ALL     = 'all';
   const CONDITION_INCLUDE_ANY     = 'any';
+  const CONDITION_INCLUDE_EXACTLY = 'exactly';
   const CONDITION_INCLUDE_NONE    = 'none';
   const CONDITION_IS_ME           = 'me';
   const CONDITION_IS_NOT_ME       = '!me';
@@ -335,6 +336,7 @@ abstract class HeraldAdapter extends Phobject {
       self::CONDITION_IS_NOT_ANY      => pht('is not any of'),
       self::CONDITION_INCLUDE_ALL     => pht('include all of'),
       self::CONDITION_INCLUDE_ANY     => pht('include any of'),
+      self::CONDITION_INCLUDE_EXACTLY => pht('is exactly'), // custom adaptation for projects
       self::CONDITION_INCLUDE_NONE    => pht('do not include'),
       self::CONDITION_IS_ME           => pht('is myself'),
       self::CONDITION_IS_NOT_ME       => pht('is not myself'),
@@ -432,6 +434,19 @@ abstract class HeraldAdapter extends Phobject {
         return (bool)array_select_keys(
           array_fuse($field_value),
           $condition_value);
+      case self::CONDITION_INCLUDE_EXACTLY:
+        if (!is_array($field_value)) {
+          throw new HeraldInvalidConditionException(
+            pht('Object produced non-array value!'));
+        }
+        if (!is_array($condition_value)) {
+          throw new HeraldInvalidConditionException(
+            pht('Expected condition value to be an array.'));
+        }
+
+        $have = array_select_keys(array_fuse($field_value), $condition_value);
+        return (count($have) == count($condition_value) &&
+                count($have) == count(array_fuse($field_value)));
       case self::CONDITION_INCLUDE_NONE:
         return !array_select_keys(
           array_fuse($field_value),
@@ -592,6 +607,7 @@ abstract class HeraldAdapter extends Phobject {
       case self::CONDITION_IS_NOT_ANY:
       case self::CONDITION_INCLUDE_ALL:
       case self::CONDITION_INCLUDE_ANY:
+      case self::CONDITION_INCLUDE_EXACTLY:
       case self::CONDITION_INCLUDE_NONE:
       case self::CONDITION_IS_ME:
       case self::CONDITION_IS_NOT_ME:
diff --git a/src/applications/herald/field/HeraldField.php b/src/applications/herald/field/HeraldField.php
index 2abed0ff1..8bbcfa3dd 100644
--- a/src/applications/herald/field/HeraldField.php
+++ b/src/applications/herald/field/HeraldField.php
@@ -58,6 +58,7 @@ abstract class HeraldField extends Phobject {
         return array(
           HeraldAdapter::CONDITION_INCLUDE_ALL,
           HeraldAdapter::CONDITION_INCLUDE_ANY,
+          HeraldAdapter::CONDITION_INCLUDE_EXACTLY,
           HeraldAdapter::CONDITION_INCLUDE_NONE,
           HeraldAdapter::CONDITION_EXISTS,
           HeraldAdapter::CONDITION_NOT_EXISTS,
diff --git a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldPHIDs.php b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldPHIDs.php
index 78c8caa5a..be267d2e4 100644
--- a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldPHIDs.php
+++ b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldPHIDs.php
@@ -235,6 +235,7 @@ abstract class PhabricatorStandardCustomFieldPHIDs
     return array(
       HeraldAdapter::CONDITION_INCLUDE_ALL,
       HeraldAdapter::CONDITION_INCLUDE_ANY,
+      HeraldAdapter::CONDITION_INCLUDE_EXACTLY,
       HeraldAdapter::CONDITION_INCLUDE_NONE,
       HeraldAdapter::CONDITION_EXISTS,
       HeraldAdapter::CONDITION_NOT_EXISTS,

Look at the README.md in the Github repo for further details.

Upvotes: 2

Related Questions