Page MenuHomePhabricator

Phriction - add "create" conduit endpoint
ClosedPublic

Authored by btrahan on Nov 7 2014, 7:30 PM.

Details

Summary

Fixes T6262. Ref T4029. Also gets us ready for T5873 for these end points. I can file something new about someday adding phriction.query, etc but I think we'll remember and can look at that post T5873.

Test Plan

made a document via conduit and it worked!

Diff Detail

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

btrahan updated this revision to Diff 25945.Nov 7 2014, 7:30 PM
btrahan retitled this revision from to Phriction - add "create" conduit endpoint.
btrahan updated this object.
btrahan edited the test plan for this revision. (Show Details)
btrahan added a reviewer: epriestley.
epriestley accepted this revision.Nov 7 2014, 7:37 PM
epriestley edited edge metadata.

I'm not sure how valuable this is before T5873, but I don't think it costs us much either. Your call either way.

Offhand, I imagine we'll probably end up with full-power endpoints (say, app.xcreate, app.xupdate) which take a list of transactions to apply (and, in the case of xupdate, an ID to update) and get full access to every possible transaction. And then maybe we'll have easy endpoints (say app.create, app.update, app.comment) which take a small list of things like "title", "content", etc., and build the transactions for you.

However, I'm not sure how valuable the easy endpoints really are, or if we need them. If we do, maybe we can get away with a few shared ones (like anything.comment which takes a PHID to comment on) and not have per-app create/update easy ones. So it's possible that after T5873 we might want to deprecate this kind of endpoint and just provide fully transaction-aware endpoints (which can presumably be very simple). Not a big deal regardless, and I don't have strong feelings about either choice being preferable.

This revision is now accepted and ready to land.Nov 7 2014, 7:37 PM

Oh, okay, I get the scope of T5873 now.

I am going to land this (later; its a branch of a branch with D10812 needing to come first) because I think some users will get some utility out of this, deleting it / editing it later seems trivial, and I don't think T5873 is coming particularly soon...

Cool, works for me.

This revision was automatically updated to reflect the committed changes.