Merge pull request #854 from consul/feature-flags-tweaks

Feature flags tweaks

app/controllers/admin/settings_controller.rb

@@ -1,7 +1,9 @@
 class Admin::SettingsController < Admin::BaseController
 
   def index
-    @settings = Setting.all
+    all_settings = (Setting.all).group_by { |s| s.feature_flag? }
+    @settings = all_settings[false]
+    @feature_flags = all_settings[true]
   end
 
   def update

app/models/setting.rb

@@ -3,15 +3,22 @@ class Setting < ActiveRecord::Base
 
   default_scope { order(id: :asc) }
 
+  def feature_flag?
+    key.start_with?('feature.')
+  end
+
+  def enabled?
+    feature_flag? && value.present?
+  end
+
   class << self
     def [](key)
-      where(key: key).pluck(:value).first
+      where(key: key).pluck(:value).first.presence
     end
 
     def []=(key, value)
       setting = where(key: key).first || new(key: key)
-      setting.value = value
-      setting.value = nil if setting.value == false
+      setting.value = value.presence
       setting.save!
       value
     end

app/views/admin/settings/index.html.erb

@@ -12,3 +12,23 @@
     </li>
   <% end %>
 </ul>
+
+<h2><%= t("admin.settings.index.feature_flags") %></h2>
+
+<ul class="admin-list">
+  <% @feature_flags.each do |feature_flag| %>
+    <li>
+      <strong><%= t("settings.#{feature_flag.key}") %></strong>
+
+      <div>
+        <%= feature_flag.enabled? ? t("admin.settings.index.features.enabled") : t("admin.settings.index.features.disabled") %>
+      <div>
+
+      <%= form_for(feature_flag, url: admin_setting_path(feature_flag), html: { id: "edit_#{dom_id(feature_flag)}"}) do |f| %>
+
+        <%= f.hidden_field :value, id: dom_id(feature_flag), value: (feature_flag.enabled? ? "" : "active") %>
+        <%= f.submit(t("admin.settings.index.features.#{feature_flag.enabled? ? 'disable' : 'enable'}"), class: "button radius tiny #{feature_flag.enabled? ? 'warning' : 'success'}", data: {confirm: t("admin.actions.confirm")}) %>
+      <% end %>
+    </li>
+  <% end %>
+</ul>

config/i18n-tasks.yml

@@ -114,6 +114,7 @@ ignore_unused:
   - 'admin.users.index.filter*'
   - 'admin.activity.show.filter*'
   - 'admin.comments.index.hidden_*'
+  - 'admin.settings.index.features.*'
   - 'moderation.comments.index.filter*'
   - 'moderation.comments.index.order*'
   - 'moderation.debates.index.filter*'

config/locales/admin.en.yml

@@ -56,7 +56,7 @@ en:
       moderators: Moderators
       officials: Officials
       organizations: Organisations
-      settings: General settings
+      settings: Configuration settings
       spending_proposals: Spending proposals
       stats: Statistics
     moderators:
@@ -121,8 +121,14 @@ en:
       flash:
         updated: Value updated
       index:
-        title: General settings
+        title: Configuration settings
         update_setting: Update
+        feature_flags: Features
+        features:
+          enabled: "Feature enabled"
+          disabled: "Feature disabled"
+          enable: "Enable"
+          disable: "Disable"
     shared:
       proposal_search:
         button: Search

config/locales/admin.es.yml

@@ -123,6 +123,12 @@ es:
       index:
         title: Configuración global
         update_setting: Actualizar
+        feature_flags: Funcionalidades
+        features:
+          enabled: "Funcionalidad activada"
+          disabled: "Funcionalidad desactivada"
+          enable: "Activar"
+          disable: "Desactivar"
     shared:
       proposal_search:
         button: Buscar

config/locales/settings.en.yml

@@ -11,4 +11,7 @@ en:
     proposal_code_prefix: "Prefix for Proposal codes"
     votes_for_proposal_success: "Number of votes necessary for approval of a Proposal"
     email_domain_for_officials: "Email domain for public officials"
-    per_page_javascript: "Code to be included on every page"
+    per_page_code: "Code to be included on every page"
+    feature:
+      debates: Debates
+      spending_proposals: Spending proposals

config/locales/settings.es.yml

@@ -12,3 +12,6 @@ es:
     votes_for_proposal_success: "Número de votos necesarios para aprobar una Propuesta"
     email_domain_for_officials: "Dominio de email para cargos públicos"
     per_page_code: "Código a incluir en cada página"
+    feature:
+      debates: Debates
+      spending_proposals: Propuestas de gasto

spec/features/admin/feature_flags_spec.rb

@@ -0,0 +1,63 @@
+require 'rails_helper'
+
+feature 'Admin feature flags' do
+
+  background do
+    login_as(create(:administrator).user)
+  end
+
+  scenario 'Enabled features are listed on menu' do
+    visit admin_root_path
+
+    within('#admin_menu') do
+      expect(page).to have_link "Spending proposals"
+      expect(page).to have_link "Hidden debates"
+    end
+  end
+
+  scenario 'Disable a feature' do
+    setting_id = Setting.find_by(key: 'feature.spending_proposals').id
+
+    visit admin_settings_path
+
+    within("#edit_setting_#{setting_id}") do
+      expect(page).to have_button "Disable"
+      expect(page).to_not have_button "Enable"
+      click_button 'Disable'
+    end
+
+    visit admin_root_path
+
+    within('#admin_menu') do
+      expect(page).not_to have_link "Spending proposals"
+    end
+
+    expect{ visit spending_proposals_path }.to raise_exception(FeatureFlags::FeatureDisabled)
+    expect{ visit admin_spending_proposals_path }.to raise_exception(FeatureFlags::FeatureDisabled)
+  end
+
+  scenario 'Enable a disabled feature' do
+    Setting['feature.spending_proposals'] = nil
+    setting_id = Setting.find_by(key: 'feature.spending_proposals').id
+
+    visit admin_root_path
+
+    within('#admin_menu') do
+      expect(page).not_to have_link "Spending proposals"
+    end
+
+    visit admin_settings_path
+
+    within("#edit_setting_#{setting_id}") do
+      expect(page).to have_button "Enable"
+      expect(page).to_not have_button "Disable"
+      click_button 'Enable'
+    end
+
+    visit admin_root_path
+
+    within('#admin_menu') do
+      expect(page).to have_link "Spending proposals"
+    end
+  end
+end

spec/models/setting_spec.rb

@@ -16,4 +16,42 @@ describe Setting do
   it "should persist a setting on the db" do
     expect(Setting.where(key: 'official_level_1_name', value: 'Stormtrooper')).to exist
   end
+
+  describe "#feature_flag?" do
+    it "should be true if key starts with 'feature.'" do
+      setting = Setting.create(key: 'feature.whatever')
+      expect(setting.feature_flag?).to eq true
+    end
+
+    it "should be false if key does not start with 'feature.'" do
+      setting = Setting.create(key: 'whatever')
+      expect(setting.feature_flag?).to eq false
+    end
+  end
+
+  describe "#enabled?" do
+    it "should be true if feature_flag and value present" do
+      setting = Setting.create(key: 'feature.whatever', value: 1)
+      expect(setting.enabled?).to eq true
+
+      setting.value = "true"
+      expect(setting.enabled?).to eq true
+
+      setting.value = "whatever"
+      expect(setting.enabled?).to eq true
+    end
+
+    it "should be false if feature_flag and value blank" do
+      setting = Setting.create(key: 'feature.whatever')
+      expect(setting.enabled?).to eq false
+
+      setting.value = ""
+      expect(setting.enabled?).to eq false
+    end
+
+    it "should be false if not feature_flag" do
+      setting = Setting.create(key: 'whatever', value: "whatever")
+      expect(setting.enabled?).to eq false
+    end
+  end
 end