Skip to content
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

Relay refetching support #106

Merged
merged 5 commits into from
Jan 24, 2024
Merged

Relay refetching support #106

merged 5 commits into from
Jan 24, 2024

Conversation

rbino
Copy link
Contributor

@rbino rbino commented Jan 18, 2024

Add support for Relay global IDs and the node query, see https://relay.dev/graphql/objectidentification.htm

Contributor checklist

  • Features include unit/acceptance tests

@rbino
Copy link
Contributor Author

rbino commented Jan 18, 2024

Opened this as draft to collect some early feedback, global IDs are already working, I will tackle the node query in the next days

Part 1 of what is needed for ash-project#99.
The other half is implementing the node query.
@rbino
Copy link
Contributor Author

rbino commented Jan 22, 2024

@zachdaniel I think we're almost there but right now only the ID gets selected because it seems the fields contained in the type-specific fragments don't get included in the selected fields by Ash. Is there something where, e.g., interfaces are implemented that I can take as reference to see if I can make this work?

lib/graphql/resolver.ex Outdated Show resolved Hide resolved
The is_type_of implementation wasn't actually working
Use Ash.Error.Invalid.InvalidPrimaryKey and provide its protocol implementation
to render it to a Graphql error
@rbino rbino marked this pull request as ready for review January 24, 2024 18:11
@rbino
Copy link
Contributor Author

rbino commented Jan 24, 2024

@zachdaniel this is now ready for review, now I implement the node interface for resources with a primary key get.

I also had to change some of the code regarding field selection for the reason I mentioned above (fields coming from fragment weren't being selected), it is now working correctly.

@@ -4118,4 +4154,11 @@ defmodule AshGraphql.Resource do
name
end
end

def primary_key_get_query(resource) do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think one of the things we'll need to do soon here is make this configurable, but the strategy of choosing an existing gql query for it to mimic is pretty neat.

@zachdaniel
Copy link
Contributor

looks like some credo failures to address, then we can merge it :)

This allows picking also up fields coming from fragments in queries returning an
interface
Allow retrieving a resource implementing the Node interface given its Relay
global id.

Close ash-project#99
@rbino
Copy link
Contributor Author

rbino commented Jan 24, 2024

It was complaining about the two TODOs I had left, removed them

@zachdaniel
Copy link
Contributor

😆 of course. We can also just delete that credo rule.

@zachdaniel zachdaniel merged commit 365b3ae into ash-project:main Jan 24, 2024
13 checks passed
@zachdaniel
Copy link
Contributor

awesome work! 🚀 Thank you for your contribution! 🚀

rbino added a commit to rbino/ash_graphql that referenced this pull request Jan 25, 2024
Slight API improvement over ash-project#106.

This makes it more ergonomic to partially match on the return value (and it also
makes it more explicit by explicitly labeling the two parts)
rbino added a commit to rbino/ash_graphql that referenced this pull request Jan 25, 2024
Slight API improvement over ash-project#106.

This makes it more ergonomic to partially match on the return value (and it also
makes it more explicit by explicitly labeling the two parts).

Also add tests for relay id encoding/decoding.
zachdaniel pushed a commit that referenced this pull request Jan 25, 2024
Slight API improvement over #106.

This makes it more ergonomic to partially match on the return value (and it also
makes it more explicit by explicitly labeling the two parts).

Also add tests for relay id encoding/decoding.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants