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.

Sign up for GitHub

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

Jump to bottom

CREATE_MISSING_FLAGS should be handled in .get() instead of is_active() #400

Open
honzakral opened this issue Jun 22, 2021 · 3 comments · Fixed by #427
Open

CREATE_MISSING_FLAGS should be handled in .get() instead of is_active() #400

honzakral opened this issue Jun 22, 2021 · 3 comments · Fixed by #427

Comments

@honzakral
Copy link
Contributor

I am using the Flags in other scenarios aside from just views (celery tasks for example) and I was surprised that the CREATE_MISSING_FLAGS setting wasn't respected (I am manually calling is_active_for_user and other, custom, methods I created based on the example) and the methods were failing with error:

ValueError: "<CustomFlag: flag_name>" needs to have a value for field "id" before this many-to-many relationship can be used.

In my mind there should be a way to check if a flag is active using similar mechanism to waffle.flag_is_active which would work and respect the CREATE_MISSING_FLAGS setting, simplest solution seems to move the logic from is_active to get(). I would be happy to submit a PR if this would be considered a useful addition.
@btel
Copy link

btel commented Mar 2, 2022

I have same issue. The solution suggested by @honzakral seems reasonable. Is there any interest in such a PR?

@clintonb
Copy link
Collaborator

clintonb commented Mar 2, 2022

Yes, PRs are welcome.

honzakral added a commit to honzakral/django-waffle that referenced this issue Mar 2, 2022
@honzakral
Make CREATE_MISSING_FLAGS logic universal (jazzband#400)
fff498f 
Before CREATE_MISSING_FLAGS was only respected when `is_active(request)`
method was used, now any check against a flag (calling `get(name)`) will
trigger the automatic creation of the `Flag` model.
@honzakral honzakral mentioned this issue Mar 2, 2022
Merged
honzakral added a commit to honzakral/django-waffle that referenced this issue Mar 3, 2022
@honzakral
Make CREATE_MISSING_FLAGS logic universal (jazzband#400)
35ba0e6 
Before CREATE_MISSING_FLAGS was only respected when `is_active(request)`
method was used, now any check against a flag (calling `get(name)`) will
trigger the automatic creation of the `Flag` model.
clintonb pushed a commit that referenced this issue Mar 3, 2022
@honzakral
Make CREATE_MISSING_FLAGS logic universal (#400) (#427)
947195b 
Before CREATE_MISSING_FLAGS was only respected when `is_active(request)` method was used, now any check against a flag (calling `get(name)`) will trigger the automatic creation of the `Flag` model.
clintonb added a commit that referenced this issue Mar 28, 2022
@clintonb
Revert "Make CREATE_MISSING_FLAGS logic universal (#400) (#427)"
47ec8b9 
This reverts commit 947195b.
clintonb added a commit that referenced this issue Mar 28, 2022
@clintonb
Revert "Make CREATE_MISSING_FLAGS logic universal (#400) (#427)" (#436)
856644e 
This reverts commit 947195b.
@clintonb
Copy link
Collaborator

This feature was reverted. See #435 for some details. Please modify the logic to ensure it supports flags, switches, and samples.

@clintonb clintonb reopened this Mar 28, 2022
honzakral added a commit to honzakral/django-waffle that referenced this issue Mar 28, 2022
@honzakral
Make CREATE_MISSING_FLAGS logic universal (jazzband#400) (jazzband#427)
9331bc4 
Before CREATE_MISSING_FLAGS was only respected when `is_active(request)` method was used, now any check against a flag (calling `get(name)`) will trigger the automatic creation of the `Flag` model.
@honzakral honzakral mentioned this issue Mar 28, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@honzakral @btel @clintonb