Hi,

I am trying to add a custom herald action to my firm's phabricator, since what we would like to implement isn't in the herald built-in conditions. But it appears we cannot create a custom task which rejects a push.

The point of this is to reject a push if someone has too much code reviews waiting for him, for example, in order not to let him pile up too much reviews, and block someone else from the firm.

For a testing purpose, I tried to duplicate the built-in "block push with message" action (phabricator/src/applications/diffusion/herald/DiffusionBlockHeraldAction.php), changed its name, and added it to the /src/extensions directory.
Here, an error displayed on my phabricator :

Two objects (of classes "CustomHeraldAction" and "DiffusionBlockHeraldAction", descendants of ancestor class "HeraldAction") returned the same key from "getActionConstant()" ("diffusion.block"), but each object in this class map must be identified by a unique key.

Apparently, 2 actions cannot have the same ACTIONCONST (ok, why not).

I looked up where the push reject for the built-in method was done, and found this starting at line 320 (https://github.com/phacility/phabricator/blob/master/src/applications/diffusion/engine/DiffusionCommitHookEngine.php) :

$block_action = DiffusionBlockHeraldAction::ACTIONCONST;
if ($blocking_effect === null) {
  foreach ($effects as $effect) {
    if ($effect->getAction() == $block_action) {
      $blocking_effect = $effect;
      $blocked_update = $update;
      $blocking_xscript = $xscript;
      break;
    }
  }
}

The push reject only seems to trigger when the $block_action is "diffusion.block", defined in DiffusionBlockHeraldAction.php.

What do you think would be the best way to bypass this problem, and being able to create a custom action which would reject a push ?
I thought about several ways of going, so that it could be pushed to the up-stream from your side :

• Instead of defining the $block_action this way : $block_action = DiffusionBlockHeraldAction::ACTIONCONNT; why not do a test on the syntax of the different ACTIONCONST that could be defined : For example, : if ($effect->getAction() starts with diffusion.block), then do the block.

• Allow several actions to have the same ACTIONCONNT (I didn't study this one, I do not know if it is possible at all. Please explain me why this uniqueness way of going if you have time).

• Maybe return the result of another action from a custom action ? If (my condition in my custom action is true) then do built-in action "Block push with message".

Please tell me if I am not clear enough.

Thank you for your help, and keep up for good work,

Yann