Spike: can we simplify the GraphQL types? #3024
Conversation
| @@ -23,8 +47,12 @@ class WithdrawnNotice < Types::BaseObject | |||
| field :rendering_app, String | |||
| field :scheduled_publishing_delay_seconds, Int | |||
| field :schema_name, String | |||
| field :slug, String, null: false | |||
| field :started_on, GraphQL::Types::ISO8601DateTime | |||
| field :supports_historical_accounts, Boolean | |||
There was a problem hiding this comment.
thought: approach
I'd be interested in discussing what our priorities are in terms of simplification vs clarity. Here are some questions off the top of my head (I haven't formed an opinion yet!):
- Here we might end up with one big edition type with fields that might or might not exist on a given document based on its type: will we then lose the ability to know whether a given document should have certain fields/links?
- Does the distinction between nullish data and invalid properties matter?
- Do we want GraphQL query clients to be able to make use of a complex schema for introspection, so that the clients can guide us on what could/should exist on a given document based on its type (this change would presumably mean they'd just suggest every field for every edition/link)?
- Do we want client apps to know via error if they're asking for data that will never exist on a given type (perhaps they're using it in a view and not realising it will always be null)?
- Is the benefit here mostly that we don't need to define fields on multiple types?
- Is this better than just moving the shared stuff into the edition type?
- Is everything technically shared, and just null on certain types?
There was a problem hiding this comment.
Is everything technically shared, and just null on certain types?
This was my assumption when I started looking into this, on the basis that Publishing API can theoretically have any of these things on any document type in it's database, it's just we exclude them from being allowed by validating against a schema when the publishing application makes a request.
I don't know if it's possible (or even a good idea), but maybe we could validate GraphQL queries against the schemas too, so you can't request a field that isn't in that document's schema?
(Writing the answer to this question here, so I don't forgot before we mob)
| EDITION_TYPES = [Types::EditionType, Types::RoleType, Types::WorldIndexType, Types::MinistersIndexType].freeze | ||
| EDITION_TYPES = [Types::EditionType].freeze |
There was a problem hiding this comment.
suggestion: tidy up
If we do go down this route, I'd have thought this class/type resolution could be fully deleted. I don't think it's doing anything meaningful with the edition type descendants removed
brucebolt
force-pushed
the
spike-flatten-graphql-edition
branch
from
a44fa80 to
d2c455d
Compare
brucebolt
force-pushed
the
spike-flatten-graphql-edition
branch
from
d2c455d to
f4d6952
Compare
|
Closing in favour of #3063. |
This application is owned by the publishing platform team. Please let us know in #govuk-publishing-platform when you raise any PRs.
Follow these steps if you are doing a Rails upgrade.