Initial GraphQL implementation #11
Conversation
This adds the GraphQL gem and generates a GraphQL installation.
Why these changes are being introduced: We need a GraphQL interface to record SearchEvents from TACOS consumers. Relevant ticket(s): https://mitlibraries.atlassian.net/browse/ENGX-234 How this addresses that need: This adds a SearchEvent type and corresponding query field that returns information about the search event and creates a new SearchEvent (and Term, if applicable). Side effects of this change: * Removed a couple of generated GraphQL test fields. * Miscellaneous rubocop fixes.
|
To address the coverage issues seems like we would be testing the gem. I'm curious about differing perspectives on this, and whether additional coverage is needed outside of what's being reported. |
The introspection query was erroring `Type Mutation must define one or more fields`. Since we don't currently provide a mutation, this should be fine.
| description: "An example field added by the generator" | ||
| def test_field | ||
| "Hello World!" | ||
| field :log_search_event, SearchEventType, null: false, |
There was a problem hiding this comment.
I don't want to change this now, but I think we may want to be open to changing this name as we start integrating this work with other systems.
TACOS cares about the logging portion of this, but consumers of TACOS in the future are hopefully less concerned with logging and more interested in the structured data we can return. It's also possible there are multiple access points, this one that just logs and a different one in the future that would log and return data... hence my not wanting to worry about the name quite yet as this is currently is well named :)
As the current targeted use is entirely internal for the near future, we can easily update this if we come up with a name that feels more accurate.
| # frozen_string_literal: true | ||
|
|
||
| module Types | ||
| class SearchEventType < Types::BaseObject |
There was a problem hiding this comment.
Thought for the future: I'm waffly (and thus not requesting a change) as to whether it might be good to include the Term.phrase as part of this externally facing object. I know from the data perspective, it is a relation but from the API perspective we may want this to feel like a single object. Hmmm.
There was a problem hiding this comment.
I had the same thought and, being similarly ambivalent, stuck to what is contained in the SearchEvent model for now. My expectation is that this type will eventually include a lot of data that it does not currently, including the search phrase. (As you noted, the field name will probably also change to reflect this.) For now, sticking with a basic implementation feels to me like a good approach, given that we'll be extending it in the hopefully near future.
| @@ -1,4 +1,8 @@ | |||
| Rails.application.routes.draw do | |||
| if Rails.env.development? | |||
| mount GraphiQL::Rails::Engine, at: "/graphiql", graphql_path: "/graphql" | |||
There was a problem hiding this comment.
In TIMDEX we've decided to implement GraphiQL as an HTML page instead of via the Gem. I don't remember why, so this isn't a request for a change but just something we should keep our eye on to know whether we should update TIMDEX or TACOS to match the other.
https://github.com/MITLibraries/timdex/blob/main/public/playground.html
If we can configure the Gem version, it may be better as it would fit into our dependency update patterns more easily than the standalone HTML does. 🤷🏻
|
@jazairi I added a commit to address the mutation warning. I am not (currently at least) concerned with the code coverage as that is all generated code as you noted. |
Why these changes are being introduced:
We need a GraphQL interface to record SearchEvents from TACOS
consumers.
Relevant ticket(s):
https://mitlibraries.atlassian.net/browse/ENGX-234
How this addresses that need:
returns information about the search event and creates a new
SearchEvent (and Term, if applicable).
Side effects of this change: