flipper icon indicating copy to clipboard operation
flipper copied to clipboard

Add #features_for_actor to return all enabled features for a given actor

Open MagoMathew opened this issue 1 year ago • 6 comments
trafficstars

Adds #features_for_actor

With this change, you can get all features for the given actor as a param.

I've found we need that feature on my team, where we just load all the features once.

Even tho, the name could be just features_for but wasn't sure about it.

Please feel free to share your comments and thoughts, Probably there's a better way to do it.

Thanks for the great work.

MagoMathew avatar Dec 07 '23 15:12 MagoMathew

@jnunemaker My approach could not be the most performant way to do what the PR does, so please feel free to suggest/apply any changes to make it work better.

I'm sure there's a way to get all the enabled features for a given actor with a single call to the DB, instead of calling one by one the features with the enabled? method.

MagoMathew avatar Dec 18 '23 09:12 MagoMathew

@MagoMathew thanks! This looks good. I've been mostly off for the holidays. Sorry for slow response.

Only thing I'm trying to decide on is if we should roll with what you have or maybe something like Flipper.features.enabled?(...). I'd have to make a proxy for that, so probably not worth it.

The other thought is maybe something that does Flipper.features_for_actor(...) but returns a Hash with each feature key as the key and true/false as the value.

@bkeepers any feelings on this method name or other ideas? If not, let me know and we can just merge as is.

jnunemaker avatar Jan 03 '24 22:01 jnunemaker

The other thought is maybe something that does Flipper.features_for_actor(...) but returns a Hash with each feature key as the key and true/false as the value

Hi @jnunemaker

What you propose above is actually what I needed on my project. But I solved it in a different way,

On my actor I have this method

 def features
    sql = "SELECT feature_key  FROM flipper_gates WHERE value = '#{self.flipper_id}'"
    res = ActiveRecord::Base.connection_pool.with_connection { |con| con.exec_query(sql) }.rows.flatten.map(&:to_sym)
    ::Feature.all.map { |f| { key: f.key, active: res.include?(f.key) } }
  end  

Do you think is a better/cleaner way to do it? Regards

MagoMathew avatar Jan 04 '24 11:01 MagoMathew