Merge pull request #3839 from consul/generate_stats

Improve restrictions for poll stats

app/controllers/polls_controller.rb

@@ -13,7 +13,7 @@ class PollsController < ApplicationController
 
   def index
     @polls = Kaminari.paginate_array(
-      @polls.public_polls.not_budget.send(@current_filter).includes(:geozones).sort_for_list
+      @polls.created_by_admin.not_budget.send(@current_filter).includes(:geozones).sort_for_list
     ).page(params[:page])
   end
 

app/helpers/polls_helper.rb

@@ -49,19 +49,15 @@ module PollsHelper
   end
 
   def link_to_poll(text, poll)
-    if poll.results_enabled?
+    if can?(:results, poll)
       link_to text, results_poll_path(id: poll.slug || poll.id)
-    elsif poll.stats_enabled?
+    elsif can?(:stats, poll)
       link_to text, stats_poll_path(id: poll.slug || poll.id)
     else
       link_to text, poll_path(id: poll.slug || poll.id)
     end
   end
 
-  def show_stats_or_results?
-    @poll.expired? && (@poll.results_enabled? || @poll.stats_enabled?)
-  end
-
   def results_menu?
     controller_name == "polls" && action_name == "results"
   end

app/models/abilities/administrator.rb

@@ -75,7 +75,7 @@ module Abilities
 
       can [:index, :create, :edit, :update, :destroy], Geozone
 
-      can [:read, :create, :update, :destroy, :add_question, :search_booths, :search_officers, :booth_assignments, :results, :stats], Poll
+      can [:read, :create, :update, :destroy, :add_question, :search_booths, :search_officers, :booth_assignments], Poll
       can [:read, :create, :update, :destroy, :available], Poll::Booth
       can [:search, :create, :index, :destroy], ::Poll::Officer
       can [:create, :destroy, :manage], ::Poll::BoothAssignment

app/models/abilities/everyone.rb

@@ -7,20 +7,16 @@ module Abilities
       can [:read, :map, :summary, :share], Proposal
       can :read, Comment
       can :read, Poll
-      can :results, Poll do |poll|
-        poll.expired? && poll.results_enabled?
-      end
-      can :stats, Poll do |poll|
-        poll.expired? && poll.stats_enabled?
-      end
+      can :results, Poll, id: Poll.expired.results_enabled.not_budget.ids
+      can :stats, Poll, id: Poll.expired.stats_enabled.not_budget.ids
       can :read, Poll::Question
       can :read, User
       can [:read, :welcome], Budget
       can [:read], Budget
       can [:read], Budget::Group
       can [:read, :print, :json_data], Budget::Investment
-      can(:read_results, Budget) { |budget| budget.results_enabled? && budget.finished? }
-      can(:read_stats, Budget) { |budget| budget.stats_enabled? && budget.valuating_or_later? }
+      can :read_results, Budget, id: Budget.finished.results_enabled.ids
+      can :read_stats, Budget, id: Budget.valuating_or_later.stats_enabled.ids
       can :read_executions, Budget, phase: "finished"
       can :new, DirectMessage
       can [:read, :debate, :draft_publication, :allegations, :result_publication,

app/models/budget.rb

@@ -47,6 +47,7 @@ class Budget < ApplicationRecord
   scope :reviewing, -> { where(phase: "reviewing") }
   scope :selecting, -> { where(phase: "selecting") }
   scope :valuating, -> { where(phase: "valuating") }
+  scope :valuating_or_later, -> { where(phase: Budget::Phase.kind_or_later("valuating")) }
   scope :publishing_prices, -> { where(phase: "publishing_prices") }
   scope :balloting, -> { where(phase: "balloting") }
   scope :reviewing_ballots, -> { where(phase: "reviewing_ballots") }

app/models/budget/phase.rb

@@ -32,6 +32,10 @@ class Budget
       define_singleton_method(phase) { find_by(kind: phase) }
     end
 
+    def self.kind_or_later(phase)
+      PHASE_KINDS[PHASE_KINDS.index(phase)..-1]
+    end
+
     def next_enabled_phase
       next_phase&.enabled? ? next_phase : next_phase&.next_enabled_phase
     end
@@ -92,7 +96,7 @@ class Budget
       end
 
       def in_phase_or_later?(phase)
-        PHASE_KINDS.index(kind) >= PHASE_KINDS.index(phase)
+        self.class.kind_or_later(phase).include?(kind)
       end
   end
 end

app/models/concerns/reportable.rb

@@ -4,6 +4,10 @@ module Reportable
   included do
     has_one :report, as: :process, inverse_of: :process, dependent: :destroy
     accepts_nested_attributes_for :report
+
+    Report::KINDS.each do |kind|
+      scope "#{kind}_enabled", -> { joins(:report).where("reports.#{kind}": true) }
+    end
   end
 
   def report

app/models/poll.rb

@@ -40,7 +40,6 @@ class Poll < ApplicationRecord
   accepts_nested_attributes_for :questions, reject_if: :all_blank, allow_destroy: true
 
   scope :for, ->(element) { where(related: element) }
-  scope :public_polls, -> { where(related: nil) }
   scope :current,  -> { where("starts_at <= ? and ? <= ends_at", Date.current.beginning_of_day, Date.current.beginning_of_day) }
   scope :expired,  -> { where("ends_at < ?", Date.current.beginning_of_day) }
   scope :recounting, -> { where(ends_at: (Date.current.beginning_of_day - RECOUNT_DURATION)..Date.current.beginning_of_day) }

app/views/polls/_poll_subnav.html.erb

@@ -1,8 +1,8 @@
-<% if show_stats_or_results? %>
+<% if can?(:stats, @poll) || can?(:results, @poll) %>
   <div class="row margin-top">
     <div class="small-12 column">
       <ul class="menu simple clear">
-        <% if @poll.results_enabled? %>
+        <% if can?(:results, @poll) %>
           <% if results_menu? %>
             <li class="is-active">
               <h2><%= t("polls.show.results_menu") %></h2>
@@ -12,7 +12,7 @@
           <% end %>
         <% end %>
 
-        <% if @poll.stats_enabled? %>
+        <% if can?(:stats, @poll) %>
           <% if stats_menu? %>
             <li class="is-active">
               <h2><%= t("polls.show.stats_menu") %></h2>

lib/tasks/stats.rake

@@ -2,13 +2,14 @@ namespace :stats do
   desc "Generates stats which are not cached yet"
   task generate: :environment do
     ApplicationLogger.new.info "Updating budget and poll stats"
+    admin_ability = Ability.new(Administrator.first.user)
 
-    Budget.find_each do |budget|
+    Budget.accessible_by(admin_ability, :read_stats).find_each do |budget|
       Budget::Stats.new(budget).generate
       print "."
     end
 
-    Poll.find_each do |poll|
+    Poll.accessible_by(admin_ability, :stats).find_each do |poll|
       Poll::Stats.new(poll).generate
       print "."
     end

spec/features/polls/polls_spec.rb

@@ -110,17 +110,17 @@ describe "Polls" do
     end
 
     scenario "Poll title link to stats if enabled" do
-      poll = create(:poll, name: "Poll with stats", stats_enabled: true)
+      poll = create(:poll, :expired, name: "Poll with stats", stats_enabled: true)
 
-      visit polls_path
+      visit polls_path(filter: "expired")
 
       expect(page).to have_link("Poll with stats", href: stats_poll_path(poll.slug))
     end
 
     scenario "Poll title link to results if enabled" do
-      poll = create(:poll, name: "Poll with results", stats_enabled: true, results_enabled: true)
+      poll = create(:poll, :expired, name: "Poll with results", stats_enabled: true, results_enabled: true)
 
-      visit polls_path
+      visit polls_path(filter: "expired")
 
       expect(page).to have_link("Poll with results", href: results_poll_path(poll.slug))
     end

spec/models/abilities/administrator_spec.rb

@@ -6,7 +6,6 @@ describe Abilities::Administrator do
 
   let(:user) { administrator.user }
   let(:administrator) { create(:administrator) }
-  let(:poll) { create(:poll, :current, stats_enabled: false, results_enabled: false) }
 
   let(:other_user) { create(:user) }
   let(:hidden_user) { create(:user, :hidden) }
@@ -89,9 +88,6 @@ describe Abilities::Administrator do
   it { should_not be_able_to(:destroy, budget_investment_document) }
   it { should be_able_to(:manage, Dashboard::Action) }
 
-  it { should be_able_to(:stats, poll) }
-  it { should be_able_to(:results, poll) }
-
   it { should be_able_to(:read, Poll::Question) }
   it { should be_able_to(:create, Poll::Question) }
   it { should be_able_to(:update, Poll::Question) }

spec/models/abilities/everyone_spec.rb

@@ -31,81 +31,21 @@ describe Abilities::Everyone do
   it { should_not be_able_to(:create, LocalCensusRecords::Import) }
   it { should_not be_able_to(:show, LocalCensusRecords::Import) }
 
-  context "when accessing poll results" do
-    let(:results_enabled) { true }
-    let(:poll) { create(:poll, :expired, results_enabled: results_enabled) }
+  it { should be_able_to(:results, create(:poll, :expired, results_enabled: true)) }
+  it { should_not be_able_to(:results, create(:poll, :expired, results_enabled: false)) }
+  it { should_not be_able_to(:results, create(:poll, :current, results_enabled: true)) }
+  it { should_not be_able_to(:results, create(:poll, :for_budget, :expired, results_enabled: true)) }
 
-    it { should be_able_to(:results, poll) }
+  it { should be_able_to(:stats, create(:poll, :expired, stats_enabled: true)) }
+  it { should_not be_able_to(:stats, create(:poll, :expired, stats_enabled: false)) }
+  it { should_not be_able_to(:stats, create(:poll, :current, stats_enabled: true)) }
+  it { should_not be_able_to(:stats, create(:poll, :for_budget, :expired, stats_enabled: true)) }
 
-    context "and results disabled" do
-      let(:results_enabled) { false }
+  it { should be_able_to(:read_results, create(:budget, :finished, results_enabled: true)) }
+  it { should_not be_able_to(:read_results, create(:budget, :finished, results_enabled: false)) }
+  it { should_not be_able_to(:read_results, create(:budget, :reviewing_ballots, results_enabled: true)) }
 
-      it { should_not be_able_to(:results, poll) }
-    end
-
-    context "and not expired" do
-      let(:poll) { create(:poll, :current, results_enabled: true) }
-
-      it { should_not be_able_to(:results, poll) }
-    end
-  end
-
-  context "when accessing poll stats" do
-    let(:stats_enabled) { true }
-    let(:poll) { create(:poll, :expired, stats_enabled: stats_enabled) }
-
-    it { should be_able_to(:stats, poll) }
-
-    context "and stats disabled" do
-      let(:stats_enabled) { false }
-
-      it { should_not be_able_to(:stats, poll) }
-    end
-
-    context "and not expired" do
-      let(:poll) { create(:poll, :current, stats_enabled: true) }
-
-      it { should_not be_able_to(:stats, poll) }
-    end
-  end
-
-  context "when accessing budget results" do
-    context "budget is not finished" do
-      let(:budget) { create(:budget, :reviewing_ballots, results_enabled: true) }
-
-      it { should_not be_able_to(:read_results, budget) }
-    end
-
-    context "budget is finished" do
-      let(:budget) { create(:budget, :finished) }
-
-      it { should be_able_to(:read_results, budget) }
-    end
-
-    context "results disabled" do
-      let(:budget) { create(:budget, :finished, results_enabled: false) }
-
-      it { should_not be_able_to(:read_results, budget) }
-    end
-  end
-
-  context "when accessing budget stats" do
-    context "supports phase is not finished" do
-      let(:budget) { create(:budget, :selecting, stats_enabled: true) }
-
-      it { should_not be_able_to(:read_stats, budget) }
-    end
-
-    context "supports phase is finished" do
-      let(:budget) { create(:budget, :valuating, stats_enabled: true) }
-
-      it { should be_able_to(:read_stats, budget) }
-    end
-
-    context "stats disabled" do
-      let(:budget) { create(:budget, :valuating, stats_enabled: false) }
-
-      it { should_not be_able_to(:read_stats, budget) }
-    end
-  end
+  it { should be_able_to(:read_stats, create(:budget, :valuating, stats_enabled: true)) }
+  it { should_not be_able_to(:read_stats, create(:budget, :valuating, stats_enabled: false)) }
+  it { should_not be_able_to(:read_stats, create(:budget, :selecting, stats_enabled: true)) }
 end

spec/models/budget_spec.rb

@@ -7,6 +7,33 @@ describe Budget do
   it_behaves_like "reportable"
   it_behaves_like "globalizable", :budget
 
+  describe "scopes" do
+    describe ".open" do
+      it "returns all budgets that are not in the finished phase" do
+        (Budget::Phase::PHASE_KINDS - ["finished"]).each do |phase|
+          budget = create(:budget, phase: phase)
+          expect(Budget.open).to include(budget)
+        end
+      end
+    end
+
+    describe ".valuating_or_later" do
+      it "returns budgets valuating or later" do
+        valuating = create(:budget, :valuating)
+        finished = create(:budget, :finished)
+
+        expect(Budget.valuating_or_later).to match_array([valuating, finished])
+      end
+
+      it "does not return budgets which haven't reached valuation" do
+        create(:budget, :drafting)
+        create(:budget, :selecting)
+
+        expect(Budget.valuating_or_later).to be_empty
+      end
+    end
+  end
+
   describe "name" do
     before do
       budget.update(name_en: "object name")
@@ -172,15 +199,6 @@ describe Budget do
     end
   end
 
-  describe "#open" do
-    it "returns all budgets that are not in the finished phase" do
-      (Budget::Phase::PHASE_KINDS - ["finished"]).each do |phase|
-        budget = create(:budget, phase: phase)
-        expect(Budget.open).to include(budget)
-      end
-    end
-  end
-
   describe "heading_price" do
     it "returns the heading price if the heading provided is part of the budget" do
       heading = create(:budget_heading, price: 100, budget: budget)

spec/models/concerns/reportable.rb

@@ -1,6 +1,36 @@
 shared_examples "reportable" do
   let(:reportable) { create(model_name(described_class)) }
 
+  describe "scopes" do
+    describe ".results_enabled" do
+      it "includes records with results enabled" do
+        reportable.update!(results_enabled: true)
+
+        expect(described_class.results_enabled).to eq [reportable]
+      end
+
+      it "does not include records without results enabled" do
+        reportable.update!(results_enabled: false)
+
+        expect(described_class.results_enabled).to be_empty
+      end
+    end
+
+    describe ".stats_enabled" do
+      it "includes records with stats enabled" do
+        reportable.update!(stats_enabled: true)
+
+        expect(described_class.stats_enabled).to eq [reportable]
+      end
+
+      it "does not include records without stats enabled" do
+        reportable.update!(stats_enabled: false)
+
+        expect(described_class.stats_enabled).to be_empty
+      end
+    end
+  end
+
   describe "#results_enabled" do
     it "can write and read the attribute" do
       reportable.results_enabled = true