From 66ecb2835bced218eef8615aa633473b8de223eb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Wed, 20 Mar 2019 20:36:42 +0100
Subject: [PATCH] Show only available budget stats phases

This way we can show statistics for the supports phase before the vote
phase is over.
---
 app/assets/stylesheets/stats.scss     |  4 +-
 app/models/abilities/administrator.rb |  1 -
 app/models/abilities/everyone.rb      |  2 +-
 app/models/budget/stats.rb            | 50 ++++++++++++++++--
 app/views/budgets/stats/show.html.erb | 36 ++++++-------
 config/i18n-tasks.yml                 |  1 +
 config/locales/en/stats.yml           |  8 +--
 config/locales/es/stats.yml           |  8 +--
 spec/features/budgets/stats_spec.rb   | 17 ++-----
 spec/models/budget/stats_spec.rb      | 73 ++++++++++++++++++++++++++-
 10 files changed, 150 insertions(+), 50 deletions(-)

diff --git a/app/assets/stylesheets/stats.scss b/app/assets/stylesheets/stats.scss
index be084755b..61b86e1dd 100644
--- a/app/assets/stylesheets/stats.scss
+++ b/app/assets/stylesheets/stats.scss
@@ -190,12 +190,10 @@ table {
 
     tr th {
       border: 1px solid $border;
-      padding-left: 0;
-      padding-right: 0;
       text-align: center;
     }
 
-    .tiny {
+    .phase-subheader {
       font-size: rem-calc(11);
       font-weight: normal;
     }
diff --git a/app/models/abilities/administrator.rb b/app/models/abilities/administrator.rb
index 9f8c7e965..a72997dbf 100644
--- a/app/models/abilities/administrator.rb
+++ b/app/models/abilities/administrator.rb
@@ -66,7 +66,6 @@ module Abilities
       can [:hide, :update, :toggle_selection], Budget::Investment
       can [:valuate, :comment_valuation], Budget::Investment
       can :create, Budget::ValuatorAssignment
-      can :read_stats, Budget, phase: "reviewing_ballots"
 
       can [:search, :edit, :update, :create, :index, :destroy], Banner
 
diff --git a/app/models/abilities/everyone.rb b/app/models/abilities/everyone.rb
index 626be1c0a..46dc19585 100644
--- a/app/models/abilities/everyone.rb
+++ b/app/models/abilities/everyone.rb
@@ -21,7 +21,7 @@ module Abilities
       can [:read], Budget::Group
       can [:read, :print, :json_data], Budget::Investment
       can [:read_results, :read_executions], Budget, phase: "finished"
-      can :read_stats, Budget, phase: "finished"
+      can(:read_stats, Budget) { |budget| budget.valuating_or_later? }
       can :new, DirectMessage
       can [:read, :debate, :draft_publication, :allegations, :result_publication,
            :proposals, :milestones], Legislation::Process, published: true
diff --git a/app/models/budget/stats.rb b/app/models/budget/stats.rb
index 536c99b12..f627c59e7 100644
--- a/app/models/budget/stats.rb
+++ b/app/models/budget/stats.rb
@@ -3,14 +3,42 @@ class Budget::Stats
   alias_method :budget, :resource
 
   def self.stats_methods
-    super +
-      %i[total_participants_support_phase total_participants_vote_phase
-         total_budget_investments total_votes total_selected_investments
-         total_unfeasible_investments headings]
+    super + support_phase_methods + vote_phase_methods
+  end
+
+  def self.support_phase_methods
+    %i[total_participants_support_phase total_budget_investments
+       total_selected_investments total_unfeasible_investments headings]
+  end
+
+  def self.vote_phase_methods
+    %i[total_votes total_participants_vote_phase]
+  end
+
+  def stats_methods
+    base_stats_methods + participation_methods + phase_methods
+  end
+
+  def phases
+    %w[support vote].select { |phase| send("#{phase}_phase_finished?") }
+  end
+
+  def all_phases
+    return phases unless phases.many?
+
+    [*phases, "every"]
+  end
+
+  def support_phase_finished?
+    budget.valuating_or_later?
+  end
+
+  def vote_phase_finished?
+    budget.finished?
   end
 
   def participants
-    User.where(id: (authors + voters + balloters + poll_ballot_voters).uniq.compact)
+    User.where(id: phases.map { |phase| send("participant_ids_#{phase}_phase") }.flatten.uniq)
   end
 
   def total_participants
@@ -66,6 +94,18 @@ class Budget::Stats
 
   private
 
+    def phase_methods
+      phases.map { |phase| self.class.send("#{phase}_phase_methods") }.flatten
+    end
+
+    def participant_ids_support_phase
+      (authors + voters).uniq
+    end
+
+    def participant_ids_vote_phase
+      (balloters + poll_ballot_voters).uniq
+    end
+
     def authors
       budget.investments.pluck(:author_id)
     end
diff --git a/app/views/budgets/stats/show.html.erb b/app/views/budgets/stats/show.html.erb
index b944fe893..65430a026 100644
--- a/app/views/budgets/stats/show.html.erb
+++ b/app/views/budgets/stats/show.html.erb
@@ -80,11 +80,12 @@
         <div id="stats_by_phase" class="stats-group">
           <h4><%= t("stats.budgets.by_phase") %></h4>
 
-          <%= number_with_info_tags(@stats.total_participants_support_phase,
-                                    t("stats.budgets.participants_support_phase")) %>
-          <%= number_with_info_tags(@stats.total_participants_vote_phase,
-                                   t("stats.budgets.participants_voting_phase")) %>
-
+          <% @stats.phases.each do |phase| %>
+            <%= number_with_info_tags(
+              @stats.send("total_participants_#{phase}_phase"),
+              t("stats.budgets.participants_#{phase}_phase")
+            ) %>
+          <% end %>
         </div>
 
         <div id="stats_by_heading" class="stats-group">
@@ -95,20 +96,19 @@
               <tr>
                 <th scope="col" rowspan="2"><%= t("stats.budgets.heading") %></th>
                 <th scope="col" rowspan="2"><%= t("stats.budgets.investments_sent_html") %></th>
-                <th scope="col" colspan="3"><%= t("stats.budgets.participants_support_phase") %></th>
-                <th scope="col" colspan="3"><%= t("stats.budgets.participants_voting_phase") %></th>
-                <th scope="col" colspan="3"><%= t("stats.budgets.participants_total") %></th>
+
+                <% @stats.all_phases.each do |phase| %>
+                  <th scope="col" colspan="3">
+                    <%= t("stats.budgets.participants_#{phase}_phase") %>
+                  </th>
+                <% end %>
               </tr>
               <tr>
-                <th scope="col" class="tiny"><%= t("stats.budgets.total") %></th>
-                <th scope="col" class="tiny"><%= t("stats.budgets.percent_total_participants_html") %></th>
-                <th scope="col" class="tiny"><%= t("stats.budgets.percent_heading_census_html") %></th>
-                <th scope="col" class="tiny"><%= t("stats.budgets.total") %></th>
-                <th scope="col" class="tiny"><%= t("stats.budgets.percent_total_participants_html") %></th>
-                <th scope="col" class="tiny"><%= t("stats.budgets.percent_heading_census_html") %></th>
-                <th scope="col" class="tiny"><%= t("stats.budgets.total") %></th>
-                <th scope="col" class="tiny"><%= t("stats.budgets.percent_total_participants_html") %></th>
-                <th scope="col" class="tiny"><%= t("stats.budgets.percent_heading_census_html") %></th>
+                <% @stats.all_phases.each do %>
+                  <th scope="col" class="phase-subheader"><%= t("stats.budgets.total") %></th>
+                  <th scope="col" class="phase-subheader"><%= t("stats.budgets.percent_total_participants") %></th>
+                  <th scope="col" class="phase-subheader"><%= t("stats.budgets.percent_heading_census") %></th>
+                <% end %>
               </tr>
             </thead>
             <tbody id="headings">
@@ -122,7 +122,7 @@
                     <%= @stats.headings[heading.id][:total_investments_count] %>
                   </td>
 
-                  <% ["support", "vote", "every"].each do |phase| %>
+                  <% @stats.all_phases.each do |phase| %>
                     <td id="total_participants_<%= phase %>_phase_heading_<%= heading.id %>"
                         class="border-left text-center">
                       <%= @stats.headings[heading.id]["total_participants_#{phase}_phase".to_sym] %>
diff --git a/config/i18n-tasks.yml b/config/i18n-tasks.yml
index 35d06d172..736f12c0d 100644
--- a/config/i18n-tasks.yml
+++ b/config/i18n-tasks.yml
@@ -194,6 +194,7 @@ ignore_unused:
   - "admin.site_customization.pages.page.status_*"
   - "admin.legislation.processes.process.*"
   - "legislation.processes.index.*"
+  - "stats.budgets.participants_*_phase"
   - "votes.budget_investments.different_heading_assigned*"
   - "*.form.map_skip_checkbox"
 # - "{devise,kaminari,will_paginate}.*"
diff --git a/config/locales/en/stats.yml b/config/locales/en/stats.yml
index cd6eae23f..3e38d765b 100644
--- a/config/locales/en/stats.yml
+++ b/config/locales/en/stats.yml
@@ -29,10 +29,10 @@ en:
       heading: "Heading"
       investments_sent_html: "Investment proposals sent"
       participants_support_phase: "Participants support phase"
-      participants_voting_phase: "Participants voting phase"
-      participants_total: "Total Participants"
-      percent_total_participants_html: "% <br>Total<br>Participants"
-      percent_heading_census_html: "% <br>Heading<br>Census"
+      participants_vote_phase: "Participants voting phase"
+      participants_every_phase: "Total Participants"
+      percent_total_participants: "% Total Participants"
+      percent_heading_census: "% Heading Census"
       participatory_disclaimer: "** The numbers of total participants refer to persons that created, supported or voted investment proposals."
       heading_disclaimer: "*** Data about headings refer to the heading where each user voted, not necessarily the one that person is registered on."
     polls:
diff --git a/config/locales/es/stats.yml b/config/locales/es/stats.yml
index 0425c7ec8..82a4ee801 100644
--- a/config/locales/es/stats.yml
+++ b/config/locales/es/stats.yml
@@ -29,10 +29,10 @@ es:
       heading: "Distrito"
       investments_sent_html: "Propuestas<br>enviadas"
       participants_support_phase: Participantes fase apoyos
-      participants_voting_phase: Participantes fase votación
-      participants_total: Total de participantes
-      percent_total_participants_html: "% <br>Total<br>Participantes"
-      percent_heading_census_html: "% <br>Censo<br>Distrito"
+      participants_vote_phase: Participantes fase votación
+      participants_every_phase: Total de participantes
+      percent_total_participants: "% Total Participantes"
+      percent_heading_census: "% Censo Distrito"
       participatory_disclaimer: "** Las cifras de total de participantes se refieren a personas que han creado, apoyado o votado propuestas."
       heading_disclaimer: "*** Los datos de distrito se refieren al distrito en el que el usuario ha votado, no necesariamente en el que está empadronado."
     polls:
diff --git a/spec/features/budgets/stats_spec.rb b/spec/features/budgets/stats_spec.rb
index ce1982cdf..6318b1cea 100644
--- a/spec/features/budgets/stats_spec.rb
+++ b/spec/features/budgets/stats_spec.rb
@@ -8,25 +8,16 @@ feature "Stats" do
 
   describe "Show" do
 
-    it "is not accessible to normal users if phase is not 'finished'" do
-      budget.update(phase: "reviewing_ballots")
+    it "is not accessible if supports phase is not finished" do
+      budget.update(phase: "selecting")
 
       visit budget_stats_path(budget.id)
       expect(page).to have_content "You do not have permission to carry out the action "\
                                    "'read_stats' on budget."
     end
 
-    it "is accessible to normal users if phase is 'finished'" do
-      budget.update(phase: "finished")
-
-      visit budget_stats_path(budget.id)
-      expect(page).to have_content "Stats"
-    end
-
-    it "is accessible to administrators when budget has phase 'reviewing_ballots'" do
-      budget.update(phase: "reviewing_ballots")
-
-      login_as(create(:administrator).user)
+    it "is accessible if supports phase is finished" do
+      budget.update(phase: "valuating")
 
       visit budget_stats_path(budget.id)
       expect(page).to have_content "Stats"
diff --git a/spec/models/budget/stats_spec.rb b/spec/models/budget/stats_spec.rb
index 3e293a410..2a154f448 100644
--- a/spec/models/budget/stats_spec.rb
+++ b/spec/models/budget/stats_spec.rb
@@ -1,7 +1,7 @@
 require "rails_helper"
 
 describe Budget::Stats do
-  let(:budget) { create(:budget) }
+  let(:budget) { create(:budget, :finished) }
   let(:stats) { Budget::Stats.new(budget) }
   let(:investment) { create(:budget_investment, :selected, budget: budget) }
 
@@ -213,4 +213,75 @@ describe Budget::Stats do
       expect(heading_stats[:percentage_district_population_every_phase]).to be 0.243
     end
   end
+
+  describe "#support_phase_finished?" do
+    context "support phase isn't finished" do
+      before { budget.phase = "selecting" }
+
+      it "is false" do
+        expect(stats.support_phase_finished?).to be false
+      end
+    end
+
+    context "support phase is finished" do
+      before { budget.phase = "valuating" }
+
+      it "is false" do
+        expect(stats.support_phase_finished?).to be true
+      end
+    end
+  end
+
+  describe "#vote_phase_finished" do
+    context "support phase isn't finished" do
+      before { budget.phase = "reviewing_ballots" }
+
+      it "is false" do
+        expect(stats.vote_phase_finished?).to be false
+      end
+    end
+
+    context "vote phase is finished" do
+      before { budget.phase = "finished" }
+
+      it "is false" do
+        expect(stats.vote_phase_finished?).to be true
+      end
+    end
+  end
+
+  describe "#all_phases" do
+    context "no phases are finished" do
+      before do
+        allow(stats).to receive(:support_phase_finished?).and_return(false)
+        allow(stats).to receive(:vote_phase_finished?).and_return(false)
+      end
+
+      it "returns an empty array" do
+        expect(stats.all_phases).to eq []
+      end
+    end
+
+    context "one phase is finished" do
+      before do
+        allow(stats).to receive(:support_phase_finished?).and_return(true)
+        allow(stats).to receive(:vote_phase_finished?).and_return(false)
+      end
+
+      it "returns the finished phase" do
+        expect(stats.all_phases).to eq ["support"]
+      end
+    end
+
+    context "all phases are finished" do
+      before do
+        allow(stats).to receive(:support_phase_finished?).and_return(true)
+        allow(stats).to receive(:vote_phase_finished?).and_return(true)
+      end
+
+      it "returns the finished phases and a total phase" do
+        expect(stats.all_phases).to eq ["support", "vote", "every"]
+      end
+    end
+  end
 end