Do not interpret Enum members called 'description' as description properties

[mike-roberts-healx]

This is a workaround for TypeErrors being raised when initialising schemas with Enum members named description or deprecation_reason.

It is still not ideal as you can't have an enum with both a "description" member and a description property. Fixing this would require some more thought though. Potentially we could allow something like __description__ as an alternative way of naming the description property in cases where it conflicts with one of the members.

Fixes #1321 (ish)

[codecov]

Codecov Report

Base: 95.94% // Head: 95.95% // Increases project coverage by +0.01% 🎉

Coverage data is based on head (8af988a) compared to base (f09b2e5).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1478      +/-   ##
==========================================
+ Coverage   95.94%   95.95%   +0.01%     
==========================================
  Files          50       50              
  Lines        1726     1731       +5     
==========================================
+ Hits         1656     1661       +5     
  Misses         70       70              

Impacted Files	Coverage Δ
graphene/types/schema.py	95.41% <100.00%> (+0.09%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[erikwrede]

This might help users that need to define an enum member called description in their schema.
I agree that this is not an ideal solution. Maybe looking into supporting aenum for this special case using a special enum type could be an option?
Other than that, it looks like we can't do much about this issue without making breaking changes to graphene.Enum like defining custom enum members that have a description field.

[mike-roberts-healx]

This might help users that need to define an enum member called description in their schema. I agree that this is not an ideal solution. Maybe looking into supporting aenum for this special case using a special enum type could be an option? Other than that, it looks like we can't do much about this issue without making breaking changes to graphene.Enum like defining custom enum members that have a description field.

Yeah, we are hitting this issue in one of our graphQL APIs. It auto-generates a schema at initialisation time from the database schema, which has elements named "description". I've worked around it for now by registering Enum as a valid type for enum descriptions but that is really an ugly hack.

[erikwrede]

Maybe we can include a description dict[enum_value, description] in the EnumOptions as a fix.
Alternatively, adding a docs class instead containing the docstrings as class attributes instead of a dict would also work.
Both approaches have major drawbacks (including a lot of verbosity) and don't promote clean code either.

The significant disadvantage of all non-breaking options is that the style of implementing documentation differs from the rest of graphene, where a Field or Argument with a description argument is used.
If we decided to provide an alternative enum doc style to users not adhering to graphene's field doc standard, devs would use yet another non-optimal solution that might be subject to change in the future.

[mike-roberts-healx]

I can't think of an elegant solution that keeps descriptions working the way they currently do while also allowing for Enum members called "description" to have descriptions.

This PR is a fairly conservative change in that it shouldn't break any existing functionality, but fixes the case where there is an Enum with a member called "description" that doesn't have a description.

[erikwrede]

Took some time to play around with alternatives, and I agree that this is the only non breaking solution we have without reworking enums while changing as little as possible. Thanks for PRing this!

[mike-roberts-healx]

Took some time to play around with alternatives, and I agree that this is the only non breaking solution we have without reworking enums while changing as little as possible. Thanks for PRing this!

Thanks for the review and for helping to maintain graphene. It's much appreciated!

[julianmoji]

Hello @mike-roberts-healx, Nice fix I was struggling with it some hours ago, is it already available for version 3.1.1? I've already installed it, but I just can't get your change

[erikwrede]

@julianmoji Ill release a patch soon, for now you can also install this PR in pip/poetry as a workaround:)

[erikwrede]

This is now released in 3.2.0: https://github.com/graphql-python/graphene/releases/tag/v3.2.0

[julianmoji]

@erikwrede Thanks you so much man :)