Fixes T6734. This is a very generic fix, which basically attaches the subscribers if necessary. This seems like a good idea given there's some crazy generic code doing this sort of thing? This would end up being a new pattern for these types of objects that may be loaded by a general object query but then get some editor action against them.
Details
- Reviewers
epriestley - Maniphest Tasks
- T6734: PhabricatorDataNotAttachedException when awarding a token
- Commits
- Restricted Diffusion Commit
rPdda8e64a3e31: Maniphest - load subscribers in getApplicationTransactionsObject
I can't actually reproduce this in my sandbox so I'll verify live again.
Diff Detail
- Repository
- rP Phabricator
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
How about doing this in Herald instead? That would be more consistent with how Herald loads other types of data.
I feel like that's a bit cleaner than loading in getApplicationTransactionObject().
This error in T6734 is from PhabricatorTokenGivenEditor, in the publishTransaction method.
(AFAIK Herald is okay because it gets invoked via the ManiphestEditor now, and the ManiphestTask is properly loaded.)
Oh, hrrm -- this is the trace I'm seeing (definitely looks like a Herald issue?) -- do you have a different one?
I think to get this to repro, add a Herald rule which acts on subscribers, like this one:
I don't get this to reproduce with a herald rule:
I can give tokens to tasks with no errors regardless of whether xerxes is subscribed, not subscribed, unsubscribed, or subscribed with others.
(also I guess all this is sort of a hint at how this can go awry with projects conditionally loading eventually...)