Skip to content

fix: editing of FlagField via EnumFlagField form field #97

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 4 commits into from
Mar 19, 2025

Conversation

crgwbr
Copy link
Contributor

@crgwbr crgwbr commented Mar 13, 2025

When attempting to edit an EnumFlagField through a model form (e.g. in django admin), the form would always be invalid on submit with an invalid_choice error. This was due to the form attempting to set the model field to a list of enum members (rather than bitwise-OR'ing the flags together).

This change handles this case by overriding FlagField._coerce_to_value_type and performing the appropriate bitwise OR there.

When attempting to edit an EnumFlagField through a model form (e.g. in django
admin), the form would always be invalid on submit with an invalid_choice error.
This was due to the form attempting to set the model field to a list of enum
members (rather than bitwise-OR'ing the flags together).

This change handles this case by overriding FlagField._coerce_to_value_type and
performing the appropriate bitwise OR there.
@bckohan bckohan self-requested a review March 13, 2025 19:58
@bckohan bckohan self-assigned this Mar 13, 2025
@codecov-commenter
Copy link

codecov-commenter commented Mar 13, 2025

Codecov Report

Attention: Patch coverage is 96.42857% with 1 line in your changes missing coverage. Please review.

Project coverage is 99.87%. Comparing base (4a1688d) to head (549a1f1).

Files with missing lines Patch % Lines
src/django_enum/forms.py 96.15% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##              main      #97      +/-   ##
===========================================
- Coverage   100.00%   99.87%   -0.13%     
===========================================
  Files            9        9              
  Lines          806      831      +25     
  Branches       117      123       +6     
===========================================
+ Hits           806      830      +24     
- Partials         0        1       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bckohan
Copy link
Member

bckohan commented Mar 19, 2025

bug confirmed #102

@bckohan
Copy link
Member

bckohan commented Mar 19, 2025

Thank you for the report and contribution @crgwbr!

I'm going to move your _coerce_to_value_type implementation over to the form class. This is because we delegate all EnumField construction logic to the Enum's constructor. Custom construction logic may be implemented in Enum._missing_ - it could be the case that an enum decides to do something different with a sequence of values and users would not expect setting the model field to behave differently. The bug really resides in the django_enum.forms.EnumFlagField class.

@bckohan bckohan merged commit b13be16 into django-commons:main Mar 19, 2025
55 checks passed
@crgwbr
Copy link
Contributor Author

crgwbr commented Mar 21, 2025

Thanks for improving and merging this, @bckohan! Would you possibly be able to cut a patch release with this fix soon? Hoping to remove from my project the temporary monkey patch I used to workaround the bug.

@bckohan
Copy link
Member

bckohan commented Mar 22, 2025

Yes, I'm hoping to get one out this weekend - a few other fixes are in progress wrt how the forms work by default.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants