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.
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