Details
- Reviewers
amckinley - Maniphest Tasks
- T13151: Plans: 2018 Week 23 - Week 30 Bonus Content
- Commits
- rPcac3dc4983c3: Give "create" transactions a readable type in "transaction.search"
- Created a new task.
- Called transaction.search on it.
- Saw the labelled "create" transaction.
Diff Detail
- Repository
- rP Phabricator
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Will this affect commits i.e. will newly imported commits have a "create" transaction type?
Commits and Phriction wiki pages currently don't get a "create" transaction, since the "create" transaction is built by EditEngine and these objects don't create via EditEngine at the moment.
Commits will (probably?) never create via EditEngine so I'll likely just add an explicit TYPE_CREATE transaction to the transaction stack they get during creation. This is simple but sometimes requires fiddling with the rendering of other transactions to avoid redundant stories. In older code, transactions like "title" would sometimes be special cased into "if the old value is null, render 'x created this object'; if the old value is not null, render 'x changed the title from y to z.'". I don't think commits have a transaction like this, but if they do it will need a little tweaking. Still, this is likely on the order of 5-10 minutes of work.
Phriction pages should create via EditEngine some day and it would be good to generally push them forward toward that. I haven't looked at the code yet so I'm not sure how involved this is. Might be an hour; might be a day or two. If it's on the messier side I can just stick a TYPE_CREATE transaction into the stack for them too, but if it's only an hour or two of work to modernize them I'd rather do it right.
I expect I can get most of PHI725 out the door in next week's release, there's just very little time left to sneak stuff into the release for this week since it's promoting in a few hours.
Would it be useful for everything that implements ModularTransaction to just extract TRANSACTIONTYPE and stick it in the conduit output? Or is that where the existing type data is coming from? I can't really discern a rhyme or reason for when the type field is null for the examples in PHI725.
We intentionally don't do that because many type fields are internally some string like core.custom:std:transaction/epriestley-tmp.v2:!!:bugfix or similar. Not quite that bad, but nonsensical or otherwise not the best values for API consumers (inconsistent; namespaced in an unhelpful way; old names waiting for migration, etc). For example, the actual getTransactionType() value here is core.create, but there's no real value in exposing the core.<...> part over the API.
(We could expose it in some sort of unstable.do-not-trust-this-value field but then I suspect everyone would use that field and complain when it proved unstable and not trustworthy.)