Many Conduit methods for updating or reading objects are now out of date. In particular, they generally do not support ApplicationTransactions or CustomFields. This means some data (like inline comments, and usually custom field values) often can not be read, and some data can not be written. In action-oriented applications like Differential and Diffusion, some actions can not be taken.
We also have uneven support for querying attached objects. Sometimes these are just loaded and provided explicitly; sometimes they are inaccessible.
A number of open tasks request access to these fields or edges. We want to move forward in a general way which accommodates ApplicationTransactions and CustomFields, rather than adding one-off properties to objects. However, this requires that we think through the design carefully. It is also likely to make backward-incompatible changes in at least some methods which have incompatible partial support with these systems.
Reading Custom Fields
This is probably the simplest thing to do. Letting custom fields elect to appear in Conduit responses and exposing them from app.query methods seems straightforward. Open issues:
- differential.query emits an auxiliary dictionary with custom field values.
- maniphest.query emits an auxiliary dictionary with custom field values.
- Maniphest data is keyed by field key (often nonobvious and internal). Differential data is keyed by "conduit field key" (better-ish but distinct).
- Maybe pulling this data should be optional?
- Should we have keys which are easier to use? Probably...
This is easy to implement as a low-level API, like transactions.query. Open issues:
- Transaction formats are very hard for third-party code to consume. Lots of internal datastructures, and nothing is rendered.
- For one example, Differential inline comments are bound to changesetIDs, which probably can not be retrieved over the API.
- Transaction formats aren't very stable.
- differential.getrevisioncomments provides access to Differential comments only.
- maniphest.gettasktransactions provides access to Maniphest transactions only.
- Neither expose all of the information (like comment edits / removal).
- Putting this in "transactions.query" instead of "application.querytransactions" might make it hard for users to find.
- Overall, the data format is probably not totally unreasonable. It's not pristine, but most technical users can likely make sense of it.
- Expose this, call it "low-level", make some effort to pretty it up a bit (add extra data, like a text rendering of the transaction)?
- Maybe also expose a light/easy way to query just comments? Not sure this is useful.
- We can expose edges.query, but this makes it harder to use than just getting the data back on the objects.
- However, putting taskPHIDs, revisionPHIDs, commitPHIDs, etc., on every object which is related to other objects seems clearly unworkable, and won't ever work with third-party applications.
- A few relationships are not edges (like workboard columns, and I worry reviewers/auditors should maybe not really be edges).
- Edges have meaningless constant identifiers.
- Some methods already expose related objects.
- I think this should probably be in edges.query and we can just smooth over the other stuff as best we can (e.g., let edge types define readable names).
- The other alternative I can come up with is adding a new "connections" idea, which is a generalization over edges but could include other connections which are not technically stored as edges, like workboard columns. Primary query methods like differential.query could then have an option to pull these connections, and/or we could pull them all by default. I kind of like this, but we could build edges.query first and then look into this as an ease-of-use refinement.
We can not allow direct edge writes, since you can violate security in all sorts of ways by performing them.
Writing Custom Fields
We should not allow direct writes to custom fields, except maybe as a convenience. Write transactions instead.
This is the biggest mess. In particular:
- Transactions don't have as much type validation as they should to be exposed as public APIs.
- Constructing transaction values is very much type-specific, inconsistent black magic (for example, the magic + and - keys when doing subscriptions and edges).
- Some transactions currently require the caller to provide both oldValue and newValue, but we generally should not permit callers to control oldValue.
- Transaction constants are not consistently meaningful and not stable.
- Transaction formats are not stable.
- I think this probably needs a layer of glue code to make the API usable, with operations like "comment", "subscribers.add", "subscribers.remove", "subscribers.set", etc., and less black magic? This will be somewhat involved to develop.