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

Add rake tasks to manage features #30

Merged

Conversation

kevinnio
Copy link
Contributor

@kevinnio kevinnio commented May 4, 2019

Hey guys, I noticed Flipflop doesn't have a way to manage features via console out of the box, so here's this PR built from work I had to do in one of my projects.

This adds a set of rake tasks that allow the user to control features the same way they can currently do it in the Dashboard.

These are the new tasks:

rake flipflop:features                    # Shows features table
rake flipflop:turn_on[feature,strategy]   # Enables a feature with the specified strategy
rake flipflop:turn_off[feature,strategy]  # Disables a feature with the specified strategy

For example, let's say you have a config/features.rb file like this:

Flipflop.configure do
  strategy :cookie
  strategy :active_record
  strategy :default

  feature :world_domination, default: false
  feature :peace_among_worlds, default: false
end

To enable :peace_among_worlds using :active_record you would do:

bundle exec rake flipflop:turn_on[peace_among_worlds,active_record]
# => Feature :peace_among_worlds enabled!

To disable :world_domination using :cookie you would do:

bundle exec rake flipflop:turn_off[world_domination,cookie]
# => Feature :world_domination disabled!

To see the current state of your features you would do:

bundle exec rake flipflop:features

# => ┌──────────────────┬───────────┬──────┬─────────────┬───────┐
# => │feature           │description│cookie│active_record│default│
# => ├──────────────────┼───────────┼──────┼─────────────┼───────┤
# => │world_domination  │           │OFF   │             │OFF    │
# => ├──────────────────┼───────────┼──────┼─────────────┼───────┤
# => │peace_among_worlds│           │      │ON           │OFF    │
# => └──────────────────┴───────────┴──────┴─────────────┴───────┘

Thanks for such a great gem, guys!

@kevinnio kevinnio force-pushed the add-rake-tasks-to-flipflop-features branch 2 times, most recently from 15f7c67 to 2e96260 Compare May 5, 2019 02:07
@kevinnio
Copy link
Contributor Author

kevinnio commented May 5, 2019

@rolftimmermans If you could do a quick review of this, I'll appreciate it. There's also #27 waiting for review. 😉

@rolftimmermans
Copy link
Member

Love this idea! I wonder how to deal with strategies that cannot be changed via the console though, especially those that require a request context (like session & cookie strategies).

Another concern I have is the new gem dependency; I prefer to have minimal dependencies where possible; tty-table brings in 5 more gem dependencies on its own.

Anyway, I would love to see this implemented so if you have any thoughts on this I would love to hear it!

@kevinnio
Copy link
Contributor Author

kevinnio commented May 6, 2019

Thanks for the quick reply, @rolftimmermans.

I added tty-table as a dependency to render the features table more easily, but I guess I can found another way to do it without introducing so many new gems to the project.

About the other strategies, I originally built this for a project that was using only :default and :active_record so I didn't pay much attention to non-switchable ones like :lambda or the ones that depend on request. From the top of my head I'd say we exclude such strategies from console management, but I want to review the code and see if I can make this work somehow. I'll post an update later this week.

@rolftimmermans
Copy link
Member

Just looked at the code for the strategies in more detail and it seems all request-based strategies will default to nil for all features when no request is present. So there is no immediate need to exclude them. Also the switchable? method will return false so if you check that before attempting to set a feature value (and display an error appropriately) then everything should be fine.

@kevinnio kevinnio force-pushed the add-rake-tasks-to-flipflop-features branch from 2e96260 to 38ef23d Compare May 7, 2019 17:02
kevinnio added 3 commits May 7, 2019 12:04
Flipflop already provides a GUI to manage feature statuses. This adds
rake tasks that allow the user to do the same from the console.
Not all strategies are switchable. This adds a check to see if the
strategy is switchable before trying to switch on/off a feature using
the new rake tasks.
@kevinnio kevinnio force-pushed the add-rake-tasks-to-flipflop-features branch from 38ef23d to 130531a Compare May 7, 2019 17:04
@kevinnio
Copy link
Contributor Author

kevinnio commented May 7, 2019

@rolftimmermans I replaced tty-table with the most lightweight replacement I could find. Also added a check for switchable?.

What do you think?

@rolftimmermans
Copy link
Member

Starting to look good. The only remaining comment I have when looking at the code is that features are 'ternary' – they can be explicitly enabled (true), explicitly disabled (false) or unset (nil) by a strategy. If you look at the dashboard you see that they can be on/off, or cleared. Right now the code only checks for truthiness but I think it should display one of three states, perhaps ON for true, OFF for false and - for nil. Does that make sense?

@kevinnio
Copy link
Contributor Author

kevinnio commented May 9, 2019

Yeah, totally makes sense. I'll check for nil and produce the respective output.

By the way, do you think adding a rake flipflop:unset[feature,strategy] is a good idea?

When printing the features table using rake flipflop:features, this
shows an empty whenever cell a feature in unset for a strategy.
@rolftimmermans
Copy link
Member

Unsetting/clearing does seem potentially useful. I think the dashboard refers to this as clearing a feature.

@kevinnio
Copy link
Contributor Author

OK, then. I'll add a rake flipflop:clear[feature,strategy] task as soon as I can. Thanks.

This task replicates the "Clear" feature from the dashboard.
With it, a feature can be cleared (unset) using the specified
strategy.
@kevinnio
Copy link
Contributor Author

Added the clear task. Working on some unit tests for all of this...

@kevinnio kevinnio force-pushed the add-rake-tasks-to-flipflop-features branch from 788187f to fd19b5a Compare May 11, 2019 06:16
@kevinnio kevinnio force-pushed the add-rake-tasks-to-flipflop-features branch 2 times, most recently from 9d52b71 to 1e9aa01 Compare May 11, 2019 06:48
@rolftimmermans
Copy link
Member

LGTM. Would you say it's ready?

@kevinnio kevinnio force-pushed the add-rake-tasks-to-flipflop-features branch from 1e9aa01 to d433525 Compare May 12, 2019 16:00
@kevinnio
Copy link
Contributor Author

Yeah, totally. Ready to merge whenever you are!

@rolftimmermans rolftimmermans merged commit 687ea8b into voormedia:master May 13, 2019
@kevinnio kevinnio deleted the add-rake-tasks-to-flipflop-features branch May 13, 2019 13:58
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

Successfully merging this pull request may close these issues.

2 participants