From d5f4313f592d5532ead90470300b26159927d316 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 13 Jun 2021 01:18:42 +0200 Subject: [PATCH 1/7] Simplify getting URL to support an investment We're also changing the method name to `vote_path` in order to be consistent with the way Rails uses `_path` for relative URLs. --- .../budgets/investments/votes_component.html.erb | 2 +- .../budgets/investments/votes_component.rb | 16 ++++++++++++---- app/helpers/budgets_helper.rb | 9 --------- .../budgets/investments/_investment.html.erb | 3 +-- .../investments/_investment_show.html.erb | 3 +-- app/views/budgets/investments/_votes.html.erb | 3 +-- app/views/budgets/investments/vote.js.erb | 3 +-- .../management/budgets/investments/vote.js.erb | 3 +-- .../budgets/investments/votes_component_spec.rb | 4 +--- 9 files changed, 19 insertions(+), 27 deletions(-) diff --git a/app/components/budgets/investments/votes_component.html.erb b/app/components/budgets/investments/votes_component.html.erb index 255d551d0..5e9e78a6a 100644 --- a/app/components/budgets/investments/votes_component.html.erb +++ b/app/components/budgets/investments/votes_component.html.erb @@ -10,7 +10,7 @@ <%= t("budgets.investments.investment.already_supported") %> <% elsif investment.should_show_votes? %> - <%= button_to t("budgets.investments.investment.give_support"), vote_url, + <%= button_to t("budgets.investments.investment.give_support"), vote_path, class: "button button-support small expanded", title: t("budgets.investments.investment.support_title"), method: "post", diff --git a/app/components/budgets/investments/votes_component.rb b/app/components/budgets/investments/votes_component.rb index b8a4f6571..7fb20dba3 100644 --- a/app/components/budgets/investments/votes_component.rb +++ b/app/components/budgets/investments/votes_component.rb @@ -1,12 +1,20 @@ class Budgets::Investments::VotesComponent < ApplicationComponent - attr_reader :investment, :investment_votes, :vote_url - delegate :current_user, :voted_for?, :image_absolute_url, + attr_reader :investment, :investment_votes + 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:, vote_url:) + def initialize(investment, investment_votes:) @investment = investment @investment_votes = investment_votes - @vote_url = vote_url + end + + def vote_path + case namespace + when "management" + vote_management_budget_investment_path(investment.budget, investment, value: "yes") + else + vote_budget_investment_path(investment.budget, investment, value: "yes") + end end private diff --git a/app/helpers/budgets_helper.rb b/app/helpers/budgets_helper.rb index f7c196bbb..6f51f5f30 100644 --- a/app/helpers/budgets_helper.rb +++ b/app/helpers/budgets_helper.rb @@ -15,15 +15,6 @@ module BudgetsHelper end end - def namespaced_budget_investment_vote_path(investment, options = {}) - case namespace - when "management" - vote_management_budget_investment_path(investment.budget, investment, options) - else - vote_budget_investment_path(investment.budget, investment, options) - end - end - def css_for_ballot_heading(heading) return "" if current_ballot.blank? || @current_filter == "unfeasible" diff --git a/app/views/budgets/investments/_investment.html.erb b/app/views/budgets/investments/_investment.html.erb index ef96f1f3f..4d3b68acc 100644 --- a/app/views/budgets/investments/_investment.html.erb +++ b/app/views/budgets/investments/_investment.html.erb @@ -40,8 +40,7 @@ <%= "data-equalizer-watch" if feature?(:allow_images) && investment.image.present? %>> <%= render "/budgets/investments/votes", investment: investment, - investment_votes: investment_votes, - vote_url: namespaced_budget_investment_vote_path(investment, value: "yes") %> + investment_votes: investment_votes %> <% elsif investment.should_show_vote_count? %>
<%= render "/budgets/investments/votes", investment: investment, - investment_votes: investment_votes, - vote_url: namespaced_budget_investment_vote_path(investment, value: "yes") %> + investment_votes: investment_votes %>
<% elsif investment.should_show_vote_count? %> diff --git a/app/views/budgets/investments/_votes.html.erb b/app/views/budgets/investments/_votes.html.erb index d134506c1..2717c924e 100644 --- a/app/views/budgets/investments/_votes.html.erb +++ b/app/views/budgets/investments/_votes.html.erb @@ -1,5 +1,4 @@ <%= render Budgets::Investments::VotesComponent.new( investment, - investment_votes: investment_votes, - vote_url: vote_url + investment_votes: investment_votes ) %> diff --git a/app/views/budgets/investments/vote.js.erb b/app/views/budgets/investments/vote.js.erb index 74ce74ccf..76fabc128 100644 --- a/app/views/budgets/investments/vote.js.erb +++ b/app/views/budgets/investments/vote.js.erb @@ -1,4 +1,3 @@ $("#<%= dom_id(@investment) %>_votes").html("<%= j render("/budgets/investments/votes", investment: @investment, - investment_votes: @investment_votes, - vote_url: namespaced_budget_investment_vote_path(@investment, value: "yes")) %>"); + investment_votes: @investment_votes) %>"); diff --git a/app/views/management/budgets/investments/vote.js.erb b/app/views/management/budgets/investments/vote.js.erb index 74ce74ccf..76fabc128 100644 --- a/app/views/management/budgets/investments/vote.js.erb +++ b/app/views/management/budgets/investments/vote.js.erb @@ -1,4 +1,3 @@ $("#<%= dom_id(@investment) %>_votes").html("<%= j render("/budgets/investments/votes", investment: @investment, - investment_votes: @investment_votes, - vote_url: namespaced_budget_investment_vote_path(@investment, value: "yes")) %>"); + investment_votes: @investment_votes) %>"); diff --git a/spec/components/budgets/investments/votes_component_spec.rb b/spec/components/budgets/investments/votes_component_spec.rb index 0b85f7f20..19d54ad65 100644 --- a/spec/components/budgets/investments/votes_component_spec.rb +++ b/spec/components/budgets/investments/votes_component_spec.rb @@ -4,9 +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) do - Budgets::Investments::VotesComponent.new(investment, investment_votes: [], vote_url: "/") - end + let(:component) { Budgets::Investments::VotesComponent.new(investment, investment_votes: []) } before { allow(investment).to receive(:should_show_votes?).and_return(true) } 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 2/7] 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) } From 0214184b2df0272d24744051eb5bd944372f5f55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 13 Jun 2021 04:23:43 +0200 Subject: [PATCH 3/7] Remove investment supports query optimizations In the previous commit I mentioned: > 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. Now let's discuss the case when there are several investments, like in the investments index: * There are 10 investments per page by default * Each query takes less than a millisecond * We still make a query per investment to check whether the current user voted in a different group * AFAIK, there have been no performance tests showing these optimizations make the request to the investments index significantly faster * These optimizations make the code way more complex than it is without them Considering all these points, I'm removing the optimizations. I'm fine with adding `includes` calls to preload records and avoid N+1 queries even if there are no performance tests showing they make the application faster because the effect on the code complexity is negligible. But that's not the case here. Note we're using `defined?` instead of the `||=` operator because the `||=` operator will not serve its purpose when the result of the operation returns `false`. --- .../budgets/investments/votes_component.rb | 12 ++++-------- app/controllers/budgets/investments_controller.rb | 5 ----- .../management/budgets/investments_controller.rb | 6 ------ app/models/user.rb | 5 ----- app/views/budgets/investments/_investment.html.erb | 3 +-- app/views/budgets/investments/index.html.erb | 1 - .../management/budgets/investments/index.html.erb | 1 - .../management/budgets/investments/print.html.erb | 1 - 8 files changed, 5 insertions(+), 29 deletions(-) diff --git a/app/components/budgets/investments/votes_component.rb b/app/components/budgets/investments/votes_component.rb index 1350f5be6..e7e601c18 100644 --- a/app/components/budgets/investments/votes_component.rb +++ b/app/components/budgets/investments/votes_component.rb @@ -1,11 +1,10 @@ class Budgets::Investments::VotesComponent < ApplicationComponent - attr_reader :investment, :investment_votes + attr_reader :investment 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: nil) + def initialize(investment) @investment = investment - @investment_votes = investment_votes end def vote_path @@ -28,11 +27,8 @@ class Budgets::Investments::VotesComponent < ApplicationComponent end def user_voted_for? - @user_voted_for ||= if investment_votes - voted_for?(investment_votes, investment) - else - current_user&.voted_for?(investment) - end + @user_voted_for = current_user&.voted_for?(investment) unless defined?(@user_voted_for) + @user_voted_for end def display_support_alert? diff --git a/app/controllers/budgets/investments_controller.rb b/app/controllers/budgets/investments_controller.rb index 4f25c78a5..c0eec21bb 100644 --- a/app/controllers/budgets/investments_controller.rb +++ b/app/controllers/budgets/investments_controller.rb @@ -47,7 +47,6 @@ module Budgets @investment_ids = @investments.pluck(:id) @investments_map_coordinates = MapLocation.where(investment: investments).map(&:json_data) - load_investment_votes(@investments) @tag_cloud = tag_cloud @remote_translations = detect_remote_translations(@investments) end @@ -128,10 +127,6 @@ module Budgets "budget_investment" end - def load_investment_votes(investments) - @investment_votes = current_user ? current_user.budget_investment_votes(investments) : {} - end - def investment_params attributes = [:heading_id, :tag_list, :organization_name, :location, :terms_of_service, :related_sdg_list, diff --git a/app/controllers/management/budgets/investments_controller.rb b/app/controllers/management/budgets/investments_controller.rb index 2cecdb6c3..ba32122fd 100644 --- a/app/controllers/management/budgets/investments_controller.rb +++ b/app/controllers/management/budgets/investments_controller.rb @@ -15,7 +15,6 @@ class Management::Budgets::InvestmentsController < Management::BaseController def index @investments = @investments.apply_filters_and_search(@budget, params).page(params[:page]) - load_investment_votes(@investments) end def new @@ -49,15 +48,10 @@ class Management::Budgets::InvestmentsController < Management::BaseController def print @investments = @investments.apply_filters_and_search(@budget, params).order(cached_votes_up: :desc).for_render.limit(15) - load_investment_votes(@investments) end private - def load_investment_votes(investments) - @investment_votes = managed_user ? managed_user.budget_investment_votes(investments) : {} - end - def investment_params attributes = [:external_url, :heading_id, :tag_list, :organization_name, :location, image_attributes: image_attributes, diff --git a/app/models/user.rb b/app/models/user.rb index 15159f727..e6304a318 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -161,11 +161,6 @@ class User < ApplicationRecord voted.each_with_object({}) { |v, h| h[v.votable_id] = v.value } end - def budget_investment_votes(budget_investments) - voted = votes.for_budget_investments(budget_investments) - voted.each_with_object({}) { |v, h| h[v.votable_id] = v.value } - end - def comment_flags(comments) comment_flags = flags.for_comments(comments) comment_flags.each_with_object({}) { |f, h| h[f.flaggable_id] = true } diff --git a/app/views/budgets/investments/_investment.html.erb b/app/views/budgets/investments/_investment.html.erb index 79b6d8cf2..86c66d9fe 100644 --- a/app/views/budgets/investments/_investment.html.erb +++ b/app/views/budgets/investments/_investment.html.erb @@ -38,8 +38,7 @@
> - <%= render Budgets::Investments::VotesComponent.new(investment, - investment_votes: investment_votes) %> + <%= render Budgets::Investments::VotesComponent.new(investment) %>
<% elsif investment.should_show_vote_count? %>
<% end %> <% else %> diff --git a/app/views/management/budgets/investments/index.html.erb b/app/views/management/budgets/investments/index.html.erb index e44f904de..ad00520b7 100644 --- a/app/views/management/budgets/investments/index.html.erb +++ b/app/views/management/budgets/investments/index.html.erb @@ -21,7 +21,6 @@ <%= render "/budgets/investments/investment", investment: investment, investment_ids: @investment_ids, - investment_votes: @investment_votes, ballot: @ballot %> <% end %> diff --git a/app/views/management/budgets/investments/print.html.erb b/app/views/management/budgets/investments/print.html.erb index f6fcc4bab..f2ef18e22 100644 --- a/app/views/management/budgets/investments/print.html.erb +++ b/app/views/management/budgets/investments/print.html.erb @@ -26,7 +26,6 @@ <%= render "/budgets/investments/investment", investment: investment, investment_ids: @investment_ids, - investment_votes: @investment_votes, ballot: @ballot %> <% end %> From 071da81be64878ac873c9cad25f094c48e0cd0da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 13 Jun 2021 14:40:56 +0200 Subject: [PATCH 4/7] Add notice when supporting an investment It was hard to notice what was going on when supporting one investment which was at the bottom of the investment index page. I wonder whether we should add the title of the investment to this text; I'm not doing so because we don't do that anywhere else. --- app/controllers/budgets/investments_controller.rb | 7 ++++++- .../management/budgets/investments_controller.rb | 7 ++++++- config/locales/en/responders.yml | 1 + config/locales/es/responders.yml | 1 + spec/system/budgets/votes_spec.rb | 2 ++ spec/system/management/budget_investments_spec.rb | 1 + 6 files changed, 17 insertions(+), 2 deletions(-) diff --git a/app/controllers/budgets/investments_controller.rb b/app/controllers/budgets/investments_controller.rb index c0eec21bb..b07d25b24 100644 --- a/app/controllers/budgets/investments_controller.rb +++ b/app/controllers/budgets/investments_controller.rb @@ -92,8 +92,13 @@ module Budgets def vote @investment.register_selection(current_user) + respond_to do |format| - format.html { redirect_to budget_investments_path(heading_id: @investment.heading.id) } + format.html do + redirect_to budget_investments_path(heading_id: @investment.heading.id), + notice: t("flash.actions.create.support") + end + format.js end end diff --git a/app/controllers/management/budgets/investments_controller.rb b/app/controllers/management/budgets/investments_controller.rb index ba32122fd..68abcbb84 100644 --- a/app/controllers/management/budgets/investments_controller.rb +++ b/app/controllers/management/budgets/investments_controller.rb @@ -40,8 +40,13 @@ class Management::Budgets::InvestmentsController < Management::BaseController def vote @investment.register_selection(managed_user) + respond_to do |format| - format.html { redirect_to management_budget_investments_path(heading_id: @investment.heading.id) } + format.html do + redirect_to management_budget_investments_path(heading_id: @investment.heading.id), + notice: t("flash.actions.create.support") + end + format.js end end diff --git a/config/locales/en/responders.yml b/config/locales/en/responders.yml index 82ee36271..7fb544b97 100644 --- a/config/locales/en/responders.yml +++ b/config/locales/en/responders.yml @@ -14,6 +14,7 @@ en: proposal_notification: "Your message has been sent correctly." budget_investment: "Budget Investment created successfully." signature_sheet: "Signature sheet created successfully" + support: "Investment supported successfully" topic: "Topic created successfully." valuator_group: "Valuator group created successfully" save_changes: diff --git a/config/locales/es/responders.yml b/config/locales/es/responders.yml index fc12f9961..6092cf9ff 100644 --- a/config/locales/es/responders.yml +++ b/config/locales/es/responders.yml @@ -14,6 +14,7 @@ es: proposal_notification: "Tu mensaje ha sido enviado correctamente." budget_investment: "Proyecto de gasto creado correctamente." signature_sheet: "Hoja de firmas creada correctamente" + support: "Proyecto de gasto apoyado correctamente" topic: "Tema creado correctamente." valuator_group: "Grupo de evaluadores creado correctamente" save_changes: diff --git a/spec/system/budgets/votes_spec.rb b/spec/system/budgets/votes_spec.rb index 5e1aff243..2a9de4659 100644 --- a/spec/system/budgets/votes_spec.rb +++ b/spec/system/budgets/votes_spec.rb @@ -127,6 +127,8 @@ describe "Votes" do "Share it!" end + expect(page).to have_content "Investment supported successfully" + visit budget_investments_path(budget, heading_id: san_francisco.id) within("#budget_investment_#{san_francisco_investment.id}") do diff --git a/spec/system/management/budget_investments_spec.rb b/spec/system/management/budget_investments_spec.rb index eafb51d94..382ff4a63 100644 --- a/spec/system/management/budget_investments_spec.rb +++ b/spec/system/management/budget_investments_spec.rb @@ -329,6 +329,7 @@ describe "Budget Investments" do expect(page).to have_content "1 support" expect(page).to have_content "You have already supported this investment project. Share it!" + expect(page).to have_content "Investment supported successfully" expect(page).to have_content "CONSUL\nMANAGEMENT" end From 758cdaf8d7ebfa78232496beb66cae03e1b49769 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 13 Jun 2021 20:38:42 +0200 Subject: [PATCH 5/7] Extract controllers to support investments Since we're going to add an action to remove supports, having a separate controller makes things easier. Note there was a strange piece of code which assumed users were not verified if they couldn't vote investments. Now the code is also strange, since it assumes users are not verified if they can't create votes. We might need to revisit these conditions if our logic changes in the future. --- .../budgets/investments/votes_component.rb | 4 ++-- .../budgets/investments/votes_controller.rb | 21 +++++++++++++++++++ .../budgets/investments_controller.rb | 13 ------------ .../budgets/investments/votes_controller.rb | 17 +++++++++++++++ .../budgets/investments_controller.rb | 13 ------------ app/models/abilities/common.rb | 5 ++++- app/models/budget/investment.rb | 2 +- .../{vote.js.erb => votes/create.js.erb} | 0 .../{vote.js.erb => votes/create.js.erb} | 0 config/routes/budget.rb | 3 ++- config/routes/management.rb | 3 ++- spec/models/abilities/common_spec.rb | 6 +++--- 12 files changed, 52 insertions(+), 35 deletions(-) create mode 100644 app/controllers/budgets/investments/votes_controller.rb create mode 100644 app/controllers/management/budgets/investments/votes_controller.rb rename app/views/budgets/investments/{vote.js.erb => votes/create.js.erb} (100%) rename app/views/management/budgets/investments/{vote.js.erb => votes/create.js.erb} (100%) diff --git a/app/components/budgets/investments/votes_component.rb b/app/components/budgets/investments/votes_component.rb index e7e601c18..1ab12de59 100644 --- a/app/components/budgets/investments/votes_component.rb +++ b/app/components/budgets/investments/votes_component.rb @@ -10,9 +10,9 @@ class Budgets::Investments::VotesComponent < ApplicationComponent def vote_path case namespace when "management" - vote_management_budget_investment_path(investment.budget, investment, value: "yes") + management_budget_investment_votes_path(investment.budget, investment) else - vote_budget_investment_path(investment.budget, investment, value: "yes") + budget_investment_votes_path(investment.budget, investment) end end diff --git a/app/controllers/budgets/investments/votes_controller.rb b/app/controllers/budgets/investments/votes_controller.rb new file mode 100644 index 000000000..26921d437 --- /dev/null +++ b/app/controllers/budgets/investments/votes_controller.rb @@ -0,0 +1,21 @@ +module Budgets + module Investments + class VotesController < ApplicationController + load_and_authorize_resource :budget + load_and_authorize_resource :investment, through: :budget, class: "Budget::Investment" + + def create + @investment.register_selection(current_user) + + respond_to do |format| + format.html do + redirect_to budget_investments_path(heading_id: @investment.heading.id), + notice: t("flash.actions.create.support") + end + + format.js + end + end + end + end +end diff --git a/app/controllers/budgets/investments_controller.rb b/app/controllers/budgets/investments_controller.rb index b07d25b24..6286fe13b 100644 --- a/app/controllers/budgets/investments_controller.rb +++ b/app/controllers/budgets/investments_controller.rb @@ -90,19 +90,6 @@ module Budgets redirect_to user_path(current_user, filter: "budget_investments"), notice: t("flash.actions.destroy.budget_investment") end - def vote - @investment.register_selection(current_user) - - respond_to do |format| - format.html do - redirect_to budget_investments_path(heading_id: @investment.heading.id), - notice: t("flash.actions.create.support") - end - - format.js - end - end - def suggest @resource_path_method = :namespaced_budget_investment_path @resource_relation = resource_model.where(budget: @budget).apply_filters_and_search(@budget, params, @current_filter) diff --git a/app/controllers/management/budgets/investments/votes_controller.rb b/app/controllers/management/budgets/investments/votes_controller.rb new file mode 100644 index 000000000..f96ec1ed6 --- /dev/null +++ b/app/controllers/management/budgets/investments/votes_controller.rb @@ -0,0 +1,17 @@ +class Management::Budgets::Investments::VotesController < Management::BaseController + load_resource :budget, find_by: :slug_or_id + load_resource :investment, through: :budget, class: "Budget::Investment" + + def create + @investment.register_selection(managed_user) + + respond_to do |format| + format.html do + redirect_to management_budget_investments_path(heading_id: @investment.heading.id), + notice: t("flash.actions.create.support") + end + + format.js + end + end +end diff --git a/app/controllers/management/budgets/investments_controller.rb b/app/controllers/management/budgets/investments_controller.rb index 68abcbb84..627964a69 100644 --- a/app/controllers/management/budgets/investments_controller.rb +++ b/app/controllers/management/budgets/investments_controller.rb @@ -38,19 +38,6 @@ class Management::Budgets::InvestmentsController < Management::BaseController def show end - def vote - @investment.register_selection(managed_user) - - respond_to do |format| - format.html do - redirect_to management_budget_investments_path(heading_id: @investment.heading.id), - notice: t("flash.actions.create.support") - end - - format.js - end - end - def print @investments = @investments.apply_filters_and_search(@budget, params).order(cached_votes_up: :desc).for_render.limit(15) end diff --git a/app/models/abilities/common.rb b/app/models/abilities/common.rb index dc2563492..24f86638f 100644 --- a/app/models/abilities/common.rb +++ b/app/models/abilities/common.rb @@ -99,7 +99,10 @@ module Abilities can :update, Budget::Investment, budget: { phase: "accepting" }, author_id: user.id can :suggest, Budget::Investment, budget: { phase: "accepting" } can :destroy, Budget::Investment, budget: { phase: ["accepting", "reviewing"] }, author_id: user.id - can :vote, Budget::Investment, budget: { phase: "selecting" } + can :create, ActsAsVotable::Vote, + voter_id: user.id, + votable_type: "Budget::Investment", + votable: { budget: { phase: "selecting" }} can [:show, :create], Budget::Ballot, budget: { phase: "balloting" } can [:create, :destroy], Budget::Ballot::Line, budget: { phase: "balloting" } diff --git a/app/models/budget/investment.rb b/app/models/budget/investment.rb index 75e83f5ff..79f0e6cde 100644 --- a/app/models/budget/investment.rb +++ b/app/models/budget/investment.rb @@ -277,7 +277,7 @@ class Budget def permission_problem(user) return :not_logged_in unless user return :organization if user.organization? - return :not_verified unless user.can?(:vote, Budget::Investment) + return :not_verified unless user.can?(:create, ActsAsVotable::Vote) nil end diff --git a/app/views/budgets/investments/vote.js.erb b/app/views/budgets/investments/votes/create.js.erb similarity index 100% rename from app/views/budgets/investments/vote.js.erb rename to app/views/budgets/investments/votes/create.js.erb diff --git a/app/views/management/budgets/investments/vote.js.erb b/app/views/management/budgets/investments/votes/create.js.erb similarity index 100% rename from app/views/management/budgets/investments/vote.js.erb rename to app/views/management/budgets/investments/votes/create.js.erb diff --git a/config/routes/budget.rb b/config/routes/budget.rb index bed9b7324..82794855f 100644 --- a/config/routes/budget.rb +++ b/config/routes/budget.rb @@ -2,12 +2,13 @@ resources :budgets, only: [:show, :index] do resources :groups, controller: "budgets/groups", only: [:show] resources :investments, controller: "budgets/investments" do member do - post :vote put :flag put :unflag end collection { get :suggest } + + resources :votes, controller: "budgets/investments/votes", only: :create end resource :ballot, only: :show, controller: "budgets/ballots" do diff --git a/config/routes/management.rb b/config/routes/management.rb index 7b35850b9..77afca888 100644 --- a/config/routes/management.rb +++ b/config/routes/management.rb @@ -39,8 +39,9 @@ namespace :management do end resources :investments, only: [:index, :new, :create, :show, :destroy], controller: "budgets/investments" do - post :vote, on: :member get :print, on: :collection + + resources :votes, controller: "budgets/investments/votes", only: :create end end end diff --git a/spec/models/abilities/common_spec.rb b/spec/models/abilities/common_spec.rb index 2cd3815f4..fc1bd62eb 100644 --- a/spec/models/abilities/common_spec.rb +++ b/spec/models/abilities/common_spec.rb @@ -248,9 +248,9 @@ describe Abilities::Common do it { should_not be_able_to(:create, investment_in_selecting_budget) } it { should_not be_able_to(:create, investment_in_balloting_budget) } - it { should be_able_to(:vote, investment_in_selecting_budget) } - it { should_not be_able_to(:vote, investment_in_accepting_budget) } - it { should_not be_able_to(:vote, investment_in_balloting_budget) } + it { should be_able_to(:create, user.votes.build(votable: investment_in_selecting_budget)) } + it { should_not be_able_to(:create, user.votes.build(votable: investment_in_accepting_budget)) } + it { should_not be_able_to(:create, user.votes.build(votable: investment_in_balloting_budget)) } it { should_not be_able_to(:destroy, investment_in_accepting_budget) } it { should_not be_able_to(:destroy, investment_in_reviewing_budget) } From 368023986a814b5333ed3521d15fe648d4d1c303 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 13 Jun 2021 23:34:34 +0200 Subject: [PATCH 6/7] Group investment support translation keys together This way it's easier to find the keys: keys related to the `Budgets::Investments::VotesComponent` class are in the `budgets.investments.votes` namespace. We're making a couple of exceptions; we're not modifying the `supports` nor the `support_title` keys because they're used in other places. --- .../budgets/investments/votes_component.html.erb | 4 ++-- .../budgets/investments/votes_component.rb | 4 ++-- config/locales/en/budgets.yml | 13 +++++++------ config/locales/es/budgets.yml | 13 +++++++------ 4 files changed, 18 insertions(+), 16 deletions(-) diff --git a/app/components/budgets/investments/votes_component.html.erb b/app/components/budgets/investments/votes_component.html.erb index 5e9e78a6a..3c9937de1 100644 --- a/app/components/budgets/investments/votes_component.html.erb +++ b/app/components/budgets/investments/votes_component.html.erb @@ -7,10 +7,10 @@
<% if user_voted_for? %>
- <%= t("budgets.investments.investment.already_supported") %> + <%= t("budgets.investments.votes.already_supported") %>
<% elsif investment.should_show_votes? %> - <%= button_to t("budgets.investments.investment.give_support"), vote_path, + <%= button_to t("budgets.investments.votes.support"), vote_path, class: "button button-support small expanded", title: t("budgets.investments.investment.support_title"), method: "post", diff --git a/app/components/budgets/investments/votes_component.rb b/app/components/budgets/investments/votes_component.rb index 1ab12de59..a6ff29983 100644 --- a/app/components/budgets/investments/votes_component.rb +++ b/app/components/budgets/investments/votes_component.rb @@ -38,10 +38,10 @@ class Budgets::Investments::VotesComponent < ApplicationComponent end def confirm_vote_message - t("budgets.investments.investment.confirm_group", count: investment.group.max_votable_headings) + t("budgets.investments.votes.confirm_group", count: investment.group.max_votable_headings) end def support_aria_label - t("budgets.investments.investment.support_label", investment: investment.title) + t("budgets.investments.votes.support_label", investment: investment.title) end end diff --git a/config/locales/en/budgets.yml b/config/locales/en/budgets.yml index 5b143a513..4cafad9e4 100644 --- a/config/locales/en/budgets.yml +++ b/config/locales/en/budgets.yml @@ -156,23 +156,24 @@ en: investment: add: Vote already_added: You have already added this investment project - already_supported: You have already supported this investment project. Share it! support_title: Support this project - support_label: "Support %{investment}" - confirm_group: - one: "You can only support investments in %{count} district. If you continue you cannot change the election of your district. Are you sure?" - other: "You can only support investments in %{count} districts. If you continue you cannot change the election of your district. Are you sure?" supports: one: 1 support other: "%{count} supports" zero: No supports - give_support: Support header: check_ballot: "Submit my ballot" different_heading_assigned: "You have active votes in another heading: %{heading_link}" change_ballot: "If you change your mind you can remove your votes in %{check_ballot} and start again." check_ballot_link: "submit my ballot" price: "Total budget" + votes: + already_supported: "You have already supported this investment project. Share it!" + confirm_group: + one: "You can only support investments in %{count} district. If you continue you cannot change the election of your district. Are you sure?" + other: "You can only support investments in %{count} districts. If you continue you cannot change the election of your district. Are you sure?" + support: "Support" + support_label: "Support %{investment}" investments_list: investment: price: "Price" diff --git a/config/locales/es/budgets.yml b/config/locales/es/budgets.yml index 1921ddb88..b03967178 100644 --- a/config/locales/es/budgets.yml +++ b/config/locales/es/budgets.yml @@ -156,23 +156,24 @@ es: investment: add: Votar already_added: Ya has añadido este proyecto de gasto - already_supported: Ya has apoyado este proyecto de gasto. ¡Compártelo! support_title: Apoyar este proyecto - support_label: "Apoyar %{investment}" - confirm_group: - one: "Sólo puedes apoyar proyectos en %{count} distrito. Si sigues adelante no podrás cambiar la elección de este distrito. ¿Estás seguro/a?" - other: "Sólo puedes apoyar proyectos en %{count} distritos. Si sigues adelante no podrás cambiar la elección de este distrito. ¿Estás seguro/a?" supports: zero: Sin apoyos one: 1 apoyo other: "%{count} apoyos" - give_support: Apoyar header: check_ballot: Revisar y confirmar mis votos different_heading_assigned: "Ya apoyaste proyectos de otra sección del presupuesto: %{heading_link}" change_ballot: "Si cambias de opinión puedes borrar tus votos en %{check_ballot} y volver a empezar." check_ballot_link: "revisar y confirmar mis votos" price: "Presupuesto total" + votes: + already_supported: "Ya has apoyado este proyecto de gasto. ¡Compártelo!" + confirm_group: + one: "Sólo puedes apoyar proyectos en %{count} distrito. Si sigues adelante no podrás cambiar la elección de este distrito. ¿Estás seguro/a?" + other: "Sólo puedes apoyar proyectos en %{count} distritos. Si sigues adelante no podrás cambiar la elección de este distrito. ¿Estás seguro/a?" + support: "Apoyar" + support_label: "Apoyar %{investment}" investments_list: investment: price: "Coste" From a851048d56b05e6027deb1328f15754b96efcd1d Mon Sep 17 00:00:00 2001 From: decabeza Date: Thu, 25 Jun 2020 17:58:52 +0200 Subject: [PATCH 7/7] Allow users to remove their support on investments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Note we don't cast negative votes when users remove their support. That way we provide compatibility for institutions who have implemented real negative votes (in case there are / will be any), and we also keep the database meaningful: it's not that users downvoted something; they simply removed their upvote. Co-Authored-By: Javi Martín Co-Authored-By: Julian Nicolas Herrero --- .../investments/votes_component.html.erb | 35 ++++++++----- .../budgets/investments/votes_component.rb | 17 ++++++- .../budgets/supports_info_component.html.erb | 1 - .../budgets/investments/votes_controller.rb | 11 +++- .../budgets/investments/votes_controller.rb | 11 +++- app/models/abilities/common.rb | 2 +- .../votes/{create.js.erb => show.js.erb} | 0 .../votes/{create.js.erb => show.js.erb} | 0 config/locales/en/budgets.yml | 3 +- config/locales/es/budgets.yml | 3 +- config/routes/budget.rb | 2 +- config/routes/management.rb | 2 +- .../investments/votes_component_spec.rb | 13 +++++ spec/models/abilities/common_spec.rb | 3 ++ spec/system/budgets/budgets_spec.rb | 36 +++++++++++++ spec/system/budgets/investments_spec.rb | 42 ++++++++++++++++ .../management/budget_investments_spec.rb | 50 +++++++++++++++++++ 17 files changed, 209 insertions(+), 22 deletions(-) rename app/views/budgets/investments/votes/{create.js.erb => show.js.erb} (100%) rename app/views/management/budgets/investments/votes/{create.js.erb => show.js.erb} (100%) diff --git a/app/components/budgets/investments/votes_component.html.erb b/app/components/budgets/investments/votes_component.html.erb index 3c9937de1..0aedbe37e 100644 --- a/app/components/budgets/investments/votes_component.html.erb +++ b/app/components/budgets/investments/votes_component.html.erb @@ -5,19 +5,28 @@
- <% if user_voted_for? %> -
- <%= t("budgets.investments.votes.already_supported") %> -
- <% elsif investment.should_show_votes? %> - <%= button_to t("budgets.investments.votes.support"), vote_path, - class: "button button-support small expanded", - title: t("budgets.investments.investment.support_title"), - method: "post", - remote: !display_support_alert?, - data: ({ confirm: confirm_vote_message } if display_support_alert?), - disabled: !current_user, - "aria-label": support_aria_label %> + <% if investment.should_show_votes? %> + <% if user_voted_for? %> +
+
+ <%= t("budgets.investments.votes.already_supported") %> +
+ <%= button_to t("budgets.investments.votes.remove_support"), remove_support_path, + class: "button button-remove-support expanded", + method: "delete", + remote: true, + "aria-label": remove_support_aria_label %> +
+ <% else %> + <%= button_to t("budgets.investments.votes.support"), support_path, + class: "button button-support expanded", + title: t("budgets.investments.investment.support_title"), + method: "post", + remote: !display_support_alert?, + data: ({ confirm: confirm_vote_message } if display_support_alert?), + disabled: !current_user, + "aria-label": support_aria_label %> + <% end %> <% end %>
diff --git a/app/components/budgets/investments/votes_component.rb b/app/components/budgets/investments/votes_component.rb index a6ff29983..57a948bca 100644 --- a/app/components/budgets/investments/votes_component.rb +++ b/app/components/budgets/investments/votes_component.rb @@ -7,7 +7,7 @@ class Budgets::Investments::VotesComponent < ApplicationComponent @investment = investment end - def vote_path + def support_path case namespace when "management" management_budget_investment_votes_path(investment.budget, investment) @@ -16,6 +16,17 @@ class Budgets::Investments::VotesComponent < ApplicationComponent end end + def remove_support_path + vote = investment.votes_for.find_by!(voter: current_user) + + case namespace + when "management" + management_budget_investment_vote_path(investment.budget, investment, vote) + else + budget_investment_vote_path(investment.budget, investment, vote) + end + end + private def reason @@ -44,4 +55,8 @@ class Budgets::Investments::VotesComponent < ApplicationComponent def support_aria_label t("budgets.investments.votes.support_label", investment: investment.title) end + + def remove_support_aria_label + t("budgets.investments.votes.remove_support_label", investment: investment.title) + end end diff --git a/app/components/budgets/supports_info_component.html.erb b/app/components/budgets/supports_info_component.html.erb index e9c55e837..19fff395f 100644 --- a/app/components/budgets/supports_info_component.html.erb +++ b/app/components/budgets/supports_info_component.html.erb @@ -5,7 +5,6 @@

<%= t("budgets.supports_info.next") %>

-

<%= sanitize(t("budgets.supports_info.remember")) %>

<%= t("budgets.supports_info.different") %>

diff --git a/app/controllers/budgets/investments/votes_controller.rb b/app/controllers/budgets/investments/votes_controller.rb index 26921d437..5b30a17c7 100644 --- a/app/controllers/budgets/investments/votes_controller.rb +++ b/app/controllers/budgets/investments/votes_controller.rb @@ -3,6 +3,7 @@ module Budgets class VotesController < ApplicationController load_and_authorize_resource :budget load_and_authorize_resource :investment, through: :budget, class: "Budget::Investment" + load_and_authorize_resource through: :investment, through_association: :votes_for, only: :destroy def create @investment.register_selection(current_user) @@ -13,7 +14,15 @@ module Budgets notice: t("flash.actions.create.support") end - format.js + format.js { render :show } + end + end + + def destroy + @investment.unliked_by(current_user) + + respond_to do |format| + format.js { render :show } end end end diff --git a/app/controllers/management/budgets/investments/votes_controller.rb b/app/controllers/management/budgets/investments/votes_controller.rb index f96ec1ed6..1ab8f8056 100644 --- a/app/controllers/management/budgets/investments/votes_controller.rb +++ b/app/controllers/management/budgets/investments/votes_controller.rb @@ -1,6 +1,7 @@ class Management::Budgets::Investments::VotesController < Management::BaseController load_resource :budget, find_by: :slug_or_id load_resource :investment, through: :budget, class: "Budget::Investment" + load_and_authorize_resource through: :investment, through_association: :votes_for, only: :destroy def create @investment.register_selection(managed_user) @@ -11,7 +12,15 @@ class Management::Budgets::Investments::VotesController < Management::BaseContro notice: t("flash.actions.create.support") end - format.js + format.js { render :show } + end + end + + def destroy + @investment.unliked_by(managed_user) + + respond_to do |format| + format.js { render :show } end end end diff --git a/app/models/abilities/common.rb b/app/models/abilities/common.rb index 24f86638f..8e341a39d 100644 --- a/app/models/abilities/common.rb +++ b/app/models/abilities/common.rb @@ -99,7 +99,7 @@ module Abilities can :update, Budget::Investment, budget: { phase: "accepting" }, author_id: user.id can :suggest, Budget::Investment, budget: { phase: "accepting" } can :destroy, Budget::Investment, budget: { phase: ["accepting", "reviewing"] }, author_id: user.id - can :create, ActsAsVotable::Vote, + can [:create, :destroy], ActsAsVotable::Vote, voter_id: user.id, votable_type: "Budget::Investment", votable: { budget: { phase: "selecting" }} diff --git a/app/views/budgets/investments/votes/create.js.erb b/app/views/budgets/investments/votes/show.js.erb similarity index 100% rename from app/views/budgets/investments/votes/create.js.erb rename to app/views/budgets/investments/votes/show.js.erb diff --git a/app/views/management/budgets/investments/votes/create.js.erb b/app/views/management/budgets/investments/votes/show.js.erb similarity index 100% rename from app/views/management/budgets/investments/votes/create.js.erb rename to app/views/management/budgets/investments/votes/show.js.erb diff --git a/config/locales/en/budgets.yml b/config/locales/en/budgets.yml index 4cafad9e4..e2c8e65fd 100644 --- a/config/locales/en/budgets.yml +++ b/config/locales/en/budgets.yml @@ -172,6 +172,8 @@ en: confirm_group: one: "You can only support investments in %{count} district. If you continue you cannot change the election of your district. Are you sure?" other: "You can only support investments in %{count} districts. If you continue you cannot change the election of your district. Are you sure?" + remove_support: "Remove your support" + remove_support_label: "Remove your support to %{investment}" support: "Support" support_label: "Support %{investment}" investments_list: @@ -186,7 +188,6 @@ en: supports_info: different: "You may support on as many different projects as you would like." next: "Support the projects you would like to see move on to the next phase." - remember: "Remember! You can only cast your support once for each project and each support is irreversible." scrolling: "Keep scrolling to see all ideas" share: "You can share the projects you have supported on through social media and attract more attention and support to them!" supported: diff --git a/config/locales/es/budgets.yml b/config/locales/es/budgets.yml index b03967178..3f232f6f2 100644 --- a/config/locales/es/budgets.yml +++ b/config/locales/es/budgets.yml @@ -172,6 +172,8 @@ es: confirm_group: one: "Sólo puedes apoyar proyectos en %{count} distrito. Si sigues adelante no podrás cambiar la elección de este distrito. ¿Estás seguro/a?" other: "Sólo puedes apoyar proyectos en %{count} distritos. Si sigues adelante no podrás cambiar la elección de este distrito. ¿Estás seguro/a?" + remove_support: "Eliminar apoyo" + remove_support_label: "Eliminar tu apoyo a %{investment}" support: "Apoyar" support_label: "Apoyar %{investment}" investments_list: @@ -186,7 +188,6 @@ es: supports_info: different: "Puedes apoyar tantos proyectos diferentes como quieras." next: "Apoya proyectos que te gustaría ver en la siguiente fase." - remember: "¡Recuerda! Solo puedes emitir tu apoyo una vez por cada proyecto y cada apoyo es irreversible." scrolling: "Sigue desplazándote para ver todas las ideas" share: "Puedes compartir los proyectos que has apoyado en las redes sociales y ¡atraer más atención y apoyos para ellos!" supported: diff --git a/config/routes/budget.rb b/config/routes/budget.rb index 82794855f..416bdf88e 100644 --- a/config/routes/budget.rb +++ b/config/routes/budget.rb @@ -8,7 +8,7 @@ resources :budgets, only: [:show, :index] do collection { get :suggest } - resources :votes, controller: "budgets/investments/votes", only: :create + resources :votes, controller: "budgets/investments/votes", only: [:create, :destroy] end resource :ballot, only: :show, controller: "budgets/ballots" do diff --git a/config/routes/management.rb b/config/routes/management.rb index 77afca888..9de4ef731 100644 --- a/config/routes/management.rb +++ b/config/routes/management.rb @@ -41,7 +41,7 @@ namespace :management do resources :investments, only: [:index, :new, :create, :show, :destroy], controller: "budgets/investments" do get :print, on: :collection - resources :votes, controller: "budgets/investments/votes", only: :create + resources :votes, controller: "budgets/investments/votes", only: [:create, :destroy] end end end diff --git a/spec/components/budgets/investments/votes_component_spec.rb b/spec/components/budgets/investments/votes_component_spec.rb index c1f44f50e..a3b3fdae0 100644 --- a/spec/components/budgets/investments/votes_component_spec.rb +++ b/spec/components/budgets/investments/votes_component_spec.rb @@ -23,8 +23,21 @@ describe Budgets::Investments::VotesComponent, type: :component do render_inline component + expect(page).to have_button count: 1, disabled: :all expect(page).to have_button "Support", disabled: true end + + it "shows the button to remove support when users have supported the investment" do + user = create(:user) + user.up_votes(investment) + allow(controller).to receive(:current_user).and_return(user) + + render_inline component + + expect(page).to have_button count: 1, disabled: :all + expect(page).to have_button "Remove your support" + expect(page).to have_button "Remove your support to Renovate sidewalks in Main Street" + end end end end diff --git a/spec/models/abilities/common_spec.rb b/spec/models/abilities/common_spec.rb index fc1bd62eb..8cce034f2 100644 --- a/spec/models/abilities/common_spec.rb +++ b/spec/models/abilities/common_spec.rb @@ -251,6 +251,9 @@ describe Abilities::Common do it { should be_able_to(:create, user.votes.build(votable: investment_in_selecting_budget)) } it { should_not be_able_to(:create, user.votes.build(votable: investment_in_accepting_budget)) } it { should_not be_able_to(:create, user.votes.build(votable: investment_in_balloting_budget)) } + it { should be_able_to(:destroy, create(:vote, voter: user, votable: investment_in_selecting_budget)) } + it { should_not be_able_to(:destroy, create(:vote, voter: user, votable: investment_in_accepting_budget)) } + it { should_not be_able_to(:destroy, create(:vote, voter: user, votable: investment_in_balloting_budget)) } it { should_not be_able_to(:destroy, investment_in_accepting_budget) } it { should_not be_able_to(:destroy, investment_in_reviewing_budget) } diff --git a/spec/system/budgets/budgets_spec.rb b/spec/system/budgets/budgets_spec.rb index 9aa52b13e..9214ff311 100644 --- a/spec/system/budgets/budgets_spec.rb +++ b/spec/system/budgets/budgets_spec.rb @@ -432,6 +432,42 @@ describe "Budgets" do expect(page).to have_content "It's time to support projects!" expect(page).to have_content "So far you've supported 3 projects." end + + scenario "Show supports only if the support has not been removed" do + voter = create(:user, :level_two) + budget = create(:budget, phase: "selecting") + investment = create(:budget_investment, :selected, budget: budget) + + login_as(voter) + + visit budget_path(budget) + + expect(page).to have_content "So far you've supported 0 projects." + + visit budget_investment_path(budget, investment) + + within("#budget_investment_#{investment.id}_votes") do + click_button "Support" + + expect(page).to have_content "You have already supported this investment project." + end + + visit budget_path(budget) + + expect(page).to have_content "So far you've supported 1 project." + + visit budget_investment_path(budget, investment) + + within("#budget_investment_#{investment.id}_votes") do + click_button "Remove your support" + + expect(page).to have_content "No supports" + end + + visit budget_path(budget) + + expect(page).to have_content "So far you've supported 0 projects." + end end context "In Drafting phase" do diff --git a/spec/system/budgets/investments_spec.rb b/spec/system/budgets/investments_spec.rb index 2b1d3ae48..ea9075c93 100644 --- a/spec/system/budgets/investments_spec.rb +++ b/spec/system/budgets/investments_spec.rb @@ -1177,6 +1177,48 @@ describe "Budget Investments" do expect(page).to have_content "SUPPORTS" end end + + scenario "Remove a support from show view" do + investment = create(:budget_investment, budget: budget) + + login_as(author) + visit budget_investment_path(budget, investment) + + within("aside") do + expect(page).to have_content "No supports" + + click_button "Support" + + expect(page).to have_content "1 support" + expect(page).to have_content "You have already supported this investment project." + + click_button "Remove your support" + + expect(page).to have_content "No supports" + expect(page).to have_button "Support" + end + end + + scenario "Remove a support from index view" do + investment = create(:budget_investment, budget: budget) + + login_as(author) + visit budget_investments_path(budget) + + within("#budget_investment_#{investment.id}") do + expect(page).to have_content "No supports" + + click_button "Support" + + expect(page).to have_content "1 support" + expect(page).to have_content "You have already supported this investment project." + + click_button "Remove your support" + + expect(page).to have_content "No supports" + expect(page).to have_button "Support" + end + end end context "Evaluating Phase" do diff --git a/spec/system/management/budget_investments_spec.rb b/spec/system/management/budget_investments_spec.rb index 382ff4a63..b814066e8 100644 --- a/spec/system/management/budget_investments_spec.rb +++ b/spec/system/management/budget_investments_spec.rb @@ -333,6 +333,56 @@ describe "Budget Investments" do expect(page).to have_content "CONSUL\nMANAGEMENT" end + scenario "Remove support on behalf of someone else in index view" do + create(:budget_investment, heading: heading) + + login_managed_user(user) + login_as_manager(manager) + + visit management_budget_investments_path(budget) + click_button "Support" + + expect(page).to have_content "1 support" + expect(page).to have_content "You have already supported this investment project. Share it!" + expect(page).not_to have_button "Support" + + click_button "Remove your support" + + expect(page).to have_content "No supports" + expect(page).to have_button "Support" + expect(page).not_to have_button "Remove your support" + end + + scenario "Remove support on behalf of someone else in show view" do + create(:budget_investment, heading: heading, title: "Don't support me!") + + login_managed_user(user) + login_as_manager(manager) + + visit management_budget_investments_path(budget) + click_link "Don't support me!" + + expect(page).to have_css "h1", exact_text: "Don't support me!" + + click_button "Support" + + expect(page).to have_content "1 support" + expect(page).to have_content "You have already supported this investment project. Share it!" + expect(page).not_to have_button "Support" + + click_button "Remove your support" + + expect(page).to have_content "No supports" + expect(page).to have_button "Support" + expect(page).not_to have_button "Remove your support" + + refresh + + expect(page).to have_content "No supports" + expect(page).to have_button "Support" + expect(page).not_to have_button "Remove your support" + end + scenario "Should not allow unverified users to vote proposals" do login_managed_user(create(:user)) create(:budget_investment, budget: budget)