-
Notifications
You must be signed in to change notification settings - Fork 211
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
deactivate_user does not deactivate if feature was activated using activate_group ? #87
Comments
I had the same issue. I suppose that this is is the behaviour right now but I do believe that a black-list approach would be great. |
This doesn't seem like too much of a bug. If you've decided to activate a feature for everyone you probably shouldn't be deactivating it on a per user basis. |
I'd also love to see this fixed. It just doesn't make sense to model things this way. I think you should be able to disable a single user even if they are in a group. |
+1 |
+1 This is a common use case. Imagine you enable a feature for all of your users and then you figure out it's giving problems to only one user. I don't want to turn everyone off. |
@reneklacan thoughts? |
+1 Any plans to address this? @reneklacan @johnbaku |
@AhmedBelal I hit up @reneklacan and he will check it out! Sorry about that! |
This is probably a lot more complicated that it looks like. Mentioned use case is clear: $rollout.activate_group(:chat, :all)
$rollout.deactivate_user(:chat, user)
$rollout.active?(:chat, user) # should be true but isn't But what if you consider $rollout.activate_group(:chat, :group_including_user)
$rollout.deactivate_user(:chat, user)
$rollout.activate_group(:chat, :another_group_including_user)
$rollout.active?(:chat, user) # what should outcome be? As you know groups can be overlapping so what should you do in this case? Should you still keep feature disabled for user based on Sames applies to $rollout.activate_group(:chat, :group_including_user)
$rollout.deactivate_user(:chat, user)
$rollout.activate_group(:chat, :all)
$rollout.active?(:chat, user) # what should outcome be? Or slightly different use case: let's say you enable feature for specific user to test it, then you disable it and you go make some final tweaks, then you know it's ready and you enable it for everybody $rollout.activate_user(:chat, user)
$rollout.deactivate_user(:chat, user)
$rollout.activate_group(:chat, :all)
$rollout.active?(:chat, user) # what should outcome be? I'm kind of afraid this would introduce even more unexpected behaviour if it's not well thought out. How do you think this should work? |
@alfonsocora
If feature is enabled for all, you can deprecate group rule and only use per-user flags probably? |
$rollout.activate_group(:chat, :group_including_user) # user should be active
$rollout.deactivate_user(:chat, user) # overrides the current state so user should be deactivated, all other members active
$rollout.activate_group(:chat, :another_group_including_user) # overrides the current state so user should be active
$rollout.active?(:chat, user) # return true $rollout.activate_group(:chat, :group_including_user) #user should be active
$rollout.deactivate_user(:chat, user) #overrides the current state so user should be deactivated
$rollout.activate_group(:chat, :all) # overrides the current state so user should be active
$rollout.active?(:chat, user) # what should outcome be? WDYT ? |
I'm rather fond of a transparencies approach like they used to use for presentations before powerpoint. We layer up settings and the one on top can cover bits of the ones below. Another way to say this is most specific wins: User > Group > Global
This also alleviates the need for extensive callbacks to find every activation/de-activation state for each user in a group that gets a flag toggled. |
Maybe I'm using it wrong, or its mentioned somewhere in the docs, and I have not read it properly.
But if you do this :
The text was updated successfully, but these errors were encountered: