From 4d0638d5df23dbb3864b73b39f47b46d98c922ff Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Fri, 10 Sep 2021 01:44:09 +0200
Subject: [PATCH] Specify group in the "No headings" message

Since the message might appear several times on the same page, it's
useful to give a bit more context. Besides, usability tests show that
when there's a group with no headings, there's no clear indication on
the page that the group is actually a group and not a heading.

We're also adding some emphasis to the group name in the "Showing
headings" message, to be consistent with the emphasis we've
added the the group name in the "No headings" message.
---
 .../budget_headings/headings_component.html.erb      |  2 +-
 .../admin/budget_headings/headings_component.rb      |  4 ++--
 .../headings/group_switcher_component.html.erb       |  4 ++--
 .../headings/group_switcher_component.rb             |  4 ++++
 config/locales/en/admin.yml                          |  4 ++--
 config/locales/es/admin.yml                          |  4 ++--
 .../admin/budget_headings/headings_component_spec.rb | 12 ++++++++++++
 spec/system/admin/budget_headings_spec.rb            |  4 +++-
 spec/system/admin/budgets_wizard/wizard_spec.rb      |  8 ++++----
 9 files changed, 32 insertions(+), 14 deletions(-)
 create mode 100644 spec/components/admin/budget_headings/headings_component_spec.rb

diff --git a/app/components/admin/budget_headings/headings_component.html.erb b/app/components/admin/budget_headings/headings_component.html.erb
index 189d7166d..cde24117b 100644
--- a/app/components/admin/budget_headings/headings_component.html.erb
+++ b/app/components/admin/budget_headings/headings_component.html.erb
@@ -28,6 +28,6 @@
   </table>
 <% else %>
   <div class="callout primary clear">
-    <%= t("admin.budget_headings.no_headings") %>
+    <%= sanitize(t("admin.budget_headings.no_headings", group: group.name)) %>
   </div>
 <% end %>
diff --git a/app/components/admin/budget_headings/headings_component.rb b/app/components/admin/budget_headings/headings_component.rb
index 3b6594f23..396944392 100644
--- a/app/components/admin/budget_headings/headings_component.rb
+++ b/app/components/admin/budget_headings/headings_component.rb
@@ -8,10 +8,10 @@ class Admin::BudgetHeadings::HeadingsComponent < ApplicationComponent
   private
 
     def group
-      @group ||= headings.first.group
+      @group ||= headings.proxy_association.owner
     end
 
     def budget
-      @budget ||= headings.first.budget
+      @budget ||= group.budget
     end
 end
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 4fe2ba830..1c04c5af5 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
@@ -1,12 +1,12 @@
 <div class="budget-group-switcher">
   <% if other_groups.one? %>
     <p>
-      <%= t("admin.budget_headings.group_switcher.currently_showing", group: group.name) %>
+      <%= currently_showing_text %>
       <%= link_to t("admin.budget_headings.group_switcher.the_other_group", group: other_groups.first.name),
                   headings_path(other_groups.first) %>
     </p>
   <% else %>
-    <p><%= t("admin.budget_headings.group_switcher.currently_showing", group: group.name) %></p>
+    <p><%= currently_showing_text %></p>
     <ul class="dropdown menu" data-dropdown-menu data-disable-hover="true" data-click-open="true">
       <li class="has-submenu">
         <button type="button"><%= t("admin.budget_headings.group_switcher.different_group") %></button>
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 b6e490ad9..ffb85ffda 100644
--- a/app/components/admin/budgets_wizard/headings/group_switcher_component.rb
+++ b/app/components/admin/budgets_wizard/headings/group_switcher_component.rb
@@ -22,4 +22,8 @@ class Admin::BudgetsWizard::Headings::GroupSwitcherComponent < ApplicationCompon
     def headings_path(group)
       admin_budgets_wizard_budget_group_headings_path(budget, group)
     end
+
+    def currently_showing_text
+      sanitize(t("admin.budget_headings.group_switcher.currently_showing", group: group.name))
+    end
 end
diff --git a/config/locales/en/admin.yml b/config/locales/en/admin.yml
index 6a468d3b8..b05115d70 100644
--- a/config/locales/en/admin.yml
+++ b/config/locales/en/admin.yml
@@ -185,7 +185,7 @@ en:
       index:
         new_button: "Add new group"
     budget_headings:
-      no_headings: "There are no headings."
+      no_headings: "There are no headings in the <strong>%{group}</strong> group."
       create:
         notice: "Heading created successfully!"
       update:
@@ -202,7 +202,7 @@ en:
         edit: "Edit heading"
         submit: "Save heading"
       group_switcher:
-        currently_showing: "Showing headings from the %{group} group."
+        currently_showing: "Showing headings from the <strong>%{group}</strong> group."
         different_group: "Manage headings from a different group"
         the_other_group: "Manage headings from the %{group} group."
       index:
diff --git a/config/locales/es/admin.yml b/config/locales/es/admin.yml
index 8459dfa9e..7fd142ed2 100644
--- a/config/locales/es/admin.yml
+++ b/config/locales/es/admin.yml
@@ -185,7 +185,7 @@ es:
       index:
         new_button: "Añadir un grupo nuevo"
     budget_headings:
-      no_headings: "No hay partidas."
+      no_headings: "El grupo <strong>%{group}</strong> no tiene ninguna partida."
       create:
         notice: "¡Partida presupuestaria creada con éxito!"
       update:
@@ -202,7 +202,7 @@ es:
         edit: "Editar partida"
         submit: "Guardar partida"
       group_switcher:
-        currently_showing: "Mostrando las partidas del grupo: %{group}."
+        currently_showing: "Mostrando las partidas del grupo <strong>%{group}</strong>."
         different_group: "Ir a partidas de otro grupo"
         the_other_group: "Ir a partidas del grupo %{group}."
       index:
diff --git a/spec/components/admin/budget_headings/headings_component_spec.rb b/spec/components/admin/budget_headings/headings_component_spec.rb
new file mode 100644
index 000000000..e45549ce2
--- /dev/null
+++ b/spec/components/admin/budget_headings/headings_component_spec.rb
@@ -0,0 +1,12 @@
+require "rails_helper"
+
+describe Admin::BudgetHeadings::HeadingsComponent do
+  it "includes group name in the message when there are no headings" do
+    group = create(:budget_group, name: "Whole planet")
+
+    render_inline Admin::BudgetHeadings::HeadingsComponent.new(group.headings)
+
+    expect(page.text.strip).to eq "There are no headings in the Whole planet group."
+    expect(page).to have_css "strong", exact_text: "Whole planet"
+  end
+end
diff --git a/spec/system/admin/budget_headings_spec.rb b/spec/system/admin/budget_headings_spec.rb
index fa130efe6..10569bb5a 100644
--- a/spec/system/admin/budget_headings_spec.rb
+++ b/spec/system/admin/budget_headings_spec.rb
@@ -19,10 +19,12 @@ describe "Admin budget headings", :admin do
 
   context "List of headings in the budget page" do
     scenario "Displaying no headings for group" do
+      group.update!(name: "Universities")
+
       visit admin_budget_path(budget)
 
       within "section", text: "Groups and headings" do
-        expect(page).to have_content "There are no headings."
+        expect(page).to have_content "There are no headings in the Universities group."
       end
     end
 
diff --git a/spec/system/admin/budgets_wizard/wizard_spec.rb b/spec/system/admin/budgets_wizard/wizard_spec.rb
index cfadc5869..a5b3f2330 100644
--- a/spec/system/admin/budgets_wizard/wizard_spec.rb
+++ b/spec/system/admin/budgets_wizard/wizard_spec.rb
@@ -62,7 +62,7 @@ describe "Budgets creation wizard", :admin do
     click_link "Continue to headings"
 
     expect(page).to have_content "Showing headings from the All city group"
-    expect(page).to have_content "There are no headings."
+    expect(page).to have_content "There are no headings in the All city group."
 
     click_button "Add new heading"
     fill_in "Heading name", with: "All city"
@@ -71,10 +71,10 @@ describe "Budgets creation wizard", :admin do
 
     expect(page).to have_content "Heading created successfully!"
     within_table("Headings in All city") { expect(page).to have_content "All city" }
-    expect(page).not_to have_content "There are no headings."
+    expect(page).not_to have_content "There are no headings"
 
     click_link "Manage headings from the Districts group."
-    expect(page).to have_content "There are no headings."
+    expect(page).to have_content "There are no headings in the Districts group."
 
     click_button "Add new heading"
     fill_in "Heading name", with: "North"
@@ -83,7 +83,7 @@ describe "Budgets creation wizard", :admin do
 
     expect(page).to have_content "Heading created successfully!"
     within_table("Headings in Districts") { expect(page).to have_content "North" }
-    expect(page).not_to have_content "There are no headings."
+    expect(page).not_to have_content "There are no headings"
 
     click_button "Add new heading"
     fill_in "Heading name", with: "South"