recaptime.dev / phorge
@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Conduit: Print useful error when setting non-existing object subscribers

Summary:
Passing bogus values to the Conduit endpoints 'subscribers.remove', 'subscribers.add', 'subscribers.set', do not silently fail or do not emit confusing `Edges are not available for objects of type '????'!`, but throw a useful error message on empty/invalid values like
`"ERR-CONDUIT-CORE: <maniphest.edit> Subscriber transactions must be existing PHIDs or usernames (found key \"nonsense\" on transaction of type \"core:subscribers\")."`

Closes T16311

Test Plan:
* Edit the subscribers of an existing task in the web UI, e.g. add/remove existing users and projects
* Run a bunch of API calls passing valid and invalid usernames/project tags and see that they now fail with informative error output telling you which value is invalid, e.g.
```
echo '{"transactions": [{"type": "subscribers.add", "value":["user1","user2","someExistingProjectName"]}],"objectIdentifier":"T3955"}' | /var/www/html/phorge/arcanist/bin/arc call-conduit --conduit-uri http://phorge.localhost --conduit-token "cli-xxx" maniphest.edit --

echo '{"transactions": [{"type": "subscribers.add", "value":["PHID-xxxinvalid","user1",""]},{"type": "subscribers.remove", "value":["user4","usernonexisting"]}],"objectIdentifier":"T3955"}' | /var/www/html/phorge/arcanist/bin/arc call-conduit --conduit-uri http://phorge.localhost --conduit-token "cli-xxx" maniphest.edit --

echo '{"transactions": [{"type": "subscribers.add", "value":["user9"]},{"type": "subscribers.remove", "value":["user3","usernonexisting"]}],"objectIdentifier":"T3955"}' | /var/www/html/phorge/arcanist/bin/arc call-conduit --conduit-uri http://phorge.localhost --conduit-token "cli-xxx" maniphest.edit --

echo '{"transactions": [{"type": "subscribers.add", "value":["user9"]},{"type": "subscribers.remove", "value":["user3"]}],"objectIdentifier":"T3955"}' | /var/www/html/phorge/arcanist/bin/arc call-conduit --conduit-uri http://phorge.localhost --conduit-token "cli-xxx" maniphest.edit --
```

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T16311

Differential Revision: https://we.phorge.it/D26696

Andre Klapper babeed68 83813e21

+14
+14
src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
··· 602 602 case PhabricatorTransactions::TYPE_COMMENT: 603 603 return null; 604 604 case PhabricatorTransactions::TYPE_SUBSCRIBERS: 605 + $new_value = $xaction->getNewValue(); 606 + foreach ($new_value as $subscribers) { 607 + foreach ($subscribers as $key => $subscriber) { 608 + if (phid_get_type($key) === 609 + PhabricatorPHIDConstants::PHID_TYPE_UNKNOWN) { 610 + throw new Exception( 611 + pht( 612 + 'Subscriber transactions must be existing PHIDs or '. 613 + 'usernames (found key "%s" on transaction of type "%s").', 614 + $key, 615 + $type)); 616 + } 617 + } 618 + } 605 619 return $this->getPHIDTransactionNewValue($xaction); 606 620 case PhabricatorTransactions::TYPE_VIEW_POLICY: 607 621 case PhabricatorTransactions::TYPE_EDIT_POLICY: