Add descriptions support for features

[juanroldan1989]

1. Simple way to provide insights/details about a specific feature within Flipper::UI.

2. Avoids generating feature keys too long when developers/admins/users need more context about a feature's behaviour:

• What is it meant to do ?
• What is it not meant to do ?
• Which sections of the platform it applies to ?
• How it differentiates itself from similar features ?
• What user roles does it apply to ?
• Etc.

[annaswims]

I like this idea, for the most part but Id suggest making the yaml file a bit more generic.

At VA.gov we have a features.yml file and we use it for initializing features in all environments, descriptions, specifying the actor (sometimes we want a toggle for logged in users, sometimes we want a toggle for all users with a cookie value). Our latest addition to the features.yml file is that we want to specify default values for the toggle by environment (i.e. most, but not all features are enabled by default in development when they're initialized).

[juanroldan1989]

I like this idea, for the most part but Id suggest making the yaml file a bit more generic.

At VA.gov we have a features.yml file and we use it for initializing features in all environments, descriptions, specifying the actor (sometimes we want a toggle for logged in users, sometimes we want a toggle for all users with a cookie value). Our latest addition to the features.yml file is that we want to specify default values for the toggle by environment (i.e. most, but not all features are enabled by default in development when they're initialized).

Thank you @annaswims for your feedback! really appreciated : ) adjustments made 🚀

[jnunemaker]

I’ll try to give this a look over tomorrow. Sorry for delay.

[juanroldan1989]

I’ll try to give this a look over tomorrow. Sorry for delay.

Thank you @jnunemaker ! really appreciated : )

[juanroldan1989]

Failing tests are un-related to this PR - There's a separate PR addressing that: #462

[juanroldan1989]

Failing tests are un-related to this PR - There's a separate PR addressing that: #462

PR #462 rebased on this PR - test suite green again : )

[jnunemaker]

@juanroldan1989 you are a fantastic contributor. You've setup a great PR and completed a lot of other work to get things green and help out. Thanks.

My struggle with this PR is that it uses YAML. What if someone wanted to store the descriptions somewhere that someone could update them like mysql or postgres? They'd be out of luck. Part of what took me so long to respond was thinking through what I might do instead.

Flipper::UI.configure do |config|
  # keys would be an Array
  config.description_fetcher = ->(keys) do
    # do your yaml load or query mysql or whatever you want
    # return would be hash of {String key => String description}
  end
end

I don't love the description_fetcher name but I'm lacking a better name currently. description_loader and description_finder were other thoughts. I hate them too. 😄

Something like above would allow you to use yaml and allow anyone else to use whatever they want. I'd rather have native storage of other attributes like description, but I totally get this is useful and want to make it possible for you so I think something like above is what I'd lean toward so it is more flexible.

What are your thoughts? Would you be interested in take a stab at it? If so, do you even have time? If not, let me know and I can try to whip something together. I just want to make sure whatever we do works for everyone who uses flipper rather than on specific use case.

[annaswims]

I think the feature toggles straddle the awkward line between what's code and data. If I wanted to keep a list of all features that may be toggled, I'd lean towards doing that as code. That way, I commit the feature toggle name and description in the same PR that I could write the code to check the feature value. When a new feature is added, I'd prefer to have it added to the interface and defaulted to disabled rather than not visible in the interface at all.

It doesn't seem like the right flow for me for a developer to merge a PR that references a feature toggle, wait for deploy, then add the feature to the interface for staging and prod (or add the feature as a migration).

[juanroldan1989]

@juanroldan1989 you are a fantastic contributor. You've setup a great PR and completed a lot of other work to get things green and help out. Thanks.

Thank you @jnunemaker ! I really appreciate that : ) the tool is such a gem that is really hard not to take care of it honestly 🚀

My struggle with this PR is that it uses YAML. What if someone wanted to store the descriptions somewhere that someone could update them like mysql or postgres? They'd be out of luck. Part of what took me so long to respond was thinking through what I might do instead.

It makes total sense now I think of it.

Honestly, I went with the "descriptions loaded from a YAML file" idea simply because that's how several libraries that I've used over time do it, e.g: devise, figaro, activeadmin, simple_form, wicked, wicked-pdf, money_rails.

Allowing descriptions to be loaded with as many options as possible is key, no pun intended haha, to keep this feature available for everyone who wants to improve their experience using it. I like that! 💯

Flipper::UI.configure do |config|
  # keys would be an Array
  config.description_fetcher = ->(keys) do
    # do your yaml load or query mysql or whatever you want
    # return would be hash of {String key => String description}
  end
end

I don't love the description_fetcher name but I'm lacking a better name currently. description_loader and description_finder were other thoughts. I hate them too. 😄

How about config.descriptions_source = ->(keys) do ? 🤔

Something like above would allow you to use yaml and allow anyone else to use whatever they want. I'd rather have native storage of other attributes like description, but I totally get this is useful and want to make it possible for you so I think something like above is what I'd lean toward so it is more flexible.

Totally agree with this. This is the second company I worked with where the subject comes up at some point and questions emerge within team members:

• "Yeah I can read the key name but ... what was this feature for?"
• "This key is really too long but hey, works for me and the rest of the Admins"
• "Maybe we should add another *_extra_info section for keys to make it more understandable"

That's the reason why I wanted to give it a shot and see how far we could go to solve this issue, or at least provide folks with an option to make things clearer : ) hope that makes sense.

What are your thoughts? Would you be interested in take a stab at it? If so, do you even have time? If not, let me know and I can try to whip something together. I just want to make sure whatever we do works for everyone who uses flipper rather than on specific use case.

I'm not sure where to start regarding setting up more sources for Flipper::UI. More questions come to me than answers:

• How many options to connect should we provide? E.g: Redis, Postgres, MySQL, ...
• Maybe we should define an entry_point for the database within Flipper::UI configuration module?
• Should we define an entry_point_type for connectors, with possible values like: yaml, pg, mysql, redis. This entry_point_type will be used to decided which source read descriptions from.

I would love to see your approach on this and participate on that process 🚀

I really appreciate the feedback on this and all the possibilities involved! Thank you ! 🍻

[jnunemaker]

How about config.descriptions_source

👍 Works for me.

I'm not sure where to start regarding setting up more sources

I wouldn't worry about building all the adapters for different data stores. I'd just focus on adding description_source as a block that takes keys and returns key => description. Then making the UI use that block to get the descriptions to show. Does that simplify things? Just ruby. No yaml or mysql or anything. You could easily add an example in the examples/ui folder that shows how to use it with yaml or something if you wanted.

[jnunemaker]

For example, once the descriptions_source is in Flipper::UI with a default empty block, you could do this in the features action:

module Flipper
  module UI
    module Actions
      class Features < UI::Action
        route %r{\A/features/?\Z}

        def get
          @page_title = 'Features'
          descriptions = Flipper::UI.configuration.description_source(flipper.features.map(&:key))
          @features = flipper.features.map do |feature|
            decorator = Decorators::Feature.new(feature)
            decorator.description = descriptions[feature.key]
            decorator
          end.sort
  # ... clipped for brevity ...

You could then do the same thing for showing a feature. You would need an attr_accessor :description on the feature decorator or something like that.

Does that help?

[juanroldan1989]

I'd just focus on adding description_source as a block that takes keys and returns key => description. Then making the UI use that block to get the descriptions to show.

@jnunemaker oooh now I get the approach : ) we'd be providing Flipper::UI with just data, no matter its origin, so we can read that data and show it whenever is need it, gotcha 👌

Yes! that helps for sure 🚀 will take a stab at it tonight and see how it goes. Thank you !

[juanroldan1989]

@jnunemaker I've added the latest changes 🚀

While working on these, one question came up:

Should we or should we not include a default message whenever a description is not found within whatever source provided for a specific key?

Currently we show Description not provided message in both feature and features pages.

Leaving this as is now, whenever a developer updates the gem to its most up to date version, all of a sudden such message would pop up on features listing and feature pages.

The positive side of this would be: developers realising they can now add/edit descriptions for features, prompting them to go to the docs and see how it can be done. (besides of course acknowledging the fact that developers using this gem for the first time would read docs about how the gem works).

The not-so-positive side of this would be: for projects with the gem already implemented, in case developers/users/admins don't like the Description not provided message to be displayed, they would need to provide let's say a - content for keys they don't need a description for.

Perhaps, we could just leave this default description as an empty string and expect/hope/know-for-sure developers in need of descriptions for their features will read the docs and find out about the existing configuration to provide them ?

Anyway, food for thought : )

[hstebbins-dailypay]

Love this idea, thanks for putting in the work to get this PR up. Is there anything I can help with to get it over the finish line?

Not that my opinion matters much, but an empty string makes more sense to me as a default. It makes it feel more like an optional feature rather than something that's required/expected, if that makes sense.

Looking forward to upgrading ASAP once this is merged 🙂

[juanroldan1989]

@hstebbins-dailypay thanks for your feedback ! greatly appreciated as well : D

I've made up my mind thinking again about that 🚀

I've just updated the PR so whenever a key does not have a description associated no message is displayed.

This way developers will only need to care about keys/features they do want to have a description available for.

[jnunemaker]

This is looking a lot better! Thanks for the tweaks. I still have some questions, but I feel like this is close. My primary concerns are 1) the difference between docs and code and 2) forcing users to restart/redeploy to get updated descriptions.

[jnunemaker]

A few parts weren't actually wired up right in the specs and the UI but it was close enough so I pulled it down, got it working and merged it. Thanks for all your help! I'll release and blog about it soon hopefully.

[juanroldan1989]

A few parts weren't actually wired up right in the specs and the UI but it was close enough so I pulled it down, got it working and merged it. Thanks for all your help! I'll release and blog about it soon hopefully.

Thank you @jnunemaker ! I was hoping it was getting there haha 🎊 I really appreciate your guidance and insights on this feature : D

Kind Regards 🍻