Page MenuHomePhabricator

Modularize Almanac Binding transactions
ClosedPublic

Authored by epriestley on Apr 10 2018, 12:15 PM.
Tags
None
Referenced Files
F18584055: D19321.id46262.diff
Thu, Sep 11, 11:50 AM
F18584052: D19321.id46262.diff
Thu, Sep 11, 11:49 AM
F18577395: D19321.id46227.diff
Wed, Sep 10, 6:40 PM
F18574413: D19321.id46227.diff
Wed, Sep 10, 10:38 AM
F18509536: D19321.id.diff
Fri, Sep 5, 3:27 AM
F18502615: D19321.diff
Thu, Sep 4, 10:33 PM
F18447621: D19321.diff
Sun, Aug 31, 10:51 PM
F18411191: D19321.id.diff
Sat, Aug 30, 5:45 AM
Subscribers
None

Details

Summary

Depends on D19320. Ref T13120. Ref T12414. Move transactions for Almanac Bindings to ModularTransactions.

Test Plan
  • Created a new binding.
  • Tried to create a duplicate binding, got an error.
  • Edited a binding to rebind it to a different device.
  • Disabled and enabled bindings.
  • Grepped for AlmanacBindingTransaction:: constants.

When a binding is created, it currently renders a bad "changed the interface from ??? to X" transaction. This is because creation isn't currently using EditEngine. I plan to swap it shortly, which will turn this into a real "Create" transaction and fix the issue.

Diff Detail

Repository
rP Phabricator
Branch
almanac6
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 20051
Build 27194: Run Core Tests
Build 27193: arc lint + arc unit

Event Timeline

Note that, for now, Bindings have the same issue as Services in D19317: editing properties works correctly, but doesn't render the right transaction stories in the timeline. Since this is just a display issue, I plan to deal with it after converting everything.

amckinley added inline comments.
src/applications/almanac/xaction/AlmanacBindingInterfaceTransaction.php
61–62

renderOldValue()/renderNewValue()?

This revision is now accepted and ready to land.Apr 10 2018, 3:51 PM
src/applications/almanac/xaction/AlmanacBindingInterfaceTransaction.php
61–62

This one's right -- since the old and new values are PHIDs, this renders nice named links for them.

This revision was automatically updated to reflect the committed changes.