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

deactivate_user does not deactivate if feature was activated using activate_group ? #87

Open
murtuzakz opened this issue Oct 12, 2015 · 11 comments

Comments

@murtuzakz
Copy link

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 :

user = User.first
$rollout.activate_group(:chat, :all)      # You'd assume this activates feature for everyone, and it does.
$rollout.deactivate_user(:chat, user)  # I'm assuming this should only deactivate the feature for this user, everyone else should still have it active.
$rollout.active?(:chat, user)                # This should return false, but returns true.

@MarceloCajueiro
Copy link

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.

@perfetti
Copy link

perfetti commented Jan 2, 2016

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.

@hachibu
Copy link

hachibu commented Mar 16, 2016

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.

@petrgazarov
Copy link

+1

@alfius
Copy link

alfius commented May 3, 2016

+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.

@johnbaku
Copy link
Member

johnbaku commented May 3, 2016

@reneklacan thoughts?

@AhmedBelal
Copy link

AhmedBelal commented Aug 3, 2016

+1

Any plans to address this? @reneklacan @johnbaku

@johnbaku
Copy link
Member

johnbaku commented Aug 3, 2016

@AhmedBelal I hit up @reneklacan and he will check it out! Sorry about that!

@reneklacan
Copy link
Member

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 deactivate_user?

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?

@zharikovpro
Copy link

zharikovpro commented Oct 10, 2016

@alfonsocora

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.

If feature is enabled for all, you can deprecate group rule and only use per-user flags probably?

@dorongutman
Copy link

@reneklacan

How do you think this should work?
I think it should consider the last action made on a user. So in your examples:

$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 ?

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

No branches or pull requests