From 4954038076f62d097c3a844c3c8f86dc56f47d21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 7 Mar 2023 16:15:34 +0100 Subject: [PATCH] Only re-render my ballot after voting We were rendering the whole sidebar again, which wasn't necessary since most of it remains unchanged. This resulted in complicated code to pass the necessary information to render the same map that was already rendered. Furthermore, when users had been moving the map around or zooming out, we were resetting the map to its default settings after voting, which was potentially annoying. This also fixes the wrong investments being displayed in the map after voting; only the investments on the current page were displayed in this case while the index displayed all of them. --- .../investments/my_ballot_component.rb | 8 ++++++-- .../budgets/ballot/lines_controller.rb | 8 -------- app/views/budgets/ballot/lines/create.js.erb | 10 +++++++--- app/views/budgets/ballot/lines/destroy.js.erb | 10 +++++++--- .../budgets/investments/_sidebar.html.erb | 4 ++-- spec/factories/budgets.rb | 4 ++++ spec/system/budgets/ballots_spec.rb | 19 +++++++++++++++++-- 7 files changed, 43 insertions(+), 20 deletions(-) diff --git a/app/components/budgets/investments/my_ballot_component.rb b/app/components/budgets/investments/my_ballot_component.rb index cbbfa2413..6bc06273f 100644 --- a/app/components/budgets/investments/my_ballot_component.rb +++ b/app/components/budgets/investments/my_ballot_component.rb @@ -1,14 +1,18 @@ class Budgets::Investments::MyBallotComponent < ApplicationComponent attr_reader :ballot, :heading, :investment_ids, :assigned_heading - delegate :heading_link, to: :helpers + delegate :can?, :heading_link, to: :helpers - def initialize(ballot:, heading:, investment_ids:, assigned_heading:) + def initialize(ballot:, heading:, investment_ids:, assigned_heading: nil) @ballot = ballot @heading = heading @investment_ids = investment_ids @assigned_heading = assigned_heading end + def render? + heading && can?(:show, ballot) + end + private def budget diff --git a/app/controllers/budgets/ballot/lines_controller.rb b/app/controllers/budgets/ballot/lines_controller.rb index 684f14110..dc7e8cfe3 100644 --- a/app/controllers/budgets/ballot/lines_controller.rb +++ b/app/controllers/budgets/ballot/lines_controller.rb @@ -15,7 +15,6 @@ module Budgets def create load_investment load_heading - load_map @ballot.add_investment(@investment) end @@ -23,7 +22,6 @@ module Budgets def destroy @investment = @line.investment load_heading - load_map @line.destroy! load_investments @@ -69,12 +67,6 @@ module Budgets def load_categories @categories = Tag.category.order(:name) end - - def load_map - @investments ||= [] - @investments_map_coordinates = MapLocation.where(investment: @investments).map(&:json_data) - @map_location = MapLocation.load_from_heading(@heading) - end end end end diff --git a/app/views/budgets/ballot/lines/create.js.erb b/app/views/budgets/ballot/lines/create.js.erb index 48e56054f..d85a738bf 100644 --- a/app/views/budgets/ballot/lines/create.js.erb +++ b/app/views/budgets/ballot/lines/create.js.erb @@ -1,8 +1,12 @@ $("#progress_bar").html("<%= j render("/budgets/ballot/progress_bar", ballot: @ballot, heading: @heading) %>"); -$("#sidebar").html("<%= j render("/budgets/investments/sidebar") %>"); +$("#my_ballot").html("<%= j render( + Budgets::Investments::MyBallotComponent.new( + ballot: @ballot, + heading: @heading, + investment_ids: @investment_ids + ) +) %>"); <%= render "refresh_ballots", investment_ids: @investment_ids, ballot: @ballot %> - -App.Map.initialize(); diff --git a/app/views/budgets/ballot/lines/destroy.js.erb b/app/views/budgets/ballot/lines/destroy.js.erb index c93517414..75af79e09 100644 --- a/app/views/budgets/ballot/lines/destroy.js.erb +++ b/app/views/budgets/ballot/lines/destroy.js.erb @@ -1,9 +1,13 @@ $("#progress_bar").html("<%= j render("budgets/ballot/progress_bar", ballot: @ballot, heading: @heading) %>"); -$("#sidebar").html("<%= j render("budgets/investments/sidebar") %>"); +$("#my_ballot").html("<%= j render( + Budgets::Investments::MyBallotComponent.new( + ballot: @ballot, + heading: @heading, + investment_ids: @investment_ids + ) +) %>"); $("#ballot").html("<%= j render("budgets/ballot/ballot") %>") <%= render "refresh_ballots", investment_ids: @investment_ids, ballot: @ballot %> - -App.Map.initialize(); diff --git a/app/views/budgets/investments/_sidebar.html.erb b/app/views/budgets/investments/_sidebar.html.erb index 83126cef7..a45ea333f 100644 --- a/app/views/budgets/investments/_sidebar.html.erb +++ b/app/views/budgets/investments/_sidebar.html.erb @@ -12,14 +12,14 @@ <% end %> <% end %> -<% if @heading && can?(:show, @ballot) %> +
<%= render Budgets::Investments::MyBallotComponent.new( ballot: @ballot, heading: @heading, investment_ids: @investment_ids, assigned_heading: @assigned_heading ) %> -<% end %> +
<%= render Budgets::Investments::ContentBlocksComponent.new(@heading) %> diff --git a/spec/factories/budgets.rb b/spec/factories/budgets.rb index 59ae4b977..2e57a920b 100644 --- a/spec/factories/budgets.rb +++ b/spec/factories/budgets.rb @@ -193,6 +193,10 @@ FactoryBot.define do valuators { [create(:valuator)] } end + trait :with_map_location do + map_location + end + trait :flagged do after :create do |investment| Flag.flag(create(:user), investment) diff --git a/spec/system/budgets/ballots_spec.rb b/spec/system/budgets/ballots_spec.rb index 35e1a8fc8..62653fdb0 100644 --- a/spec/system/budgets/ballots_spec.rb +++ b/spec/system/budgets/ballots_spec.rb @@ -161,15 +161,28 @@ describe "Ballots" do end scenario "map and content block shoud be visible before and after" do - create(:budget_investment, :selected, heading: new_york, price: 10000, title: "More bridges") + stub_const("#{Budgets::InvestmentsController}::PER_PAGE", 1) + + create(:budget_investment, :selected, :with_map_location, + heading: new_york, + price: 10000, + title: "More bridges", + ) + create(:budget_investment, :selected, :with_map_location, + heading: new_york, + price: 5000, + title: "Less bridges" + ) + create(:heading_content_block, heading: new_york, body: "
  • New Block
    • ") new_york.update!(allow_custom_content: true) - visit budget_investments_path(budget, heading_id: new_york) + visit budget_investments_path(budget, heading_id: new_york, order: :price) within("#sidebar") do expect(page).to have_content "OpenStreetMap" expect(page).to have_content "New Block" + expect(page).to have_css ".map-icon", visible: :all, count: 2 end add_to_ballot("More bridges") @@ -178,6 +191,7 @@ describe "Ballots" do expect(page).to have_content "More bridges" expect(page).to have_content "OpenStreetMap" expect(page).to have_content "New Block" + expect(page).to have_css ".map-icon", visible: :all, count: 2 end within(".budget-investment", text: "More bridges") do @@ -188,6 +202,7 @@ describe "Ballots" do expect(page).not_to have_content "More bridges" expect(page).to have_content "OpenStreetMap" expect(page).to have_content "New Block" + expect(page).to have_css ".map-icon", visible: :all, count: 2 end end end