-
Notifications
You must be signed in to change notification settings - Fork 769
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
Provide setting to enable/disable converting choices to enums globally #1477
Provide setting to enable/disable converting choices to enums globally #1477
Conversation
95fb235
to
e9632d1
Compare
e9632d1
to
c9af33a
Compare
): | ||
if registry is not None: | ||
converted = registry.get_converted_field(field) | ||
if converted: | ||
return converted | ||
choices = getattr(field, "choices", None) | ||
if convert_choices_to_enum is None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A reason why I moved it into convert_django_field_with_choices
and not implemented it on construct_fields
or DjangoObjectType
is, that django-graphene-plus
uses convert_django_field_with_choices
directly and would skip/ignore the setting or would need to manually look it up.
See: https://github.com/0soft/graphene-django-plus/blob/master/graphene_django_plus/input_types.py#L28
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Flauschbaellchen for the PR. This sounds like a valid use case to me and the changes you proposed look backward-compatible too. I don't see a problem with merging this. Thoughts @sjdemartini @firaskafri?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes look good to me! Thanks for adding the thorough tests and for suggesting this; the motivation makes sense to me. I support merging 👍
graphql-python#1477) Co-authored-by: Firas Kafri <3097061+firaskafri@users.noreply.github.com> Co-authored-by: Kien Dang <mail@kien.ai>
In our project we want to disable the auto-convertion of Django choice fields to Enums for every model.
To specify
convert_choices_to_enum = False
on everyType
is a bit cumbersome so a global setting would help a lot.Any feedback is highly appreciated.
Closes #1480