-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Updates for GraphQL api spec #198
Conversation
cafca
commented
Jul 14, 2022
•
edited
Loading
edited
- Make document metadata field generic over schemas: We don't have anything schema-specific in this type so it can be generic.
- Use GraphQL language (e.g. "field argument" instead of "input variable") and camel case
- Specify types of field values
- Simplify GraphQL type and field names
We don't have anything schema-specific in this type so it can be generic.
How about calling it just |
That's what I called it before and then I made it |
Metadata makes me think of AI or some big data company 😅 it's technically not wrong, but I like DocumentMeta better haha |
You should own that term! I love metadata, it's the best :) Don't leave it to big tech. |
- in this specification we use `<schema_id>` as a placeholder for the string-encoded schema id of actual schemas | ||
- For every schema that can be queried, nodes generate fields, which are made available on the _root query type_, as well as types to represent the schema's documents. | ||
- Together, these allow clients to request documents including their materialised views and metadata. | ||
- As fields and types that contain schema-specific data are dynamically generated, every node may contain a different set of these, depending on which schemas are available. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if it's clear what varies here: is it the response (node a has 15 documents, b has only 3), the offered queries (node a offers schema p, node b offers schema q and p) or features of them (node a offers meta information about authors, b doesn't)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is pretty unambiguous already. It says that the set of schema-specific fields and types varies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It says that the set of schema-specific fields and types varies.
I think I'm still lost here, haha. Do you want to say that every node offers different schemas?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, maybe the confusion comes from GraphQL terminology? The structure of a GraphQL schema is that at the root you have the root query type which is an object. Like every object in GraphQL it has fields that can be queried to access: either another object with more fields or a scalar type (that actually has a concrete value).
These fields are what you might think of as "queries" in another context, e.g. nextArgs
in GraphQL is not a query, it's a field on the query object.
So all the fields on the root query type change depending on what schemas are available. But also in other places: for example the available fields on inbound
also change depending on the available schemas. That's why I wrote it like this. I will try and rephrase the sentence to try and make this clearer, please let me know if it works :D
- either the `id` or `viewId` field argument must be set | ||
- if `id` contains a document id, the response must contain the [_latest document view_][latest-document-view] for that document | ||
- if `viewId` contains a document view id, the response must contain this document view | ||
- if both field arguments are given the query must return an error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change this to current behaviour (return the view)
|
||
all_<schema_id>( | ||
""" | ||
filter collection of documents |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These filters are not implemented (right? 🤣 ) should we still include them?
@@ -294,7 +315,7 @@ type <schema_id>Filter { | |||
<field_name>_lte: <value> | |||
} | |||
|
|||
type <schema_id>PageResponse { | |||
type <schema_id>Page { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same, should we include unimplemented stuff?