Page MenuHomePhabricator

Provide "almanac.binding.search" and "almanac.binding.edit"
ClosedPublic

Authored by epriestley on Apr 11 2018, 1:40 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Nov 24, 6:55 AM
Unknown Object (File)
Thu, Nov 14, 7:04 AM
Unknown Object (File)
Mon, Nov 11, 9:02 PM
Unknown Object (File)
Sat, Nov 9, 3:13 PM
Unknown Object (File)
Thu, Nov 7, 11:19 PM
Unknown Object (File)
Wed, Nov 6, 5:55 AM
Unknown Object (File)
Thu, Oct 31, 6:42 AM
Unknown Object (File)
Oct 24 2024, 10:57 PM
Subscribers
None

Details

Summary

Depends on D19338. Ref T13120. Ref T12414. These are the last of the new API methods.

This stuff still doesn't work:

  • You can't actually enable/disable bindings yet. I want to take a look at the use cases and consider changing "disabled" to "status", or providing a different way to solve the problem.
  • You can't edit properties via the API. I expect to enable this for all AlmanacPropertyInterface objects with an extension in a future change.
Test Plan
  • Searched for bindings via API.
  • Viewed binding web UI for API methods.
  • Created bindings via API.

Diff Detail

Repository
rP Phabricator
Branch
almanac22
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 20074
Build 27234: Run Core Tests
Build 27233: arc lint + arc unit

Event Timeline

amckinley added inline comments.
src/applications/almanac/xaction/AlmanacBindingServiceTransaction.php
39

Just out of curiosity, why isn't this executeOne()?

This revision is now accepted and ready to land.Apr 11 2018, 3:57 PM

executeOne() raises an immediate policy exception if the object exists but can't be seen, which is less specific ("You can't see a service.") than the error we'll be able to raise ("You can't see the service you tried to bind to.") if we continue.

This pattern is sort of "executeOne(), but return null instead of raising a policy exception if the object exists and can't be seen". Possibly it should be more of a first-class part of the API like executeOneMaybeOrMaybeNot(). There's setRaisePolicyExceptions() but it has some weird behavior that makes it not quite the same, I think. This is relatively rare overall and I don't have a great name for it so I haven't gotten as far as actually adding it to the API.

Conceptually, it would also be kind of nice to differentiate more clearly between "invalid" and "restricted". The API currently makes this tough. An executeOneMaybe() could perhaps return some kind of object wrapping the result that had methods like isRestricted(), isNonexistent(), etc. But I'm not sure if we'd realistically write out all the branches of error messages every time. Still, it might be nice if we had the option more readily.

This revision was automatically updated to reflect the committed changes.