From e6127decac0d120d242eb6a0789aecb2afa575d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 20 Aug 2021 21:33:48 +0200 Subject: [PATCH 1/7] Move featured settings form partial to a component --- .../settings/featured_settings_form_component.html.erb} | 2 +- .../admin/settings/featured_settings_form_component.rb | 8 ++++++++ .../admin/settings/_featured_settings_table.html.erb | 2 +- 3 files changed, 10 insertions(+), 2 deletions(-) rename app/{views/admin/settings/_featured_settings_form.html.erb => components/admin/settings/featured_settings_form_component.html.erb} (88%) create mode 100644 app/components/admin/settings/featured_settings_form_component.rb diff --git a/app/views/admin/settings/_featured_settings_form.html.erb b/app/components/admin/settings/featured_settings_form_component.html.erb similarity index 88% rename from app/views/admin/settings/_featured_settings_form.html.erb rename to app/components/admin/settings/featured_settings_form_component.html.erb index ea9c3732a..84b3c5f73 100644 --- a/app/views/admin/settings/_featured_settings_form.html.erb +++ b/app/components/admin/settings/featured_settings_form_component.html.erb @@ -1,5 +1,5 @@ <%= form_for(feature, url: admin_setting_path(feature), html: { id: "edit_#{dom_id(feature)}" }) do |f| %> - <%= f.hidden_field :tab, value: tab if defined?(tab) %> + <%= f.hidden_field :tab, value: tab if tab %> <%= f.hidden_field :value, id: dom_id(feature), value: (feature.enabled? ? "" : "active") %> <%= f.submit(t("admin.settings.index.features.#{feature.enabled? ? "disable" : "enable"}"), class: "button expanded #{feature.enabled? ? "hollow alert" : "success"}", diff --git a/app/components/admin/settings/featured_settings_form_component.rb b/app/components/admin/settings/featured_settings_form_component.rb new file mode 100644 index 000000000..db2382678 --- /dev/null +++ b/app/components/admin/settings/featured_settings_form_component.rb @@ -0,0 +1,8 @@ +class Admin::Settings::FeaturedSettingsFormComponent < ApplicationComponent + attr_reader :feature, :tab + + def initialize(feature, tab: nil) + @feature = feature + @tab = tab + end +end diff --git a/app/views/admin/settings/_featured_settings_table.html.erb b/app/views/admin/settings/_featured_settings_table.html.erb index 8936b3cae..4bde70d30 100644 --- a/app/views/admin/settings/_featured_settings_table.html.erb +++ b/app/views/admin/settings/_featured_settings_table.html.erb @@ -32,7 +32,7 @@ - <%= render "admin/settings/featured_settings_form", feature: feature, tab: tab %> + <%= render Admin::Settings::FeaturedSettingsFormComponent.new(feature, tab: tab) %> <% end %> From d9076b9d026a62b30ed3494b4101e906cb77b119 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 20 Aug 2021 21:46:43 +0200 Subject: [PATCH 2/7] Simplify attributes in settings forms Rails automatically assigns that id and that URL to forms for existing records. --- .../admin/settings/featured_settings_form_component.html.erb | 2 +- app/views/admin/settings/_settings_form.html.erb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/components/admin/settings/featured_settings_form_component.html.erb b/app/components/admin/settings/featured_settings_form_component.html.erb index 84b3c5f73..5ba0c392e 100644 --- a/app/components/admin/settings/featured_settings_form_component.html.erb +++ b/app/components/admin/settings/featured_settings_form_component.html.erb @@ -1,4 +1,4 @@ -<%= form_for(feature, url: admin_setting_path(feature), html: { id: "edit_#{dom_id(feature)}" }) do |f| %> +<%= form_for([:admin, feature]) do |f| %> <%= f.hidden_field :tab, value: tab if tab %> <%= f.hidden_field :value, id: dom_id(feature), value: (feature.enabled? ? "" : "active") %> <%= f.submit(t("admin.settings.index.features.#{feature.enabled? ? "disable" : "enable"}"), diff --git a/app/views/admin/settings/_settings_form.html.erb b/app/views/admin/settings/_settings_form.html.erb index 865c409f5..5a57e413c 100644 --- a/app/views/admin/settings/_settings_form.html.erb +++ b/app/views/admin/settings/_settings_form.html.erb @@ -1,4 +1,4 @@ -<%= form_for(setting, url: admin_setting_path(setting), html: { id: "edit_#{dom_id(setting)}" }) do |f| %> +<%= form_for([:admin, setting]) do |f| %> <%= f.hidden_field :tab, value: tab if defined?(tab) %>
<%= f.text_area :value, label: false, id: dom_id(setting), lines: 1 %> From 71aa651f6feb5f887c1c9b6e7e0cd97b4a2c1b64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 20 Aug 2021 22:03:57 +0200 Subject: [PATCH 3/7] Fix invalid HTML in admin settings page There were duplicate IDs and the `lines` attribute doesn't do anything for textareas (I guess it was accidentally used instead of the `rows` attribute; I'm just removing it so the page looks the same way it did until now). Even though the `value` field didn't generate duplicate IDs, we're also changing it because we usually set an element with the `dom_id` of a record when it contains the whole information about a record, and not just one piece of it. For instance, in some places we assign this ID to the table row related to the record. --- .../settings/featured_settings_form_component.html.erb | 4 ++-- .../settings/_content_types_settings_form.html.erb | 2 +- app/views/admin/settings/_settings_form.html.erb | 4 ++-- spec/system/admin/settings_spec.rb | 10 +++++----- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/app/components/admin/settings/featured_settings_form_component.html.erb b/app/components/admin/settings/featured_settings_form_component.html.erb index 5ba0c392e..e3b5c6f46 100644 --- a/app/components/admin/settings/featured_settings_form_component.html.erb +++ b/app/components/admin/settings/featured_settings_form_component.html.erb @@ -1,6 +1,6 @@ <%= form_for([:admin, feature]) do |f| %> - <%= f.hidden_field :tab, value: tab if tab %> - <%= f.hidden_field :value, id: dom_id(feature), value: (feature.enabled? ? "" : "active") %> + <%= f.hidden_field :tab, id: dom_id(feature, :tab), value: tab if tab %> + <%= f.hidden_field :value, id: dom_id(feature, :value), value: (feature.enabled? ? "" : "active") %> <%= f.submit(t("admin.settings.index.features.#{feature.enabled? ? "disable" : "enable"}"), class: "button expanded #{feature.enabled? ? "hollow alert" : "success"}", data: { confirm: t("admin.actions.confirm") }) %> diff --git a/app/views/admin/settings/_content_types_settings_form.html.erb b/app/views/admin/settings/_content_types_settings_form.html.erb index 0711adbb4..4c3cb347b 100644 --- a/app/views/admin/settings/_content_types_settings_form.html.erb +++ b/app/views/admin/settings/_content_types_settings_form.html.erb @@ -1,5 +1,5 @@ <%= form_tag admin_update_content_types_path, method: :put, id: "edit_#{dom_id(setting)}" do %> - <%= hidden_field_tag "id", setting.id %> + <%= hidden_field_tag "id", setting.id, id: dom_id(setting, :id) %>
<% group = setting.content_type_group %> diff --git a/app/views/admin/settings/_settings_form.html.erb b/app/views/admin/settings/_settings_form.html.erb index 5a57e413c..1b8bc2027 100644 --- a/app/views/admin/settings/_settings_form.html.erb +++ b/app/views/admin/settings/_settings_form.html.erb @@ -1,7 +1,7 @@ <%= form_for([:admin, setting]) do |f| %> - <%= f.hidden_field :tab, value: tab if defined?(tab) %> + <%= f.hidden_field :tab, id: dom_id(setting, :tab), value: tab if defined?(tab) %>
- <%= f.text_area :value, label: false, id: dom_id(setting), lines: 1 %> + <%= f.text_area :value, label: false, id: dom_id(setting, :value) %>
<%= f.submit(t("admin.settings.index.update_setting"), class: "button hollow expanded") %> diff --git a/spec/system/admin/settings_spec.rb b/spec/system/admin/settings_spec.rb index 4c06b9251..95f7831d3 100644 --- a/spec/system/admin/settings_spec.rb +++ b/spec/system/admin/settings_spec.rb @@ -19,7 +19,7 @@ describe "Admin settings", :admin do visit admin_settings_path within("#edit_setting_#{setting.id}") do - fill_in "setting_#{setting.id}", with: "Super Users of level 1" + fill_in "value_setting_#{setting.id}", with: "Super Users of level 1" click_button "Update" end @@ -179,7 +179,7 @@ describe "Admin settings", :admin do find("#remote-census-tab").click within("#edit_setting_#{remote_census_setting.id}") do - fill_in "setting_#{remote_census_setting.id}", with: "New value" + fill_in "value_setting_#{remote_census_setting.id}", with: "New value" click_button "Update" end @@ -195,7 +195,7 @@ describe "Admin settings", :admin do find("#tab-configuration").click within("#edit_setting_#{configuration_setting.id}") do - fill_in "setting_#{configuration_setting.id}", with: "New value" + fill_in "value_setting_#{configuration_setting.id}", with: "New value" click_button "Update" end @@ -215,7 +215,7 @@ describe "Admin settings", :admin do click_link "Map configuration" within("#edit_setting_#{map_setting.id}") do - fill_in "setting_#{map_setting.id}", with: "New value" + fill_in "value_setting_#{map_setting.id}", with: "New value" click_button "Update" end @@ -231,7 +231,7 @@ describe "Admin settings", :admin do find("#proposals-tab").click within("#edit_setting_#{proposal_dashboard_setting.id}") do - fill_in "setting_#{proposal_dashboard_setting.id}", with: "New value" + fill_in "value_setting_#{proposal_dashboard_setting.id}", with: "New value" click_button "Update" end From fabe97e506922f870a63e72960aa72bacdb2a605 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 20 Aug 2021 21:50:23 +0200 Subject: [PATCH 4/7] Use a switch control to enable/disable features We were using buttons with the "Enable" and "Disable" texts to enable/disable settings. However, when machine learning settings were introduced in commit 4d27bbeba, a switch control was introduced to enable/disable them. In order to keep the interface consistent, we're now using switch controls in other sections where settings are enabled/disabled. We can even use the same code in the machine learning settings as well. We're also removing the confirmation dialog to enable/disable a setting, since the dialog is really annoying when changing several settings and this action can be undone immediately. The only setting which might need a confirmation is the "Skip user verification" one; we might add it in the future. Removing the confirmation here doesn't make things worse, though; the "Are you sure?" confirmation dialog was also pretty useless and users would most likely blindly accept it. Note Capybara doesn't support finding a button by its `aria-labelledby` atrribute. Ideally we'd write `click_button "Participatory budgeting"` instead of `click_button "Yes"`, since from the user's point of view the "Yes" or "No" texts aren't button labels but indicators of the status of the setting. This makes the code a little brittle since tests would pass even if the element referenced by `aria-labelledby` didn't exist. --- .../admin/machine_learning/setting.scss | 40 -------------- .../settings/featured_settings_form.scss | 6 ++ .../settings/featured_settings_table.scss | 6 ++ app/assets/stylesheets/mixins/buttons.scss | 40 ++++++++++++++ .../setting_component.html.erb | 13 +---- .../featured_settings_form_component.html.erb | 8 +-- .../featured_settings_form_component.rb | 19 +++++++ .../_featured_settings_table.html.erb | 25 ++------- config/locales/en/admin.yml | 5 +- config/locales/es/admin.yml | 5 +- .../featured_settings_form_component_spec.rb | 55 +++++++++++++++++++ spec/system/admin/feature_flags_spec.rb | 47 ++++------------ spec/system/admin/settings_spec.rb | 34 +++--------- 13 files changed, 157 insertions(+), 146 deletions(-) create mode 100644 app/assets/stylesheets/admin/settings/featured_settings_form.scss create mode 100644 app/assets/stylesheets/admin/settings/featured_settings_table.scss create mode 100644 spec/components/admin/settings/featured_settings_form_component_spec.rb diff --git a/app/assets/stylesheets/admin/machine_learning/setting.scss b/app/assets/stylesheets/admin/machine_learning/setting.scss index f9afcc079..3bf902365 100644 --- a/app/assets/stylesheets/admin/machine_learning/setting.scss +++ b/app/assets/stylesheets/admin/machine_learning/setting.scss @@ -9,46 +9,6 @@ } } - [aria-pressed] { - @include regular-button; - border-radius: $line-height; - font-weight: bold; - min-width: rem-calc(100); - position: relative; - - &::after { - background: $white; - border-radius: 100%; - content: ""; - display: block; - height: 1.75em; - position: absolute; - transform: translateY(-50%); - top: 50%; - width: 1.75em; - } - - &[aria-pressed=true] { - background: $primary-color; - padding-right: 2.5em; - text-align: left; - - &::after { - right: 0.5em; - } - } - - &[aria-pressed=false] { - background: $dark-gray; - padding-left: 2.5em; - text-align: right; - - &::after { - left: 0.5em; - } - } - } - dl { @include callout-size(map-get($callout-sizes, small)); } diff --git a/app/assets/stylesheets/admin/settings/featured_settings_form.scss b/app/assets/stylesheets/admin/settings/featured_settings_form.scss new file mode 100644 index 000000000..c002824d5 --- /dev/null +++ b/app/assets/stylesheets/admin/settings/featured_settings_form.scss @@ -0,0 +1,6 @@ +.admin .featured-settings-form { + + [aria-pressed] { + @include switch; + } +} diff --git a/app/assets/stylesheets/admin/settings/featured_settings_table.scss b/app/assets/stylesheets/admin/settings/featured_settings_table.scss new file mode 100644 index 000000000..ef0df6709 --- /dev/null +++ b/app/assets/stylesheets/admin/settings/featured_settings_table.scss @@ -0,0 +1,6 @@ +.admin .featured-settings-table { + + td { + max-width: $global-width / 3; + } +} diff --git a/app/assets/stylesheets/mixins/buttons.scss b/app/assets/stylesheets/mixins/buttons.scss index ff69c30e8..b5e1aee6c 100644 --- a/app/assets/stylesheets/mixins/buttons.scss +++ b/app/assets/stylesheets/mixins/buttons.scss @@ -40,3 +40,43 @@ text-decoration: underline; } } + +@mixin switch { + @include regular-button; + border-radius: $line-height; + font-weight: bold; + min-width: rem-calc(100); + position: relative; + + &::after { + background: $white; + border-radius: 100%; + content: ""; + display: block; + height: 1.75em; + position: absolute; + transform: translateY(-50%); + top: 50%; + width: 1.75em; + } + + &[aria-pressed=true] { + background: $primary-color; + padding-right: 2.5em; + text-align: left; + + &::after { + right: 0.5em; + } + } + + &[aria-pressed=false] { + background: $dark-gray; + padding-left: 2.5em; + text-align: right; + + &::after { + left: 0.5em; + } + } +} diff --git a/app/components/admin/machine_learning/setting_component.html.erb b/app/components/admin/machine_learning/setting_component.html.erb index 5509c1040..c9ac35348 100644 --- a/app/components/admin/machine_learning/setting_component.html.erb +++ b/app/components/admin/machine_learning/setting_component.html.erb @@ -1,19 +1,12 @@
-

<%= t("admin.machine_learning.#{kind}") %>

+

<%= t("admin.machine_learning.#{kind}") %>

-

<%= t("admin.machine_learning.#{kind}_description") %>

+

<%= t("admin.machine_learning.#{kind}_description") %>

<% if ml_info.present? %> - <%= form_for(setting, url: admin_setting_path(setting), method: :put) do |f| %> - <%= f.hidden_field :tab, value: "#settings", id: "setting_tab_#{kind}" %> - <%= f.hidden_field :value, value: (setting.enabled? ? "" : "active"), id: "setting_value_#{kind}" %> - <%= f.button(t("shared.#{setting.enabled? ? "yes" : "no"}"), - "aria-labelledby": "machine_learning_#{kind}", - "aria-describedby": "machine_learning_#{kind}_description", - "aria-pressed": setting.enabled?) %> - <% end %> + <%= render Admin::Settings::FeaturedSettingsFormComponent.new(setting, tab: "#settings") %>
<%= t("admin.machine_learning.last_execution") %>
diff --git a/app/components/admin/settings/featured_settings_form_component.html.erb b/app/components/admin/settings/featured_settings_form_component.html.erb index e3b5c6f46..bb5ae5096 100644 --- a/app/components/admin/settings/featured_settings_form_component.html.erb +++ b/app/components/admin/settings/featured_settings_form_component.html.erb @@ -1,7 +1,5 @@ -<%= form_for([:admin, feature]) do |f| %> +<%= form_for([:admin, feature], html: { class: "featured-settings-form" }) do |f| %> <%= f.hidden_field :tab, id: dom_id(feature, :tab), value: tab if tab %> - <%= f.hidden_field :value, id: dom_id(feature, :value), value: (feature.enabled? ? "" : "active") %> - <%= f.submit(t("admin.settings.index.features.#{feature.enabled? ? "disable" : "enable"}"), - class: "button expanded #{feature.enabled? ? "hollow alert" : "success"}", - data: { confirm: t("admin.actions.confirm") }) %> + <%= f.hidden_field :value, id: dom_id(feature, :value), value: (enabled? ? "" : "active") %> + <%= f.button text, options %> <% end %> diff --git a/app/components/admin/settings/featured_settings_form_component.rb b/app/components/admin/settings/featured_settings_form_component.rb index db2382678..c5835bc09 100644 --- a/app/components/admin/settings/featured_settings_form_component.rb +++ b/app/components/admin/settings/featured_settings_form_component.rb @@ -1,8 +1,27 @@ class Admin::Settings::FeaturedSettingsFormComponent < ApplicationComponent attr_reader :feature, :tab + delegate :enabled?, to: :feature def initialize(feature, tab: nil) @feature = feature @tab = tab end + + private + + def text + if enabled? + t("shared.yes") + else + t("shared.no") + end + end + + def options + { + "aria-labelledby": dom_id(feature, :title), + "aria-describedby": dom_id(feature, :description), + "aria-pressed": enabled? + } + end end diff --git a/app/views/admin/settings/_featured_settings_table.html.erb b/app/views/admin/settings/_featured_settings_table.html.erb index 4bde70d30..649d43cec 100644 --- a/app/views/admin/settings/_featured_settings_table.html.erb +++ b/app/views/admin/settings/_featured_settings_table.html.erb @@ -1,37 +1,22 @@ - +
- - + <% features.each do |feature| %> - - - diff --git a/config/locales/en/admin.yml b/config/locales/en/admin.yml index 3284f41f9..b797b26a4 100644 --- a/config/locales/en/admin.yml +++ b/config/locales/en/admin.yml @@ -1298,8 +1298,7 @@ en: images_and_documents: "Images and documents" feature_flags: Features features: - enabled: "Feature enabled" - disabled: "Feature disabled" + enabled: "Enabled" enable: "Enable" disable: "Disable" map: @@ -1322,9 +1321,7 @@ en: remote_census_request_name: Request Data remote_census_response_name: Response Data setting: Feature - setting_actions: Actions setting_name: Setting - setting_status: Status setting_value: Value no_description: "No description" shared: diff --git a/config/locales/es/admin.yml b/config/locales/es/admin.yml index be84208a3..da1b4b075 100644 --- a/config/locales/es/admin.yml +++ b/config/locales/es/admin.yml @@ -1297,8 +1297,7 @@ es: images_and_documents: "Imágenes y documentos" feature_flags: Funcionalidades features: - enabled: "Funcionalidad activada" - disabled: "Funcionalidad desactivada" + enabled: "Activada" enable: "Activar" disable: "Desactivar" map: @@ -1321,9 +1320,7 @@ es: remote_census_request_name: Datos Petición remote_census_response_name: Datos Respuesta setting: Funcionalidad - setting_actions: Acciones setting_name: Configuración - setting_status: Estado setting_value: Valor no_description: "Sin descripción" shared: diff --git a/spec/components/admin/settings/featured_settings_form_component_spec.rb b/spec/components/admin/settings/featured_settings_form_component_spec.rb new file mode 100644 index 000000000..f01455405 --- /dev/null +++ b/spec/components/admin/settings/featured_settings_form_component_spec.rb @@ -0,0 +1,55 @@ +require "rails_helper" + +describe Admin::Settings::FeaturedSettingsFormComponent do + let(:setting) { create(:setting, key: "feature.goodness") } + let(:component) { Admin::Settings::FeaturedSettingsFormComponent.new(setting) } + + it "includes an aria-labelledby attribute" do + render_inline component + + expect(page).to have_button count: 1 + expect(page).to have_css "button[aria-labelledby='title_setting_#{setting.id}']" + end + + it "includes an aria-describedby attribute" do + render_inline component + + expect(page).to have_css "button[aria-describedby='description_setting_#{setting.id}']" + end + + describe "aria-pressed attribute" do + it "is true when the setting is enabled" do + setting.update!(value: "active") + + render_inline component + + expect(page).to have_css "button[aria-pressed='true']" + end + + it "is false when the setting is disabled" do + setting.update!(value: "") + + render_inline component + + expect(page).to have_css "button[aria-pressed='false']" + end + end + + describe "button text" do + it "reflects the status when the setting is enabled" do + setting.update!(value: "active") + + render_inline component + + expect(page).to have_button "Yes" + end + + it "reflects the status when the setting is disabled" do + setting.update!(value: "") + + render_inline component + + expect(page).to have_button "No" + end + end +end diff --git a/spec/system/admin/feature_flags_spec.rb b/spec/system/admin/feature_flags_spec.rb index 5fd48a934..28e439e32 100644 --- a/spec/system/admin/feature_flags_spec.rb +++ b/spec/system/admin/feature_flags_spec.rb @@ -15,18 +15,12 @@ describe "Admin feature flags", :admin do end scenario "Disable a participatory process", :show_exceptions do - setting = Setting.find_by(key: "process.budgets") budget = create(:budget) visit admin_settings_path within("#settings-tabs") { click_link "Participation processes" } - within("#edit_setting_#{setting.id}") do - expect(page).to have_button "Disable" - expect(page).not_to have_button "Enable" - - accept_confirm { click_button "Disable" } - end + within("tr", text: "Participatory budgeting") { click_button "Yes" } expect(page).to have_content "Value updated" @@ -46,7 +40,6 @@ describe "Admin feature flags", :admin do scenario "Enable a disabled participatory process" do Setting["process.budgets"] = nil - setting = Setting.find_by(key: "process.budgets") visit admin_root_path @@ -56,13 +49,7 @@ describe "Admin feature flags", :admin do visit admin_settings_path within("#settings-tabs") { click_link "Participation processes" } - - within("#edit_setting_#{setting.id}") do - expect(page).to have_button "Enable" - expect(page).not_to have_button "Disable" - - accept_confirm { click_button "Enable" } - end + within("tr", text: "Participatory budgeting") { click_button "No" } expect(page).to have_content "Value updated" @@ -72,44 +59,30 @@ describe "Admin feature flags", :admin do end scenario "Disable a feature" do - setting = Setting.find_by(key: "feature.twitter_login") - visit admin_settings_path click_link "Features" - within("#edit_setting_#{setting.id}") do - expect(page).to have_button "Disable" - expect(page).not_to have_button "Enable" - - accept_confirm { click_button "Disable" } - end + within("tr", text: "Twitter login") { click_button "Yes" } expect(page).to have_content "Value updated" - within("#edit_setting_#{setting.id}") do - expect(page).to have_button "Enable" - expect(page).not_to have_button "Disable" + within("tr", text: "Twitter login") do + expect(page).to have_button "No" + expect(page).not_to have_button "Yes" end end scenario "Enable a disabled feature" do - setting = Setting.find_by(key: "feature.map") - visit admin_settings_path click_link "Features" - within("#edit_setting_#{setting.id}") do - expect(page).to have_button "Enable" - expect(page).not_to have_button "Disable" - - accept_confirm { click_button "Enable" } - end + within("tr", text: "Proposals and budget investments geolocation") { click_button "No" } expect(page).to have_content "Value updated" - within("#edit_setting_#{setting.id}") do - expect(page).to have_button "Disable" - expect(page).not_to have_button "Enable" + within("tr", text: "Proposals and budget investments geolocation") do + expect(page).to have_button "Yes" + expect(page).not_to have_button "No" end end end diff --git a/spec/system/admin/settings_spec.rb b/spec/system/admin/settings_spec.rb index 95f7831d3..4b29ec014 100644 --- a/spec/system/admin/settings_spec.rb +++ b/spec/system/admin/settings_spec.rb @@ -240,28 +240,22 @@ describe "Admin settings", :admin do end scenario "On #tab-participation-processes" do - process_setting = Setting.create!(key: "process.whatever") + Setting.create!(key: "process.whatever") visit admin_settings_path find("#participation-processes-tab").click - - accept_alert do - find("#edit_setting_#{process_setting.id} .button").click - end + within("tr", text: "Whatever") { click_button "No" } expect(page).to have_current_path(admin_settings_path) expect(page).to have_css("div#tab-participation-processes.is-active") end scenario "On #tab-feature-flags" do - feature_setting = Setting.create!(key: "feature.whatever") + Setting.create!(key: "feature.whatever") visit admin_settings_path find("#features-tab").click - - accept_alert do - find("#edit_setting_#{feature_setting.id} .button").click - end + within("tr", text: "Whatever") { click_button "No" } expect(page).to have_current_path(admin_settings_path) expect(page).to have_css("div#tab-feature-flags.is-active") @@ -274,11 +268,9 @@ describe "Admin settings", :admin do visit admin_settings_path click_link "SDG configuration" + within("tr", text: "Whatever") { click_button "No" } - accept_alert do - within("tr", text: "Whatever") { click_button "Enable" } - end - + expect(page).to have_content "Value updated" expect(page).to have_current_path(admin_settings_path) expect(page).to have_css("h2", exact_text: "SDG configuration") end @@ -287,32 +279,22 @@ describe "Admin settings", :admin do describe "Skip verification" do scenario "deactivate skip verification" do Setting["feature.user.skip_verification"] = "true" - setting = Setting.find_by(key: "feature.user.skip_verification") visit admin_settings_path find("#features-tab").click - - accept_alert do - find("#edit_setting_#{setting.id} .button").click - end + within("tr", text: "Skip user verification") { click_button "Yes" } expect(page).to have_content "Value updated" end scenario "activate skip verification" do Setting["feature.user.skip_verification"] = nil - setting = Setting.find_by(key: "feature.user.skip_verification") visit admin_settings_path find("#features-tab").click - - accept_alert do - find("#edit_setting_#{setting.id} .button").click - end + within("tr", text: "Skip user verification") { click_button "No" } expect(page).to have_content "Value updated" - - Setting["feature.user.skip_verification"] = nil end end From 7b8e892f9cd784a619bb585cc63f99ce253adad3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 28 Aug 2021 03:05:44 +0200 Subject: [PATCH 5/7] Use a switch to enable/disable homepage features So it's consistent with the way we enable/disable other features. --- .../settings/featured_settings_form_component.rb | 8 +++++--- app/views/admin/homepage/_feed.html.erb | 2 +- app/views/admin/homepage/_setting.html.erb | 11 +---------- app/views/admin/homepage/show.html.erb | 2 +- config/i18n-tasks.yml | 1 - config/locales/en/admin.yml | 2 -- config/locales/es/admin.yml | 2 -- .../featured_settings_form_component_spec.rb | 14 +++++++++++--- spec/system/admin/homepage/homepage_spec.rb | 14 +++++++------- 9 files changed, 26 insertions(+), 30 deletions(-) diff --git a/app/components/admin/settings/featured_settings_form_component.rb b/app/components/admin/settings/featured_settings_form_component.rb index c5835bc09..b785f8716 100644 --- a/app/components/admin/settings/featured_settings_form_component.rb +++ b/app/components/admin/settings/featured_settings_form_component.rb @@ -1,10 +1,12 @@ class Admin::Settings::FeaturedSettingsFormComponent < ApplicationComponent - attr_reader :feature, :tab + attr_reader :feature, :tab, :describedby + alias_method :describedby?, :describedby delegate :enabled?, to: :feature - def initialize(feature, tab: nil) + def initialize(feature, tab: nil, describedby: true) @feature = feature @tab = tab + @describedby = describedby end private @@ -20,7 +22,7 @@ class Admin::Settings::FeaturedSettingsFormComponent < ApplicationComponent def options { "aria-labelledby": dom_id(feature, :title), - "aria-describedby": dom_id(feature, :description), + "aria-describedby": (dom_id(feature, :description) if describedby?), "aria-pressed": enabled? } end diff --git a/app/views/admin/homepage/_feed.html.erb b/app/views/admin/homepage/_feed.html.erb index ff0a6f812..480fe49b1 100644 --- a/app/views/admin/homepage/_feed.html.erb +++ b/app/views/admin/homepage/_feed.html.erb @@ -1,6 +1,6 @@
-

<%= t("admin.homepage.feeds.#{feed.kind}") %>

+

<%= t("admin.homepage.feeds.#{feed.kind}") %>

<%= render "setting", setting: feed.setting %> diff --git a/app/views/admin/homepage/_setting.html.erb b/app/views/admin/homepage/_setting.html.erb index f875e3e7d..34aafe3b4 100644 --- a/app/views/admin/homepage/_setting.html.erb +++ b/app/views/admin/homepage/_setting.html.erb @@ -1,10 +1 @@ -
- <%= form_for(setting, url: admin_setting_path(setting), method: :put) do |f| %> - - <%= f.hidden_field :value, - value: (setting.enabled? ? "" : "active") %> - - <%= f.submit(t("admin.settings.index.features.#{setting.enabled? ? "disable" : "enable"}"), - class: "button #{setting.enabled? ? "hollow alert" : "success"}") %> - <% end %> -
+<%= render Admin::Settings::FeaturedSettingsFormComponent.new(setting, describedby: false) %> diff --git a/app/views/admin/homepage/show.html.erb b/app/views/admin/homepage/show.html.erb index 3c933c961..367d5cd9e 100644 --- a/app/views/admin/homepage/show.html.erb +++ b/app/views/admin/homepage/show.html.erb @@ -38,7 +38,7 @@
-

<%= t("settings.#{@recommendations.key}") %>

+

<%= t("settings.#{@recommendations.key}") %>

<%= render "setting", setting: @recommendations %>
diff --git a/config/i18n-tasks.yml b/config/i18n-tasks.yml index 42a20478c..b5ee4bd08 100644 --- a/config/i18n-tasks.yml +++ b/config/i18n-tasks.yml @@ -160,7 +160,6 @@ ignore_unused: - "admin.legislation.draft_versions.*.submit_button" - "admin.legislation.questions.*.submit_button" - "admin.hidden_comments.index.hidden_*" - - "admin.settings.index.features.*" - "admin.polls.*.submit_button" - "admin.booths.*.submit_button" - "admin.admin_notifications.*.submit_button" diff --git a/config/locales/en/admin.yml b/config/locales/en/admin.yml index b797b26a4..0749e67c6 100644 --- a/config/locales/en/admin.yml +++ b/config/locales/en/admin.yml @@ -1299,8 +1299,6 @@ en: feature_flags: Features features: enabled: "Enabled" - enable: "Enable" - disable: "Disable" map: title: Map configuration help: Here you can customize the way the map is displayed to users. Drag map marker or click anywhere over the map, set desired zoom and click button "Update". diff --git a/config/locales/es/admin.yml b/config/locales/es/admin.yml index da1b4b075..5ffce5ea8 100644 --- a/config/locales/es/admin.yml +++ b/config/locales/es/admin.yml @@ -1298,8 +1298,6 @@ es: feature_flags: Funcionalidades features: enabled: "Activada" - enable: "Activar" - disable: "Desactivar" map: title: Configuración del mapa help: Aquí puedes personalizar la manera en la que se muestra el mapa a los usuarios. Arrastra el marcador o pulsa sobre cualquier parte del mapa, ajusta el zoom y pulsa el botón 'Actualizar'. diff --git a/spec/components/admin/settings/featured_settings_form_component_spec.rb b/spec/components/admin/settings/featured_settings_form_component_spec.rb index f01455405..bc04198ea 100644 --- a/spec/components/admin/settings/featured_settings_form_component_spec.rb +++ b/spec/components/admin/settings/featured_settings_form_component_spec.rb @@ -11,10 +11,18 @@ describe Admin::Settings::FeaturedSettingsFormComponent do expect(page).to have_css "button[aria-labelledby='title_setting_#{setting.id}']" end - it "includes an aria-describedby attribute" do - render_inline component + describe "aria-describedby attribute" do + it "is rendered by default" do + render_inline component - expect(page).to have_css "button[aria-describedby='description_setting_#{setting.id}']" + expect(page).to have_css "button[aria-describedby='description_setting_#{setting.id}']" + end + + it "is not rendered when the describedby option is false" do + render_inline Admin::Settings::FeaturedSettingsFormComponent.new(setting, describedby: false) + + expect(page).not_to have_css "[aria-describedby]" + end end describe "aria-pressed attribute" do diff --git a/spec/system/admin/homepage/homepage_spec.rb b/spec/system/admin/homepage/homepage_spec.rb index 3783002a8..f7f89a233 100644 --- a/spec/system/admin/homepage/homepage_spec.rb +++ b/spec/system/admin/homepage/homepage_spec.rb @@ -33,7 +33,7 @@ describe "Homepage", :admin do within("#widget_feed_#{proposals_feed.id}") do select "1", from: "widget_feed_limit" - click_button "Enable" + click_button "No" end visit root_path @@ -52,7 +52,7 @@ describe "Homepage", :admin do visit admin_homepage_path within("#widget_feed_#{debates_feed.id}") do select "2", from: "widget_feed_limit" - click_button "Enable" + click_button "No" end visit root_path @@ -73,12 +73,12 @@ describe "Homepage", :admin do within("#widget_feed_#{proposals_feed.id}") do select "3", from: "widget_feed_limit" - click_button "Enable" + click_button "No" end within("#widget_feed_#{debates_feed.id}") do select "3", from: "widget_feed_limit" - click_button "Enable" + click_button "No" end visit root_path @@ -100,7 +100,7 @@ describe "Homepage", :admin do visit admin_homepage_path within("#widget_feed_#{processes_feed.id}") do select "3", from: "widget_feed_limit" - click_button "Enable" + click_button "No" end visit root_path @@ -153,8 +153,8 @@ describe "Homepage", :admin do create(:proposal, tag_list: "Sport") visit admin_homepage_path - within("#setting_#{user_recommendations.id}") do - click_button "Enable" + within("#edit_setting_#{user_recommendations.id}") do + click_button "No" end expect(page).to have_content "Value updated" From ead5eac67fbedbdb413f5fab9e5a466ad99d567f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 28 Aug 2021 14:00:45 +0200 Subject: [PATCH 6/7] Update settings using an AJAX requests Having to wait for a whole page refresh after updating each setting was painful when modifying several settings. Even though the navigation is updated immediately to reflect which sections have been enabled/disabled, there's one gotcha. Changing the "SDG" setting will not update the user menu (which contains a link to SDG content) nor the "SDG Configuration" tab; refreshing the page will be necessary to check these changes. The same happens with the map and remote census tabs. So in these cases we're making an exception and sending the form. We might find a better solution in the future. For this reason, we aren't using the `switch` ARIA role. Some users might not expect a switch control to refresh the page, just like they usually don't expect checkboxes to refresh the page. Furthermore, screen reader support for the `switch` role seems to be inconsistent. For instance, NVDA with Chrome announces the control as a checkbox instead of a switch. Note AJAX is only used for feature settings. Other settings are still updated with regular HTTP requests. Since we're now using AJAX requests, we have to make sure to add an expectation in the homepage tests in order to make sure the request has finished before starting a new one. --- .../featured_settings_form_component.html.erb | 3 +- .../featured_settings_form_component.rb | 5 +++ app/controllers/admin/settings_controller.rb | 6 ++- app/views/admin/settings/update.js.erb | 11 +++++ spec/system/admin/feature_flags_spec.rb | 25 +++++------ spec/system/admin/homepage/homepage_spec.rb | 15 ++++++- spec/system/admin/settings_spec.rb | 41 ++++++++++++++++--- 7 files changed, 84 insertions(+), 22 deletions(-) create mode 100644 app/views/admin/settings/update.js.erb diff --git a/app/components/admin/settings/featured_settings_form_component.html.erb b/app/components/admin/settings/featured_settings_form_component.html.erb index bb5ae5096..e1b8840c9 100644 --- a/app/components/admin/settings/featured_settings_form_component.html.erb +++ b/app/components/admin/settings/featured_settings_form_component.html.erb @@ -1,5 +1,6 @@ -<%= form_for([:admin, feature], html: { class: "featured-settings-form" }) do |f| %> +<%= form_for([:admin, feature], remote: remote?, html: { class: "featured-settings-form" }) do |f| %> <%= f.hidden_field :tab, id: dom_id(feature, :tab), value: tab if tab %> + <%= f.hidden_field :describedby, id: dom_id(feature, :describedby), value: describedby if describedby %> <%= f.hidden_field :value, id: dom_id(feature, :value), value: (enabled? ? "" : "active") %> <%= f.button text, options %> <% end %> diff --git a/app/components/admin/settings/featured_settings_form_component.rb b/app/components/admin/settings/featured_settings_form_component.rb index b785f8716..a4c19154f 100644 --- a/app/components/admin/settings/featured_settings_form_component.rb +++ b/app/components/admin/settings/featured_settings_form_component.rb @@ -21,9 +21,14 @@ class Admin::Settings::FeaturedSettingsFormComponent < ApplicationComponent def options { + data: { disable_with: text }, "aria-labelledby": dom_id(feature, :title), "aria-describedby": (dom_id(feature, :description) if describedby?), "aria-pressed": enabled? } end + + def remote? + !%w[feature.map feature.remote_census feature.sdg].include?(feature.key) + end end diff --git a/app/controllers/admin/settings_controller.rb b/app/controllers/admin/settings_controller.rb index cdc2278ba..e225d6557 100644 --- a/app/controllers/admin/settings_controller.rb +++ b/app/controllers/admin/settings_controller.rb @@ -16,7 +16,11 @@ class Admin::SettingsController < Admin::BaseController def update @setting = Setting.find(params[:id]) @setting.update!(settings_params) - redirect_to request_referer, notice: t("admin.settings.flash.updated") + + respond_to do |format| + format.html { redirect_to request_referer, notice: t("admin.settings.flash.updated") } + format.js + end end def update_map diff --git a/app/views/admin/settings/update.js.erb b/app/views/admin/settings/update.js.erb new file mode 100644 index 000000000..503ef165d --- /dev/null +++ b/app/views/admin/settings/update.js.erb @@ -0,0 +1,11 @@ +var form = $("<%= j render Admin::Settings::FeaturedSettingsFormComponent.new( + @setting, + tab: params[:setting][:tab], + describedby: params[:setting][:describedby] +) %>"); + +$("#" + form.attr("id")).html(form.html()).find("[type='submit']").focus(); + +<% if @setting.type == "feature" || @setting.type == "process" %> + $("#side_menu").html("<%= j render Admin::MenuComponent.new %>").foundation(); +<% end %> diff --git a/spec/system/admin/feature_flags_spec.rb b/spec/system/admin/feature_flags_spec.rb index 28e439e32..397a056b8 100644 --- a/spec/system/admin/feature_flags_spec.rb +++ b/spec/system/admin/feature_flags_spec.rb @@ -20,9 +20,11 @@ describe "Admin feature flags", :admin do visit admin_settings_path within("#settings-tabs") { click_link "Participation processes" } - within("tr", text: "Participatory budgeting") { click_button "Yes" } + within("tr", text: "Participatory budgeting") do + click_button "Yes" - expect(page).to have_content "Value updated" + expect(page).to have_button "No" + end within("#side_menu") do expect(page).not_to have_link "Participatory budgets" @@ -49,9 +51,12 @@ describe "Admin feature flags", :admin do visit admin_settings_path within("#settings-tabs") { click_link "Participation processes" } - within("tr", text: "Participatory budgeting") { click_button "No" } - expect(page).to have_content "Value updated" + within("tr", text: "Participatory budgeting") do + click_button "No" + + expect(page).to have_button "Yes" + end within("#side_menu") do expect(page).to have_link "Participatory budgets" @@ -62,11 +67,9 @@ describe "Admin feature flags", :admin do visit admin_settings_path click_link "Features" - within("tr", text: "Twitter login") { click_button "Yes" } - - expect(page).to have_content "Value updated" - within("tr", text: "Twitter login") do + click_button "Yes" + expect(page).to have_button "No" expect(page).not_to have_button "Yes" end @@ -76,11 +79,9 @@ describe "Admin feature flags", :admin do visit admin_settings_path click_link "Features" - within("tr", text: "Proposals and budget investments geolocation") { click_button "No" } - - expect(page).to have_content "Value updated" - within("tr", text: "Proposals and budget investments geolocation") do + click_button "No" + expect(page).to have_button "Yes" expect(page).not_to have_button "No" end diff --git a/spec/system/admin/homepage/homepage_spec.rb b/spec/system/admin/homepage/homepage_spec.rb index f7f89a233..314e68247 100644 --- a/spec/system/admin/homepage/homepage_spec.rb +++ b/spec/system/admin/homepage/homepage_spec.rb @@ -34,6 +34,8 @@ describe "Homepage", :admin do within("#widget_feed_#{proposals_feed.id}") do select "1", from: "widget_feed_limit" click_button "No" + + expect(page).to have_button "Yes" end visit root_path @@ -53,6 +55,8 @@ describe "Homepage", :admin do within("#widget_feed_#{debates_feed.id}") do select "2", from: "widget_feed_limit" click_button "No" + + expect(page).to have_button "Yes" end visit root_path @@ -74,11 +78,15 @@ describe "Homepage", :admin do within("#widget_feed_#{proposals_feed.id}") do select "3", from: "widget_feed_limit" click_button "No" + + expect(page).to have_button "Yes" end within("#widget_feed_#{debates_feed.id}") do select "3", from: "widget_feed_limit" click_button "No" + + expect(page).to have_button "Yes" end visit root_path @@ -101,6 +109,8 @@ describe "Homepage", :admin do within("#widget_feed_#{processes_feed.id}") do select "3", from: "widget_feed_limit" click_button "No" + + expect(page).to have_button "Yes" end visit root_path @@ -153,11 +163,12 @@ describe "Homepage", :admin do create(:proposal, tag_list: "Sport") visit admin_homepage_path + within("#edit_setting_#{user_recommendations.id}") do click_button "No" - end - expect(page).to have_content "Value updated" + expect(page).to have_button "Yes" + end login_as(user) visit root_path diff --git a/spec/system/admin/settings_spec.rb b/spec/system/admin/settings_spec.rb index 4b29ec014..74c90a760 100644 --- a/spec/system/admin/settings_spec.rb +++ b/spec/system/admin/settings_spec.rb @@ -268,9 +268,13 @@ describe "Admin settings", :admin do visit admin_settings_path click_link "SDG configuration" - within("tr", text: "Whatever") { click_button "No" } - expect(page).to have_content "Value updated" + within("tr", text: "Whatever") do + click_button "No" + + expect(page).to have_button "Yes" + end + expect(page).to have_current_path(admin_settings_path) expect(page).to have_css("h2", exact_text: "SDG configuration") end @@ -282,9 +286,12 @@ describe "Admin settings", :admin do visit admin_settings_path find("#features-tab").click - within("tr", text: "Skip user verification") { click_button "Yes" } - expect(page).to have_content "Value updated" + within("tr", text: "Skip user verification") do + click_button "Yes" + + expect(page).to have_button "No" + end end scenario "activate skip verification" do @@ -292,9 +299,12 @@ describe "Admin settings", :admin do visit admin_settings_path find("#features-tab").click - within("tr", text: "Skip user verification") { click_button "No" } - expect(page).to have_content "Value updated" + within("tr", text: "Skip user verification") do + click_button "No" + + expect(page).to have_button "Yes" + end end end @@ -320,5 +330,24 @@ describe "Admin settings", :admin do "Sustainable Development Goals you must " \ 'enable "SDG" on "Features" tab.' end + + scenario "is enabled right after enabling the feature" do + Setting["feature.sdg"] = false + login_as(create(:administrator).user) + + visit admin_settings_path + + click_link "Features" + + within("tr", text: "SDG") do + click_button "No" + + expect(page).to have_button "Yes" + end + + click_link "SDG configuration" + + expect(page).to have_css "h2", exact_text: "SDG configuration" + end end end From 2ca5f5c815d2ca673f36be787ae11b052fca6f0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 29 Aug 2021 02:05:24 +0200 Subject: [PATCH 7/7] Add ARIA label and description to settings fields These fields have no label associated to them. While it's more or less obvious for sighted users that these fields are associated with the table cell next to them, visually impaired users might not get that association when using the screen reader in forms mode. Note we're using `aria-label` instead of `aria-labelledby`. IMHO in this case `aria-labelledby` is the superior method because it guarantees the text is present for both sighted and visually impaired users. However, testing for fields with no label other than the one provided by `aria-labelledby` is hard since Capybara has no support for this attribute. So we're using `aria-label` and testing the presence of the text on the page (with the `within "tr", text:` statements) as well as the ARIA label (with the `fill_in` statements). --- .../admin/settings/_settings_form.html.erb | 6 +++- .../admin/settings/_settings_table.html.erb | 2 +- spec/system/admin/settings_spec.rb | 30 +++++++++---------- 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/app/views/admin/settings/_settings_form.html.erb b/app/views/admin/settings/_settings_form.html.erb index 1b8bc2027..33e9ab8f1 100644 --- a/app/views/admin/settings/_settings_form.html.erb +++ b/app/views/admin/settings/_settings_form.html.erb @@ -1,7 +1,11 @@ <%= form_for([:admin, setting]) do |f| %> <%= f.hidden_field :tab, id: dom_id(setting, :tab), value: tab if defined?(tab) %>
- <%= f.text_area :value, label: false, id: dom_id(setting, :value) %> + <%= f.text_area :value, + label: false, + id: dom_id(setting, :value), + "aria-label": strip_tags(t("settings.#{setting.key}")), + "aria-describedby": dom_id(setting, :description) %>
<%= f.submit(t("admin.settings.index.update_setting"), class: "button hollow expanded") %> diff --git a/app/views/admin/settings/_settings_table.html.erb b/app/views/admin/settings/_settings_table.html.erb index d46c5237d..b3015525f 100644 --- a/app/views/admin/settings/_settings_table.html.erb +++ b/app/views/admin/settings/_settings_table.html.erb @@ -11,7 +11,7 @@
diff --git a/spec/system/admin/settings_spec.rb b/spec/system/admin/settings_spec.rb index 74c90a760..8160d4043 100644 --- a/spec/system/admin/settings_spec.rb +++ b/spec/system/admin/settings_spec.rb @@ -14,12 +14,12 @@ describe "Admin settings", :admin do end scenario "Update" do - setting = create(:setting, key: "super.users.first") + create(:setting, key: "super.users.first") visit admin_settings_path - within("#edit_setting_#{setting.id}") do - fill_in "value_setting_#{setting.id}", with: "Super Users of level 1" + within "tr", text: "First" do + fill_in "First", with: "Super Users of level 1" click_button "Update" end @@ -173,13 +173,13 @@ describe "Admin settings", :admin do end scenario "On #tab-remote-census-configuration" do - remote_census_setting = create(:setting, key: "remote_census.general.whatever") + create(:setting, key: "remote_census.general.whatever") visit admin_settings_path find("#remote-census-tab").click - within("#edit_setting_#{remote_census_setting.id}") do - fill_in "value_setting_#{remote_census_setting.id}", with: "New value" + within "tr", text: "Whatever" do + fill_in "Whatever", with: "New value" click_button "Update" end @@ -189,13 +189,13 @@ describe "Admin settings", :admin do end scenario "On #tab-configuration" do - configuration_setting = Setting.create!(key: "whatever") + Setting.create!(key: "whatever") visit admin_settings_path find("#tab-configuration").click - within("#edit_setting_#{configuration_setting.id}") do - fill_in "value_setting_#{configuration_setting.id}", with: "New value" + within "tr", text: "Whatever" do + fill_in "Whatever", with: "New value" click_button "Update" end @@ -209,13 +209,13 @@ describe "Admin settings", :admin do end scenario "On #tab-map-configuration" do - map_setting = Setting.create!(key: "map.whatever") + Setting.create!(key: "map.whatever") visit admin_settings_path click_link "Map configuration" - within("#edit_setting_#{map_setting.id}") do - fill_in "value_setting_#{map_setting.id}", with: "New value" + within "tr", text: "Whatever" do + fill_in "Whatever", with: "New value" click_button "Update" end @@ -225,13 +225,13 @@ describe "Admin settings", :admin do end scenario "On #tab-proposals" do - proposal_dashboard_setting = Setting.create!(key: "proposals.whatever") + Setting.create!(key: "proposals.whatever") visit admin_settings_path find("#proposals-tab").click - within("#edit_setting_#{proposal_dashboard_setting.id}") do - fill_in "value_setting_#{proposal_dashboard_setting.id}", with: "New value" + within "tr", text: "Whatever" do + fill_in "Whatever", with: "New value" click_button "Update" end