From 88ad7113306d363c7823fa12ce121aca6bee8d10 Mon Sep 17 00:00:00 2001 From: decabeza Date: Wed, 11 Nov 2020 09:06:56 +0100 Subject: [PATCH] Hide group name only on budgets with one group In the form of creating a new investment was hiding the name of the group if it had only one heading, but could be confusing to users if there are, for example, five different groups of one heading. The solution: - If the budget has one group and one heading, the heading selector is hidden. - If the budget has one group and more than one heading, the group name is hidden. - If the budget has more than one group, the group name appears regardless of the number of headings. --- app/models/budget.rb | 6 +++- app/models/budget/group.rb | 4 --- app/models/budget/heading.rb | 2 +- spec/models/budget/heading_spec.rb | 29 +++++++++++++++++++ spec/system/admin/budget_investments_spec.rb | 4 +-- spec/system/budgets/investments_spec.rb | 29 +++++++++++++++---- .../management/budget_investments_spec.rb | 2 +- 7 files changed, 61 insertions(+), 15 deletions(-) diff --git a/app/models/budget.rb b/app/models/budget.rb index a1ee86954..eaf35aae4 100644 --- a/app/models/budget.rb +++ b/app/models/budget.rb @@ -162,8 +162,12 @@ class Budget < ApplicationRecord current_phase&.balloting_or_later? end + def single_group? + groups.one? + end + def single_heading? - groups.one? && headings.one? + single_group? && headings.one? end def heading_price(heading) diff --git a/app/models/budget/group.rb b/app/models/budget/group.rb index bcf217182..22138b04a 100644 --- a/app/models/budget/group.rb +++ b/app/models/budget/group.rb @@ -30,10 +30,6 @@ class Budget all.sort_by(&:name) end - def single_heading_group? - headings.count == 1 - end - private def generate_slug? diff --git a/app/models/budget/heading.rb b/app/models/budget/heading.rb index 4e310f81f..94c61511e 100644 --- a/app/models/budget/heading.rb +++ b/app/models/budget/heading.rb @@ -48,7 +48,7 @@ class Budget end def name_scoped_by_group - group.single_heading_group? ? name : "#{group.name}: #{name}" + budget.single_group? ? name : "#{group.name}: #{name}" end def can_be_deleted? diff --git a/spec/models/budget/heading_spec.rb b/spec/models/budget/heading_spec.rb index a2b77a7d5..a5cc098b4 100644 --- a/spec/models/budget/heading_spec.rb +++ b/spec/models/budget/heading_spec.rb @@ -332,4 +332,33 @@ describe Budget::Heading do expect(build(:budget_heading, max_ballot_lines: 0)).not_to be_valid end end + + describe "#name_scoped_by_group" do + it "returns heading name in budgets with a single heading" do + heading = create(:budget_heading, group: group, name: "One and only") + + expect(heading.name_scoped_by_group).to eq "One and only" + end + + it "returns heading name in budgets with one group and many headings" do + schools = create(:budget_heading, group: group, name: "Schools") + universities = create(:budget_heading, group: group, name: "Universities") + + expect(schools.name_scoped_by_group).to eq "Schools" + expect(universities.name_scoped_by_group).to eq "Universities" + end + + it "returns heading name in groups with many headings in budgets with many groups" do + education = create(:budget_group, budget: budget, name: "Education") + health = create(:budget_group, budget: budget, name: "Health") + + schools = create(:budget_heading, group: education, name: "Schools") + universities = create(:budget_heading, group: education, name: "Universities") + supplies = create(:budget_heading, group: health, name: "Medical supplies") + + expect(schools.name_scoped_by_group).to eq "Education: Schools" + expect(universities.name_scoped_by_group).to eq "Education: Universities" + expect(supplies.name_scoped_by_group).to eq "Health: Medical supplies" + end + end end diff --git a/spec/system/admin/budget_investments_spec.rb b/spec/system/admin/budget_investments_spec.rb index 00291061d..3782dd530 100644 --- a/spec/system/admin/budget_investments_spec.rb +++ b/spec/system/admin/budget_investments_spec.rb @@ -101,7 +101,7 @@ describe "Admin budget investments", :admin do expect(page).to have_link("Change name") expect(page).to have_link("Plant trees") - select "Central Park", from: "heading_id" + select "Parks: Central Park", from: "heading_id" click_button "Filter" expect(page).not_to have_link("Realocate visitors") @@ -1040,7 +1040,7 @@ describe "Admin budget investments", :admin do fill_in "Title", with: "Potatoes" fill_in_ckeditor "Description", with: "Carrots" - select "#{budget_investment.group.name}: Barbate", from: "budget_investment[heading_id]" + select "Barbate", from: "budget_investment[heading_id]" uncheck "budget_investment_incompatible" check "budget_investment_selected" diff --git a/spec/system/budgets/investments_spec.rb b/spec/system/budgets/investments_spec.rb index 773e9d4ee..c73bdf3ba 100644 --- a/spec/system/budgets/investments_spec.rb +++ b/spec/system/budgets/investments_spec.rb @@ -570,18 +570,35 @@ describe "Budget Investments" do expect(page).to have_content "Build a skyscraper" end - scenario "Create with multiple headings" do - create(:budget_heading, budget: budget, name: "Medical supplies") - create(:budget_heading, budget: budget, name: "Even more hospitals") + scenario "Create with single group and multiple headings" do + create(:budget_heading, group: group, name: "Medical supplies") + create(:budget_heading, group: group, name: "Even more hospitals") + + login_as(author) + + visit new_budget_investment_path(budget) + + expect(page).to have_select "Heading", + options: ["", "More hospitals", "Medical supplies", "Even more hospitals"] + expect(page).not_to have_content "Health" + end + + scenario "Create with multiple groups" do + education = create(:budget_group, budget: budget, name: "Education") + + create(:budget_heading, group: group, name: "Medical supplies") + create(:budget_heading, group: education, name: "Schools") + login_as(author) visit new_budget_investment_path(budget) expect(page).not_to have_content("#{heading.name} (#{budget.formatted_heading_price(heading)})") expect(page).to have_select "Heading", - options: ["", "More hospitals", "Medical supplies", "Even more hospitals"] + options: ["", "Health: More hospitals", "Health: Medical supplies", "Education: Schools"] + + select "Health: Medical supplies", from: "Heading" - select "Medical supplies", from: "Heading" fill_in "Title", with: "Build a skyscraper" fill_in_ckeditor "Description", with: "I want to live in a high tower over the clouds" fill_in "Location additional info", with: "City center" @@ -755,7 +772,7 @@ describe "Budget Investments" do select_options = find("#budget_investment_heading_id").all("option").map(&:text) expect(select_options).to eq ["", - "Toda la ciudad", + "Toda la ciudad: Toda la ciudad", "Health: More health professionals", "Health: More hospitals"] end diff --git a/spec/system/management/budget_investments_spec.rb b/spec/system/management/budget_investments_spec.rb index 04b0cd87a..4dd949584 100644 --- a/spec/system/management/budget_investments_spec.rb +++ b/spec/system/management/budget_investments_spec.rb @@ -396,7 +396,7 @@ describe "Budget Investments" do expect(page).to have_content(low_investment.title) end - select "Whole city: District Nine", from: "heading_id" + select "District Nine", from: "heading_id" click_button("Search") within "#budget-investments" do