Implement an edition type for all GraphQL edition-like documents #3063
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
brucebolt
force-pushed
the
flatten-graphql-edition
branch
10 times, most recently
from
4f83c8d to
b6b3749
Compare
brucebolt
added a commit
to alphagov/government-frontend
that referenced
this pull request
Dec 31, 2024
Publishing API was updated in alphagov/publishing-api#3063 to have a single edition type for most content, instead of a specific type for news articles. Updating the query used by this application in making such requests to Publishing API.
brucebolt
added a commit
to alphagov/collections
that referenced
this pull request
Dec 31, 2024
Publishing API was updated in alphagov/publishing-api#3063 to have a single edition type for most content, instead of a specific type for roles. Updating the query used by this application in making such requests to Publishing API.
brucebolt
added a commit
to alphagov/collections
that referenced
this pull request
Dec 31, 2024
Publishing API was updated in alphagov/publishing-api#3063 to have a single edition type for most content, instead of a specific type for the world index page. Updating the query used by this application in making such requests to Publishing API.
brucebolt
changed the title
brucebolt
force-pushed
the
flatten-graphql-edition
branch
from
b6b3749 to
213b539
Compare
mike3985
reviewed
Jan 2, 2025
Comment on lines
+11
to
+13
| all_links = @object | ||
| .document | ||
| .reverse_links |
There was a problem hiding this comment.
Are we missing some eager loading here?
.reverse_links
.includes(source_documents: @content_store)There was a problem hiding this comment.
Yeah, I've added that in and it's substantially reduced the number of queries being made.
Comment on lines
3
to
6
| target_document = create(:edition) | ||
| source_document_1 = create(:edition) | ||
| source_document_2 = create(:edition) | ||
| source_document_3 = create(:edition) |
There was a problem hiding this comment.
It might be easier on my brain if these were called "source_edition"s
There was a problem hiding this comment.
Good idea. I'll update the existing spec too.
|
|
||
| edition_links = Link | ||
| .where(target_content_id: @object.content_id, link_type: link_types) | ||
| .where.not(edition: nil) |
There was a problem hiding this comment.
I've never worked with edition links. To me it looks like the first Active Record query ("link_set_links = ...") would return both link set links and edition links. Am I missing something?
There was a problem hiding this comment.
You're absolutely right. I haven't noticed that the reverse_links relationship didn't involve LinkSet in any way. I'll update this to just do the one query then grab the edition differently depending on the link type.
These are editions, not documents, so should be named as such to avoid confusion.
This allows us to define reverse links on a type in a generic way, in the same format as direct links.
This allows us to return the direct edition links for editions using GraphQL. Also means we can now support multi-level edition links, which is currently not possible using the content item version of editions.
This allows us to return the reverse edition links for editions using GraphQL.
This removes the hardcoding and duplication amongst the varieties of Edition we have currently implemented. These changes will make it easier to implement new document types, as the fields and links have already been defined. Since all links are now returned as the Edition type, any field can be selected from them, not just those that have been defined in the link type. This makes queries much more flexible, as the frontend application can request any fields it likes from the links, without any changes being made in Publishing API. The Minister Index Type is being retained, since that contains custom optimisations specific to the links on that page. In future, it may be possible to decouple that type from editions entirely, meaning we wouldn't need to ever publish that page from Whitehall. Note 1: this requires selection of all fields within `details` for any edition. Previously this was only required when `details` was nested within a link, making it consistent whether the edition is returned at the top-level or as a link. Note 2: role appointment reverse links will be added in a separate commit, as they require some logic.
This reverse link uses a different link type depending on the parent object. Therefore adding some logic here to select which link type is required.
brucebolt
force-pushed
the
flatten-graphql-edition
branch
from
213b539 to
e3b484b
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This moves all edition types into a single type, to reduce duplication and minimise the amount of code required when adding support for content types to GraphQL.
The bespoke code for getting
role_appointmentson the ministers index page has been retained, as this was added to improve performance for that page due to the number of links and their complexity.When we want to add a new content type that doesn't need any bespoke code, we just need to ensure all the relevant
detailsfields are present in the Edition type. All the link types have already been added and any links can be requested for any document type. Without making the changes in this PR, we would need to create a new type for each document type, which would include a lot of code duplication where similar things exist across the types.Also adds support for edition links and multi-level edition links.
Trello card