From 16fc9998c439ababdded3c1bf8b2d83e0feaf129 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 3 Apr 2024 23:51:35 +0200 Subject: [PATCH 01/25] Use labels in controls to add and select languages The absence of labels in these controls made them hard to use, particularly for people who use screen readers. Note we're removing the "Choose language" prompt, since we always automatically choose a language and not choosing a language doesn't really make sense. The only scenario where the prompt was used took place when all languages had been removed but, in that case, the "Choose language" prompt was misleading because there were no languages to choose from. --- .../globalize_locales_component.html.erb | 5 +- config/locales/en/general.yml | 2 +- config/locales/es/general.yml | 2 +- .../globalize_locales_component_spec.rb | 6 +- spec/system/admin/banners_spec.rb | 2 +- spec/system/admin/budget_groups_spec.rb | 4 +- spec/system/admin/budget_headings_spec.rb | 4 +- spec/system/admin/budgets_spec.rb | 4 +- .../admin/legislation/questions_spec.rb | 10 +-- .../information_texts_spec.rb | 12 +-- spec/system/admin/translatable_spec.rb | 78 +++++++++---------- spec/system/translatable_spec.rb | 16 ++-- 12 files changed, 73 insertions(+), 72 deletions(-) diff --git a/app/components/shared/globalize_locales_component.html.erb b/app/components/shared/globalize_locales_component.html.erb index 664d298db..d173c76ba 100644 --- a/app/components/shared/globalize_locales_component.html.erb +++ b/app/components/shared/globalize_locales_component.html.erb @@ -6,9 +6,9 @@ <%= selected_languages_description %> + <%= label_tag :select_language, t("shared.translations.current_language") %> <%= select_tag :select_language, options_for_select_language, - prompt: t("shared.translations.select_language_prompt"), class: "js-select-language" %> <%= select_language_error %>
@@ -24,9 +24,10 @@
<% if manage_languages %> + <%= label_tag :add_language, t("shared.translations.add_language") %> <%= select_tag :add_language, options_for_add_language, - prompt: t("shared.translations.add_language"), + prompt: "", class: "js-add-language" %> <% end %>
diff --git a/config/locales/en/general.yml b/config/locales/en/general.yml index 95a9db662..e1c4cd82d 100644 --- a/config/locales/en/general.yml +++ b/config/locales/en/general.yml @@ -742,7 +742,7 @@ en: see_more: See more recommendations hide: Hide recommendations translations: - select_language_prompt: Choose language + current_language: Current language remove_language: Remove language add_language: Add language languages_in_use: diff --git a/config/locales/es/general.yml b/config/locales/es/general.yml index 05071f11f..fd203c641 100644 --- a/config/locales/es/general.yml +++ b/config/locales/es/general.yml @@ -742,7 +742,7 @@ es: see_more: Ver más recomendaciones hide: Ocultar recomendaciones translations: - select_language_prompt: Seleccionar idioma + current_language: Idioma actual remove_language: Eliminar idioma add_language: Añadir idioma languages_in_use: diff --git a/spec/components/shared/globalize_locales_component_spec.rb b/spec/components/shared/globalize_locales_component_spec.rb index da2dae47f..9b53599fb 100644 --- a/spec/components/shared/globalize_locales_component_spec.rb +++ b/spec/components/shared/globalize_locales_component_spec.rb @@ -8,13 +8,13 @@ describe Shared::GlobalizeLocalesComponent do I18n.with_locale(:en) do render_inline Shared::GlobalizeLocalesComponent.new - expect(page).to have_select options: ["Choose language", "English"] + expect(page).to have_select "Current language", options: ["English"] end I18n.with_locale(:es) do render_inline Shared::GlobalizeLocalesComponent.new - expect(page).to have_select options: ["Seleccionar idioma"] + expect(page).to have_select "Idioma actual", options: [] end end end @@ -43,7 +43,7 @@ describe Shared::GlobalizeLocalesComponent do render_inline Shared::GlobalizeLocalesComponent.new - expect(page).to have_select options: ["Add language", "English", "Nederlands"] + expect(page).to have_select "Add language", options: ["", "English", "Nederlands"] end end end diff --git a/spec/system/admin/banners_spec.rb b/spec/system/admin/banners_spec.rb index 33d67c405..c2bbab091 100644 --- a/spec/system/admin/banners_spec.rb +++ b/spec/system/admin/banners_spec.rb @@ -95,7 +95,7 @@ describe "Admin banners magement", :admin do expect_to_have_language_selected "English" click_link "Remove language" - select "Français", from: "add_language" + select "Français", from: "Add language" fill_in "Title", with: "En Français" fill_in "Description", with: "Link en Français" diff --git a/spec/system/admin/budget_groups_spec.rb b/spec/system/admin/budget_groups_spec.rb index 2040d3f54..484d2470e 100644 --- a/spec/system/admin/budget_groups_spec.rb +++ b/spec/system/admin/budget_groups_spec.rb @@ -144,7 +144,7 @@ describe "Admin budget groups", :admin do visit edit_admin_budget_group_path(budget, group) - select "Español", from: :add_language + select "Español", from: "Add language" fill_in "Group name", with: "Spanish name" click_button "Save group" @@ -156,7 +156,7 @@ describe "Admin budget groups", :admin do visit edit_admin_budget_group_path(budget, group) - select "English", from: :select_language + select "English", from: "Current language" fill_in "Group name", with: "New English Name" click_button "Save group" diff --git a/spec/system/admin/budget_headings_spec.rb b/spec/system/admin/budget_headings_spec.rb index 07763a208..04b00a40b 100644 --- a/spec/system/admin/budget_headings_spec.rb +++ b/spec/system/admin/budget_headings_spec.rb @@ -181,7 +181,7 @@ describe "Admin budget headings", :admin do visit edit_admin_budget_group_heading_path(budget, group, heading) - select "Español", from: :add_language + select "Español", from: "Add language" fill_in "Heading name", with: "Spanish name" click_button "Save heading" @@ -193,7 +193,7 @@ describe "Admin budget headings", :admin do visit edit_admin_budget_group_heading_path(budget, group, heading) - select "English", from: :select_language + select "English", from: "Current language" fill_in "Heading name", with: "New English Name" click_button "Save heading" diff --git a/spec/system/admin/budgets_spec.rb b/spec/system/admin/budgets_spec.rb index 06e8610b2..b05f40851 100644 --- a/spec/system/admin/budgets_spec.rb +++ b/spec/system/admin/budgets_spec.rb @@ -315,7 +315,7 @@ describe "Admin budgets", :admin do visit edit_admin_budget_path(budget) - select "Español", from: :add_language + select "Español", from: "Add language" fill_in "Name", with: "Spanish name" click_button "Update Budget" @@ -327,7 +327,7 @@ describe "Admin budgets", :admin do visit edit_admin_budget_path(budget) - select "English", from: :select_language + select "English", from: "Current language" fill_in "Name", with: "New English Name" click_button "Update Budget" diff --git a/spec/system/admin/legislation/questions_spec.rb b/spec/system/admin/legislation/questions_spec.rb index 45b39d088..d19b42f33 100644 --- a/spec/system/admin/legislation/questions_spec.rb +++ b/spec/system/admin/legislation/questions_spec.rb @@ -149,7 +149,7 @@ describe "Admin legislation questions", :admin do find("#nested_question_options input").set("Option 1") - select "Español", from: :select_language + select "Español", from: "Current language" find("#nested_question_options input").set("Opción 1") @@ -158,7 +158,7 @@ describe "Admin legislation questions", :admin do expect(page).to have_field(field_en[:id], with: "Option 1") - select "Español", from: :select_language + select "Español", from: "Current language" expect(page).to have_field(field_es[:id], with: "Opción 1") end @@ -166,13 +166,13 @@ describe "Admin legislation questions", :admin do scenario "Add new question option after changing active locale" do visit edit_question_url - select "Español", from: :select_language + select "Español", from: "Current language" click_link "Add option" find("#nested_question_options input").set("Opción 1") - select "English", from: :select_language + select "English", from: "Current language" find("#nested_question_options input").set("Option 1") @@ -182,7 +182,7 @@ describe "Admin legislation questions", :admin do expect(page).to have_field(field_en[:id], with: "Option 1") - select "Español", from: :select_language + select "Español", from: "Current language" expect(page).to have_field(field_es[:id], with: "Opción 1") end diff --git a/spec/system/admin/site_customization/information_texts_spec.rb b/spec/system/admin/site_customization/information_texts_spec.rb index ff2251271..206295aa7 100644 --- a/spec/system/admin/site_customization/information_texts_spec.rb +++ b/spec/system/admin/site_customization/information_texts_spec.rb @@ -67,7 +67,7 @@ describe "Admin custom information texts", :admin do visit admin_site_customization_information_texts_path - select "Français", from: :add_language + select "Français", from: "Add language" fill_in "contents[content_#{key}]values[value_fr]", with: "Aide personalise sur les débats" click_button "Save" @@ -75,7 +75,7 @@ describe "Admin custom information texts", :admin do expect(page).to have_content "Translation updated successfully" visit admin_site_customization_information_texts_path - select "Français", from: :select_language + select "Français", from: "Current language" expect(page).to have_content "Aide personalise sur les débats" expect(page).not_to have_content "Aide sur les débats" @@ -87,14 +87,14 @@ describe "Admin custom information texts", :admin do visit admin_site_customization_information_texts_path(tab: "proposals") - select "Français", from: :select_language + select "Français", from: "Current language" fill_in "contents_content_#{key}values_value_fr", with: "Partager personalise" click_button "Save" expect(page).to have_content "Translation updated successfully" visit admin_site_customization_information_texts_path(tab: "proposals") - select "Français", from: :select_language + select "Français", from: "Current language" expect(page).to have_content "Partager personalise" expect(page).not_to have_content "Partager la proposition" @@ -111,14 +111,14 @@ describe "Admin custom information texts", :admin do visit admin_site_customization_information_texts_path(tab: "debates") - select "Español", from: :select_language + select "Español", from: "Current language" click_link "Remove language" click_button "Save" expect(page).not_to have_link "Español" visit admin_site_customization_information_texts_path(tab: "debates") - select "English", from: :select_language + select "English", from: "Current language" expect(page).to have_content "Start a new debate" expect(page).to have_content "Custom featured" diff --git a/spec/system/admin/translatable_spec.rb b/spec/system/admin/translatable_spec.rb index 51e25ce9a..c9deea0e1 100644 --- a/spec/system/admin/translatable_spec.rb +++ b/spec/system/admin/translatable_spec.rb @@ -22,7 +22,7 @@ describe "Admin edit translatable records", :admin do scenario "Maintains existing translations" do visit path - select "Français", from: :add_language + select "Français", from: "Add language" fill_in "Heading name", with: "Nom en Français" click_button "Save heading" @@ -30,11 +30,11 @@ describe "Admin edit translatable records", :admin do expect(page).to have_field "Heading name", with: "Heading name in English" - select "Español", from: :select_language + select "Español", from: "Current language" expect(page).to have_field "Heading name", with: "Nombre de la partida en español" - select "Français", from: :select_language + select "Français", from: "Current language" expect(page).to have_field "Heading name", with: "Nom en Français" end @@ -47,7 +47,7 @@ describe "Admin edit translatable records", :admin do scenario "Maintains existing translations" do visit path - select "Français", from: :add_language + select "Français", from: "Add language" fill_in "Title", with: "Titre en Français" fill_in "Subtitle", with: "Sous-titres en Français" fill_in_ckeditor "Content", with: "Contenu en Français" @@ -57,11 +57,11 @@ describe "Admin edit translatable records", :admin do expect(page).to have_ckeditor "Content", with: "Content in English" - select "Español", from: :select_language + select "Español", from: "Current language" expect(page).to have_ckeditor "Content", with: "Contenido en español" - select "Français", from: :select_language + select "Français", from: "Current language" expect(page).to have_ckeditor "Content", with: "Contenu en Français" end @@ -74,7 +74,7 @@ describe "Admin edit translatable records", :admin do scenario "Maintains existing translations" do visit path - select "Français", from: :add_language + select "Français", from: "Add language" fill_in "Version title", with: "Titre en Français" click_link class: "fullscreen-toggle" fill_in "Text", with: "Texte en Français" @@ -87,13 +87,13 @@ describe "Admin edit translatable records", :admin do expect(page).to have_field "Text", with: "Text in English" click_link class: "fullscreen-toggle" - select "Español", from: :select_language + select "Español", from: "Current language" click_link class: "fullscreen-toggle" expect(page).to have_field "Text", with: "Texto en español" click_link class: "fullscreen-toggle" - select "Français", from: :select_language + select "Français", from: "Current language" click_link class: "fullscreen-toggle" expect(page).to have_field "Text", with: "Texte en Français" @@ -107,7 +107,7 @@ describe "Admin edit translatable records", :admin do scenario "Adds a translation for that locale" do visit path - select "Português brasileiro", from: :add_language + select "Português brasileiro", from: "Add language" fill_in "Question", with: "Português" click_button "Save changes" @@ -128,13 +128,13 @@ describe "Admin edit translatable records", :admin do scenario "Shows validation erros" do visit edit_admin_budget_path(translatable) - select "Français", from: :add_language + select "Français", from: "Add language" fill_in "Name", with: "" click_button "Update Budget" expect(page).to have_css "#error_explanation" - select "Français", from: :select_language + select "Français", from: "Current language" expect(page).to have_field "Name", with: "", class: "is-invalid-input" end @@ -146,14 +146,14 @@ describe "Admin edit translatable records", :admin do scenario "Shows validation errors" do visit edit_admin_budget_budget_investment_path(translatable.budget, translatable) - select "Français", from: :add_language + select "Français", from: "Add language" fill_in "Title", with: "Titre en Français" fill_in_ckeditor "Description", with: "" click_button "Update" expect(page).to have_css "#error_explanation" - select "Français", from: :select_language + select "Français", from: "Current language" expect(page).to have_ckeditor "Description", with: "" end @@ -165,7 +165,7 @@ describe "Admin edit translatable records", :admin do scenario "Shows validation errors" do visit edit_admin_legislation_process_draft_version_path(translatable.process, translatable) - select "Français", from: :add_language + select "Français", from: "Add language" fill_in "Version title", with: "Titre en Français" click_link class: "fullscreen-toggle" fill_in "Text", with: "" @@ -174,7 +174,7 @@ describe "Admin edit translatable records", :admin do expect(page).to have_css "#error_explanation" - select "Français", from: :select_language + select "Français", from: "Current language" click_link class: "fullscreen-toggle" expect(page).to have_field "Text", with: "", class: "is-invalid-input" @@ -190,7 +190,7 @@ describe "Admin edit translatable records", :admin do scenario "Changes the existing translation" do visit path - select "Español", from: :select_language + select "Español", from: "Current language" within(".translatable-fields") do fill_in "Title", with: "Título corregido" @@ -221,7 +221,7 @@ describe "Admin edit translatable records", :admin do scenario "Changes the existing translation" do visit path - select "Español", from: :select_language + select "Español", from: "Current language" within(".translatable-fields") do fill_in "Answer", with: "Respuesta corregida" @@ -266,7 +266,7 @@ describe "Admin edit translatable records", :admin do scenario "Show validation errors" do visit edit_admin_banner_path(translatable) - select "Español", from: :select_language + select "Español", from: "Current language" expect(page).to have_field "Title", with: "Title en español" @@ -275,7 +275,7 @@ describe "Admin edit translatable records", :admin do expect(page).to have_css "#error_explanation" - select "Español", from: :select_language + select "Español", from: "Current language" expect(page).to have_field "Title", with: "", class: "is-invalid-input" end @@ -287,7 +287,7 @@ describe "Admin edit translatable records", :admin do scenario "Shows validation errors" do visit edit_admin_legislation_process_draft_version_path(translatable.process, translatable) - select "Español", from: :select_language + select "Español", from: "Current language" click_link class: "fullscreen-toggle" expect(page).to have_field "Text", with: "Texto en español" @@ -298,7 +298,7 @@ describe "Admin edit translatable records", :admin do expect(page).to have_css "#error_explanation" - select "Español", from: :select_language + select "Español", from: "Current language" click_link class: "fullscreen-toggle" expect(page).to have_field "Text", with: "" @@ -338,17 +338,17 @@ describe "Admin edit translatable records", :admin do scenario "Keeps the other languages" do visit path - select "Español", from: :select_language + select "Español", from: "Current language" click_link "Remove language" - expect(page).not_to have_select :select_language, with_options: ["Español"] + expect(page).not_to have_select "Current language", with_options: ["Español"] click_button "Save group" visit path - expect(page).not_to have_select :select_language, with_options: ["Español"] - expect(page).to have_select :select_language, with_options: ["English"] + expect(page).not_to have_select "Current language", with_options: ["Español"] + expect(page).to have_select "Current language", with_options: ["English"] end end @@ -389,10 +389,10 @@ describe "Admin edit translatable records", :admin do scenario "Doesn't remove the translation" do visit path - select "Español", from: :select_language + select "Español", from: "Current language" click_link "Remove language" - select "English", from: :select_language + select "English", from: "Current language" fill_in "Question", with: "" click_button "Save" @@ -402,7 +402,7 @@ describe "Admin edit translatable records", :admin do expect_not_to_have_language "Español" visit path - select "Español", from: :select_language + select "Español", from: "Current language" expect(page).to have_field "Question", with: "Pregunta en español" end @@ -417,9 +417,9 @@ describe "Admin edit translatable records", :admin do visit edit_admin_admin_notification_path(translatable) - select "English", from: :select_language + select "English", from: "Current language" click_link "Remove language" - select "Español", from: :select_language + select "Español", from: "Current language" click_link "Remove language" click_button "Update notification" @@ -436,9 +436,9 @@ describe "Admin edit translatable records", :admin do visit edit_admin_budget_budget_phase_path(translatable.budget, translatable) - select "English", from: :select_language + select "English", from: "Current language" click_link "Remove language" - select "Español", from: :select_language + select "Español", from: "Current language" click_link "Remove language" click_button "Save changes" @@ -458,9 +458,9 @@ describe "Admin edit translatable records", :admin do visit edit_admin_active_polls_path(translatable) - select "English", from: :select_language + select "English", from: "Current language" click_link "Remove language" - select "Español", from: :select_language + select "Español", from: "Current language" click_link "Remove language" click_button "Save" @@ -498,7 +498,7 @@ describe "Admin edit translatable records", :admin do expect(page).to have_field "contents_content_#{content.key}values_value_en", with: "Value in English" - select "Español", from: :select_language + select "Español", from: "Current language" expect(page).to have_field "contents_content_#{content.key}values_value_es", with: "Value en español" end @@ -506,10 +506,10 @@ describe "Admin edit translatable records", :admin do scenario "Select a locale and add it to the form" do visit path - select "Français", from: :add_language + select "Français", from: "Add language" expect_to_have_language_selected "Français" - expect(page).to have_select :add_language, selected: "Add language" + expect(page).to have_select "Add language", selected: "" expect(page).to have_field "contents_content_#{content.key}values_value_fr" end @@ -523,7 +523,7 @@ describe "Admin edit translatable records", :admin do scenario "Increase description count after add new language" do visit path - select "Français", from: :add_language + select "Français", from: "Add language" expect(page).to have_content "3 languages in use" end diff --git a/spec/system/translatable_spec.rb b/spec/system/translatable_spec.rb index d502fbd4a..1054808c8 100644 --- a/spec/system/translatable_spec.rb +++ b/spec/system/translatable_spec.rb @@ -43,7 +43,7 @@ describe "Public area translatable records" do fill_in_new_investment_title with: "My awesome project" fill_in_ckeditor "Description", with: "Everything is awesome!" - select "Français", from: :add_language + select "Français", from: "Add language" fill_in_new_investment_title with: "Titre en Français" fill_in_ckeditor "Description", with: "Contenu en Français" @@ -56,7 +56,7 @@ describe "Public area translatable records" do scenario "Add only single translation at once not having the current locale" do visit new_proposal_path click_link "Remove language" - select "Français", from: :add_language + select "Français", from: "Add language" fill_in_new_proposal_title with: "Titre en Français" fill_in "Proposal summary", with: "Résumé en Français" @@ -71,7 +71,7 @@ describe "Public area translatable records" do visit new_budget_investment_path(budget) click_link "Remove language" - select "Português brasileiro", from: :add_language + select "Português brasileiro", from: "Add language" fill_in_new_investment_title with: "Titre en Français" fill_in_ckeditor "Description", with: "Contenu en Français" @@ -123,7 +123,7 @@ describe "Public area translatable records" do scenario "Select a locale and add it to the form" do visit new_budget_investment_path(create(:budget)) - select "Français", from: :add_language + select "Français", from: "Add language" expect(page).to have_field "Title", with: "" end @@ -147,7 +147,7 @@ describe "Public area translatable records" do scenario "Increase description count after add new language" do visit new_proposal_path - select "Español", from: :add_language + select "Español", from: "Add language" expect(page).to have_content "2 languages in use" end @@ -195,7 +195,7 @@ describe "Public area translatable records" do scenario "Changes the existing translation" do visit path - select "Español", from: :select_language + select "Español", from: "Current language" fill_in "Debate title", with: "Título corregido" fill_in_ckeditor "Initial debate text", with: "Texto corregido" @@ -218,7 +218,7 @@ describe "Public area translatable records" do scenario "Show validation errors" do visit edit_proposal_path(translatable) - select "Español", from: :select_language + select "Español", from: "Current language" expect(page).to have_field "Proposal title", with: "Título en español" @@ -227,7 +227,7 @@ describe "Public area translatable records" do expect(page).to have_css "#error_explanation" - select "Español", from: :select_language + select "Español", from: "Current language" expect(page).to have_field "Proposal title", with: "", class: "is-invalid-input" end From e850ae2ff948515faad6db2a2d864c652f56c1ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 11 Oct 2024 19:58:10 +0200 Subject: [PATCH 02/25] Move banner form partial to a component Other than simplifying the controller, this'll make it easier to write tests for this code. --- .../admin/banners/form_component.html.erb} | 10 +++++----- app/components/admin/banners/form_component.rb | 15 +++++++++++++++ app/controllers/admin/banners_controller.rb | 6 ------ app/views/admin/banners/edit.html.erb | 2 +- app/views/admin/banners/new.html.erb | 2 +- 5 files changed, 22 insertions(+), 13 deletions(-) rename app/{views/admin/banners/_form.html.erb => components/admin/banners/form_component.html.erb} (88%) create mode 100644 app/components/admin/banners/form_component.rb diff --git a/app/views/admin/banners/_form.html.erb b/app/components/admin/banners/form_component.html.erb similarity index 88% rename from app/views/admin/banners/_form.html.erb rename to app/components/admin/banners/form_component.html.erb index cd82e3f37..7468a2ff0 100644 --- a/app/views/admin/banners/_form.html.erb +++ b/app/components/admin/banners/form_component.html.erb @@ -1,8 +1,8 @@ -<%= render "shared/globalize_locales", resource: @banner %> +<%= render "shared/globalize_locales", resource: banner %> -<%= translatable_form_for [:admin, @banner] do |f| %> +<%= translatable_form_for [:admin, banner] do |f| %> - <%= render "shared/errors", resource: @banner %> + <%= render "shared/errors", resource: banner %>
@@ -36,7 +36,7 @@
<%= t("admin.banners.banner.sections_label") %> - <%= f.collection_check_boxes(:web_section_ids, @banner_sections, :id, :name) do |b| %> + <%= f.collection_check_boxes(:web_section_ids, sections, :id, :name) do |b| %> <%= b.label do %> <%= b.check_box + t("admin.banners.banner.sections.#{b.text}") %> <% end %> @@ -79,6 +79,6 @@
- <%= render Shared::BannerComponent.new(@banner) %> + <%= render Shared::BannerComponent.new(banner) %>
<% end %> diff --git a/app/components/admin/banners/form_component.rb b/app/components/admin/banners/form_component.rb new file mode 100644 index 000000000..26c51f9ba --- /dev/null +++ b/app/components/admin/banners/form_component.rb @@ -0,0 +1,15 @@ +class Admin::Banners::FormComponent < ApplicationComponent + include TranslatableFormHelper + include GlobalizeHelper + attr_reader :banner + + def initialize(banner) + @banner = banner + end + + private + + def sections + WebSection.all + end +end diff --git a/app/controllers/admin/banners_controller.rb b/app/controllers/admin/banners_controller.rb index 72b085a5d..3ae9df5f8 100644 --- a/app/controllers/admin/banners_controller.rb +++ b/app/controllers/admin/banners_controller.rb @@ -3,8 +3,6 @@ class Admin::BannersController < Admin::BaseController has_filters %w[all with_active with_inactive], only: :index - before_action :banner_sections, only: [:edit, :new, :create, :update] - respond_to :html, :js load_and_authorize_resource @@ -47,10 +45,6 @@ class Admin::BannersController < Admin::BaseController web_section_ids: []] end - def banner_sections - @banner_sections = WebSection.all - end - def resource @banner ||= Banner.find(params[:id]) end diff --git a/app/views/admin/banners/edit.html.erb b/app/views/admin/banners/edit.html.erb index 26b4f49b0..a3d17da83 100644 --- a/app/views/admin/banners/edit.html.erb +++ b/app/views/admin/banners/edit.html.erb @@ -5,6 +5,6 @@

<%= t("admin.banners.edit.editing") %>

- <%= render "form" %> + <%= render Admin::Banners::FormComponent.new(@banner) %>
diff --git a/app/views/admin/banners/new.html.erb b/app/views/admin/banners/new.html.erb index 1174fc81c..27df19519 100644 --- a/app/views/admin/banners/new.html.erb +++ b/app/views/admin/banners/new.html.erb @@ -5,6 +5,6 @@

<%= t("admin.banners.new.creating") %>

- <%= render "form" %> + <%= render Admin::Banners::FormComponent.new(@banner) %> From c1fbcb4e0f113f1af8b050db210cd3eb115bca5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 8 Nov 2024 14:50:41 +0100 Subject: [PATCH 03/25] Remove obsolete resource method in controllers This method was used by controllers using the `Translatable` concern. We forgot to remove it in commit 71601bd3f. --- app/controllers/admin/admin_notifications_controller.rb | 4 ---- app/controllers/admin/banners_controller.rb | 4 ---- .../admin/legislation/draft_versions_controller.rb | 4 ---- app/controllers/admin/legislation/homepages_controller.rb | 4 ---- app/controllers/admin/legislation/processes_controller.rb | 4 ---- app/controllers/admin/legislation/questions_controller.rb | 4 ---- .../admin/site_customization/information_texts_controller.rb | 4 ---- app/controllers/admin/site_customization/pages_controller.rb | 4 ---- 8 files changed, 32 deletions(-) diff --git a/app/controllers/admin/admin_notifications_controller.rb b/app/controllers/admin/admin_notifications_controller.rb index f76a4a756..484326942 100644 --- a/app/controllers/admin/admin_notifications_controller.rb +++ b/app/controllers/admin/admin_notifications_controller.rb @@ -69,8 +69,4 @@ class Admin::AdminNotificationsController < Admin::BaseController def allowed_params [:link, :segment_recipient, translation_params(AdminNotification)] end - - def resource - AdminNotification.find(params[:id]) - end end diff --git a/app/controllers/admin/banners_controller.rb b/app/controllers/admin/banners_controller.rb index 3ae9df5f8..dee4fab2d 100644 --- a/app/controllers/admin/banners_controller.rb +++ b/app/controllers/admin/banners_controller.rb @@ -44,8 +44,4 @@ class Admin::BannersController < Admin::BaseController translation_params(Banner), web_section_ids: []] end - - def resource - @banner ||= Banner.find(params[:id]) - end end diff --git a/app/controllers/admin/legislation/draft_versions_controller.rb b/app/controllers/admin/legislation/draft_versions_controller.rb index 5cb6fc88e..7dae29334 100644 --- a/app/controllers/admin/legislation/draft_versions_controller.rb +++ b/app/controllers/admin/legislation/draft_versions_controller.rb @@ -45,8 +45,4 @@ class Admin::Legislation::DraftVersionsController < Admin::Legislation::BaseCont def allowed_params [:status, :final_version, translation_params(Legislation::DraftVersion)] end - - def resource - @draft_version - end end diff --git a/app/controllers/admin/legislation/homepages_controller.rb b/app/controllers/admin/legislation/homepages_controller.rb index 68d10285c..d0c8fd51c 100644 --- a/app/controllers/admin/legislation/homepages_controller.rb +++ b/app/controllers/admin/legislation/homepages_controller.rb @@ -26,8 +26,4 @@ class Admin::Legislation::HomepagesController < Admin::Legislation::BaseControll def allowed_params [:homepage, :homepage_enabled, translation_params(::Legislation::Process)] end - - def resource - @process || ::Legislation::Process.find(params[:id]) - end end diff --git a/app/controllers/admin/legislation/processes_controller.rb b/app/controllers/admin/legislation/processes_controller.rb index d35b7ca52..17c18b794 100644 --- a/app/controllers/admin/legislation/processes_controller.rb +++ b/app/controllers/admin/legislation/processes_controller.rb @@ -77,8 +77,4 @@ class Admin::Legislation::ProcessesController < Admin::Legislation::BaseControll image_attributes: image_attributes ] end - - def resource - @process || ::Legislation::Process.find(params[:id]) - end end diff --git a/app/controllers/admin/legislation/questions_controller.rb b/app/controllers/admin/legislation/questions_controller.rb index 4c285e7ab..bab3c81d2 100644 --- a/app/controllers/admin/legislation/questions_controller.rb +++ b/app/controllers/admin/legislation/questions_controller.rb @@ -55,8 +55,4 @@ class Admin::Legislation::QuestionsController < Admin::Legislation::BaseControll translation_params(::Legislation::QuestionOption)] ] end - - def resource - @question || ::Legislation::Question.find(params[:id]) - end end diff --git a/app/controllers/admin/site_customization/information_texts_controller.rb b/app/controllers/admin/site_customization/information_texts_controller.rb index 4a8591b24..54c4078e1 100644 --- a/app/controllers/admin/site_customization/information_texts_controller.rb +++ b/app/controllers/admin/site_customization/information_texts_controller.rb @@ -15,10 +15,6 @@ class Admin::SiteCustomization::InformationTextsController < Admin::SiteCustomiz private - def resource - I18nContent.find(content_params[:id]) - end - def content_params params.require(:contents).values end diff --git a/app/controllers/admin/site_customization/pages_controller.rb b/app/controllers/admin/site_customization/pages_controller.rb index 7721da3f6..11a69b9d9 100644 --- a/app/controllers/admin/site_customization/pages_controller.rb +++ b/app/controllers/admin/site_customization/pages_controller.rb @@ -43,8 +43,4 @@ class Admin::SiteCustomization::PagesController < Admin::SiteCustomization::Base [*attributes, translation_params(SiteCustomization::Page)] end - - def resource - SiteCustomization::Page.find(params[:id]) - end end From 8aff1414c51b89e216de1416e82f8067abe7a7e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 11 Oct 2024 20:12:04 +0200 Subject: [PATCH 04/25] Move legislation process form partial to a component This will make it easier to write tests for it. --- .../processes/form_component.html.erb} | 22 +++++++++---------- .../legislation/processes/form_component.rb | 11 ++++++++++ .../admin/legislation/processes/edit.html.erb | 2 +- .../admin/legislation/processes/new.html.erb | 2 +- 4 files changed, 24 insertions(+), 13 deletions(-) rename app/{views/admin/legislation/processes/_form.html.erb => components/admin/legislation/processes/form_component.html.erb} (81%) create mode 100644 app/components/admin/legislation/processes/form_component.rb diff --git a/app/views/admin/legislation/processes/_form.html.erb b/app/components/admin/legislation/processes/form_component.html.erb similarity index 81% rename from app/views/admin/legislation/processes/_form.html.erb rename to app/components/admin/legislation/processes/form_component.html.erb index 0f461bfd7..82516b498 100644 --- a/app/views/admin/legislation/processes/_form.html.erb +++ b/app/components/admin/legislation/processes/form_component.html.erb @@ -1,8 +1,8 @@ -<%= render "shared/globalize_locales", resource: @process %> +<%= render "shared/globalize_locales", resource: process %> -<%= translatable_form_for [:admin, @process], html: { class: "legislation-process-form" } do |f| %> +<%= translatable_form_for [:admin, process], html: { class: "legislation-process-form" } do |f| %> - <%= render "shared/errors", resource: @process %> + <%= render "shared/errors", resource: process %>
@@ -18,7 +18,7 @@ <%= f.date_field :draft_end_date, id: "draft_end_date" %>
- <%= f.check_box :draft_phase_enabled, checked: @process.draft_phase.enabled?, label: t("admin.legislation.processes.form.enabled") %> + <%= f.check_box :draft_phase_enabled, checked: process.draft_phase.enabled?, label: t("admin.legislation.processes.form.enabled") %>
@@ -35,7 +35,7 @@ <%= f.date_field :end_date, id: "end_date" %>
- <%= f.check_box :published, checked: @process.published?, label: t("admin.legislation.processes.form.enabled") %> + <%= f.check_box :published, checked: process.published?, label: t("admin.legislation.processes.form.enabled") %>
@@ -52,7 +52,7 @@ <%= f.date_field :debate_end_date, id: "debate_end_date" %>
- <%= f.check_box :debate_phase_enabled, checked: @process.debate_phase.enabled?, label: t("admin.legislation.processes.form.enabled") %> + <%= f.check_box :debate_phase_enabled, checked: process.debate_phase.enabled?, label: t("admin.legislation.processes.form.enabled") %>
@@ -69,7 +69,7 @@ <%= f.date_field :proposals_phase_end_date, id: "proposals_phase_end_date" %>
- <%= f.check_box :proposals_phase_enabled, checked: @process.proposals_phase.enabled?, label: t("admin.legislation.processes.form.enabled") %> + <%= f.check_box :proposals_phase_enabled, checked: process.proposals_phase.enabled?, label: t("admin.legislation.processes.form.enabled") %>
@@ -86,7 +86,7 @@ <%= f.date_field :allegations_end_date, id: "allegations_end_date" %>
- <%= f.check_box :allegations_phase_enabled, checked: @process.allegations_phase.enabled?, label: t("admin.legislation.processes.form.enabled") %> + <%= f.check_box :allegations_phase_enabled, checked: process.allegations_phase.enabled?, label: t("admin.legislation.processes.form.enabled") %>
@@ -95,7 +95,7 @@ <%= f.date_field :draft_publication_date, id: "draft_publication_date" %>
- <%= f.check_box :draft_publication_enabled, checked: @process.draft_publication.enabled?, label: t("admin.legislation.processes.form.enabled") %> + <%= f.check_box :draft_publication_enabled, checked: process.draft_publication.enabled?, label: t("admin.legislation.processes.form.enabled") %>
@@ -104,7 +104,7 @@ <%= f.date_field :result_publication_date, id: "result_publication_date" %>
- <%= f.check_box :result_publication_enabled, checked: @process.result_publication.enabled?, label: t("admin.legislation.processes.form.enabled") %> + <%= f.check_box :result_publication_enabled, checked: process.result_publication.enabled?, label: t("admin.legislation.processes.form.enabled") %>
@@ -182,7 +182,7 @@
- <%= f.submit(class: "button success expanded", value: t("admin.legislation.processes.#{admin_submit_action(@process)}.submit_button")) %> + <%= f.submit(class: "button success expanded", value: t("admin.legislation.processes.#{admin_submit_action(process)}.submit_button")) %>
<% end %> diff --git a/app/components/admin/legislation/processes/form_component.rb b/app/components/admin/legislation/processes/form_component.rb new file mode 100644 index 000000000..b0525378b --- /dev/null +++ b/app/components/admin/legislation/processes/form_component.rb @@ -0,0 +1,11 @@ +class Admin::Legislation::Processes::FormComponent < ApplicationComponent + include TranslatableFormHelper + include GlobalizeHelper + + attr_reader :process + use_helpers :admin_submit_action + + def initialize(process) + @process = process + end +end diff --git a/app/views/admin/legislation/processes/edit.html.erb b/app/views/admin/legislation/processes/edit.html.erb index 372792e7e..2d2f148f0 100644 --- a/app/views/admin/legislation/processes/edit.html.erb +++ b/app/views/admin/legislation/processes/edit.html.erb @@ -9,5 +9,5 @@ <%= render "subnav", process: @process, active: "info" %> - <%= render "form" %> + <%= render Admin::Legislation::Processes::FormComponent.new(@process) %> diff --git a/app/views/admin/legislation/processes/new.html.erb b/app/views/admin/legislation/processes/new.html.erb index 3efb091f6..aff4002b7 100644 --- a/app/views/admin/legislation/processes/new.html.erb +++ b/app/views/admin/legislation/processes/new.html.erb @@ -7,5 +7,5 @@

<%= t("admin.legislation.processes.new.title") %>

- <%= render "form" %> + <%= render Admin::Legislation::Processes::FormComponent.new(@process) %> From 9c057d56950fb62317a5d602c0733fb4b447b7c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 3 Apr 2024 23:24:18 +0200 Subject: [PATCH 05/25] Fix labels in color selection We were using the same label for two elements, but the label was only assigned to one of them. --- .../admin/banners/form_component.html.erb | 12 +++--- .../admin/geozones/form_component.html.erb | 6 +-- .../processes/form_component.html.erb | 12 +++--- .../admin/banners/form_component_spec.rb | 38 +++++++++++++++++++ .../admin/geozones/form_component_spec.rb | 22 +++++++++++ .../processes/form_component_spec.rb | 38 +++++++++++++++++++ 6 files changed, 113 insertions(+), 15 deletions(-) create mode 100644 spec/components/admin/banners/form_component_spec.rb create mode 100644 spec/components/admin/geozones/form_component_spec.rb create mode 100644 spec/components/admin/legislation/processes/form_component_spec.rb diff --git a/app/components/admin/banners/form_component.html.erb b/app/components/admin/banners/form_component.html.erb index 7468a2ff0..a3d5c3d83 100644 --- a/app/components/admin/banners/form_component.html.erb +++ b/app/components/admin/banners/form_component.html.erb @@ -45,12 +45,12 @@
-
- <%= f.label :background_color, nil, for: "background_color_input" %> +
+ <%= f.label :background_color, nil, for: "background_color_input", id: "background_color_label" %>

<%= t("admin.shared.color_help") %>

- <%= f.text_field :background_color, label: false, type: :color %> + <%= f.text_field :background_color, label: false, type: :color, "aria-labelledby": "background_color_label" %>
<%= f.text_field :background_color, label: false, id: "background_color_input" %> @@ -58,12 +58,12 @@
-
- <%= f.label :font_color, nil, for: "font_color_input" %> +
+ <%= f.label :font_color, nil, for: "font_color_input", id: "font_color_label" %>

<%= t("admin.shared.color_help") %>

- <%= f.text_field :font_color, label: false, type: :color %> + <%= f.text_field :font_color, label: false, type: :color, "aria-labelledby": "font_color_label" %>
<%= f.text_field :font_color, label: false, id: "font_color_input" %> diff --git a/app/components/admin/geozones/form_component.html.erb b/app/components/admin/geozones/form_component.html.erb index bd6a00ab9..ddf2a42d4 100644 --- a/app/components/admin/geozones/form_component.html.erb +++ b/app/components/admin/geozones/form_component.html.erb @@ -24,14 +24,14 @@ <%= f.text_area :geojson, rows: "10", hint: t("admin.geozones.geozone.geojson_help") %>
-
- <%= f.label :color, nil, for: "color_input", id: "color_input_label" %> +
+ <%= f.label :color, nil, for: "color_input", id: "color_label" %>

<%= t("admin.geozones.geozone.color_help", format_help: t("admin.shared.color_help")) %>

- <%= f.text_field :color, label: false, type: :color %> + <%= f.text_field :color, label: false, type: :color, "aria-labelledby": "color_label" %>
<%= f.text_field :color, label: false, id: "color_input" %> diff --git a/app/components/admin/legislation/processes/form_component.html.erb b/app/components/admin/legislation/processes/form_component.html.erb index 82516b498..6662148fc 100644 --- a/app/components/admin/legislation/processes/form_component.html.erb +++ b/app/components/admin/legislation/processes/form_component.html.erb @@ -121,12 +121,12 @@

<%= t("admin.legislation.processes.form.banner_title") %>

-
- <%= f.label :background_color, nil, for: "background_color_input" %> +
+ <%= f.label :background_color, nil, for: "background_color_input", id: "background_color_label" %>

<%= t("admin.shared.color_help") %>

- <%= f.text_field :background_color, label: false, type: :color %> + <%= f.text_field :background_color, label: false, type: :color, "aria-labelledby": "background_color_label" %>
<%= f.text_field :background_color, label: false, id: "background_color_input" %> @@ -134,12 +134,12 @@
-
- <%= f.label :font_color, nil, for: "font_color_input" %> +
+ <%= f.label :font_color, nil, for: "font_color_input", id: "font_color_label" %>

<%= t("admin.shared.color_help") %>

- <%= f.text_field :font_color, label: false, type: :color %> + <%= f.text_field :font_color, label: false, type: :color, "aria-labelledby": "font_color_label" %>
<%= f.text_field :font_color, label: false, id: "font_color_input" %> diff --git a/spec/components/admin/banners/form_component_spec.rb b/spec/components/admin/banners/form_component_spec.rb new file mode 100644 index 000000000..ccfae6b05 --- /dev/null +++ b/spec/components/admin/banners/form_component_spec.rb @@ -0,0 +1,38 @@ +require "rails_helper" + +describe Admin::Banners::FormComponent do + let(:banner) { build(:banner) } + let(:component) { Admin::Banners::FormComponent.new(banner) } + + describe "background color fields" do + it "renders two inputs sharing the same label" do + render_inline component + + page.find(".background-color-inputs") do |color_inputs| + expect(color_inputs).to have_css "input", count: 2 + expect(color_inputs).to have_css "label", count: 1 + + expect(color_inputs).to have_css "label#background_color_label[for=background_color_input]" + expect(color_inputs.all("input")[0][:"aria-labelledby"]).to eq "background_color_label" + expect(color_inputs.all("input")[0][:id]).not_to eq "background_color_input" + expect(color_inputs.all("input")[1][:id]).to eq "background_color_input" + end + end + end + + describe "font color fields" do + it "renders two inputs sharing the same label" do + render_inline component + + page.find(".font-color-inputs") do |color_inputs| + expect(color_inputs).to have_css "input", count: 2 + expect(color_inputs).to have_css "label", count: 1 + + expect(color_inputs).to have_css "label#font_color_label[for=font_color_input]" + expect(color_inputs.all("input")[0][:"aria-labelledby"]).to eq "font_color_label" + expect(color_inputs.all("input")[0][:id]).not_to eq "font_color_input" + expect(color_inputs.all("input")[1][:id]).to eq "font_color_input" + end + end + end +end diff --git a/spec/components/admin/geozones/form_component_spec.rb b/spec/components/admin/geozones/form_component_spec.rb new file mode 100644 index 000000000..cd236efcb --- /dev/null +++ b/spec/components/admin/geozones/form_component_spec.rb @@ -0,0 +1,22 @@ +require "rails_helper" + +describe Admin::Geozones::FormComponent do + let(:geozone) { build(:geozone) } + let(:component) { Admin::Geozones::FormComponent.new(geozone) } + + describe "color fields" do + it "renders two inputs sharing the same label" do + render_inline component + + page.find(".color-inputs") do |color_inputs| + expect(color_inputs).to have_css "input", count: 2 + expect(color_inputs).to have_css "label", count: 1 + + expect(color_inputs).to have_css "label#color_label[for=color_input]" + expect(color_inputs.all("input")[0][:"aria-labelledby"]).to eq "color_label" + expect(color_inputs.all("input")[0][:id]).not_to eq "color_input" + expect(color_inputs.all("input")[1][:id]).to eq "color_input" + end + end + end +end diff --git a/spec/components/admin/legislation/processes/form_component_spec.rb b/spec/components/admin/legislation/processes/form_component_spec.rb new file mode 100644 index 000000000..dbc6204ef --- /dev/null +++ b/spec/components/admin/legislation/processes/form_component_spec.rb @@ -0,0 +1,38 @@ +require "rails_helper" + +describe Admin::Legislation::Processes::FormComponent, :admin do + let(:process) { build(:legislation_process) } + let(:component) { Admin::Legislation::Processes::FormComponent.new(process) } + + describe "background color fields" do + it "renders two inputs sharing the same label" do + render_inline component + + page.find(".background-color-inputs") do |color_inputs| + expect(color_inputs).to have_css "input", count: 2 + expect(color_inputs).to have_css "label", count: 1 + + expect(color_inputs).to have_css "label#background_color_label[for=background_color_input]" + expect(color_inputs.all("input")[0][:"aria-labelledby"]).to eq "background_color_label" + expect(color_inputs.all("input")[0][:id]).not_to eq "background_color_input" + expect(color_inputs.all("input")[1][:id]).to eq "background_color_input" + end + end + end + + describe "font color fields" do + it "renders two inputs sharing the same label" do + render_inline component + + page.find(".font-color-inputs") do |color_inputs| + expect(color_inputs).to have_css "input", count: 2 + expect(color_inputs).to have_css "label", count: 1 + + expect(color_inputs).to have_css "label#font_color_label[for=font_color_input]" + expect(color_inputs.all("input")[0][:"aria-labelledby"]).to eq "font_color_label" + expect(color_inputs.all("input")[0][:id]).not_to eq "font_color_input" + expect(color_inputs.all("input")[1][:id]).to eq "font_color_input" + end + end + end +end From 233ba3c72f3a8fb13d18adbdc77d364233a5ca6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 11 Oct 2024 22:39:08 +0200 Subject: [PATCH 06/25] Move progress bars form partial to a component This way we can move some of the view logic to the Ruby class. It'll also make it easier to write tests for it. --- .../admin/progress_bars/form_component.html.erb} | 5 ++--- .../admin/progress_bars/form_component.rb | 15 +++++++++++++++ app/views/admin/progress_bars/edit.html.erb | 2 +- app/views/admin/progress_bars/new.html.erb | 2 +- 4 files changed, 19 insertions(+), 5 deletions(-) rename app/{views/admin/progress_bars/_form.html.erb => components/admin/progress_bars/form_component.html.erb} (81%) create mode 100644 app/components/admin/progress_bars/form_component.rb diff --git a/app/views/admin/progress_bars/_form.html.erb b/app/components/admin/progress_bars/form_component.html.erb similarity index 81% rename from app/views/admin/progress_bars/_form.html.erb rename to app/components/admin/progress_bars/form_component.html.erb index 0a479245f..f86e107d9 100644 --- a/app/views/admin/progress_bars/_form.html.erb +++ b/app/components/admin/progress_bars/form_component.html.erb @@ -1,6 +1,6 @@ -<%= render "shared/globalize_locales", resource: @progress_bar %> +<%= render "shared/globalize_locales", resource: progress_bar %> -<%= translatable_form_for @progress_bar, url: admin_polymorphic_path(@progress_bar) do |f| %> +<%= translatable_form_for progress_bar, url: admin_polymorphic_path(progress_bar) do |f| %>
@@ -16,7 +16,6 @@ <% end %>
- <% progress_options = { min: ProgressBar::RANGE.min, max: ProgressBar::RANGE.max, step: 1 } %>
<%= f.label :percentage %> diff --git a/app/components/admin/progress_bars/form_component.rb b/app/components/admin/progress_bars/form_component.rb new file mode 100644 index 000000000..a93f6c0fb --- /dev/null +++ b/app/components/admin/progress_bars/form_component.rb @@ -0,0 +1,15 @@ +class Admin::ProgressBars::FormComponent < ApplicationComponent + include TranslatableFormHelper + include GlobalizeHelper + attr_reader :progress_bar + + def initialize(progress_bar) + @progress_bar = progress_bar + end + + private + + def progress_options + { min: ProgressBar::RANGE.min, max: ProgressBar::RANGE.max, step: 1 } + end +end diff --git a/app/views/admin/progress_bars/edit.html.erb b/app/views/admin/progress_bars/edit.html.erb index 21dd27d9a..c7f280a63 100644 --- a/app/views/admin/progress_bars/edit.html.erb +++ b/app/views/admin/progress_bars/edit.html.erb @@ -12,4 +12,4 @@

<%= bar_title %>

-<%= render "form" %> +<%= render Admin::ProgressBars::FormComponent.new(@progress_bar) %> diff --git a/app/views/admin/progress_bars/new.html.erb b/app/views/admin/progress_bars/new.html.erb index 8c379ac3a..81c76b920 100644 --- a/app/views/admin/progress_bars/new.html.erb +++ b/app/views/admin/progress_bars/new.html.erb @@ -6,4 +6,4 @@

<%= t("admin.progress_bars.new.creating") %>

-<%= render "form" %> +<%= render Admin::ProgressBars::FormComponent.new(@progress_bar) %> From 9f738b8d5f5e806d84e0ddfd9d7286e71a79fb15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 10 Oct 2024 21:30:09 +0200 Subject: [PATCH 07/25] Fix labels in progress bar percentage selection We were using the same label for two elements, but the label was only assigned to one of them. --- .../progress_bars/form_component.html.erb | 5 +++-- .../progress_bars/form_component_spec.rb | 22 +++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 spec/components/admin/progress_bars/form_component_spec.rb diff --git a/app/components/admin/progress_bars/form_component.html.erb b/app/components/admin/progress_bars/form_component.html.erb index f86e107d9..ea73f5f91 100644 --- a/app/components/admin/progress_bars/form_component.html.erb +++ b/app/components/admin/progress_bars/form_component.html.erb @@ -16,12 +16,13 @@ <% end %>
-
+
- <%= f.label :percentage %> + <%= f.label :percentage, nil, id: "percentage_label" %> <%= f.text_field :percentage, { type: :range, id: "percentage_range", label: false, + "aria-labelledby": "percentage_label", class: "column" }.merge(progress_options) %>
diff --git a/spec/components/admin/progress_bars/form_component_spec.rb b/spec/components/admin/progress_bars/form_component_spec.rb new file mode 100644 index 000000000..5dabdfbf9 --- /dev/null +++ b/spec/components/admin/progress_bars/form_component_spec.rb @@ -0,0 +1,22 @@ +require "rails_helper" + +describe Admin::ProgressBars::FormComponent do + let(:progress_bar) { build(:progress_bar) } + let(:component) { Admin::ProgressBars::FormComponent.new(progress_bar) } + + describe "percentage fields" do + it "renders two inputs sharing the same label" do + render_inline component + + page.find(".percentage-inputs") do |percentage_inputs| + expect(percentage_inputs).to have_css "input:not([type=submit])", count: 2 + expect(percentage_inputs).to have_css "label", count: 1 + + expect(percentage_inputs).to have_css "label#percentage_label[for=progress_bar_percentage]" + expect(percentage_inputs.all("input")[0][:"aria-labelledby"]).to eq "percentage_label" + expect(percentage_inputs.all("input")[0][:id]).not_to eq "percentage_input" + expect(percentage_inputs.all("input")[1][:id]).to eq "progress_bar_percentage" + end + end + end +end From 431ebeda87d30710d75be2f2b17463125e0111f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 10 Oct 2024 23:38:57 +0200 Subject: [PATCH 08/25] Fix missing "for" attribute in document number label Since this attribute was missing, the label wasn't correctly associated with its field. --- app/views/verification/residence/new.html.erb | 2 +- spec/system/verification/residence_spec.rb | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/views/verification/residence/new.html.erb b/app/views/verification/residence/new.html.erb index a0d9b955e..c361d7d4c 100644 --- a/app/views/verification/residence/new.html.erb +++ b/app/views/verification/residence/new.html.erb @@ -38,7 +38,7 @@
- <%= f.label t("verification.residence.new.document_number") %> + <%= f.label :document_number, t("verification.residence.new.document_number") %>
- <%= check_box_tag "budget_investment_ids[]", investment.id, nil, id: "#{dom_id(investment)}_check" %> + <%= check_box_tag "budget_investment_ids[]", + investment.id, + nil, + id: "#{dom_id(investment)}_check", + "aria-label": investment.title %> <% end %> diff --git a/app/views/moderation/comments/index.html.erb b/app/views/moderation/comments/index.html.erb index 2381ce886..d6586313e 100644 --- a/app/views/moderation/comments/index.html.erb +++ b/app/views/moderation/comments/index.html.erb @@ -12,7 +12,9 @@ <%= comment.commentable_type.constantize.model_name.human %> - - <%= link_to comment.commentable.title, commentable_path(comment) %> + <%= link_to comment.commentable.title, + commentable_path(comment), + id: dom_id(comment, :title) %>
<%= l comment.updated_at.to_date %>  •  @@ -25,7 +27,11 @@
- <%= check_box_tag "comment_ids[]", comment.id, nil, id: "#{dom_id(comment)}_check" %> + <%= check_box_tag "comment_ids[]", + comment.id, + nil, + id: "#{dom_id(comment)}_check", + "aria-labelledby": dom_id(comment, :title) %> <% end %> diff --git a/app/views/moderation/debates/index.html.erb b/app/views/moderation/debates/index.html.erb index a3f666764..2691c3ba4 100644 --- a/app/views/moderation/debates/index.html.erb +++ b/app/views/moderation/debates/index.html.erb @@ -24,7 +24,11 @@
- <%= check_box_tag "debate_ids[]", debate.id, nil, id: "#{dom_id(debate)}_check" %> + <%= check_box_tag "debate_ids[]", + debate.id, + nil, + id: "#{dom_id(debate)}_check", + "aria-label": debate.title %> <% end %> diff --git a/app/views/moderation/proposal_notifications/index.html.erb b/app/views/moderation/proposal_notifications/index.html.erb index 2c8a71ac6..39e1addaf 100644 --- a/app/views/moderation/proposal_notifications/index.html.erb +++ b/app/views/moderation/proposal_notifications/index.html.erb @@ -23,7 +23,8 @@ <%= check_box_tag "proposal_notification_ids[]", proposal_notification.id, nil, - id: "#{dom_id(proposal_notification)}_check" %> + id: "#{dom_id(proposal_notification)}_check", + "aria-label": proposal_notification.title %> <% end %> diff --git a/app/views/moderation/proposals/index.html.erb b/app/views/moderation/proposals/index.html.erb index 387ab60d8..e284d75bc 100644 --- a/app/views/moderation/proposals/index.html.erb +++ b/app/views/moderation/proposals/index.html.erb @@ -24,7 +24,11 @@
- <%= check_box_tag "proposal_ids[]", proposal.id, nil, id: "#{dom_id(proposal)}_check" %> + <%= check_box_tag "proposal_ids[]", + proposal.id, + nil, + id: "#{dom_id(proposal)}_check", + "aria-label": proposal.title %> <% end %> diff --git a/spec/system/moderation/budget_investments_spec.rb b/spec/system/moderation/budget_investments_spec.rb index ad1bb0460..70cec4304 100644 --- a/spec/system/moderation/budget_investments_spec.rb +++ b/spec/system/moderation/budget_investments_spec.rb @@ -55,9 +55,7 @@ describe "Moderate budget investments" do visit moderation_budget_investments_path click_link "All" - within("#investment_#{investment.id}") do - check "budget_investment_#{investment.id}_check" - end + check investment.title end scenario "Hide the investment" do diff --git a/spec/system/moderation/debates_spec.rb b/spec/system/moderation/debates_spec.rb index df8ecf168..c9db5c9a6 100644 --- a/spec/system/moderation/debates_spec.rb +++ b/spec/system/moderation/debates_spec.rb @@ -49,9 +49,7 @@ describe "Moderate debates" do visit moderation_debates_path click_link "All" - within("#debate_#{debate.id}") do - check "debate_#{debate.id}_check" - end + check debate.title end scenario "Hide the debate" do diff --git a/spec/system/moderation/proposal_notifications_spec.rb b/spec/system/moderation/proposal_notifications_spec.rb index fec1a74fe..ccbff02c0 100644 --- a/spec/system/moderation/proposal_notifications_spec.rb +++ b/spec/system/moderation/proposal_notifications_spec.rb @@ -57,9 +57,7 @@ describe "Moderate proposal notifications" do visit moderation_proposal_notifications_path click_link "All" - within("#proposal_notification_#{proposal_notification.id}") do - check "proposal_notification_#{proposal_notification.id}_check" - end + check proposal_notification.title end scenario "Hide the proposal" do diff --git a/spec/system/moderation/proposals_spec.rb b/spec/system/moderation/proposals_spec.rb index 6fb8ce02d..0687e1dfc 100644 --- a/spec/system/moderation/proposals_spec.rb +++ b/spec/system/moderation/proposals_spec.rb @@ -48,9 +48,7 @@ describe "Moderate proposals" do visit moderation_proposals_path click_link "All" - within("#proposal_#{proposal.id}") do - check "proposal_#{proposal.id}_check" - end + check proposal.title end scenario "Hide the proposal" do From f8d06bcaf24c4a27912dfaee32424550473e4f76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 11 Oct 2024 01:30:24 +0200 Subject: [PATCH 17/25] Move management investments search partial to a component We're also moving it to the `management` subfolder, since it's only used in the budget investments management. --- .../budgets/investments/search_component.html.erb} | 2 +- .../management/budgets/investments/search_component.rb | 9 +++++++++ app/views/management/budgets/investments/index.html.erb | 2 +- app/views/management/budgets/investments/print.html.erb | 2 +- 4 files changed, 12 insertions(+), 3 deletions(-) rename app/{views/admin/shared/_budget_investment_search.html.erb => components/management/budgets/investments/search_component.html.erb} (98%) create mode 100644 app/components/management/budgets/investments/search_component.rb diff --git a/app/views/admin/shared/_budget_investment_search.html.erb b/app/components/management/budgets/investments/search_component.html.erb similarity index 98% rename from app/views/admin/shared/_budget_investment_search.html.erb rename to app/components/management/budgets/investments/search_component.html.erb index 7e9d3d082..cc762b754 100644 --- a/app/views/admin/shared/_budget_investment_search.html.erb +++ b/app/components/management/budgets/investments/search_component.html.erb @@ -5,7 +5,7 @@
<%= select_tag :heading_id, - options_for_select(budget_heading_select_options(@budget), + options_for_select(budget_heading_select_options(budget), params[:heading_id]), include_blank: true %>
diff --git a/app/components/management/budgets/investments/search_component.rb b/app/components/management/budgets/investments/search_component.rb new file mode 100644 index 000000000..7fc2dca05 --- /dev/null +++ b/app/components/management/budgets/investments/search_component.rb @@ -0,0 +1,9 @@ +class Management::Budgets::Investments::SearchComponent < ApplicationComponent + attr_reader :budget, :url + use_helpers :budget_heading_select_options + + def initialize(budget, url:) + @budget = budget + @url = url + end +end diff --git a/app/views/management/budgets/investments/index.html.erb b/app/views/management/budgets/investments/index.html.erb index 4dfc94848..73dd86f89 100644 --- a/app/views/management/budgets/investments/index.html.erb +++ b/app/views/management/budgets/investments/index.html.erb @@ -1,5 +1,5 @@ - <%= render "admin/shared/budget_investment_search", url: management_budget_investments_path(@budget) %> + <%= render Management::Budgets::Investments::SearchComponent.new(@budget, url: management_budget_investments_path(@budget)) %>
diff --git a/app/views/management/budgets/investments/print.html.erb b/app/views/management/budgets/investments/print.html.erb index 4b5de991d..11fcb59ba 100644 --- a/app/views/management/budgets/investments/print.html.erb +++ b/app/views/management/budgets/investments/print.html.erb @@ -1,5 +1,5 @@ - <%= render "admin/shared/budget_investment_search", url: print_management_budget_investments_path(@budget) %> + <%= render Management::Budgets::Investments::SearchComponent.new(@budget, url: print_management_budget_investments_path(@budget)) %>
From 39d68fd92832b227ed6b0ae636594c7d6f414c49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 11 Oct 2024 01:59:21 +0200 Subject: [PATCH 18/25] Use form_tag in management area investments search form Since this form isn't associated to an object, using `form_tag` instead of `form_for` simplifies the code. --- .../management/budgets/investments/search_component.html.erb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/components/management/budgets/investments/search_component.html.erb b/app/components/management/budgets/investments/search_component.html.erb index cc762b754..b3ec0c21b 100644 --- a/app/components/management/budgets/investments/search_component.html.erb +++ b/app/components/management/budgets/investments/search_component.html.erb @@ -1,4 +1,4 @@ -<%= form_for(Budget::Investment.new, url: url, as: :budget_investment, method: :get) do |f| %> +<%= form_tag(url, method: :get) do |f| %>
<%= text_field_tag :search, "" %> @@ -19,7 +19,7 @@
- <%= f.submit t("shared.search"), class: "button" %> + <%= submit_tag t("shared.search"), class: "button" %>
<% end %> From b45f9287032613e3826e1359cb10f08fe7354c94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 11 Oct 2024 02:28:22 +0200 Subject: [PATCH 19/25] Fix label width in the management area search form With a 50% width on medium and large screens, depending on the size of the text and the size of the screen, the label could unnecessarily use two rows, looking broken. --- .../management/budgets/investments/search_component.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/components/management/budgets/investments/search_component.html.erb b/app/components/management/budgets/investments/search_component.html.erb index b3ec0c21b..77339d973 100644 --- a/app/components/management/budgets/investments/search_component.html.erb +++ b/app/components/management/budgets/investments/search_component.html.erb @@ -11,7 +11,7 @@
-
+
<%= check_box_tag :unfeasible, "1", params[:unfeasible].present? %> <%= t("admin.budget_investments.search_unfeasible") %>
From 1cefc040a7a585bf62a97ba4705c9fe4477d1b77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 11 Oct 2024 02:09:15 +0200 Subject: [PATCH 20/25] Add labels to the search form in the management area The text for the unfeasible checkbox wasn't correctly defined as a label, while the fields to search investments and select the heading weren't intuitive since their purpose wasn't obvious. --- app/assets/stylesheets/application.scss | 1 + .../management/budgets/investments/search.scss | 5 +++++ .../investments/search_component.html.erb | 10 +++++++--- .../budgets/investments/search_component.rb | 17 +++++++++++++++++ config/locales/en/management.yml | 2 ++ config/locales/es/management.yml | 2 ++ .../investments/search_component_spec.rb | 17 +++++++++++++++++ .../management/budget_investments_spec.rb | 8 ++++---- 8 files changed, 55 insertions(+), 7 deletions(-) create mode 100644 app/assets/stylesheets/management/budgets/investments/search.scss create mode 100644 spec/components/management/budgets/investments/search_component_spec.rb diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index b2badb487..e949d2e61 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -47,6 +47,7 @@ @import "documents/**/*"; @import "layout/**/*"; @import "machine_learning/**/*"; +@import "management/**/*"; @import "moderation/**/*"; @import "polls/**/*"; @import "proposals/**/*"; diff --git a/app/assets/stylesheets/management/budgets/investments/search.scss b/app/assets/stylesheets/management/budgets/investments/search.scss new file mode 100644 index 000000000..ed7561cf5 --- /dev/null +++ b/app/assets/stylesheets/management/budgets/investments/search.scss @@ -0,0 +1,5 @@ +.management-investments-search { + .checkbox-label { + font-weight: normal; + } +} diff --git a/app/components/management/budgets/investments/search_component.html.erb b/app/components/management/budgets/investments/search_component.html.erb index 77339d973..932304916 100644 --- a/app/components/management/budgets/investments/search_component.html.erb +++ b/app/components/management/budgets/investments/search_component.html.erb @@ -1,9 +1,11 @@ -<%= form_tag(url, method: :get) do |f| %> +<%= form_tag(url, options) do |f| %>
+ <%= label_tag :search, search_label_text %> <%= text_field_tag :search, "" %>
+ <%= label_tag :heading_id, attribute_name(:heading_id) %> <%= select_tag :heading_id, options_for_select(budget_heading_select_options(budget), params[:heading_id]), @@ -12,8 +14,10 @@
- <%= check_box_tag :unfeasible, "1", params[:unfeasible].present? %> - <%= t("admin.budget_investments.search_unfeasible") %> + <%= label_tag :unfeasible, class: "checkbox-label" do %> + <%= check_box_tag :unfeasible, "1", params[:unfeasible].present? %> + <%= t("admin.budget_investments.search_unfeasible") %> + <% end %>
diff --git a/app/components/management/budgets/investments/search_component.rb b/app/components/management/budgets/investments/search_component.rb index 7fc2dca05..8244a2e9b 100644 --- a/app/components/management/budgets/investments/search_component.rb +++ b/app/components/management/budgets/investments/search_component.rb @@ -6,4 +6,21 @@ class Management::Budgets::Investments::SearchComponent < ApplicationComponent @budget = budget @url = url end + + private + + def options + { + method: :get, + class: "management-investments-search" + } + end + + def search_label_text + t("management.budget_investments.search.label") + end + + def attribute_name(attribute) + Budget::Investment.human_attribute_name(attribute) + end end diff --git a/config/locales/en/management.yml b/config/locales/en/management.yml index 76a777dff..3cf7506a3 100644 --- a/config/locales/en/management.yml +++ b/config/locales/en/management.yml @@ -95,6 +95,8 @@ en: unfeasible: Unfeasible investment print: print_button: Print + search: + label: "Search investments" search_results: one: " containing the term '%{search_term}'" other: " containing the term '%{search_term}'" diff --git a/config/locales/es/management.yml b/config/locales/es/management.yml index 786cf47a5..895268f16 100644 --- a/config/locales/es/management.yml +++ b/config/locales/es/management.yml @@ -95,6 +95,8 @@ es: unfeasible: Proyectos no factibles print: print_button: Imprimir + search: + label: "Buscar proyectos" search_results: one: " que contiene '%{search_term}'" other: " que contienen '%{search_term}'" diff --git a/spec/components/management/budgets/investments/search_component_spec.rb b/spec/components/management/budgets/investments/search_component_spec.rb new file mode 100644 index 000000000..bdb9eb354 --- /dev/null +++ b/spec/components/management/budgets/investments/search_component_spec.rb @@ -0,0 +1,17 @@ +require "rails_helper" + +describe Management::Budgets::Investments::SearchComponent do + include Rails.application.routes.url_helpers + + let(:budget) { create(:budget) } + let(:url) { management_budget_investments_path(budget) } + let(:component) { Management::Budgets::Investments::SearchComponent.new(budget, url: url) } + + it "renders a label for each field" do + render_inline component + + expect(page).to have_field "Search investments" + expect(page).to have_select "Heading" + expect(page).to have_field "Search unfeasible", type: :checkbox + end +end diff --git a/spec/system/management/budget_investments_spec.rb b/spec/system/management/budget_investments_spec.rb index ce2696002..7fcbe2763 100644 --- a/spec/system/management/budget_investments_spec.rb +++ b/spec/system/management/budget_investments_spec.rb @@ -143,7 +143,7 @@ describe "Budget Investments" do click_link "Support budget investments" end - fill_in "search", with: "what you got" + fill_in "Search investments", with: "what you got" click_button "Search" within("#budget-investments") do @@ -170,7 +170,7 @@ describe "Budget Investments" do click_link "Support budget investments" end - fill_in "search", with: "Area 52" + fill_in "Search investments", with: "Area 52" click_button "Search" within("#budget-investments") do @@ -523,8 +523,8 @@ describe "Budget Investments" do expect(page).to have_content(low_investment.title) end - select "District Nine", from: "heading_id" - click_button("Search") + select "District Nine", from: "Heading" + click_button "Search" within "#budget-investments" do expect(page).not_to have_content(unvoted_investment.title) From c28ff49f109e20acc9b9384359386088ff35bf42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 11 Oct 2024 23:12:17 +0200 Subject: [PATCH 21/25] Move investments search form partial to a component As a bonus, we now have a few less helper methods :). --- app/assets/stylesheets/admin.scss | 28 ------------ .../admin/budget_investments/search_form.scss | 30 +++++++++++++ .../search_form_component.html.erb} | 12 ++--- .../search_form_component.rb | 45 +++++++++++++++++++ .../admin_budget_investments_helper.rb | 36 --------------- .../admin/budget_investments/index.html.erb | 2 +- .../search_form_component_spec.rb} | 18 +++++--- 7 files changed, 95 insertions(+), 76 deletions(-) create mode 100644 app/assets/stylesheets/admin/budget_investments/search_form.scss rename app/{views/admin/budget_investments/_search_form.html.erb => components/admin/budget_investments/search_form_component.html.erb} (88%) create mode 100644 app/components/admin/budget_investments/search_form_component.rb delete mode 100644 app/helpers/admin_budget_investments_helper.rb rename spec/{helpers/admin_budget_investments_helper_spec.rb => components/admin/budget_investments/search_form_component_spec.rb} (56%) diff --git a/app/assets/stylesheets/admin.scss b/app/assets/stylesheets/admin.scss index 441cf51bc..9b500b6af 100644 --- a/app/assets/stylesheets/admin.scss +++ b/app/assets/stylesheets/admin.scss @@ -956,34 +956,6 @@ table { // 10. Budgets // ----------------- -.advanced-filters { - margin: $line-height 0; - - @include breakpoint(medium) { - margin: calc(#{$line-height} / 2) 0 0; - } -} - -.advanced-filters-content { - background: $highlight; - clear: both; - margin: $line-height 0; - padding: calc(#{$line-height} / 2); - - .filter { - display: inline-block; - margin: 0 calc(#{$line-height} / 2); - - label { - font-weight: normal; - margin: 0; - } - } - - .button { - margin-top: calc(#{$line-height} / 2); - } -} .columns-selector { diff --git a/app/assets/stylesheets/admin/budget_investments/search_form.scss b/app/assets/stylesheets/admin/budget_investments/search_form.scss new file mode 100644 index 000000000..1774491ac --- /dev/null +++ b/app/assets/stylesheets/admin/budget_investments/search_form.scss @@ -0,0 +1,30 @@ +.admin-budget-investments-search-form { + .advanced-filters { + margin: $line-height 0; + + @include breakpoint(medium) { + margin: calc(#{$line-height} / 2) 0 0; + } + } + + .advanced-filters-content { + background: $highlight; + clear: both; + margin: $line-height 0; + padding: calc(#{$line-height} / 2); + + .filter { + display: inline-block; + margin: 0 calc(#{$line-height} / 2); + + label { + font-weight: normal; + margin: 0; + } + } + + .button { + margin-top: calc(#{$line-height} / 2); + } + } +} diff --git a/app/views/admin/budget_investments/_search_form.html.erb b/app/components/admin/budget_investments/search_form_component.html.erb similarity index 88% rename from app/views/admin/budget_investments/_search_form.html.erb rename to app/components/admin/budget_investments/search_form_component.html.erb index 8b78a8f73..ee85483de 100644 --- a/app/views/admin/budget_investments/_search_form.html.erb +++ b/app/components/admin/budget_investments/search_form_component.html.erb @@ -1,5 +1,5 @@ <% init_advanced_menu %> -<%= form_tag(admin_budget_budget_investments_path(budget: @budget), method: :get, enforce_utf8: false) do %> +<%= form_tag(admin_budget_budget_investments_path(budget), method: :get, enforce_utf8: false, class: "admin-budget-investments-search-form") do %>
<%= link_to "#advanced_filters_content", data: { toggle: "advanced_filters" }, @@ -34,28 +34,28 @@
<%= select_tag :administrator_id, - options_for_select(admin_select_options(@budget), params[:administrator_id]), + options_for_select(admin_select_options, params[:administrator_id]), { prompt: t("admin.budget_investments.index.administrator_filter_all") } %>
<%= select_tag :valuator_or_group_id, - options_for_select(valuator_or_group_select_options(@budget), params[:valuator_or_group_id]), + options_for_select(valuator_or_group_select_options, params[:valuator_or_group_id]), { prompt: t("admin.budget_investments.index.valuator_filter_all") } %>
<%= select_tag :heading_id, - options_for_select(budget_heading_select_options(@budget), params[:heading_id]), + options_for_select(budget_heading_select_options(budget), params[:heading_id]), { prompt: t("admin.budget_investments.index.heading_filter_all") } %>
<%= select_tag :tag_name, - options_for_select(investment_tags_select_options(@budget, "valuation_tags"), params[:tag_name]), + options_for_select(investment_tags_select_options("valuation_tags"), params[:tag_name]), { prompt: t("admin.budget_investments.index.tags_filter_all") } %>
<%= select_tag :milestone_tag_name, - options_for_select(investment_tags_select_options(@budget, "milestone_tags"), params[:milestone_tag_name]), + options_for_select(investment_tags_select_options("milestone_tags"), params[:milestone_tag_name]), { prompt: t("admin.budget_investments.index.milestone_tags_filter_all") } %>
diff --git a/app/components/admin/budget_investments/search_form_component.rb b/app/components/admin/budget_investments/search_form_component.rb new file mode 100644 index 000000000..c48b88a50 --- /dev/null +++ b/app/components/admin/budget_investments/search_form_component.rb @@ -0,0 +1,45 @@ +class Admin::BudgetInvestments::SearchFormComponent < ApplicationComponent + attr_reader :budget + use_helpers :budget_heading_select_options + + def initialize(budget) + @budget = budget + end + + private + + def init_advanced_menu + params[:advanced_filters] = [] unless params[:advanced_filters] + end + + def advanced_menu_visibility + if params[:advanced_filters].empty? && + params["min_total_supports"].blank? && + params["max_total_supports"].blank? + "hide" + else + "" + end + end + + def admin_select_options + budget.administrators.with_user.map { |v| [v.description_or_name, v.id] }.sort_by { |a| a[0] } + end + + def valuator_or_group_select_options + valuator_group_select_options + valuator_select_options + end + + def valuator_group_select_options + ValuatorGroup.order("name ASC").map { |g| [g.name, "group_#{g.id}"] } + end + + def valuator_select_options + budget.valuators.order("description ASC").order("users.email ASC").includes(:user) + .map { |v| [v.description_or_email, "valuator_#{v.id}"] } + end + + def investment_tags_select_options(context) + budget.investments.tags_on(context).order(:name).pluck(:name) + end +end diff --git a/app/helpers/admin_budget_investments_helper.rb b/app/helpers/admin_budget_investments_helper.rb deleted file mode 100644 index 014118fd5..000000000 --- a/app/helpers/admin_budget_investments_helper.rb +++ /dev/null @@ -1,36 +0,0 @@ -module AdminBudgetInvestmentsHelper - def advanced_menu_visibility - if params[:advanced_filters].empty? && - params["min_total_supports"].blank? && - params["max_total_supports"].blank? - "hide" - else - "" - end - end - - def init_advanced_menu - params[:advanced_filters] = [] unless params[:advanced_filters] - end - - def admin_select_options(budget) - budget.administrators.with_user.map { |v| [v.description_or_name, v.id] }.sort_by { |a| a[0] } - end - - def valuator_or_group_select_options(budget) - valuator_group_select_options + valuator_select_options(budget) - end - - def valuator_select_options(budget) - budget.valuators.order("description ASC").order("users.email ASC").includes(:user) - .map { |v| [v.description_or_email, "valuator_#{v.id}"] } - end - - def valuator_group_select_options - ValuatorGroup.order("name ASC").map { |g| [g.name, "group_#{g.id}"] } - end - - def investment_tags_select_options(budget, context) - budget.investments.tags_on(context).order(:name).pluck(:name) - end -end diff --git a/app/views/admin/budget_investments/index.html.erb b/app/views/admin/budget_investments/index.html.erb index 7a8e46741..7c9f0db3c 100644 --- a/app/views/admin/budget_investments/index.html.erb +++ b/app/views/admin/budget_investments/index.html.erb @@ -8,7 +8,7 @@

<%= @budget.name %> - <%= t("admin.budget_investments.index.title") %>

-<%= render "search_form" %> +<%= render Admin::BudgetInvestments::SearchFormComponent.new(@budget) %> <%= render "/shared/filter_subnav", i18n_namespace: "admin.budget_investments.index" %> diff --git a/spec/helpers/admin_budget_investments_helper_spec.rb b/spec/components/admin/budget_investments/search_form_component_spec.rb similarity index 56% rename from spec/helpers/admin_budget_investments_helper_spec.rb rename to spec/components/admin/budget_investments/search_form_component_spec.rb index 2c8a0294d..18866fafd 100644 --- a/spec/helpers/admin_budget_investments_helper_spec.rb +++ b/spec/components/admin/budget_investments/search_form_component_spec.rb @@ -1,19 +1,23 @@ require "rails_helper" -describe AdminBudgetInvestmentsHelper do +describe Admin::BudgetInvestments::SearchFormComponent do describe "#admin_select_options" do it "includes administrators assigned to the budget" do admin = create(:administrator, user: create(:user, username: "Winston")) budget = create(:budget, administrators: [admin]) - expect(admin_select_options(budget)).to eq([["Winston", admin.id]]) + render_inline Admin::BudgetInvestments::SearchFormComponent.new(budget) + + expect(page).to have_select options: ["All administrators", "Winston"] end it "does not include other administrators" do create(:administrator, user: create(:user, username: "Winston")) budget = create(:budget, administrators: []) - expect(admin_select_options(budget)).to be_empty + render_inline Admin::BudgetInvestments::SearchFormComponent.new(budget) + + expect(page).to have_select options: ["All administrators"] end end @@ -22,14 +26,18 @@ describe AdminBudgetInvestmentsHelper do valuator = create(:valuator, description: "Kodogo") budget = create(:budget, valuators: [valuator]) - expect(valuator_select_options(budget)).to eq([["Kodogo", "valuator_#{valuator.id}"]]) + render_inline Admin::BudgetInvestments::SearchFormComponent.new(budget) + + expect(page).to have_select options: ["All valuators", "Kodogo"] end it "does not include other valuators" do create(:valuator, description: "Kodogo") budget = create(:budget, valuators: []) - expect(valuator_select_options(budget)).to be_empty + render_inline Admin::BudgetInvestments::SearchFormComponent.new(budget) + + expect(page).to have_select options: ["All valuators"] end end end From bc4fd6395007d69fe277f617d1659f20485d0037 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 11 Oct 2024 23:30:59 +0200 Subject: [PATCH 22/25] Simplify advanced filter params initialization --- .../admin/budget_investments/search_form_component.html.erb | 3 +-- .../admin/budget_investments/search_form_component.rb | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/app/components/admin/budget_investments/search_form_component.html.erb b/app/components/admin/budget_investments/search_form_component.html.erb index ee85483de..c40a9494e 100644 --- a/app/components/admin/budget_investments/search_form_component.html.erb +++ b/app/components/admin/budget_investments/search_form_component.html.erb @@ -1,4 +1,3 @@ -<% init_advanced_menu %> <%= form_tag(admin_budget_budget_investments_path(budget), method: :get, enforce_utf8: false, class: "admin-budget-investments-search-form") do %>
<%= link_to "#advanced_filters_content", @@ -14,7 +13,7 @@ <% %w[feasible selected undecided unfeasible without_admin without_valuator under_valuation valuation_finished winners].each do |filter| %>
- <%= check_box_tag "advanced_filters[]", filter, params[:advanced_filters].index(filter), id: "advanced_filters_#{filter}" %> + <%= check_box_tag "advanced_filters[]", filter, advanced_filters_params.index(filter), id: "advanced_filters_#{filter}" %> <%= label_tag "advanced_filters[#{filter}]", t("admin.budget_investments.index.filters.#{filter}") %>
<% end %> diff --git a/app/components/admin/budget_investments/search_form_component.rb b/app/components/admin/budget_investments/search_form_component.rb index c48b88a50..6323ff128 100644 --- a/app/components/admin/budget_investments/search_form_component.rb +++ b/app/components/admin/budget_investments/search_form_component.rb @@ -8,12 +8,12 @@ class Admin::BudgetInvestments::SearchFormComponent < ApplicationComponent private - def init_advanced_menu - params[:advanced_filters] = [] unless params[:advanced_filters] + def advanced_filters_params + params[:advanced_filters] ||= [] end def advanced_menu_visibility - if params[:advanced_filters].empty? && + if advanced_filters_params.empty? && params["min_total_supports"].blank? && params["max_total_supports"].blank? "hide" From 29968d1d9f0cce9925dfbc8c2bafdd6113cbc9c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 11 Oct 2024 23:34:26 +0200 Subject: [PATCH 23/25] Use a button to toggle advanced filters As mentioned in commit 5311daadf, there are several reasons to use buttons in these situations. --- .../admin/budget_investments/search_form.scss | 1 + .../search_form_component.html.erb | 9 ++--- spec/system/admin/budget_investments_spec.rb | 38 +++++++++---------- spec/system/admin/budgets_spec.rb | 2 +- 4 files changed, 25 insertions(+), 25 deletions(-) diff --git a/app/assets/stylesheets/admin/budget_investments/search_form.scss b/app/assets/stylesheets/admin/budget_investments/search_form.scss index 1774491ac..cdc7a0690 100644 --- a/app/assets/stylesheets/admin/budget_investments/search_form.scss +++ b/app/assets/stylesheets/admin/budget_investments/search_form.scss @@ -1,5 +1,6 @@ .admin-budget-investments-search-form { .advanced-filters { + @include link; margin: $line-height 0; @include breakpoint(medium) { diff --git a/app/components/admin/budget_investments/search_form_component.html.erb b/app/components/admin/budget_investments/search_form_component.html.erb index c40a9494e..2d8ada1a7 100644 --- a/app/components/admin/budget_investments/search_form_component.html.erb +++ b/app/components/admin/budget_investments/search_form_component.html.erb @@ -1,10 +1,9 @@ <%= form_tag(admin_budget_budget_investments_path(budget), method: :get, enforce_utf8: false, class: "admin-budget-investments-search-form") do %>
- <%= link_to "#advanced_filters_content", - data: { toggle: "advanced_filters" }, - class: "advanced-filters float-right clear" do %> - <%= t("admin.budget_investments.index.advanced_filters") %> - <% end %> + <%= button_tag t("admin.budget_investments.index.advanced_filters"), + type: :button, + data: { toggle: "advanced_filters" }, + class: "advanced-filters float-right clear" %>
diff --git a/spec/system/admin/budget_investments_spec.rb b/spec/system/admin/budget_investments_spec.rb index bd695df79..4af59630b 100644 --- a/spec/system/admin/budget_investments_spec.rb +++ b/spec/system/admin/budget_investments_spec.rb @@ -245,7 +245,7 @@ describe "Admin budget investments", :admin do expect(page).to have_link("Investment without admin") expect(page).to have_link("Investment with admin") - click_link "Advanced filters" + click_button "Advanced filters" check("Without assigned admin") click_button "Filter" @@ -269,7 +269,7 @@ describe "Admin budget investments", :admin do expect(page).to have_link("Investment without valuator") expect(page).to have_link("Investment with valuator") - click_link "Advanced filters" + click_button "Advanced filters" check "Without assigned valuator" click_button "Filter" @@ -305,7 +305,7 @@ describe "Admin budget investments", :admin do expect(page).to have_link("Investment without valuation") expect(page).to have_link("Investment with valuation") - click_link "Advanced filters" + click_button "Advanced filters" check "Under valuation" click_button "Filter" @@ -329,7 +329,7 @@ describe "Admin budget investments", :admin do expect(page).to have_link("Investment valuation open") expect(page).to have_link("Investment valuation finished") - click_link "Advanced filters" + click_button "Advanced filters" check "Valuation finished" click_button "Filter" @@ -353,7 +353,7 @@ describe "Admin budget investments", :admin do expect(page).to have_link("Investment winner") expect(page).to have_link("Investment without winner") - click_link "Advanced filters" + click_button "Advanced filters" check "Winners" click_button "Filter" @@ -497,7 +497,7 @@ describe "Admin budget investments", :admin do visit admin_budget_budget_investments_path(budget) - click_link "Advanced filters" + click_button "Advanced filters" check "Winners" click_button "Filter" @@ -515,7 +515,7 @@ describe "Admin budget investments", :admin do visit admin_budget_budget_investments_path(budget) - click_link "Advanced filters" + click_button "Advanced filters" check "Winners" click_button "Filter" @@ -551,7 +551,7 @@ describe "Admin budget investments", :admin do expect(page).to have_link("St. 200 supports") expect(page).to have_link("St. 300 supports") - click_link "Advanced filters" + click_button "Advanced filters" fill_in "min_total_supports", with: 180 click_button "Filter" @@ -587,7 +587,7 @@ describe "Admin budget investments", :admin do expect(page).to have_link("St. 200 supports") expect(page).to have_link("St. 300 supports") - click_link "Advanced filters" + click_button "Advanced filters" fill_in "max_total_supports", with: 180 click_button "Filter" @@ -651,7 +651,7 @@ describe "Admin budget investments", :admin do expect(page).to have_content("More schools") expect(page).to have_content("More hospitals") - click_link "Advanced filters" + click_button "Advanced filters" check("Feasible") click_button "Filter" @@ -703,7 +703,7 @@ describe "Admin budget investments", :admin do expect(page).to have_content("More hospitals") expect(page).not_to have_content("More hostals") - click_link "Advanced filters" + click_button "Advanced filters" within("#advanced_filters") { check("Feasible") } click_button("Filter") @@ -1339,7 +1339,7 @@ describe "Admin budget investments", :admin do scenario "Filtering by valuation and selection" do visit admin_budget_budget_investments_path(budget) - click_link "Advanced filters" + click_button "Advanced filters" check "Valuation finished" click_button "Filter" @@ -1384,7 +1384,7 @@ describe "Admin budget investments", :admin do scenario "Aggregating results" do visit admin_budget_budget_investments_path(budget) - click_link "Advanced filters" + click_button "Advanced filters" within("#advanced_filters") { check("Undecided") } click_button("Filter") @@ -1422,7 +1422,7 @@ describe "Admin budget investments", :admin do end end - click_link "Advanced filters" + click_button "Advanced filters" within("#advanced_filters") { check("Selected") } click_button("Filter") @@ -1438,7 +1438,7 @@ describe "Admin budget investments", :admin do scenario "Unselecting an investment" do visit admin_budget_budget_investments_path(budget) - click_link "Advanced filters" + click_button "Advanced filters" within("#advanced_filters") { check("Selected") } click_button("Filter") @@ -1507,7 +1507,7 @@ describe "Admin budget investments", :admin do investment2.update!(administrator: admin) visit admin_budget_budget_investments_path(budget) - click_link "Advanced filters" + click_button "Advanced filters" check "Under valuation" click_button "Filter" @@ -1561,7 +1561,7 @@ describe "Admin budget investments", :admin do visit admin_budget_budget_investments_path(budget) - click_link "Advanced filters" + click_button "Advanced filters" check "Under valuation" click_button "Filter" @@ -1614,7 +1614,7 @@ describe "Admin budget investments", :admin do visit admin_budget_budget_investments_path(budget) - click_link "Advanced filters" + click_button "Advanced filters" check "Under valuation" click_button "Filter" @@ -1704,7 +1704,7 @@ describe "Admin budget investments", :admin do create(:budget_investment, :finished, budget: budget, title: "Finished Investment") visit admin_budget_budget_investments_path(budget) - click_link "Advanced filters" + click_button "Advanced filters" check "Valuation finished" click_button "Filter" diff --git a/spec/system/admin/budgets_spec.rb b/spec/system/admin/budgets_spec.rb index b05f40851..82d017138 100644 --- a/spec/system/admin/budgets_spec.rb +++ b/spec/system/admin/budgets_spec.rb @@ -490,7 +490,7 @@ describe "Admin budgets", :admin do create(:budget_investment, :winner, budget: budget) visit admin_budget_budget_investments_path(budget) - click_link "Advanced filters" + click_button "Advanced filters" check "Winners" click_button "Filter" From 670f4515abdbc0373fcdb879a10af416e16bed5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 11 Nov 2024 14:24:55 +0100 Subject: [PATCH 24/25] Remove obsolete HTML class in advanced filters button The `clear` class isn't needed since commit c9f31b8e1, when we moved this button above the regular search fields. We're also moving the `float` property to the CSS file. --- .../stylesheets/admin/budget_investments/search_form.scss | 1 + .../admin/budget_investments/search_form_component.html.erb | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/admin/budget_investments/search_form.scss b/app/assets/stylesheets/admin/budget_investments/search_form.scss index cdc7a0690..71da631de 100644 --- a/app/assets/stylesheets/admin/budget_investments/search_form.scss +++ b/app/assets/stylesheets/admin/budget_investments/search_form.scss @@ -1,6 +1,7 @@ .admin-budget-investments-search-form { .advanced-filters { @include link; + float: $global-right; margin: $line-height 0; @include breakpoint(medium) { diff --git a/app/components/admin/budget_investments/search_form_component.html.erb b/app/components/admin/budget_investments/search_form_component.html.erb index 2d8ada1a7..1125af58b 100644 --- a/app/components/admin/budget_investments/search_form_component.html.erb +++ b/app/components/admin/budget_investments/search_form_component.html.erb @@ -3,7 +3,7 @@ <%= button_tag t("admin.budget_investments.index.advanced_filters"), type: :button, data: { toggle: "advanced_filters" }, - class: "advanced-filters float-right clear" %> + class: "advanced-filters" %>
From 4102330abcd61838343bac3b124e7a1e93cf17c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 11 Oct 2024 22:57:52 +0200 Subject: [PATCH 25/25] Add labels to investments filters Note that adding the labels broke the layout because the button was no longer aligned with the fields, so we're now using a flex layout. Since we're using labels, we no longer need a placeholder (which wasn't very informative, by the way) in the text field. --- .../admin/budget_investments/search_form.scss | 22 ++++++ .../search_form_component.html.erb | 73 +++++++++++-------- .../search_form_component.rb | 4 + config/locales/en/admin.yml | 4 +- config/locales/es/admin.yml | 4 +- spec/system/admin/budget_investments_spec.rb | 44 +++++------ 6 files changed, 95 insertions(+), 56 deletions(-) diff --git a/app/assets/stylesheets/admin/budget_investments/search_form.scss b/app/assets/stylesheets/admin/budget_investments/search_form.scss index 71da631de..1849f7f2e 100644 --- a/app/assets/stylesheets/admin/budget_investments/search_form.scss +++ b/app/assets/stylesheets/admin/budget_investments/search_form.scss @@ -29,4 +29,26 @@ margin-top: calc(#{$line-height} / 2); } } + + .basic-filters { + align-items: flex-end; + clear: both; + display: flex; + flex-wrap: wrap; + + > * { + @include grid-column-gutter; + width: 100%; + + @include breakpoint(medium) { + width: 25%; + } + } + + > .title-or-id-filter { + @include breakpoint(medium) { + width: 50%; + } + } + } } diff --git a/app/components/admin/budget_investments/search_form_component.html.erb b/app/components/admin/budget_investments/search_form_component.html.erb index 1125af58b..00ff3ec5e 100644 --- a/app/components/admin/budget_investments/search_form_component.html.erb +++ b/app/components/admin/budget_investments/search_form_component.html.erb @@ -30,40 +30,49 @@
-
- <%= select_tag :administrator_id, - options_for_select(admin_select_options, params[:administrator_id]), - { prompt: t("admin.budget_investments.index.administrator_filter_all") } %> -
-
- <%= select_tag :valuator_or_group_id, - options_for_select(valuator_or_group_select_options, params[:valuator_or_group_id]), - { prompt: t("admin.budget_investments.index.valuator_filter_all") } %> -
-
- <%= select_tag :heading_id, - options_for_select(budget_heading_select_options(budget), params[:heading_id]), - { prompt: t("admin.budget_investments.index.heading_filter_all") } %> -
-
- <%= select_tag :tag_name, - options_for_select(investment_tags_select_options("valuation_tags"), params[:tag_name]), - { prompt: t("admin.budget_investments.index.tags_filter_all") } %> -
+
+
+ <%= label_tag :administrator_id, attribute_name(:administrator_id) %> + <%= select_tag :administrator_id, + options_for_select(admin_select_options, params[:administrator_id]), + { prompt: t("admin.budget_investments.index.administrator_filter_all") } %> +
-
- <%= select_tag :milestone_tag_name, - options_for_select(investment_tags_select_options("milestone_tags"), params[:milestone_tag_name]), - { prompt: t("admin.budget_investments.index.milestone_tags_filter_all") } %> -
+
+ <%= label_tag :valuator_or_group_id, t("admin.budget_investments.index.basic_filters.valuator_or_group") %> + <%= select_tag :valuator_or_group_id, + options_for_select(valuator_or_group_select_options, params[:valuator_or_group_id]), + { prompt: t("admin.budget_investments.index.valuator_filter_all") } %> +
-
-
- <%= text_field_tag :title_or_id, params["title_or_id"], placeholder: t("admin.budget_investments.index.placeholder") %> +
+ <%= label_tag :heading_id, attribute_name(:heading_id) %> + <%= select_tag :heading_id, + options_for_select(budget_heading_select_options(budget), params[:heading_id]), + { prompt: t("admin.budget_investments.index.heading_filter_all") } %> +
+ +
+ <%= label_tag :tag_name, attribute_name(:valuation_tag_list) %> + <%= select_tag :tag_name, + options_for_select(investment_tags_select_options("valuation_tags"), params[:tag_name]), + { prompt: t("admin.budget_investments.index.tags_filter_all") } %> +
+ +
+ <%= label_tag :milestone_tag_name, attribute_name(:milestone_tag_list) %> + <%= select_tag :milestone_tag_name, + options_for_select(investment_tags_select_options("milestone_tags"), params[:milestone_tag_name]), + { prompt: t("admin.budget_investments.index.milestone_tags_filter_all") } %> +
+ +
+ <%= label_tag :title_or_id, t("admin.budget_investments.index.basic_filters.title_or_id") %> + <%= text_field_tag :title_or_id, params["title_or_id"] %> +
+ +
+ <%= submit_tag t("admin.budget_investments.index.buttons.filter"), class: "button expanded" %>
- -
- <%= submit_tag t("admin.budget_investments.index.buttons.filter"), class: "button expanded" %> -
<% end %> diff --git a/app/components/admin/budget_investments/search_form_component.rb b/app/components/admin/budget_investments/search_form_component.rb index 6323ff128..daa5c222e 100644 --- a/app/components/admin/budget_investments/search_form_component.rb +++ b/app/components/admin/budget_investments/search_form_component.rb @@ -8,6 +8,10 @@ class Admin::BudgetInvestments::SearchFormComponent < ApplicationComponent private + def attribute_name(attribute) + Budget::Investment.human_attribute_name(attribute) + end + def advanced_filters_params params[:advanced_filters] ||= [] end diff --git a/config/locales/en/admin.yml b/config/locales/en/admin.yml index 0fc0bd4d3..a752deb3e 100644 --- a/config/locales/en/admin.yml +++ b/config/locales/en/admin.yml @@ -235,12 +235,14 @@ en: title: "%{budget} phases" budget_investments: index: + basic_filters: + valuator_or_group: Valuator or group + title_or_id: Title or ID heading_filter_all: All headings administrator_filter_all: All administrators valuator_filter_all: All valuators tags_filter_all: All tags advanced_filters: Advanced filters - placeholder: Search projects filters: all: All without_admin: Without assigned admin diff --git a/config/locales/es/admin.yml b/config/locales/es/admin.yml index 862836854..84c27aa98 100644 --- a/config/locales/es/admin.yml +++ b/config/locales/es/admin.yml @@ -235,12 +235,14 @@ es: title: "Fases de %{budget}" budget_investments: index: + basic_filters: + valuator_or_group: Evaluador o grupo + title_or_id: Título o ID heading_filter_all: Todas las partidas administrator_filter_all: Todos los administradores valuator_filter_all: Todos los evaluadores tags_filter_all: Todas las etiquetas advanced_filters: Filtros avanzados - placeholder: Buscar proyectos filters: all: Todos without_admin: Sin administrador diff --git a/spec/system/admin/budget_investments_spec.rb b/spec/system/admin/budget_investments_spec.rb index 4af59630b..908dbe9b4 100644 --- a/spec/system/admin/budget_investments_spec.rb +++ b/spec/system/admin/budget_investments_spec.rb @@ -98,28 +98,28 @@ describe "Admin budget investments", :admin do expect(page).to have_link("Change name") expect(page).to have_link("Plant trees") - select "Parks: Central Park", from: "heading_id" + select "Parks: Central Park", from: "Heading" click_button "Filter" expect(page).not_to have_link("Realocate visitors") expect(page).not_to have_link("Change name") expect(page).to have_link("Plant trees") - select "All headings", from: "heading_id" + select "All headings", from: "Heading" click_button "Filter" expect(page).to have_link("Realocate visitors") expect(page).to have_link("Change name") expect(page).to have_link("Plant trees") - select "Streets: Main Avenue", from: "heading_id" + select "Streets: Main Avenue", from: "Heading" click_button "Filter" expect(page).to have_link("Realocate visitors") expect(page).not_to have_link("Change name") expect(page).not_to have_link("Plant trees") - select "Streets: Mercy Street", from: "heading_id" + select "Streets: Mercy Street", from: "Heading" click_button "Filter" expect(page).not_to have_link("Realocate visitors") @@ -142,28 +142,28 @@ describe "Admin budget investments", :admin do expect(page).to have_link("Realocate visitors") expect(page).to have_link("Destroy the city") - select "Admin 1", from: "administrator_id" + select "Admin 1", from: "Administrator" click_button "Filter" expect(page).to have_content("There is 1 investment") expect(page).not_to have_link("Destroy the city") expect(page).to have_link("Realocate visitors") - select "Alias", from: "administrator_id" + select "Alias", from: "Administrator" click_button "Filter" expect(page).to have_content("There are no investment projects") expect(page).not_to have_link("Destroy the city") expect(page).not_to have_link("Realocate visitors") - select "All administrators", from: "administrator_id" + select "All administrators", from: "Administrator" click_button "Filter" expect(page).to have_content("There are 2 investments") expect(page).to have_link("Destroy the city") expect(page).to have_link("Realocate visitors") - select "Admin 1", from: "administrator_id" + select "Admin 1", from: "Administrator" click_button "Filter" expect(page).to have_content("There is 1 investment") @@ -183,21 +183,21 @@ describe "Admin budget investments", :admin do expect(page).to have_link("Realocate visitors") expect(page).to have_link("Destroy the city") - select "Valuator 1", from: "valuator_or_group_id" + select "Valuator 1", from: "Valuator or group" click_button "Filter" expect(page).to have_content("There is 1 investment") expect(page).not_to have_link("Destroy the city") expect(page).to have_link("Realocate visitors") - select "All valuators", from: "valuator_or_group_id" + select "All valuators", from: "Valuator or group" click_button "Filter" expect(page).to have_content("There are 2 investments") expect(page).to have_link("Destroy the city") expect(page).to have_link("Realocate visitors") - select "Valuator 1", from: "valuator_or_group_id" + select "Valuator 1", from: "Valuator or group" click_button "Filter" expect(page).to have_content("There is 1 investment") expect(page).not_to have_link("Destroy the city") @@ -215,21 +215,21 @@ describe "Admin budget investments", :admin do expect(page).to have_link("Build a hospital") expect(page).to have_link("Build a theatre") - select "Health", from: "valuator_or_group_id" + select "Health", from: "Valuator or group" click_button "Filter" expect(page).to have_content("There is 1 investment") expect(page).to have_link("Build a hospital") expect(page).not_to have_link("Build a theatre") - select "All valuators", from: "valuator_or_group_id" + select "All valuators", from: "Valuator or group" click_button "Filter" expect(page).to have_content("There are 2 investments") expect(page).to have_link("Build a hospital") expect(page).to have_link("Build a theatre") - select "Culture", from: "valuator_or_group_id" + select "Culture", from: "Valuator or group" click_button "Filter" expect(page).to have_content("There is 1 investment") @@ -619,7 +619,7 @@ describe "Admin budget investments", :admin do expect(page).to have_content("More schools") expect(page).to have_content("More hospitals") - select "Admin 1", from: "administrator_id" + select "Admin 1", from: "Administrator" click_button "Filter" expect(page).to have_css(".budget_investment", count: 2) @@ -627,7 +627,7 @@ describe "Admin budget investments", :admin do expect(page).to have_content("More schools") expect(page).not_to have_content("More hospitals") - fill_in "title_or_id", with: educate_children.id + fill_in "Title or ID", with: educate_children.id click_button "Filter" expect(page).to have_css(".budget_investment", count: 1) @@ -661,7 +661,7 @@ describe "Admin budget investments", :admin do expect(page).to have_content("More schools") expect(page).not_to have_content("More hospitals") - fill_in "title_or_id", with: educate_children.id + fill_in "Title or ID", with: educate_children.id click_button "Filter" expect(page).to have_css(".budget_investment", count: 1) @@ -694,7 +694,7 @@ describe "Admin budget investments", :admin do expect(page).to have_content("More hospitals") expect(page).to have_content("More hostals") - select "Admin 1", from: "administrator_id" + select "Admin 1", from: "Administrator" click_button "Filter" expect(page).to have_css(".budget_investment", count: 3) @@ -714,7 +714,7 @@ describe "Admin budget investments", :admin do expect(page).not_to have_content("More hospitals") expect(page).not_to have_content("More hostals") - fill_in "title_or_id", with: educate_children.id + fill_in "Title or ID", with: educate_children.id click_button "Filter" expect(page).to have_css(".budget_investment", count: 1) @@ -759,13 +759,13 @@ describe "Admin budget investments", :admin do expect(page).to have_content("Proyecto de inversión") expect(page).to have_content("Some other investment") - fill_in "title_or_id", with: "Proyecto de inversión" + fill_in "Title or ID", with: "Proyecto de inversión" click_button "Filter" expect(page).to have_content("Proyecto de inversión") expect(page).not_to have_content("Some other investment") - fill_in "title_or_id", with: "Some other investment" + fill_in "Title or ID", with: "Some other investment" click_button "Filter" expect(page).not_to have_content("Proyecto de inversión") @@ -778,7 +778,7 @@ describe "Admin budget investments", :admin do expect(page).to have_content("Proyecto de inversión") expect(page).to have_content("Some other investment") - fill_in "title_or_id", with: first_investment.id + fill_in "Title or ID", with: first_investment.id click_button "Filter" expect(page).to have_content("Some other investment")