From 64441825d859ea9b690ee2b0397ef22cd9fcd3ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baz=C3=A1n?= Date: Mon, 25 Jan 2016 12:04:40 +0100 Subject: [PATCH 1/7] renames settings zone in admin --- config/locales/admin.en.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config/locales/admin.en.yml b/config/locales/admin.en.yml index de329a39e..d1e7e20e2 100755 --- a/config/locales/admin.en.yml +++ b/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,7 +121,7 @@ en: flash: updated: Value updated index: - title: General settings + title: Configuration settings update_setting: Update shared: proposal_search: From 853f2c6fbaaecaf1da3bc9fd13de3c5dc7667ca7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baz=C3=A1n?= Date: Mon, 25 Jan 2016 12:44:00 +0100 Subject: [PATCH 2/7] fixes disabling of features via admin interface --- app/models/setting.rb | 5 +- spec/features/admin/feature_flags_spec.rb | 61 +++++++++++++++++++++++ 2 files changed, 63 insertions(+), 3 deletions(-) create mode 100644 spec/features/admin/feature_flags_spec.rb diff --git a/app/models/setting.rb b/app/models/setting.rb index b8b29a645..b6df3daef 100644 --- a/app/models/setting.rb +++ b/app/models/setting.rb @@ -5,13 +5,12 @@ class Setting < ActiveRecord::Base 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 diff --git a/spec/features/admin/feature_flags_spec.rb b/spec/features/admin/feature_flags_spec.rb new file mode 100644 index 000000000..b4d2a006c --- /dev/null +++ b/spec/features/admin/feature_flags_spec.rb @@ -0,0 +1,61 @@ +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 + fill_in "setting_#{setting_id}", with: '' + click_button 'Update' + 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 + fill_in "setting_#{setting_id}", with: 'true' + click_button 'Update' + end + + visit admin_root_path + + within('#admin_menu') do + expect(page).to have_link "Spending proposals" + end + end +end \ No newline at end of file From 05837afd72625cadb1fbe0d83c552393ca975c98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baz=C3=A1n?= Date: Mon, 25 Jan 2016 13:45:15 +0100 Subject: [PATCH 3/7] adds #feature_flag? to Setting --- app/models/setting.rb | 4 ++++ spec/models/setting_spec.rb | 12 ++++++++++++ 2 files changed, 16 insertions(+) diff --git a/app/models/setting.rb b/app/models/setting.rb index b6df3daef..2d7d8d43c 100644 --- a/app/models/setting.rb +++ b/app/models/setting.rb @@ -3,6 +3,10 @@ class Setting < ActiveRecord::Base default_scope { order(id: :asc) } + def feature_flag? + key.start_with?('feature.') + end + class << self def [](key) where(key: key).pluck(:value).first.presence diff --git a/spec/models/setting_spec.rb b/spec/models/setting_spec.rb index 1344b0c7d..99234de95 100644 --- a/spec/models/setting_spec.rb +++ b/spec/models/setting_spec.rb @@ -16,4 +16,16 @@ 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 end From bb60e6b8e0aead257512aa87318c024e7e813fce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baz=C3=A1n?= Date: Mon, 25 Jan 2016 14:07:07 +0100 Subject: [PATCH 4/7] adds #enabled? to Setting --- app/models/setting.rb | 4 ++++ spec/models/setting_spec.rb | 26 ++++++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/app/models/setting.rb b/app/models/setting.rb index 2d7d8d43c..40659ed74 100644 --- a/app/models/setting.rb +++ b/app/models/setting.rb @@ -7,6 +7,10 @@ class Setting < ActiveRecord::Base key.start_with?('feature.') end + def enabled? + feature_flag? && value.present? + end + class << self def [](key) where(key: key).pluck(:value).first.presence diff --git a/spec/models/setting_spec.rb b/spec/models/setting_spec.rb index 99234de95..c5e8e224a 100644 --- a/spec/models/setting_spec.rb +++ b/spec/models/setting_spec.rb @@ -28,4 +28,30 @@ describe Setting do 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 From c287ae97d9309321f922f267b65459b326e8831c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baz=C3=A1n?= Date: Mon, 25 Jan 2016 14:12:16 +0100 Subject: [PATCH 5/7] fix setting key --- config/locales/settings.en.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/locales/settings.en.yml b/config/locales/settings.en.yml index 4b0cc2f77..0f784bce4 100755 --- a/config/locales/settings.en.yml +++ b/config/locales/settings.en.yml @@ -11,4 +11,4 @@ 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" From 7931ae914bcedeb0226defed230f3aff58a87962 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baz=C3=A1n?= Date: Mon, 25 Jan 2016 14:52:57 +0100 Subject: [PATCH 6/7] adds specific treatment for feature flags in admin --- app/controllers/admin/settings_controller.rb | 4 +++- app/views/admin/settings/index.html.erb | 20 ++++++++++++++++++++ config/locales/admin.en.yml | 6 ++++++ config/locales/admin.es.yml | 6 ++++++ config/locales/settings.en.yml | 3 +++ config/locales/settings.es.yml | 3 +++ spec/features/admin/feature_flags_spec.rb | 10 ++++++---- 7 files changed, 47 insertions(+), 5 deletions(-) diff --git a/app/controllers/admin/settings_controller.rb b/app/controllers/admin/settings_controller.rb index b369d2cbc..387f38177 100644 --- a/app/controllers/admin/settings_controller.rb +++ b/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 diff --git a/app/views/admin/settings/index.html.erb b/app/views/admin/settings/index.html.erb index f3b54b3be..be796d0c0 100644 --- a/app/views/admin/settings/index.html.erb +++ b/app/views/admin/settings/index.html.erb @@ -12,3 +12,23 @@ <% end %> + +

<%= t("admin.settings.index.feature_flags") %>

+ +
    + <% @feature_flags.each do |feature_flag| %> +
  • + <%= t("settings.#{feature_flag.key}") %> + +
    + <%= feature_flag.enabled? ? t("admin.settings.index.features.enabled") : t("admin.settings.index.features.disabled") %> +
    + + <%= 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 %> +
    • + <% end %> +
diff --git a/config/locales/admin.en.yml b/config/locales/admin.en.yml index d1e7e20e2..d9fcd0e2e 100755 --- a/config/locales/admin.en.yml +++ b/config/locales/admin.en.yml @@ -123,6 +123,12 @@ en: index: 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 diff --git a/config/locales/admin.es.yml b/config/locales/admin.es.yml index ff056cb83..d71cd7b0b 100644 --- a/config/locales/admin.es.yml +++ b/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 diff --git a/config/locales/settings.en.yml b/config/locales/settings.en.yml index 0f784bce4..c51eb7831 100755 --- a/config/locales/settings.en.yml +++ b/config/locales/settings.en.yml @@ -12,3 +12,6 @@ en: votes_for_proposal_success: "Number of votes necessary for approval of a Proposal" email_domain_for_officials: "Email domain for public officials" per_page_code: "Code to be included on every page" + feature: + debates: Debates + spending_proposals: Spending proposals diff --git a/config/locales/settings.es.yml b/config/locales/settings.es.yml index 48fa493eb..98ff6ae39 100644 --- a/config/locales/settings.es.yml +++ b/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 diff --git a/spec/features/admin/feature_flags_spec.rb b/spec/features/admin/feature_flags_spec.rb index b4d2a006c..63c4f1d64 100644 --- a/spec/features/admin/feature_flags_spec.rb +++ b/spec/features/admin/feature_flags_spec.rb @@ -21,8 +21,9 @@ feature 'Admin feature flags' do visit admin_settings_path within("#edit_setting_#{setting_id}") do - fill_in "setting_#{setting_id}", with: '' - click_button 'Update' + expect(page).to have_button "Disable" + expect(page).to_not have_button "Enable" + click_button 'Disable' end visit admin_root_path @@ -48,8 +49,9 @@ feature 'Admin feature flags' do visit admin_settings_path within("#edit_setting_#{setting_id}") do - fill_in "setting_#{setting_id}", with: 'true' - click_button 'Update' + expect(page).to have_button "Enable" + expect(page).to_not have_button "Disable" + click_button 'Enable' end visit admin_root_path From 3945eb1ff069004e867db7e12a01a9ddd90873e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baz=C3=A1n?= Date: Mon, 25 Jan 2016 14:59:37 +0100 Subject: [PATCH 7/7] lists dynamic keys as ignore_unused in i18n-tasks --- config/i18n-tasks.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/config/i18n-tasks.yml b/config/i18n-tasks.yml index 9941114f8..05355ec7c 100644 --- a/config/i18n-tasks.yml +++ b/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*'