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

Disabling an enabled feature for specific actor doesn't work #374

Closed
brodock opened this issue Aug 20, 2018 · 6 comments
Closed

Disabling an enabled feature for specific actor doesn't work #374

brodock opened this issue Aug 20, 2018 · 6 comments

Comments

@brodock
Copy link

brodock commented Aug 20, 2018

Here are the specs:

context 'with feature enabled for everyone but for an individual actor' do
  CustomActor = Struct.new(:flipper_id)

  let(:actor) { CustomActor.new(flipper_id: 'CustomActor:5') }
  let(:another_actor) { CustomActor.new(flipper_id: 'CustomActor:10') }

  before do
    described_class.enable(:enabled_feature_flag)

    described_class.disable(:enabled_feature_flag, actor)
  end

  it 'returns false when the same actor is informed' do
    expect(described_class.enabled?(:enabled_feature_flag, actor)).to be_falsey
  end

  it 'returns true when a different actor is informed' do
    expect(described_class.enabled?(:enabled_feature_flag, another_actor)).to be_truthy
  end

  it 'returns true when no actor is informed' do
    expect(described_class.enabled?(:enabled_feature_flag)).to be_truthy
  end
end

The first spec is failing. The expected behavior would be the most specific (disabling a single actor) to take precedence over the globally 'enabled' feature.

@jnunemaker
Copy link
Collaborator

@gmac
Copy link

gmac commented Nov 17, 2018

I find this architecture odd... It seems like gates would have ordinal specificity, as in Boolean > Group > Actor (ordered least to most specific), and each gate would store an access control – be that on, off, or a %; then feature access could be resolved by specificity.

Right now it appears that features can only be represented as enabled in the database, and disabled can only be inferred by omission? That seems very limiting.

@jnunemaker
Copy link
Collaborator

@gmac can you describe your specific use case(s) where disable would be helpful. That always helps me understand and determine if disable is really what you need or if there is another way.

Also, I'm open to pull requests. I've never needed this in all my years of using flipper. When I have looked at it as a feature request for others I remember running into many tricky situations where I wasn't sure what made sense as desired behavior.

@lneicelis
Copy link

I've bumped into this today and I agree that the it's natural to think that the more specific is the switch, the higher priority it should have.

@bkeepers
Copy link
Collaborator

@lneicelis Thanks for your feedback. We have a bunch of work coming in #557 that will pave the way for being able to deny specific users. Follow that issue to stay in the loop.

@sdhull
Copy link

sdhull commented Feb 1, 2024

I think the basic use-case is enabling a feature globally or for a % of users, but allowing individuals to opt-out. It looks like #557 has been going for quite some time, and I'm not entirely certain it offers a simple solution for this (nor am I sure how close it is to landing).

At first I thought simply adding an actors_deactivated attribute would be sufficient. Basically every time you disable a feature for a specific actor, it removes them from the actors and adds them to actors_deactivated (and vice-versa). Then when checking if the feature is active, it would be

(
  globally_active? ||
  in_active_percent?(actor) ||
  in_active_group?(actor) ||
  individually_active?(actor)
) && !deactivated_actor?(actor)

However, I think the tricky bit can be generically described as "deactivating an individual, _then later_ activating a percent, group or global state that would include that user." Consider the following pseudocode:

activate :feature, actors: [user1, vip_user2]
deactivate :feature, actors: [vip_user2]
activate :feature, groups: [:vips]

# vip_user2 is in group :vips, so now, for them ... is it active? inactive?

deactivate :feature, actors: [user1]
activate :feature, percent: 75
# user1 is in the 75% of actors that would be activated, so again: active or inactive?

After writing this all out and thinking about it some more, I still think actors_deactivated (or similar, I'm not married to the name) would be workable for most people, most of the time, with the caveat that once you deactivate an actor, they have to be specifically individually reactivated if you want them to be active again.

Maybe I'll whip up a PR to flesh the idea out a bit more. The behavior would have to be opt-in and backwards compatible. This would be enormously useful at my organization.

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

6 participants