Changeset View
Standalone View
src/applications/dashboard/xaction/dashboard/PhabricatorDashboardIconTransaction.php
- This file was added.
| <?php | |||||
| final class PhabricatorDashboardIconTransaction | |||||
| extends PhabricatorDashboardTransactionType { | |||||
| const TRANSACTIONTYPE = 'dashboard:icon'; | |||||
| public function generateOldValue($object) { | |||||
| return $object->getIcon(); | |||||
| } | |||||
| public function applyInternalEffects($object, $value) { | |||||
| $object->setIcon($value); | |||||
| } | |||||
| public function getTitle() { | |||||
| $old = $this->getOldValue(); | |||||
| $new = $this->getNewValue(); | |||||
| return pht( | |||||
| '%s changed the icon for this dashboard from %s to %s.', | |||||
| $this->renderAuthor(), | |||||
amckinley: I guess dashboards gained default icons at some point? The original implementation has a "set… | |||||
Done Inline ActionsBriefly: they'll get a default old value in the next diff, which swaps us to EditEngine. More context: In old code, when you created an object for the first time, we wrote a bunch of transactions like this: name: null -> "some name" icon: null -> "some icon" status: null -> "active" ...and so on. Then we had all the transactions do if ($old === null) { return null; }, except one of them which was chosen arbitrarily did if ($old === null) { return pht('%s created this thing.'); }. That worked okay, mostly, but it had some problems:
New code on EditEngine does this instead:
The behavior is then mostly to hide "create" transactions from the UI, except in a handful of cases. This fixes all the problems:
So new/updated code usually throws away all the null handling. The only downside to doing this is that old transactions (in the database) don't have the "create" flag, so throwing away the null handling can make them render a little weird. For common/long-lived objects like Tasks this would probably raise enough eyebrows that we still have null handling code, but for any lesser-used application no one has ever cared that some AlmanacProperty has a transaction from 2015 like alincoln changed the name of this property from "" to "x.y.z".. If we do hit this stuff, we can also migrate fairly safely. As written, this code slightly worsens some rendering since this doesn't move us to EditEngine yet so we don't get all the new handling, but that's coming up in the next diff. epriestley: Briefly: they'll get a default old value in the next diff, which swaps us to `EditEngine`. | |||||
| $this->renderOldValue(), | |||||
| $this->renderNewValue()); | |||||
| } | |||||
| } | |||||
I guess dashboards gained default icons at some point? The original implementation has a "set the icon to foo" case for the title.