From a963a99c559a19599bc90e8a8dd0fdd95ee71ded Mon Sep 17 00:00:00 2001 From: Julian Herrero Date: Fri, 8 Feb 2019 19:45:13 +0100 Subject: [PATCH] Use correct scope to sort headings by name --- app/helpers/budget_headings_helper.rb | 2 +- app/models/budget/heading.rb | 10 +++++---- app/views/budgets/groups/show.html.erb | 2 +- app/views/budgets/index.html.erb | 2 +- spec/features/budgets/budgets_spec.rb | 18 ++++++++++++---- spec/features/budgets/groups_spec.rb | 19 ++++++++++++++++ spec/models/budget/heading_spec.rb | 30 +++++++++++++------------- 7 files changed, 57 insertions(+), 26 deletions(-) create mode 100644 spec/features/budgets/groups_spec.rb diff --git a/app/helpers/budget_headings_helper.rb b/app/helpers/budget_headings_helper.rb index f3aae43f1..80c21e9e8 100644 --- a/app/helpers/budget_headings_helper.rb +++ b/app/helpers/budget_headings_helper.rb @@ -1,7 +1,7 @@ module BudgetHeadingsHelper def budget_heading_select_options(budget) - budget.headings.order_by_name.map do |heading| + budget.headings.sort_by_name.map do |heading| [heading.name_scoped_by_group, heading.id] end end diff --git a/app/models/budget/heading.rb b/app/models/budget/heading.rb index fb2e65873..ac9ac6492 100644 --- a/app/models/budget/heading.rb +++ b/app/models/budget/heading.rb @@ -27,12 +27,14 @@ class Budget delegate :budget, :budget_id, to: :group, allow_nil: true scope :i18n, -> { includes(:translations) } - scope :with_group, -> { includes(group: :translations) } - scope :order_by_group_name, -> { i18n.with_group.order("budget_group_translations.name DESC") } - scope :order_by_heading_name, -> { i18n.with_group.order("budget_heading_translations.name") } - scope :order_by_name, -> { i18n.with_group.order_by_group_name.order_by_heading_name } scope :allow_custom_content, -> { i18n.where(allow_custom_content: true).order(:name) } + def self.sort_by_name + all.sort do |heading, other_heading| + [other_heading.group.name, heading.name] <=> [heading.group.name, other_heading.name] + end + end + def name_scoped_by_group group.single_heading_group? ? name : "#{group.name}: #{name}" end diff --git a/app/views/budgets/groups/show.html.erb b/app/views/budgets/groups/show.html.erb index bd7f4bcf8..f352c15da 100644 --- a/app/views/budgets/groups/show.html.erb +++ b/app/views/budgets/groups/show.html.erb @@ -28,7 +28,7 @@
- <% @group.headings.order_by_group_name.each_slice(7) do |slice| %> + <% @group.headings.sort_by_name.each_slice(7) do |slice| %>
<% slice.each do |heading| %>

<%= group.name %>

    - <% group.headings.order_by_group_name.each do |heading| %> + <% group.headings.sort_by_name.each do |heading| %>
  • <% unless current_budget.informing? || current_budget.finished? %> <%= link_to budget_investments_path(current_budget.id, diff --git a/spec/features/budgets/budgets_spec.rb b/spec/features/budgets/budgets_spec.rb index 893772d58..b115d6e52 100644 --- a/spec/features/budgets/budgets_spec.rb +++ b/spec/features/budgets/budgets_spec.rb @@ -60,9 +60,19 @@ feature 'Budgets' do end end + scenario "Show headings ordered by name" do + group = create(:budget_group, budget: budget) + last_heading = create(:budget_heading, group: group, name: "BBB") + first_heading = create(:budget_heading, group: group, name: "AAA") + + visit budgets_path + + expect(first_heading.name).to appear_before(last_heading.name) + end + scenario "Show groups and headings for missing translations" do - group1 = create(:budget_group, budget: last_budget) - group2 = create(:budget_group, budget: last_budget) + group1 = create(:budget_group, budget: budget) + group2 = create(:budget_group, budget: budget) heading1 = create(:budget_heading, group: group1) heading2 = create(:budget_heading, group: group2) @@ -73,9 +83,9 @@ feature 'Budgets' do expect(page).to have_content group1.name expect(page).to have_content group2.name expect(page).to have_content heading1.name - expect(page).to have_content last_budget.formatted_heading_price(heading1) + expect(page).to have_content budget.formatted_heading_price(heading1) expect(page).to have_content heading2.name - expect(page).to have_content last_budget.formatted_heading_price(heading2) + expect(page).to have_content budget.formatted_heading_price(heading2) end end diff --git a/spec/features/budgets/groups_spec.rb b/spec/features/budgets/groups_spec.rb new file mode 100644 index 000000000..c19783a73 --- /dev/null +++ b/spec/features/budgets/groups_spec.rb @@ -0,0 +1,19 @@ +require "rails_helper" + +feature "Budget Groups" do + + let(:budget) { create(:budget) } + let(:group) { create(:budget_group, budget: budget) } + + context "Show" do + scenario "Headings are sorted by name" do + last_heading = create(:budget_heading, group: group, name: "BBB") + first_heading = create(:budget_heading, group: group, name: "AAA") + + visit budget_group_path(budget, group) + + expect(first_heading.name).to appear_before(last_heading.name) + end + end + +end diff --git a/spec/models/budget/heading_spec.rb b/spec/models/budget/heading_spec.rb index 94447c07e..3003d1c74 100644 --- a/spec/models/budget/heading_spec.rb +++ b/spec/models/budget/heading_spec.rb @@ -280,21 +280,8 @@ describe Budget::Heading do end end - describe "scope order_by_group_name" do - it "sorts headings using the group name (DESC) in any locale" do - last_group = create(:budget_group, name_en: "CCC", name_es: "AAA") - first_group = create(:budget_group, name_en: "DDD", name_es: "BBB") + describe ".sort_by_name" do - last_heading = create(:budget_heading, group: last_group, name: "Name") - first_heading = create(:budget_heading, group: first_group, name: "Name") - - expect(Budget::Heading.order_by_group_name.count).to be 2 - expect(Budget::Heading.order_by_group_name.first).to eq first_heading - expect(Budget::Heading.order_by_group_name.last).to eq last_heading - end - end - - describe "scope order_by_name" do it "returns headings sorted by DESC group name first and then ASC heading name" do last_group = create(:budget_group, name: "Group A") first_group = create(:budget_group, name: "Group B") @@ -305,8 +292,21 @@ describe Budget::Heading do heading1 = create(:budget_heading, group: first_group, name: "Name C") sorted_headings = [heading1, heading2, heading3, heading4] - expect(Budget::Heading.order_by_name.to_a).to eq sorted_headings + expect(Budget::Heading.sort_by_name).to eq sorted_headings end + + it "only sort headings using the group name (DESC) in the current locale" do + last_group = create(:budget_group, name_en: "CCC", name_es: "BBB") + first_group = create(:budget_group, name_en: "DDD", name_es: "AAA") + + last_heading = create(:budget_heading, group: last_group, name: "Name") + first_heading = create(:budget_heading, group: first_group, name: "Name") + + expect(Budget::Heading.sort_by_name.size).to be 2 + expect(Budget::Heading.sort_by_name.first).to eq first_heading + expect(Budget::Heading.sort_by_name.last).to eq last_heading + end + end describe "scope allow_custom_content" do