jclem / detour Goto Github PK

These all seem redundant:

  t.string  :group_type
  t.string  :percentage_type
  t.string  :flag_subject_type

I know it's nice to use the polymorphic keyword, but isn't there a better way? Only one of these would exist at a time.

For both group management and flag-ins/opt-outs, the user should not have to click an "Add" button. Rather, if all fields have a value, add another one automatically.

class User < ActiveRecord::Base
  acts_as_flaggable find_by: :email
end

Then, in the rake task:

$ rake rollout:activate[foo,User,[email protected]]

Currently, ActiveRecord::Rollout::Feature.all-with_lines can only detect feature checks of these forms:

foo.has_feature? :foo
foo.has_feature?(:foo)

Meaning that forms such as these will not register (with our without parentheses):

foo.has_feature? "foo"
foo.has_feature? "Feature Name"

Not sure if this is a bug, or if it should just validate that feature names be composed of alphanumeric characters and underscores. It's easy enough to at least update the regex to also find:

foo.has_feature? "foo_bar"

Should read "Whoops! There were some errors saving your group:"

Should read "Whoops! There were some errors saving your feature:"

Should read "Percentage must be..."

In an initializer:

ActiveRecord::Rollout.configure do |config|
  config.default_flaggable_type = "User"
end

Then, in a rake task (paired with #10):

$ rake rollout:activate[foo,[email protected]]
$ rake rollout:activate_group[foo,admins]

This could also be assumed if only a single class implements acts_as_flaggable.

Allow support for:

ActiveRecord::Rollout.any_have_feature?(current_user, current_user.organization)

and

ActiveRecord::Rollout.all_have_feature?(current_user, current_user.organization)

Update:

What's probably more valuable is a check that returns true if any of the arguments have the feature and none of them are opted out.

Before:

class User < ActiveRecord::Base
  include ActiveRecord::Rollout::Flaggable
end

After:

class User < ActiveRecord::Base
  acts_as_flaggable
end

There are currently minimal database indices defined in the migration. Need to take a second look at these and make sure we're not querying anything frequently that's not indexed.

For the same instance of Flaggable, calling has_feature?(:foo) multiple times should only actually check for :foo once. They should be memoized in an @active_record_rollout_features instance variable.

I should be able to call current_user.features and get all features for that user.