Use GraphQL for rendering Role page #3845
Conversation
The instance variable isn't being used anywhere Co-authored-by: Ynda <[email protected]>
Co-authored-by: Ynda <[email protected]>
Exercising the role page with a few alternate content item configurations, hopefully triggering all combinations of partials to be rendered Co-authored-by: Ynda <[email protected]>
Negating the use of hashes in the roles views. We've taken this opportunity to flatten some of the data that was in those hashes -- see the removal of the "details" layers. We're working towards introducing Publishing API's GraphQL endpoint as an alternate source for Role data. If the current approach is preferred, we'd be happy to build hashes from that new data, and to have those hashes reflect internal details specific to the ContentItem data. But since it didn't appear difficult to change, we thought we'd have a go at making something more neutral. --- We're not sure yet about the use of Data objects here. It seemed like a nice idea to begin with but after it was finished and working we weren't completely convinced that it wasn't a bit "too smart". Co-authored-by: Ynda <[email protected]>
We're trialling Publishing API's GraphQL endpoint as a source for Role data like we did recently for the World Index Co-authored-by: Ynda <[email protected]>
This fixture reflects how Publishing API's GraphQL endpoint represents the prime-minister role Co-authored-by: Ynda <[email protected]>
Co-authored-by: Ynda <[email protected]>
Co-authored-by: Ynda <[email protected]>
Co-authored-by: Ynda <[email protected]>
| @@ -0,0 +1,14 @@ | |||
| class RoleGraphql | |||
There was a problem hiding this comment.
I wonder if this (and the other for the world index) should be namespaced to keep them all together in one place? E.g. Graphql::Role?
| OrganisationPresenter = Data.define(:title, :base_path) do | ||
| def initialize(attributes) | ||
| super(attributes.slice(*members.map(&:to_s))) | ||
| end | ||
| end |
There was a problem hiding this comment.
I'm not convinced these sub-presenters add much value here and seem a little inconsistent with other presenters (noticed you'd noted about this in the commit message). Is there a way we can achieve the same without them?
There was a problem hiding this comment.
My aim with these sub-presenters is for the "presented" data to be used like objects with attribute reader methods instead of like hashes. Yeah, I see what you're saying, I guess that goal itself might already be the definition of inconsistency if the existing presenters all provide hash access.
As I (hope I) mentioned in the commit, the motivation for objects instead of hashes ^ is to be able to avoid having to massage the new GraphQL JSON data into nested hashes that're structured identically to the Content Store JSON.
So, we could always just take the route I avoided and massage the GraphQL JSON into nested hashes identical to the pre-existing Content Store ones. It wouldn't be a terrible thing (I mean, it's standard practice when working with unfamiliar or legacy code, and it potentially leaves less of a mess if we remove the GraphQL code later).
On the other hand, if the object access is preferred, off the top of my head the options are:
- define presenter classes
- use Structs
- use OpenStructs
- use Data objects
What appealed to me about Data objects was the minimal boilerplate (we could go further and try to hide that common initialize somewhere less distracting) and that compared to the struct types, it doesn't define setter methods out-of-the-box.
I hope the Navigation team will spot this change and have some thoughts, too
There was a problem hiding this comment.
FWIW, I've just spotted a couple of existing presenters on my travels that're using classes and structs for this same purpose (https://github.com/alphagov/collections/blob/main/app/presenters/ministers_index_presenter.rb#L59, https://github.com/alphagov/collections/blob/main/app/presenters/ministers_index_presenter.rb#L107, https://github.com/alphagov/collections/blob/main/app/presenters/embassies_index_presenter.rb#L51-L52)
This will be used to allow us to build a proof of concept for using GraphQL on GOV.UK. It runs alongside Content Store and can be switched on by setting the GRAPHQL_FEATURE_FLAG environment variable.
There should be no user facing changes.
https://trello.com/c/qBZdQMEa/1446-prime-minister-page-spike-consuming-data-required-in-frontend-app
Note: we took a lot of our cues for the approach to take from the previous GraphQL pull request.