ash-project / ash_graphql

The extension for building GraphQL APIs with Ash
https://hexdocs.pm/ash_graphql
MIT License
73 stars 49 forks source link

feat: add Relay ID translation in mutation and queries #109

Closed rbino closed 9 months ago

rbino commented 9 months ago

Adds a new option for queries and mutations that defines which arguments or attributes will use a global Relay ID and their type. This allows automatically decoding them before hitting their action.

Contributor checklist

rbino commented 9 months ago

@zachdaniel the feature is working, but it's missing some error handling and some more niceties. Opening as draft to collect feedback on the direction/naming/etc.

zachdaniel commented 9 months ago

I'm wondering if there is a way for us to figure this out automatically for cases with manage_relationship?

rbino commented 9 months ago

I think this would be orthogonal to managed_relationships, from what I understand, the main purpose of managed_relationships is generating input types from nested relationships, so I think it's mainly used for nested resource creation (e.g. create_post_with_tags) but less so with relationships that don't create the related resource (e.g. create_post_with_author, which doesn't create the author). Also, an ID could also be passed in a non-standard argument (like a map) and handled with a manual change, which wouldn't be detectable.

The high level idea I have in mind to see if this can be automatically generated is more or less this:

Given we would still need the manual way to do this to provide an escape hatch for cases not covered by the autogeneration, I don't know if you prefer fixing the manual way and then working on the autogeneration in a separate PR or start directly with the autogeneration.

zachdaniel commented 9 months ago

Yes, great points. We need both 👍 Doesn't really matter which one we start with 😄

zachdaniel commented 9 months ago

As for managed relationship type generation, you should be able to tap into our existing logic for generating types for managed relationships

rbino commented 9 months ago

@zachdaniel ready for review, this implements the manual part as discussed above. I will then open an issue to track the implementation of the automatically derived translations.

rbino commented 9 months ago

@zachdaniel rebased to fix the conflicts in the cheatsheet. I've also shortened the docs in the DSL and linked to the Relay guide instead as suggested by the Spark warning.

zachdaniel commented 9 months ago

🚀 Thank you for your contribution! 🚀