fetlife/rollout

deactivate_user does not deactivate if feature was activated using activate_group ?

murtuzakz opened this issue ยท 12 comments

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.

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

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

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

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

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

$rollout.activate_group(:chat, :group_including_user) # user should be active
$rollout.deactivate_user(:chat, user) # adds an entry in redis saying current user has the flag off
$rollout.activate_group(:chat, :another_group_including_user) # adds an entry saying another_group_including_user has the flag on
$rollout.active?(:chat, user) # finds the specific entry for the user and prioritizes that over global and group entries returns false.

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.