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

Make is_active_for_user consider when everyone is False #498

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

chess-octane
Copy link

This adds a check to the is_active_for_user method to consider when the attribute everyone is False.

Fixes #401

@@ -233,6 +233,9 @@ def is_active_for_user(self, user: AbstractBaseUser) -> bool | None:
if self.everyone:

Choose a reason for hiding this comment

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

maybe want to make this if self.everyone is True?

Copy link

@minhtrangvy minhtrangvy left a comment

Choose a reason for hiding this comment

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

actually we have a usecase where we rely on is_active_for_user NOT respecting the "everyone" attribute in order to create feature flags that are on just for some users and not all

maybe this should be a required param on whether to respect everyone or not, otherwise i could see a lot of issues when people upgrade waffle because of this behavior change

@joshkersey
Copy link

I disagree with @minhtrangvy. This is a bug in the code. The documentation makes it clear what the expectation for the Everyone attribute is:
Everyone: | Globally set the Flag, overriding all other criteria. Leave as Unknown to use other criteria.

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.

is_active_for_user() doesn't respect flag.everyone
4 participants