Page MenuHomePhabricator

Herald condition: count of project tags
Closed, InvalidPublic

Description

Is there a way to write a Herald rule that applies if a Maniphest task has no project tags or more than one project tags? Or is there a "IS EXACTLY" condition?

If no project tag is set, I want to assign the task to a specific user, and if more than one project tag is set, I would like to notify myself by email, because most tasks should only have exactly one project tag and if this is not the case I would like to get notified (as long as I am allowed to see the task).

Event Timeline

chad added a subscriber: chad.

Please read and follow Contributing Feature Requests.

Ok, read that. Here is a breakdown of our wishes and what we did to implement them:

Target Audience
Users/institutes with many small projects, distributed over multiple mutually exclusive Herald Spaces

Expectation
We have written a small patch (attached), we would appreciate feedback on if this is the "correct" way of handling this, and what (if any) things would be required to land this upstream

Describe Problems
We have hundreds of issues that we distribute over multiple spaces. Not all our developers have access to all spaces, and some tasks are sensitive enough that they should not be visible to all people. We use the "Spaces" application to handle this.
We operate in an medical environment where sensitive data cannot be posted in the default S1.
To automate the Space-assignments, we have written multiple Herald rules. (We have a homebrew extension: HeraldAction "move to space" that we can share if there is interest, we are currently even working on unit tests for it)

The problem arises when a Task is in multiple projects (semirare, but it happens).
In this case, multiple Herald rules apply, and move the task BOTH from S1:default into S2:projectA AND from S1:default into S3:ProjectB. Of course only one space ends up "winning". If A or B wins seems non-deterministic (or the logic escapes us).

Root problem: Since we have dozens of spaces, writing rules to be mutually exclusive runs into the combinatorial explosion.
It is unfeasible to write all the rules "If project includes any of Project A and project includes none of B,C,D,E,F,G,H,I.......Z". This becomes a nightmare when a new project is added (as happens regularly)
We would appreciate upstream support in this.

Current In-house solution
We have extended the Phabricator HeraldEngine/HeraldAdaptor to include a new condition type: "Is exactly".
This functions like the "is" but works on lists-vs-lists (where "is" works only for single-item to single-item, e.g. Author or Assignee).
With this new condition, we can check if the Projects List is exactly "A,B", and it will not match if it is "A,B,C" (contrary to "include all of", "include any of")

Is this new condition something Phabricator would consider adopting? (diff below)
The condition logic is not as easily extendable as HeraldActions are, so we would appreciate not carrying the patch, and we believe the condition could be useful in other situations as well.

diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php
index 78ce862..6000332 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 2abed0f..8bbcfa3 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 78c8caa..be267d2 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,

Nuance is our plan for routing tasks, we don't anticipate needing this functionality in Herald.