From 0e9190bc4338cc30e079b22000295e5cd56b9834 Mon Sep 17 00:00:00 2001
From: decabeza <alberto@decabeza.es>
Date: Mon, 5 Mar 2018 14:02:38 +0100
Subject: [PATCH 1/3] Adds heading link on different heading message

---
 app/models/budget/investment.rb               | 10 +++---
 .../budgets/investments/_ballot.html.erb      | 34 +++++++++++++------
 .../budgets/investments/_header.html.erb      |  2 +-
 config/locales/en/budgets.yml                 |  2 +-
 config/locales/es/budgets.yml                 |  2 +-
 spec/features/budgets/ballots_spec.rb         |  4 +--
 spec/models/budget/investment_spec.rb         |  2 +-
 7 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/app/models/budget/investment.rb b/app/models/budget/investment.rb
index c5d4d775d..dff2dd2e0 100644
--- a/app/models/budget/investment.rb
+++ b/app/models/budget/investment.rb
@@ -208,11 +208,11 @@ class Budget
     end
 
     def reason_for_not_being_ballotable_by(user, ballot)
-      return permission_problem(user)    if permission_problem?(user)
-      return :not_selected               unless selected?
-      return :no_ballots_allowed         unless budget.balloting?
-      return :different_heading_assigned unless ballot.valid_heading?(heading)
-      return :not_enough_money_html      if ballot.present? && !enough_money?(ballot)
+      return permission_problem(user)         if permission_problem?(user)
+      return :not_selected                    unless selected?
+      return :no_ballots_allowed              unless budget.balloting?
+      return :different_heading_assigned_html unless ballot.valid_heading?(heading)
+      return :not_enough_money_html           if ballot.present? && !enough_money?(ballot)
     end
 
     def permission_problem(user)
diff --git a/app/views/budgets/investments/_ballot.html.erb b/app/views/budgets/investments/_ballot.html.erb
index 605caebc7..f65ec6c74 100644
--- a/app/views/budgets/investments/_ballot.html.erb
+++ b/app/views/budgets/investments/_ballot.html.erb
@@ -42,17 +42,31 @@
 
   <% if reason.present? && !ballot.has_investment?(investment) %>
 
-    <div class="js-participation-not-allowed participation-not-allowed" style='display:none'>
+    <div class="js-participation-not-allowed participation-not-allowed" style="display:none">
+
+      <% verify_account = link_to(t("votes.verify_account"), verification_path) %>
+      <% signin = link_to(t("votes.signin"), new_user_session_path) %>
+      <% signup = link_to(t("votes.signup"), new_user_registration_path) %>
+      <% 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))%>
+
+      <% if @assigned_heading.present? %>
+        <% heading_link = link_to(@assigned_heading.name,
+                                  budget_investments_path(@budget,
+                                  heading_id: @assigned_heading.try(:id))) %>
+      <% end %>
+
       <p>
-        <%= t("budgets.ballots.reasons_for_not_balloting.#{reason}",
-              verify_account: link_to(t("votes.verify_account"), verification_path),
-              signin: link_to(t("votes.signin"), new_user_session_path),
-              signup: link_to(t("votes.signup"), new_user_registration_path),
-              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))).html_safe %>
+        <small>
+          <%= t("budgets.ballots.reasons_for_not_balloting.#{reason}",
+                verify_account: verify_account, signin: signin,
+                signup: signup, my_heading: my_heading,
+                change_ballot: change_ballot,
+                heading_link: heading_link).html_safe %>
+        </small>
       </p>
     </div>
 
diff --git a/app/views/budgets/investments/_header.html.erb b/app/views/budgets/investments/_header.html.erb
index c664abc01..d9f21523a 100644
--- a/app/views/budgets/investments/_header.html.erb
+++ b/app/views/budgets/investments/_header.html.erb
@@ -35,7 +35,7 @@
                 <%= t("budgets.investments.index.by_heading", heading: @heading.name) %>
               </h2>
               <div class="small-12 medium-9">
-                <div class="callout warning">
+                <div class="callout warning margin-top">
                   <%= t("budgets.investments.header.different_heading_assigned_html",
                         heading_link: link_to(
                           @assigned_heading.name,
diff --git a/config/locales/en/budgets.yml b/config/locales/en/budgets.yml
index b224cef34..b0b393b42 100644
--- a/config/locales/en/budgets.yml
+++ b/config/locales/en/budgets.yml
@@ -19,7 +19,7 @@ en:
         not_selected: Unselected investment projects can not be supported
         not_enough_money_html: "You have already assigned the available budget.<br><small>Remember you can %{change_ballot} at any time</small>"
         no_ballots_allowed: Selecting phase is closed
-        different_heading_assigned: You have already voted a different heading
+        different_heading_assigned_html: "You have already voted a different heading: %{heading_link}"
         change_ballot: change your votes
     groups:
       show:
diff --git a/config/locales/es/budgets.yml b/config/locales/es/budgets.yml
index 8686f98b0..ab1a1e4ac 100644
--- a/config/locales/es/budgets.yml
+++ b/config/locales/es/budgets.yml
@@ -19,7 +19,7 @@ es:
         not_selected: No se pueden votar proyectos inviables.
         not_enough_money_html: "Ya has asignado el presupuesto disponible.<br><small>Recuerda que puedes %{change_ballot} en cualquier momento</small>"
         no_ballots_allowed: El periodo de votación está cerrado.
-        different_heading_assigned: Ya votaste en una sección distinta del presupuesto.
+        different_heading_assigned_html: "Ya has votado proyectos de otra partida: %{heading_link}"
         change_ballot: cambiar tus votos
     groups:
       show:
diff --git a/spec/features/budgets/ballots_spec.rb b/spec/features/budgets/ballots_spec.rb
index 6a4be8696..4790de669 100644
--- a/spec/features/budgets/ballots_spec.rb
+++ b/spec/features/budgets/ballots_spec.rb
@@ -226,7 +226,7 @@ feature 'Ballots' do
         click_link "Districts"
         click_link "District 2"
 
-        expect(page).to have_content("You have active votes in another heading")
+        expect(page).to have_content("You have active votes in another heading: District 1")
       end
     end
 
@@ -296,7 +296,7 @@ feature 'Ballots' do
       visit budget_investments_path(budget, heading_id: new_york.id)
 
       expect(page).not_to have_css "#progressbar"
-      expect(page).to have_content "You have active votes in another heading:"
+      expect(page).to have_content "You have active votes in another heading: California"
       expect(page).to have_link california.name, href: budget_investments_path(budget, heading_id: california.id)
     end
 
diff --git a/spec/models/budget/investment_spec.rb b/spec/models/budget/investment_spec.rb
index 4c8ade0f5..b8b3d423b 100644
--- a/spec/models/budget/investment_spec.rb
+++ b/spec/models/budget/investment_spec.rb
@@ -756,7 +756,7 @@ describe Budget::Investment do
           ballot = create(:budget_ballot, user: user, budget: budget)
           ballot.investments << inv1
 
-          expect(inv2.reason_for_not_being_ballotable_by(user, ballot)).to eq(:different_heading_assigned)
+          expect(inv2.reason_for_not_being_ballotable_by(user, ballot)).to eq(:different_heading_assigned_html)
         end
 
         it "rejects proposals with price higher than current available money" do

From d70c46e578c210701ba30568ad1ee3d9ed7cb929 Mon Sep 17 00:00:00 2001
From: decabeza <alberto@decabeza.es>
Date: Mon, 5 Mar 2018 19:01:11 +0100
Subject: [PATCH 2/3] Extract BudgetHeadingsHelper#heading_link & use it

Why:

The logic to construct the link to a heading (if it exists) is in three
different places, this is a clear candidate for a helper method.

How:

Just checking at the helper method if `assigned_heading` and `budget`
has values and composing the link if so.
---
 app/helpers/budget_headings_helper.rb           | 6 ++++++
 app/views/budgets/investments/_ballot.html.erb  | 8 +-------
 app/views/budgets/investments/_header.html.erb  | 5 +----
 app/views/budgets/investments/_sidebar.html.erb | 4 +---
 4 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/app/helpers/budget_headings_helper.rb b/app/helpers/budget_headings_helper.rb
index 3fa1eac89..22eabfe11 100644
--- a/app/helpers/budget_headings_helper.rb
+++ b/app/helpers/budget_headings_helper.rb
@@ -6,4 +6,10 @@ module BudgetHeadingsHelper
     end
   end
 
+  def heading_link(assigned_heading = nil, budget = nil)
+    return nil unless assigned_heading && budget
+    heading_path = budget_investments_path(budget, heading_id: assigned_heading.try(:id))
+    link_to(assigned_heading.name, heading_path)
+  end
+
 end
diff --git a/app/views/budgets/investments/_ballot.html.erb b/app/views/budgets/investments/_ballot.html.erb
index f65ec6c74..c12103a0a 100644
--- a/app/views/budgets/investments/_ballot.html.erb
+++ b/app/views/budgets/investments/_ballot.html.erb
@@ -53,19 +53,13 @@
       <% change_ballot = link_to(t("budgets.ballots.reasons_for_not_balloting.change_ballot"),
                                           budget_ballot_path(@budget))%>
 
-      <% if @assigned_heading.present? %>
-        <% heading_link = link_to(@assigned_heading.name,
-                                  budget_investments_path(@budget,
-                                  heading_id: @assigned_heading.try(:id))) %>
-      <% end %>
-
       <p>
         <small>
           <%= t("budgets.ballots.reasons_for_not_balloting.#{reason}",
                 verify_account: verify_account, signin: signin,
                 signup: signup, my_heading: my_heading,
                 change_ballot: change_ballot,
-                heading_link: heading_link).html_safe %>
+                heading_link: heading_link(@assigned_heading, @budget)).html_safe %>
         </small>
       </p>
     </div>
diff --git a/app/views/budgets/investments/_header.html.erb b/app/views/budgets/investments/_header.html.erb
index d9f21523a..90d2edfd4 100644
--- a/app/views/budgets/investments/_header.html.erb
+++ b/app/views/budgets/investments/_header.html.erb
@@ -37,10 +37,7 @@
               <div class="small-12 medium-9">
                 <div class="callout warning margin-top">
                   <%= t("budgets.investments.header.different_heading_assigned_html",
-                        heading_link: link_to(
-                          @assigned_heading.name,
-                          budget_investments_path(@budget, heading_id: @assigned_heading.try(:id)))
-                       ) %>
+                        heading_link: heading_link(@assigned_heading, @budget)) %>
                   <br>
                   <small>
                     <%= t("budgets.investments.header.change_ballot",
diff --git a/app/views/budgets/investments/_sidebar.html.erb b/app/views/budgets/investments/_sidebar.html.erb
index 4ae9104d5..c020c8bbe 100644
--- a/app/views/budgets/investments/_sidebar.html.erb
+++ b/app/views/budgets/investments/_sidebar.html.erb
@@ -43,9 +43,7 @@
   <% elsif @assigned_heading.present? %>
     <p>
       <%= t("budgets.investments.index.sidebar.different_heading_assigned_html",
-              heading_link: link_to(
-                @assigned_heading.name,
-                budget_investments_path(@budget, heading_id: @assigned_heading.try(:id)))
+              heading_link: heading_link(@assigned_heading, @budget)
             ) %>
       <br>
       <small>

From 0bc0a2b1e8f233efc733ccdf914f4af02bdc75fc Mon Sep 17 00:00:00 2001
From: decabeza <alberto@decabeza.es>
Date: Wed, 7 Mar 2018 16:56:32 +0100
Subject: [PATCH 3/3] Makes smaller text on participation not allowed div

---
 app/views/budgets/investments/_votes.html.erb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/views/budgets/investments/_votes.html.erb b/app/views/budgets/investments/_votes.html.erb
index c2870d76c..d46be6ae8 100644
--- a/app/views/budgets/investments/_votes.html.erb
+++ b/app/views/budgets/investments/_votes.html.erb
@@ -29,11 +29,13 @@
   <% if reason.present? && !user_voted_for %>
     <div class="js-participation-not-allowed participation-not-allowed" style='display:none' aria-hidden="false">
       <p>
+        <small>
           <%= t("votes.budget_investments.#{reason}",
                 verify_account: link_to(t("votes.verify_account"), verification_path),
                 signin: link_to(t("votes.signin"), new_user_session_path),
                 signup: link_to(t("votes.signup"), new_user_registration_path)
            ).html_safe %>
+        </small>
       </p>
     </div>
   <% end %>