From 4e3b3f2028998ae6404730392997c9746d33fb0b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Wed, 29 Sep 2021 04:06:51 +0200
Subject: [PATCH] Extract methods in votes components

This way we can make the view code a bit easier to read.

We're also changing the order of the conditions a little bit so we only
check for the presence of a current user once.

To make sure we aren't breaking anything with these changes, we're
adding some tests. We're also replacing one system test checking content
with a component test, since component tests are much faster.
---
 .../investments/ballot_component.html.erb     | 15 +---
 .../budgets/investments/ballot_component.rb   | 19 +++++
 .../debates/votes_component.html.erb          | 13 ++--
 app/components/debates/votes_component.rb     | 14 ++++
 .../proposals/votes_component.html.erb        | 13 ++--
 .../legislation/proposals/votes_component.rb  | 14 ++++
 .../proposals/votes_component.html.erb        | 18 ++---
 app/components/proposals/votes_component.rb   | 14 ++++
 .../proposals/votes_component_spec.rb         | 77 +++++++++++++++++++
 spec/system/votes_spec.rb                     | 22 ------
 10 files changed, 163 insertions(+), 56 deletions(-)
 create mode 100644 spec/components/proposals/votes_component_spec.rb

diff --git a/app/components/budgets/investments/ballot_component.html.erb b/app/components/budgets/investments/ballot_component.html.erb
index 189236531..0bf6ee56a 100644
--- a/app/components/budgets/investments/ballot_component.html.erb
+++ b/app/components/budgets/investments/ballot_component.html.erb
@@ -1,6 +1,5 @@
-<% reason = investment.reason_for_not_being_ballotable_by(current_user, ballot) %>
 <div class="js-participation supports ballot">
-  <% if ballot.has_investment?(investment) %>
+  <% if voted? %>
     <div class="remove supported">
       <span class="icon-check-circle"
             title="<%= t("budgets.investments.investment.already_added") %>">
@@ -36,20 +35,14 @@
     </div>
   <% end %>
 
-  <% if reason.present? && !ballot.has_investment?(investment) %>
+  <% if reason.present? && !voted? %>
     <div class="js-participation-not-allowed participation-not-allowed" style="display:none">
-      <% my_heading = link_to(investment.heading.name,
-                      budget_investments_path(budget_id: investment.budget_id,
-                                              heading_id: investment.heading_id)) %>
-      <% change_ballot = link_to(t("budgets.ballots.reasons_for_not_balloting.change_ballot"),
-                                          budget_ballot_path(budget)) %>
-
       <p>
         <small>
           <%= sanitize(t("budgets.ballots.reasons_for_not_balloting.#{reason}",
                 verify_account: link_to_verify_account, signin: link_to_signin,
-                signup: link_to_signup, my_heading: my_heading,
-                change_ballot: change_ballot,
+                signup: link_to_signup, my_heading: link_to_my_heading,
+                change_ballot: link_to_change_ballot,
                 heading_link: heading_link(assigned_heading, budget))) %>
         </small>
       </p>
diff --git a/app/components/budgets/investments/ballot_component.rb b/app/components/budgets/investments/ballot_component.rb
index 16c82dc4e..016e7f4f4 100644
--- a/app/components/budgets/investments/ballot_component.rb
+++ b/app/components/budgets/investments/ballot_component.rb
@@ -15,4 +15,23 @@ class Budgets::Investments::BallotComponent < ApplicationComponent
     def budget
       ballot.budget
     end
+
+    def voted?
+      ballot.has_investment?(investment)
+    end
+
+    def reason
+      @reason ||= investment.reason_for_not_being_ballotable_by(current_user, ballot)
+    end
+
+    def link_to_my_heading
+      link_to(investment.heading.name,
+              budget_investments_path(budget_id: investment.budget_id,
+                                      heading_id: investment.heading_id))
+    end
+
+    def link_to_change_ballot
+      link_to(t("budgets.ballots.reasons_for_not_balloting.change_ballot"),
+              budget_ballot_path(budget))
+    end
 end
diff --git a/app/components/debates/votes_component.html.erb b/app/components/debates/votes_component.html.erb
index a756f579e..ce933fc54 100644
--- a/app/components/debates/votes_component.html.erb
+++ b/app/components/debates/votes_component.html.erb
@@ -1,4 +1,3 @@
-<% voted_classes = css_classes_for_vote(debate) %>
 <div class="votes">
   <div class="in-favor inline-block">
     <% if current_user %>
@@ -43,21 +42,21 @@
     <%= t("debates.debate.votes", count: debate.votes_score) %>
   </span>
 
-  <% if current_user&.organization? %>
+  <% if !current_user %>
+    <div tabindex="0">
+      <%= render "shared/login_to_vote" %>
+    </div>
+  <% elsif organization? %>
     <div class="participation-not-allowed" style="display:none" aria-hidden="false">
       <p>
         <%= t("votes.organizations") %>
       </p>
     </div>
-  <% elsif current_user && !debate.votable_by?(current_user) %>
+  <% elsif !can_vote? %>
     <div class="participation-not-allowed" style="display:none" aria-hidden="false">
       <p>
         <%= sanitize(t("votes.anonymous", verify_account: link_to_verify_account)) %>
       </p>
     </div>
-  <% elsif !current_user %>
-    <div tabindex="0">
-      <%= render "shared/login_to_vote" %>
-    </div>
   <% end %>
 </div>
diff --git a/app/components/debates/votes_component.rb b/app/components/debates/votes_component.rb
index 80fedc9ba..4f7ed8e6d 100644
--- a/app/components/debates/votes_component.rb
+++ b/app/components/debates/votes_component.rb
@@ -5,4 +5,18 @@ class Debates::VotesComponent < ApplicationComponent
   def initialize(debate)
     @debate = debate
   end
+
+  private
+
+    def voted_classes
+      @voted_classes ||= css_classes_for_vote(debate)
+    end
+
+    def can_vote?
+      debate.votable_by?(current_user)
+    end
+
+    def organization?
+      current_user&.organization?
+    end
 end
diff --git a/app/components/legislation/proposals/votes_component.html.erb b/app/components/legislation/proposals/votes_component.html.erb
index f3715f41f..078786754 100644
--- a/app/components/legislation/proposals/votes_component.html.erb
+++ b/app/components/legislation/proposals/votes_component.html.erb
@@ -1,4 +1,3 @@
-<% voted_classes = css_classes_for_vote(proposal) %>
 <div class="votes">
   <% if proposal.process.proposals_phase.open? %>
     <div class="in-favor inline-block">
@@ -45,23 +44,23 @@
     <%= t("proposals.proposal.votes", count: proposal.votes_score) %>
   </span>
 
-  <% if current_user&.organization? %>
+  <% if !current_user %>
+    <div tabindex="0">
+      <%= render "shared/login_to_vote" %>
+    </div>
+  <% elsif organization? %>
     <div class="participation-not-allowed" style="display:none" aria-hidden="false">
       <p>
         <%= t("votes.organizations") %>
       </p>
     </div>
-  <% elsif current_user && !proposal.votable_by?(current_user) %>
+  <% elsif !can_vote? %>
     <div class="participation-not-allowed" style="display:none" aria-hidden="false">
       <p>
         <%= sanitize(t("legislation.proposals.not_verified",
             verify_account: link_to_verify_account)) %>
       </p>
     </div>
-  <% elsif !current_user %>
-    <div tabindex="0">
-      <%= render "shared/login_to_vote" %>
-    </div>
   <% end %>
 
   <% if current_user&.voted_as_when_voted_for(proposal) && setting["twitter_handle"] %>
diff --git a/app/components/legislation/proposals/votes_component.rb b/app/components/legislation/proposals/votes_component.rb
index f59d11d68..fced9f731 100644
--- a/app/components/legislation/proposals/votes_component.rb
+++ b/app/components/legislation/proposals/votes_component.rb
@@ -5,4 +5,18 @@ class Legislation::Proposals::VotesComponent < ApplicationComponent
   def initialize(proposal)
     @proposal = proposal
   end
+
+  private
+
+    def voted_classes
+      @voted_classes ||= css_classes_for_vote(proposal)
+    end
+
+    def can_vote?
+      proposal.votable_by?(current_user)
+    end
+
+    def organization?
+      current_user&.organization?
+    end
 end
diff --git a/app/components/proposals/votes_component.html.erb b/app/components/proposals/votes_component.html.erb
index 5ca164d94..9a28371aa 100644
--- a/app/components/proposals/votes_component.html.erb
+++ b/app/components/proposals/votes_component.html.erb
@@ -2,11 +2,11 @@
   <%= render "proposals/supports", proposal: proposal %>
 
   <div class="in-favor">
-    <% if current_user&.voted_for?(proposal) %>
+    <% if voted? %>
       <div class="supported callout success">
         <%= t("proposals.proposal.already_supported") %>
       </div>
-    <% elsif current_user && proposal.votable_by?(current_user) %>
+    <% elsif can_vote? %>
       <%= link_to vote_url,
           class: "button button-support small expanded",
           title: t("proposals.proposal.support_title"), method: "post", remote: true do %>
@@ -19,13 +19,17 @@
     <% end %>
   </div>
 
-  <% if current_user&.organization? %>
+  <% if !current_user %>
+    <div tabindex="0">
+      <%= render "shared/login_to_vote" %>
+    </div>
+  <% elsif organization? %>
     <div class="participation-not-allowed" style="display:none" aria-hidden="false">
       <p>
         <%= t("votes.organizations") %>
       </p>
     </div>
-  <% elsif current_user && !proposal.votable_by?(current_user) %>
+  <% elsif !can_vote? %>
     <div tabindex="0">
       <div class="participation-not-allowed" style="display:none" aria-hidden="false">
         <p>
@@ -33,13 +37,9 @@
         </p>
       </div>
     </div>
-  <% elsif !current_user %>
-    <div tabindex="0">
-      <%= render "shared/login_to_vote" %>
-    </div>
   <% end %>
 
-  <% if current_user&.voted_for?(proposal) && setting["twitter_handle"] %>
+  <% if voted? && setting["twitter_handle"] %>
     <div class="share-supported">
       <%= render "proposals/social_share", proposal: proposal, share_title: false %>
     </div>
diff --git a/app/components/proposals/votes_component.rb b/app/components/proposals/votes_component.rb
index 143b32ada..bae9cf50c 100644
--- a/app/components/proposals/votes_component.rb
+++ b/app/components/proposals/votes_component.rb
@@ -10,4 +10,18 @@ class Proposals::VotesComponent < ApplicationComponent
   def vote_url
     @vote_url || vote_proposal_path(proposal, value: "yes")
   end
+
+  private
+
+    def voted?
+      current_user&.voted_for?(proposal)
+    end
+
+    def can_vote?
+      proposal.votable_by?(current_user)
+    end
+
+    def organization?
+      current_user&.organization?
+    end
 end
diff --git a/spec/components/proposals/votes_component_spec.rb b/spec/components/proposals/votes_component_spec.rb
new file mode 100644
index 000000000..b4837318d
--- /dev/null
+++ b/spec/components/proposals/votes_component_spec.rb
@@ -0,0 +1,77 @@
+require "rails_helper"
+
+describe Proposals::VotesComponent do
+  let(:proposal) { create(:proposal, title: "Create a monthly transport ticket") }
+  let(:component) { Proposals::VotesComponent.new(proposal) }
+
+  describe "support proposal link" do
+    it "is shown as plain text to unverified users" do
+      sign_in(create(:user))
+
+      render_inline component
+
+      expect(page).to have_content "Support"
+      expect(page).not_to have_link "Support"
+    end
+
+    it "is shown to verified users" do
+      sign_in(create(:user, :level_two))
+
+      render_inline component
+
+      expect(page).to have_link count: 1
+      expect(page).to have_link "Support", title: "Support this proposal"
+      expect(page).not_to have_content "You have already supported this proposal. Share it!"
+    end
+
+    it "is replaced with a success message when the proposal is already supported" do
+      sign_in(create(:user, :level_two, votables: [proposal]))
+
+      render_inline component
+
+      expect(page).to have_content "You have already supported this proposal. Share it!"
+      expect(page).not_to have_link "Support"
+    end
+  end
+
+  describe "participation not allowed" do
+    it "asks anonymous users to sign in or sign up" do
+      render_inline component
+
+      expect(page).to have_link "sign in", visible: :hidden
+      expect(page).to have_link "sign up", visible: :hidden
+    end
+
+    it "says voting is not allowed to organizations" do
+      sign_in(create(:organization, user: create(:user, :level_two)).user)
+
+      render_inline component
+
+      expect(page).to have_content "Organizations are not permitted to vote"
+      expect(page).not_to have_link "sign in", visible: :all
+      expect(page).not_to have_link "sign up", visible: :all
+    end
+
+    it "says only verified users can vote to unverified users" do
+      sign_in(create(:user))
+
+      render_inline component
+
+      expect(page).to have_content "Only verified users can vote on proposals"
+      expect(page).to have_link "verify your account", visible: :hidden
+      expect(page).not_to have_link "sign in", visible: :all
+      expect(page).not_to have_link "sign up", visible: :all
+    end
+
+    it "is not rendered for verified users" do
+      sign_in(create(:user, :level_two))
+
+      render_inline component
+
+      expect(page).not_to have_css ".participation-not-allowed"
+      expect(page).not_to have_link "verify your account", visible: :all
+      expect(page).not_to have_link "sign in", visible: :all
+      expect(page).not_to have_link "sign up", visible: :all
+    end
+  end
+end
diff --git a/spec/system/votes_spec.rb b/spec/system/votes_spec.rb
index fd78c4718..3dcfcbbb3 100644
--- a/spec/system/votes_spec.rb
+++ b/spec/system/votes_spec.rb
@@ -181,28 +181,6 @@ describe "Votes" do
   describe "Proposals" do
     before { login_as(verified) }
 
-    scenario "Index shows user votes on proposals" do
-      proposal1 = create(:proposal, voters: [verified])
-      proposal2 = create(:proposal)
-      proposal3 = create(:proposal)
-
-      visit proposals_path
-
-      within("#proposals") do
-        within("#proposal_#{proposal1.id}_votes") do
-          expect(page).to have_content "You have already supported this proposal. Share it!"
-        end
-
-        within("#proposal_#{proposal2.id}_votes") do
-          expect(page).not_to have_content "You have already supported this proposal. Share it!"
-        end
-
-        within("#proposal_#{proposal3.id}_votes") do
-          expect(page).not_to have_content "You have already supported this proposal. Share it!"
-        end
-      end
-    end
-
     describe "Single proposal" do
       let!(:proposal) { create(:proposal) }