From 3484c6b7b8af9955af231cc59984049959a2525f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?=
Date: Sun, 23 Feb 2025 21:56:02 +0100
Subject: [PATCH 1/6] Use a list of links to change group in budgets wizard
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The original implementation (which was never merged) had a `
<% end %>
diff --git a/config/locales/en/admin.yml b/config/locales/en/admin.yml
index d842a34fa..80289656d 100644
--- a/config/locales/en/admin.yml
+++ b/config/locales/en/admin.yml
@@ -212,7 +212,6 @@ en:
submit: "Save heading"
group_switcher:
currently_showing: "Showing headings from the %{group} group."
- different_group: "Manage headings from a different group"
the_other_group: "Manage headings from the %{group} group."
index:
back: "Go back to groups"
diff --git a/config/locales/es/admin.yml b/config/locales/es/admin.yml
index ffa0f58f4..3f7bac10d 100644
--- a/config/locales/es/admin.yml
+++ b/config/locales/es/admin.yml
@@ -212,7 +212,6 @@ es:
submit: "Guardar partida"
group_switcher:
currently_showing: "Mostrando las partidas del grupo %{group}."
- different_group: "Ir a partidas de otro grupo"
the_other_group: "Ir a partidas del grupo %{group}."
index:
back: "Volver a grupos"
diff --git a/spec/components/admin/budgets_wizard/headings/group_switcher_component_spec.rb b/spec/components/admin/budgets_wizard/headings/group_switcher_component_spec.rb
index 56e7e825d..486c80a1d 100644
--- a/spec/components/admin/budgets_wizard/headings/group_switcher_component_spec.rb
+++ b/spec/components/admin/budgets_wizard/headings/group_switcher_component_spec.rb
@@ -30,11 +30,14 @@ describe Admin::BudgetsWizard::Headings::GroupSwitcherComponent do
render_inline Admin::BudgetsWizard::Headings::GroupSwitcherComponent.new(group)
expect(page).to have_content "Showing headings from the Parks group"
- expect(page).to have_button "Manage headings from a different group"
- page.find("button + ul") do |list|
- expect(list).to have_link "Recreation"
- expect(list).to have_link "Entertainment"
+ page.find("ul") do |list|
+ expect(list).to have_css "li", count: 2
+ expect(list).to have_link count: 2
+ expect(list).to have_link "Manage headings from the Recreation group."
+ expect(list).to have_link "Manage headings from the Entertainment group."
+ expect(list).to have_css "strong", exact_text: "Recreation"
+ expect(list).to have_css "strong", exact_text: "Entertainment"
end
end
end
diff --git a/spec/system/admin/budgets_wizard/headings_spec.rb b/spec/system/admin/budgets_wizard/headings_spec.rb
index d56983fd5..cad0180b3 100644
--- a/spec/system/admin/budgets_wizard/headings_spec.rb
+++ b/spec/system/admin/budgets_wizard/headings_spec.rb
@@ -35,8 +35,7 @@ describe "Budgets wizard, headings step", :admin do
expect(page).not_to have_link "Health"
- click_button "Manage headings from a different group"
- click_link "Health"
+ click_link "Manage headings from the Health group."
within(".heading") do
expect(page).to have_content "Hospitals"
From a03037f0bae9665c45bb189a04ead392431b3d6f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?=
Date: Fri, 7 Mar 2025 23:01:57 +0100
Subject: [PATCH 2/6] Remove duplication in group switcher component
We can extract a method to reduce the amount of ERB code and remove the
duplication in the link texts. We also make the list consistent; now we
always use a tag in the group name, no matter whether there are
many groups or only one.
---
.../headings/group_switcher_component.html.erb | 15 ++-------------
.../headings/group_switcher_component.rb | 9 +++++++++
2 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/app/components/admin/budgets_wizard/headings/group_switcher_component.html.erb b/app/components/admin/budgets_wizard/headings/group_switcher_component.html.erb
index d30ef5e82..28e260f57 100644
--- a/app/components/admin/budgets_wizard/headings/group_switcher_component.html.erb
+++ b/app/components/admin/budgets_wizard/headings/group_switcher_component.html.erb
@@ -2,21 +2,10 @@
<% if other_groups.one? %>
+ <%= link_list(*other_groups.map { |group| link_to_group(group) }) %>
<% end %>
diff --git a/app/components/admin/budgets_wizard/headings/group_switcher_component.rb b/app/components/admin/budgets_wizard/headings/group_switcher_component.rb
index ffb85ffda..c6cc6f183 100644
--- a/app/components/admin/budgets_wizard/headings/group_switcher_component.rb
+++ b/app/components/admin/budgets_wizard/headings/group_switcher_component.rb
@@ -1,5 +1,6 @@
class Admin::BudgetsWizard::Headings::GroupSwitcherComponent < ApplicationComponent
attr_reader :group
+ use_helpers :link_list
def initialize(group)
@group = group
@@ -26,4 +27,12 @@ class Admin::BudgetsWizard::Headings::GroupSwitcherComponent < ApplicationCompon
def currently_showing_text
sanitize(t("admin.budget_headings.group_switcher.currently_showing", group: group.name))
end
+
+ def link_to_group(group)
+ link_to(link_to_group_text(group), headings_path(group))
+ end
+
+ def link_to_group_text(group)
+ sanitize(t("admin.budget_headings.group_switcher.the_other_group", group: tag.strong(group.name)))
+ end
end
From 5b5571bb546e0b5eaad54c71d2b8e9623b48b000 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?=
Date: Sun, 23 Feb 2025 22:51:20 +0100
Subject: [PATCH 3/6] Create records before a visit in dashboard actions tests
This way we avoid possible database corruption in our CI, which has
happened in the past when the process running the test accesses the
database after the process running the browser has started.
---
spec/system/admin/dashboard/actions_spec.rb | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/spec/system/admin/dashboard/actions_spec.rb b/spec/system/admin/dashboard/actions_spec.rb
index 83e67a171..6d407ad15 100644
--- a/spec/system/admin/dashboard/actions_spec.rb
+++ b/spec/system/admin/dashboard/actions_spec.rb
@@ -89,11 +89,9 @@ describe "Admin dashboard actions", :admin do
context "when destroying an action" do
let!(:action) { create(:dashboard_action) }
- before do
- visit admin_dashboard_actions_path
- end
-
scenario "deletes the action" do
+ visit admin_dashboard_actions_path
+
accept_confirm("Are you sure? This action will delete \"#{action.title}\" and can't be undone.") do
click_button "Delete"
end
@@ -101,8 +99,10 @@ describe "Admin dashboard actions", :admin do
expect(page).not_to have_content(action.title)
end
- scenario "can not delete actions that have been executed" do
- _executed_action = create(:dashboard_executed_action, action: action)
+ scenario "cannot delete actions that have been executed" do
+ create(:dashboard_executed_action, action: action)
+
+ visit admin_dashboard_actions_path
accept_confirm("Are you sure? This action will delete \"#{action.title}\" and can't be undone.") do
click_button "Delete"
From f3fcee430e319194bdf7b5e9ff893005d4907e07 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?=
Date: Sun, 23 Feb 2025 23:12:30 +0100
Subject: [PATCH 4/6] Use labels to find fields in dashboard actions tests
In the tests for the edit/update action, we were using field names, but
using labels is a better approach because it tests that the label is
correctly associated with the field.
We aren't changing the tests for the new/create action because we were
already using labels there.
---
spec/system/admin/dashboard/actions_spec.rb | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/spec/system/admin/dashboard/actions_spec.rb b/spec/system/admin/dashboard/actions_spec.rb
index 6d407ad15..645ceea0a 100644
--- a/spec/system/admin/dashboard/actions_spec.rb
+++ b/spec/system/admin/dashboard/actions_spec.rb
@@ -73,14 +73,14 @@ describe "Admin dashboard actions", :admin do
end
scenario "Updates the action" do
- fill_in "dashboard_action_title", with: "Great action!"
+ fill_in "Title", with: "Great action!"
click_button "Save"
expect(page).to have_content "Great action!"
end
scenario "Renders edit form in case data is invalid" do
- fill_in "dashboard_action_title", with: "x"
+ fill_in "Title", with: "x"
click_button "Save"
expect(page).to have_content("error prevented this Dashboard/Action from being saved.")
end
From 932d4cd6987092d6017e186a3167d1d8b1cc97da Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?=
Date: Sun, 23 Feb 2025 22:55:14 +0100
Subject: [PATCH 5/6] Fix wrong ARIA attribute in dashboard actions form
Using a `data-toggle` attribute, which we do since commit 07fd5084f,
made Foundation generate an `aria-expanded` attribute to a radio button,
but this attribute can't be present in radio buttons. This makes sense,
since the main purpose of a radio button in a form is to choose an
option, not to show/hide content.
This resulted in the following error when checking the page with Axe:
```
Found 1 accessibility violation:
aria-allowed-attr: Elements must only use supported ARIA attributes
(critical)
https://dequeuniversity.com/rules/axe/4.10/aria-allowed-attr?application=axeAPI
The following 2 nodes violate this rule:
Selector: #dashboard_action_action_type_proposed_action
HTML:
Fix all of the following:
- ARIA attribute is not allowed: aria-expanded="true"
Selector: #dashboard_action_action_type_resource
HTML:
Fix all of the following:
- ARIA attribute is not allowed: aria-expanded="true"
```
So we're using custom JavaScript instead. We're also making the
`short_description` field act as intended; since the changes in commit
07fd5084f it was never shown because it had the `hide` HTML class and it
didn't have a `data-toggler` attribute.
---
.../admin/dashboard/actions/form.js | 21 +++++++++++++++++++
app/assets/javascripts/application.js | 1 +
.../proposal_dashboard_actions_helper.rb | 4 ----
.../admin/dashboard/actions/_form.html.erb | 10 +++------
spec/system/admin/dashboard/actions_spec.rb | 10 +++++++++
5 files changed, 35 insertions(+), 11 deletions(-)
create mode 100644 app/assets/javascripts/admin/dashboard/actions/form.js
diff --git a/app/assets/javascripts/admin/dashboard/actions/form.js b/app/assets/javascripts/admin/dashboard/actions/form.js
new file mode 100644
index 000000000..1ef7f4dc1
--- /dev/null
+++ b/app/assets/javascripts/admin/dashboard/actions/form.js
@@ -0,0 +1,21 @@
+(function() {
+ "use strict";
+ App.AdminDashboardActionsForm = {
+ initialize: function() {
+ $("input[name='dashboard_action[action_type]']").on({
+ change: function() {
+ switch ($(this).val()) {
+ case "proposed_action":
+ $("#request_to_administrators").hide();
+ $("#short_description").hide();
+ break;
+ case "resource":
+ $("#request_to_administrators").show();
+ $("#short_description").show();
+ }
+ }
+ }).filter("[checked]").trigger("change");
+ }
+ };
+}).call(this);
+
diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js
index c2c3c8cfa..d1070d56a 100644
--- a/app/assets/javascripts/application.js
+++ b/app/assets/javascripts/application.js
@@ -169,6 +169,7 @@ var initialize_modules = function() {
App.ColumnsSelector.initialize();
}
App.AdminBudgetsWizardCreationStep.initialize();
+ App.AdminDashboardActionsForm.initialize();
App.AdminMachineLearningScripts.initialize();
App.AdminPollShiftsForm.initialize();
App.AdminTenantsForm.initialize();
diff --git a/app/helpers/admin/proposal_dashboard_actions_helper.rb b/app/helpers/admin/proposal_dashboard_actions_helper.rb
index 2b0cbddf2..17efbd100 100644
--- a/app/helpers/admin/proposal_dashboard_actions_helper.rb
+++ b/app/helpers/admin/proposal_dashboard_actions_helper.rb
@@ -8,8 +8,4 @@ module Admin::ProposalDashboardActionsHelper
def default_actions
%w[polls email poster]
end
-
- def css_for_resource(action)
- "hide" if action == "proposed_action"
- end
end
diff --git a/app/views/admin/dashboard/actions/_form.html.erb b/app/views/admin/dashboard/actions/_form.html.erb
index 3dd147172..a3a4301ea 100644
--- a/app/views/admin/dashboard/actions/_form.html.erb
+++ b/app/views/admin/dashboard/actions/_form.html.erb
@@ -5,9 +5,7 @@
<%= f.label :action_type %>
<% ::Dashboard::Action.action_types.keys.each do |action_type_value| %>
- <%= f.radio_button :action_type,
- action_type_value,
- data: { toggle: "request_to_administrators short_description" } %>
+ <%= f.radio_button :action_type, action_type_value %>
<% end %>
@@ -16,9 +14,7 @@
<%= f.check_box :active %>
-
+
<%= f.check_box :request_to_administrators %>
@@ -28,7 +24,7 @@
<%= f.text_field :title %>
-
+
<%= f.text_field :short_description %>
diff --git a/spec/system/admin/dashboard/actions_spec.rb b/spec/system/admin/dashboard/actions_spec.rb
index 645ceea0a..302f2edc4 100644
--- a/spec/system/admin/dashboard/actions_spec.rb
+++ b/spec/system/admin/dashboard/actions_spec.rb
@@ -73,7 +73,17 @@ describe "Admin dashboard actions", :admin do
end
scenario "Updates the action" do
+ expect(page).to have_checked_field "Proposed action"
+ expect(page).to have_unchecked_field "Resource"
+ expect(page).not_to have_field "Short description"
+ expect(page).not_to have_field "Include in the resource a button to " \
+ "request the resource from administrators"
+
+ choose "Resource"
+ check "Include in the resource a button to request the resource from administrators"
+
fill_in "Title", with: "Great action!"
+ fill_in "Short description", with: "And awesome too!"
click_button "Save"
expect(page).to have_content "Great action!"
From 224999a95fecdab7746da0042803b8b4fab1f920 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?=
Date: Mon, 24 Feb 2025 01:17:25 +0100
Subject: [PATCH 6/6] Move tabs link list outside tabs content
Just like we do everywhere else. We're also removing the wrong ARIA
attributes that we added in commit c18479e3a, which caused an
accessibility issue reported by Axe:
```
aria-required-children: Certain ARIA roles must contain particular
children (critical)
https://dequeuniversity.com/rules/axe/4.10/aria-required-children?application=axeAPI
The following 1 node violate this rule:
Selector: .tabs-content
HTML:
Fix any of the following:
- Element has children which are not allowed: ul[tabindex]
```
Although, in this case, it would probably be better to have different
pages instead of tabs, so loading the page doesn't take too long.
---
.../site_customization/information_texts/index.html.erb | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/app/views/admin/site_customization/information_texts/index.html.erb b/app/views/admin/site_customization/information_texts/index.html.erb
index c6a2fea43..39af0f84b 100644
--- a/app/views/admin/site_customization/information_texts/index.html.erb
+++ b/app/views/admin/site_customization/information_texts/index.html.erb
@@ -1,9 +1,9 @@