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
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
Diff Detail
- Repository
- rP Phabricator
- Branch
- T6734
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 3239 Build 3245: [Placeholder Plan] Wait for 30 Seconds
Event Timeline
Comment Actions
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().
Comment Actions
This error in T6734 is from PhabricatorTokenGivenEditor, in the publishTransaction method.
Comment Actions
(AFAIK Herald is okay because it gets invoked via the ManiphestEditor now, and the ManiphestTask is properly loaded.)
Comment Actions
Oh, hrrm -- this is the trace I'm seeing (definitely looks like a Herald issue?) -- do you have a different one?
Comment Actions
I think to get this to repro, add a Herald rule which acts on subscribers, like this one:
Comment Actions
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.
Comment Actions
(also I guess all this is sort of a hint at how this can go awry with projects conditionally loading eventually...)