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 options to override how Django Choice fields are converted to Enums #860

Merged
merged 12 commits into from
Mar 13, 2020

Conversation

jkimbo
Copy link
Member

@jkimbo jkimbo commented Jan 29, 2020

This PR resolves #185 and #843 by adding 2 new settings to customise the naming of the Enum types generated from Django Choice fields.

  • DJANGO_CHOICE_FIELD_ENUM_V3_NAMING changes all enum type names to a naming convention that should prevent duplicates. This format will be the default in v3.
  • DJANGO_CHOICE_FIELD_ENUM_CUSTOM_NAME allows you to define a custom function to determine what the enum type name should be

Copy link
Contributor

@MisterGlass MisterGlass left a comment

Choose a reason for hiding this comment

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

Looks great!

Copy link
Collaborator

@zbyte64 zbyte64 left a comment

Choose a reason for hiding this comment

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

While introducing new settings is to be avoided, both have their rationale.

DJANGO_CHOICE_FIELD_ENUM_V3_NAMING is a path forward to conflict free defaults.

DJANGO_CHOICE_FIELD_ENUM_CUSTOM_NAME does take away control from the applications for extreme remedies. However it appears to encourage importing models into the settings, this should be avoided. Instead we should support a string to be imported using https://docs.djangoproject.com/en/3.0/_modules/django/utils/module_loading/

@jkimbo
Copy link
Member Author

jkimbo commented Jan 31, 2020

@zbyte64 good point about using module loading rather than direct references. I'll update the PR.

Also I don't understand the rest of your comment? Are you suggesting changes to the documentation or just re-iterating what the purpose of the change is?

@zbyte64
Copy link
Collaborator

zbyte64 commented Feb 4, 2020

Was just re-iterating / thinking out loud. Updating the settings to allow a string specified import was my only recommend action.

@jkimbo
Copy link
Member Author

jkimbo commented Feb 23, 2020

@zbyte64 PR is ready for another review

Copy link
Collaborator

@zbyte64 zbyte64 left a comment

Choose a reason for hiding this comment

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

Thank you for tackling this

@bennett-jacob
Copy link

When is this going to be merged?

@rarenatoe
Copy link

Someone merge and deploy this please

@jkimbo
Copy link
Member Author

jkimbo commented Mar 13, 2020

Sorry for the delay with this (I've been on holiday). Shipping this now

@jkimbo jkimbo merged commit b8e598d into master Mar 13, 2020
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.

Different types with the same name in the schema - Django fields with choices
6 participants