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

Add required_output_field mutation option #134

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

mbuvarp
Copy link
Collaborator

@mbuvarp mbuvarp commented Nov 7, 2024

Adds a mutation option, required_output_field.

For all relevant mutations, the graphene.Field for the mutation output is set without required=True. This leads to the relevant GraphQL type being FishNode, and not FishNode! for a mutation like this:

mutation CreateFish($input: CreateFishInput!) {
  createFish(input: $input) {
    fish { # This is nullable, even though we know it should not be null
      id
      name
    }
  }
}

This is not normally an issue. However, a type generator (like GraphQL-Codegen), will then naturally make the type of the mutation result T | null, which is annoying to work around.

This PR solves the problem by introducing the required_output_field meta option for all mutations. If True, it sets required=True for all output model fields.

One question is whether this should be the default, since there are no scenarios (as far as I know) when the output field will actually be null if the above createFish is not null. This would technically be a breaking change, as it would alter the output schema of all apps using Graphene Django CUD, however I believe it would be extremely unlikely that it would actually break anything. It could perhaps also be a global configuration option.

Another question is the naming. required_output_field is perhaps not the best name. But naming is hard.

@mbuvarp mbuvarp requested a review from tOgg1 November 7, 2024 16:01
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.

1 participant