From 5fab8431848d10002b0f9eccf063ec5de2939757 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 13 Jun 2021 02:55:09 +0200 Subject: [PATCH] Simplify managing investment votes If I'm right, the `investment_votes` instance variable only exists to avoid several database queries to get whether the current user has supported each of the investments. However, that doesn't make much sense when only one investment is shown. In this case, the number of queries stays the same, and so we can simplify the code by rendering the component with an optional parameter. --- app/components/budgets/investments/votes_component.rb | 8 ++++++-- app/controllers/budgets/investments_controller.rb | 2 -- .../management/budgets/investments_controller.rb | 2 -- app/views/budgets/investments/_investment.html.erb | 5 ++--- app/views/budgets/investments/_investment_show.html.erb | 6 ++---- app/views/budgets/investments/_votes.html.erb | 4 ---- app/views/budgets/investments/show.html.erb | 1 - app/views/budgets/investments/vote.js.erb | 5 ++--- app/views/management/budgets/investments/show.html.erb | 4 +--- app/views/management/budgets/investments/vote.js.erb | 5 ++--- .../budgets/investments/votes_component_spec.rb | 2 +- 11 files changed, 16 insertions(+), 28 deletions(-) delete mode 100644 app/views/budgets/investments/_votes.html.erb diff --git a/app/components/budgets/investments/votes_component.rb b/app/components/budgets/investments/votes_component.rb index 7fb20dba3..1350f5be6 100644 --- a/app/components/budgets/investments/votes_component.rb +++ b/app/components/budgets/investments/votes_component.rb @@ -3,7 +3,7 @@ class Budgets::Investments::VotesComponent < ApplicationComponent delegate :namespace, :current_user, :voted_for?, :image_absolute_url, :link_to_verify_account, :link_to_signin, :link_to_signup, to: :helpers - def initialize(investment, investment_votes:) + def initialize(investment, investment_votes: nil) @investment = investment @investment_votes = investment_votes end @@ -28,7 +28,11 @@ class Budgets::Investments::VotesComponent < ApplicationComponent end def user_voted_for? - @user_voted_for ||= voted_for?(investment_votes, investment) + @user_voted_for ||= if investment_votes + voted_for?(investment_votes, investment) + else + current_user&.voted_for?(investment) + end end def display_support_alert? diff --git a/app/controllers/budgets/investments_controller.rb b/app/controllers/budgets/investments_controller.rb index e681160bc..4f25c78a5 100644 --- a/app/controllers/budgets/investments_controller.rb +++ b/app/controllers/budgets/investments_controller.rb @@ -60,7 +60,6 @@ module Budgets @comment_tree = CommentTree.new(@commentable, params[:page], @current_order) @related_contents = Kaminari.paginate_array(@investment.relationed_contents).page(params[:page]).per(5) set_comment_flags(@comment_tree.comments) - load_investment_votes(@investment) @investment_ids = [@investment.id] @remote_translations = detect_remote_translations([@investment], @comment_tree.comments) end @@ -94,7 +93,6 @@ module Budgets def vote @investment.register_selection(current_user) - load_investment_votes(@investment) respond_to do |format| format.html { redirect_to budget_investments_path(heading_id: @investment.heading.id) } format.js diff --git a/app/controllers/management/budgets/investments_controller.rb b/app/controllers/management/budgets/investments_controller.rb index 3a642e173..2cecdb6c3 100644 --- a/app/controllers/management/budgets/investments_controller.rb +++ b/app/controllers/management/budgets/investments_controller.rb @@ -37,12 +37,10 @@ class Management::Budgets::InvestmentsController < Management::BaseController end def show - load_investment_votes(@investment) end def vote @investment.register_selection(managed_user) - load_investment_votes(@investment) respond_to do |format| format.html { redirect_to management_budget_investments_path(heading_id: @investment.heading.id) } format.js diff --git a/app/views/budgets/investments/_investment.html.erb b/app/views/budgets/investments/_investment.html.erb index 4d3b68acc..79b6d8cf2 100644 --- a/app/views/budgets/investments/_investment.html.erb +++ b/app/views/budgets/investments/_investment.html.erb @@ -38,9 +38,8 @@
> - <%= render "/budgets/investments/votes", - investment: investment, - investment_votes: investment_votes %> + <%= render Budgets::Investments::VotesComponent.new(investment, + investment_votes: investment_votes) %>
<% elsif investment.should_show_vote_count? %>
+ current_user&.voted_for?(investment)] do %>
@@ -32,9 +32,7 @@

<%= t("budgets.investments.show.supports") %>

- <%= render "/budgets/investments/votes", - investment: investment, - investment_votes: investment_votes %> + <%= render Budgets::Investments::VotesComponent.new(investment) %>
<% elsif investment.should_show_vote_count? %> diff --git a/app/views/budgets/investments/_votes.html.erb b/app/views/budgets/investments/_votes.html.erb deleted file mode 100644 index 2717c924e..000000000 --- a/app/views/budgets/investments/_votes.html.erb +++ /dev/null @@ -1,4 +0,0 @@ -<%= render Budgets::Investments::VotesComponent.new( - investment, - investment_votes: investment_votes -) %> diff --git a/app/views/budgets/investments/show.html.erb b/app/views/budgets/investments/show.html.erb index bc3976ead..8c342c5ef 100644 --- a/app/views/budgets/investments/show.html.erb +++ b/app/views/budgets/investments/show.html.erb @@ -6,7 +6,6 @@ <%= render "/budgets/investments/investment_show", investment: @investment, investment_ids: @investment_ids, - investment_votes: @investment_votes, ballot: @ballot %>
diff --git a/app/views/budgets/investments/vote.js.erb b/app/views/budgets/investments/vote.js.erb index 76fabc128..bd86e7547 100644 --- a/app/views/budgets/investments/vote.js.erb +++ b/app/views/budgets/investments/vote.js.erb @@ -1,3 +1,2 @@ -$("#<%= dom_id(@investment) %>_votes").html("<%= j render("/budgets/investments/votes", - investment: @investment, - investment_votes: @investment_votes) %>"); +$("#<%= dom_id(@investment) %>_votes") + .html("<%= j render Budgets::Investments::VotesComponent.new(@investment) %>"); diff --git a/app/views/management/budgets/investments/show.html.erb b/app/views/management/budgets/investments/show.html.erb index 99dea2eaa..903b7ae39 100644 --- a/app/views/management/budgets/investments/show.html.erb +++ b/app/views/management/budgets/investments/show.html.erb @@ -2,6 +2,4 @@ <%= render "/shared/print" %> -<%= render "/budgets/investments/investment_show", - investment: @investment, - investment_votes: @investment_votes %> +<%= render "/budgets/investments/investment_show", investment: @investment %> diff --git a/app/views/management/budgets/investments/vote.js.erb b/app/views/management/budgets/investments/vote.js.erb index 76fabc128..bd86e7547 100644 --- a/app/views/management/budgets/investments/vote.js.erb +++ b/app/views/management/budgets/investments/vote.js.erb @@ -1,3 +1,2 @@ -$("#<%= dom_id(@investment) %>_votes").html("<%= j render("/budgets/investments/votes", - investment: @investment, - investment_votes: @investment_votes) %>"); +$("#<%= dom_id(@investment) %>_votes") + .html("<%= j render Budgets::Investments::VotesComponent.new(@investment) %>"); diff --git a/spec/components/budgets/investments/votes_component_spec.rb b/spec/components/budgets/investments/votes_component_spec.rb index 19d54ad65..c1f44f50e 100644 --- a/spec/components/budgets/investments/votes_component_spec.rb +++ b/spec/components/budgets/investments/votes_component_spec.rb @@ -4,7 +4,7 @@ describe Budgets::Investments::VotesComponent, type: :component do describe "vote link" do context "when investment shows votes" do let(:investment) { create(:budget_investment, title: "Renovate sidewalks in Main Street") } - let(:component) { Budgets::Investments::VotesComponent.new(investment, investment_votes: []) } + let(:component) { Budgets::Investments::VotesComponent.new(investment) } before { allow(investment).to receive(:should_show_votes?).and_return(true) }