Shows a timeline of all modified settings Fixes T6545
Will show all settings (no pagination, should be not so difficult to add if needed but most installs won't have hundreds of settings changes)
I'm not happy by how the PhabricatorConfigTransaction object is instructed to render the config keys but i don't see any other reasonable way.
We could always show the keys though.
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- T6545: Show unified config edit log in Config web UI
- Commits
- Restricted Diffusion Commit
rP86eb7c0ec446: Settings History
Changed settings and called the history page
Diff Detail
- Repository
- rP Phabricator
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
This looks great.
I think we can make the "displayKeys" part a little less hacky by doing this:
- Add a setRenderAsFeed() method to PhabricatorApplicationTransactionView.
- If the flag is set, in PhabricatorApplicationTransactionView->renderEvent(), call getTitleForFeed() instead of getTitle().
- You'll have to pass a $story, but I think you can just create a dummy object. As far as I can tell, no callsites actually use this parameter, and I can't think of a reason why any would. We can likely remove it at some point.
- Then, implement getTitleForFeed() in PhabricatorConfigTransaction. You can look at other Transaction classes for an example, but basically it's a copy of getTitle() that just renders alincoln closed T123 instead of alincoln closed this task, similar to what you've done already.
Basically the same thing as this diff, but that approach would be a bit more general and should work for any kind of object.
Remove all the hacks from last year ;)
calling getTitleForFeed without a Parameter is still bad and might break other use cases.
However making up a dummy $story is tricky as it expects a PhabricatorFeedStoryData Object which is a DAO...
Ok, i've checked all the implementations of getTitleForFeed and getApplicationTransactionTitleForFeed which get $story passed and it is never used anywhere...
Want me to submit a diff removing it?
I'll just land this and we can do the removal in a followup, I think it's OK to have this slightly-skethcy = null stuff for a little while.