From 4243de306240c0d78ceeff2e2052ac1de1275e8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 16 Sep 2021 12:15:55 +0200 Subject: [PATCH 1/4] Move ballot partial to a component This way it'll be easier to extract methods from its code and modify them. --- .../budgets/ballot/ballot_component.html.erb | 62 ++++++++++++++++++ .../budgets/ballot/ballot_component.rb | 11 ++++ app/views/budgets/ballot/_ballot.html.erb | 64 +------------------ .../budgets/ballot/ballot_component_spec.rb | 19 ++++++ spec/system/budgets/ballots_spec.rb | 9 --- 5 files changed, 93 insertions(+), 72 deletions(-) create mode 100644 app/components/budgets/ballot/ballot_component.html.erb create mode 100644 app/components/budgets/ballot/ballot_component.rb create mode 100644 spec/components/budgets/ballot/ballot_component_spec.rb diff --git a/app/components/budgets/ballot/ballot_component.html.erb b/app/components/budgets/ballot/ballot_component.html.erb new file mode 100644 index 000000000..cdd1f0641 --- /dev/null +++ b/app/components/budgets/ballot/ballot_component.html.erb @@ -0,0 +1,62 @@ +
+ <%= back_link_to session[:ballot_referer] %> + +

<%= t("budgets.ballots.show.title") %>

+ +
+

+ <%= sanitize(t("budgets.ballots.show.voted", count: ballot.investments.count)) %> +

+

+ <%= t("budgets.ballots.show.voted_info") %> +

+

<%= t("budgets.ballots.show.voted_info_2") %>

+
+
+ +
+ <% ballot_groups = ballot.groups.sort_by_name %> + <% ballot_groups.each do |group| %> + <% heading = ballot.heading_for_group(group) %> +
+
+
+

+ <%= group.name %> - <%= heading.name %> +

+ <%= link_to sanitize(ballot.amount_available_info(heading)), + budget_group_path(budget, group) %> +
+ <% if ballot.has_lines_in_group?(group) %> +

+ <%= sanitize(ballot.amount_spent_info(heading)) %> +

+ <% else %> +

+ <%= t("budgets.ballots.show.zero") %>
+

+ <% end %> + +
    + <%= render Budgets::Ballot::InvestmentComponent.with_collection( + ballot.investments.by_group(group.id) + ) %> +
+
+
+ <% end %> + + <% no_balloted_groups = budget.groups.sort_by_name - ballot_groups %> + <% no_balloted_groups.each do |group| %> +
+
+
+

+ <%= group.name %> +

+ <%= link_to t("budgets.ballots.show.no_balloted_group_yet"), budget_group_path(budget, group) %> +
+
+
+ <% end %> +
diff --git a/app/components/budgets/ballot/ballot_component.rb b/app/components/budgets/ballot/ballot_component.rb new file mode 100644 index 000000000..7e2b77aaf --- /dev/null +++ b/app/components/budgets/ballot/ballot_component.rb @@ -0,0 +1,11 @@ +class Budgets::Ballot::BallotComponent < ApplicationComponent + attr_reader :ballot + + def initialize(ballot) + @ballot = ballot + end + + def budget + ballot.budget + end +end diff --git a/app/views/budgets/ballot/_ballot.html.erb b/app/views/budgets/ballot/_ballot.html.erb index 54d7a0358..dcbc06b68 100644 --- a/app/views/budgets/ballot/_ballot.html.erb +++ b/app/views/budgets/ballot/_ballot.html.erb @@ -1,63 +1 @@ -
- <%= back_link_to session[:ballot_referer] %> - -

<%= t("budgets.ballots.show.title") %>

- -
-

- <%= sanitize(t("budgets.ballots.show.voted", count: @ballot.investments.count)) %> -

-

- <%= t("budgets.ballots.show.voted_info") %> -

-

<%= t("budgets.ballots.show.voted_info_2") %>

-
-
- -
- <% ballot_groups = @ballot.groups.sort_by_name %> - <% ballot_groups.each do |group| %> - <% heading = @ballot.heading_for_group(group) %> -
-
-
-

- <%= group.name %> - <%= heading.name %> -

- <%= link_to sanitize(@ballot.amount_available_info(heading)), - budget_group_path(@budget, group) %> -
- <% if @ballot.has_lines_in_group?(group) %> -

- <%= sanitize(@ballot.amount_spent_info(heading)) %> -

- <% else %> -

- <%= t("budgets.ballots.show.zero") %>
-

- <% end %> - -
    - <%= render Budgets::Ballot::InvestmentComponent.with_collection( - @ballot.investments.by_group(group.id) - ) %> -
-
-
- <% end %> - - <% no_balloted_groups = @budget.groups.sort_by_name - ballot_groups %> - <% no_balloted_groups.each do |group| %> -
-
-
-

- <%= group.name %> -

- <%= link_to t("budgets.ballots.show.no_balloted_group_yet"), budget_group_path(@budget, group) %> -
-
-
- <% end %> - -
+<%= render Budgets::Ballot::BallotComponent.new(@ballot) %> diff --git a/spec/components/budgets/ballot/ballot_component_spec.rb b/spec/components/budgets/ballot/ballot_component_spec.rb new file mode 100644 index 000000000..e26edde7b --- /dev/null +++ b/spec/components/budgets/ballot/ballot_component_spec.rb @@ -0,0 +1,19 @@ +require "rails_helper" + +describe Budgets::Ballot::BallotComponent do + include Rails.application.routes.url_helpers + before { request.session[:ballot_referer] = "/" } + + describe "link to group" do + let(:budget) { create(:budget, :balloting) } + let(:ballot) { create(:budget_ballot, user: create(:user), budget: budget) } + + it "displays links to vote on groups with no investments voted yet" do + group = create(:budget_group, budget: budget) + + render_inline Budgets::Ballot::BallotComponent.new(ballot) + + expect(page).to have_link "You have not voted on this group yet, go vote!", href: budget_group_path(budget, group) + end + end +end diff --git a/spec/system/budgets/ballots_spec.rb b/spec/system/budgets/ballots_spec.rb index 56678662a..8a253b613 100644 --- a/spec/system/budgets/ballots_spec.rb +++ b/spec/system/budgets/ballots_spec.rb @@ -344,15 +344,6 @@ describe "Ballots" do expect(page).to have_content "Still available to you €35" end end - - scenario "Display links to vote on groups with no investments voted yet" do - group = create(:budget_group, budget: budget) - - login_as(user) - visit budget_ballot_path(budget) - - expect(page).to have_link "You have not voted on this group yet, go vote!", href: budget_group_path(budget, group) - end end scenario "Removing investments from ballot" do From 9f94e9568940c124eb6177a79e2235d2ce5f7c06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 16 Sep 2021 12:24:55 +0200 Subject: [PATCH 2/4] Extract methods in ballot component Assigning variables in the view makes the code harder to read. --- .../budgets/ballot/ballot_component.html.erb | 2 -- app/components/budgets/ballot/ballot_component.rb | 10 ++++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/app/components/budgets/ballot/ballot_component.html.erb b/app/components/budgets/ballot/ballot_component.html.erb index cdd1f0641..132431c74 100644 --- a/app/components/budgets/ballot/ballot_component.html.erb +++ b/app/components/budgets/ballot/ballot_component.html.erb @@ -15,7 +15,6 @@
- <% ballot_groups = ballot.groups.sort_by_name %> <% ballot_groups.each do |group| %> <% heading = ballot.heading_for_group(group) %>
@@ -46,7 +45,6 @@
<% end %> - <% no_balloted_groups = budget.groups.sort_by_name - ballot_groups %> <% no_balloted_groups.each do |group| %>
diff --git a/app/components/budgets/ballot/ballot_component.rb b/app/components/budgets/ballot/ballot_component.rb index 7e2b77aaf..3eb6e78e4 100644 --- a/app/components/budgets/ballot/ballot_component.rb +++ b/app/components/budgets/ballot/ballot_component.rb @@ -8,4 +8,14 @@ class Budgets::Ballot::BallotComponent < ApplicationComponent def budget ballot.budget end + + private + + def ballot_groups + ballot.groups.sort_by_name + end + + def no_balloted_groups + budget.groups.sort_by_name - ballot.groups + end end From f669b476f8f619847b08fe028b3a9138a0c8b56c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 16 Sep 2021 12:48:25 +0200 Subject: [PATCH 3/4] Go to investments list from a single heading group There's no real point in linking to a page offering users to choose a heading when there's only one heading to choose. So we're linking to the investments index instead. --- .../budgets/ballot/ballot_component.html.erb | 5 +-- .../budgets/ballot/ballot_component.rb | 8 ++++ .../budgets/ballot/ballot_component_spec.rb | 42 +++++++++++++++++-- 3 files changed, 48 insertions(+), 7 deletions(-) diff --git a/app/components/budgets/ballot/ballot_component.html.erb b/app/components/budgets/ballot/ballot_component.html.erb index 132431c74..6834a3059 100644 --- a/app/components/budgets/ballot/ballot_component.html.erb +++ b/app/components/budgets/ballot/ballot_component.html.erb @@ -23,8 +23,7 @@

<%= group.name %> - <%= heading.name %>

- <%= link_to sanitize(ballot.amount_available_info(heading)), - budget_group_path(budget, group) %> + <%= link_to sanitize(ballot.amount_available_info(heading)), group_path(group) %>
<% if ballot.has_lines_in_group?(group) %>

@@ -52,7 +51,7 @@

<%= group.name %>

- <%= link_to t("budgets.ballots.show.no_balloted_group_yet"), budget_group_path(budget, group) %> + <%= link_to t("budgets.ballots.show.no_balloted_group_yet"), group_path(group) %>
diff --git a/app/components/budgets/ballot/ballot_component.rb b/app/components/budgets/ballot/ballot_component.rb index 3eb6e78e4..fbb8b1d75 100644 --- a/app/components/budgets/ballot/ballot_component.rb +++ b/app/components/budgets/ballot/ballot_component.rb @@ -18,4 +18,12 @@ class Budgets::Ballot::BallotComponent < ApplicationComponent def no_balloted_groups budget.groups.sort_by_name - ballot.groups end + + def group_path(group) + if group.multiple_headings? + budget_group_path(budget, group) + else + budget_investments_path(budget, heading_id: group.headings.first) + end + end end diff --git a/spec/components/budgets/ballot/ballot_component_spec.rb b/spec/components/budgets/ballot/ballot_component_spec.rb index e26edde7b..132b90b46 100644 --- a/spec/components/budgets/ballot/ballot_component_spec.rb +++ b/spec/components/budgets/ballot/ballot_component_spec.rb @@ -6,14 +6,48 @@ describe Budgets::Ballot::BallotComponent do describe "link to group" do let(:budget) { create(:budget, :balloting) } + let(:group) { create(:budget_group, budget: budget) } let(:ballot) { create(:budget_ballot, user: create(:user), budget: budget) } - it "displays links to vote on groups with no investments voted yet" do - group = create(:budget_group, budget: budget) + context "group with a single heading" do + let!(:heading) { create(:budget_heading, group: group, price: 1000) } - render_inline Budgets::Ballot::BallotComponent.new(ballot) + it "displays a link to vote investments when there aren't any investments in the ballot" do + render_inline Budgets::Ballot::BallotComponent.new(ballot) - expect(page).to have_link "You have not voted on this group yet, go vote!", href: budget_group_path(budget, group) + expect(page).to have_link "You have not voted on this group yet, go vote!", + href: budget_investments_path(budget, heading_id: heading.id) + end + + it "displays a link to continue voting when there are investments in the ballot" do + ballot.investments << create(:budget_investment, :selected, heading: heading, price: 200) + + render_inline Budgets::Ballot::BallotComponent.new(ballot) + + expect(page).to have_link "Still available to you €800", + href: budget_investments_path(budget, heading_id: heading.id) + end + end + + context "group with multiple headings" do + let!(:heading) { create(:budget_heading, group: group, price: 1000) } + before { create(:budget_heading, group: group) } + + it "displays a link to vote on a heading when there aren't any investments in the ballot" do + render_inline Budgets::Ballot::BallotComponent.new(ballot) + + expect(page).to have_link "You have not voted on this group yet, go vote!", + href: budget_group_path(budget, group) + end + + it "displays a link to change the heading when there are invesments in the ballot" do + ballot.investments << create(:budget_investment, :selected, heading: heading, price: 200) + + render_inline Budgets::Ballot::BallotComponent.new(ballot) + + expect(page).to have_link "Still available to you €800", + href: budget_group_path(budget, group) + end end end end From caebaac1cce081efd0760d9438935cc87f0450c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 17 Sep 2021 00:55:37 +0200 Subject: [PATCH 4/4] Fix investments link in single heading budgets The link to "See all investments" didn't have the `heading_id` parameter, which resulted in the ballot information not being displayed when in the voting phase. We could modify the link to include the `heading_id` parameter, but IMHO it's more robust to select the heading automatically when there's only one heading. That way manually accessing the page without a `heading_id` parameter will still work as if the heading had been selected. --- app/controllers/budgets/investments_controller.rb | 3 +++ spec/system/budgets/investments_spec.rb | 3 +++ 2 files changed, 6 insertions(+) diff --git a/app/controllers/budgets/investments_controller.rb b/app/controllers/budgets/investments_controller.rb index e3d82cb6f..3953def8c 100644 --- a/app/controllers/budgets/investments_controller.rb +++ b/app/controllers/budgets/investments_controller.rb @@ -135,6 +135,9 @@ module Budgets @heading = @budget.headings.find_by_slug_or_id! params[:heading_id] @assigned_heading = @ballot&.heading_for_group(@heading.group) load_map + elsif @budget.single_heading? + @heading = @budget.headings.first + load_map end end diff --git a/spec/system/budgets/investments_spec.rb b/spec/system/budgets/investments_spec.rb index 002d17dc2..615cf78ec 100644 --- a/spec/system/budgets/investments_spec.rb +++ b/spec/system/budgets/investments_spec.rb @@ -1356,6 +1356,9 @@ describe "Budget Investments" do expect(page).to have_content investment2.title expect(page).to have_content "€20,000" end + + expect(page).to have_link "Submit my ballot" + expect(page).to have_content "STILL AVAILABLE TO YOU €666,666" end scenario "Order by cost (only when balloting)" do