these transactions should never merge since they are always created for a 1:1 replacement. (ie any merging would be implicitly erroneous). Fixes T4081
Details
Details
- Reviewers
epriestley - Maniphest Tasks
- T4081: Pholio doesn't allow you to replace multiple images in one shot
- Commits
- Restricted Diffusion Commit
rPd0de4dab2416: Pholio - fix a bug replacing multiple images
made a mock with three images and replaced all three successfully. replaced image A with image B, did not save, replaced image B with image C, then saved and verified transaction correctly showed image A replaced with image C.
Diff Detail
Diff Detail
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
| src/applications/pholio/editor/PholioMockEditor.php | ||
|---|---|---|
| 294–295 | an alternative fix is $u->getOldValue() == $v->getOldValue() ...which means these transactions are replacing the same image and can be merged. Again though, that never happens. I guess though this alternate fix is probably more correct given how editors are generally tolerable of garbage in. | |
| src/applications/pholio/editor/PholioMockEditor.php | ||
|---|---|---|
| 294–295 | Yeah, I think this alternative is slightly more correct if you want to go with that. | |
Comment Actions
alternate version
note its slightly funkier than what i commented on inline since transactions haven't had their values massaged at the time of merging. ergo, do the slightest amount of work to compare the correct values, which resembles the code in getCustomOldValue. Note $this->getCustomOldValue($object, $txn) will not work since we don't have $object handy here
Comment Actions
(We could change the signature to include $object here, but this seems wholly reasonable.)