From bcc9fd97f59ff08951a374498d828150876f1bd9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Mon, 8 Apr 2024 19:27:31 +0200
Subject: [PATCH 1/9] Revert "Extract class to manage GeozoneStats"

Back in commit 383909e16, we said:

> Even if this class looks very simple now, we're trying a few things
> related to these stats. Having a class for it makes future changes
> easier and, if there weren't any future changes, at least it makes
> current experiments easier.

Since there haven't been any changes in the last 5 years and we've found
cases where using the GeozoneStats class results in a slightly worse
performance, we're removing this class. The code is now a bit easier to
read, and is consistent with the way we calculate participants by age.

This reverts commit 383909e16.
---
 app/lib/geozone_stats.rb             | 24 ---------------
 app/lib/percentage_calculator.rb     |  7 -----
 app/models/concerns/statisticable.rb | 18 ++++++------
 spec/lib/geozone_stats_spec.rb       | 44 ----------------------------
 4 files changed, 9 insertions(+), 84 deletions(-)
 delete mode 100644 app/lib/geozone_stats.rb
 delete mode 100644 app/lib/percentage_calculator.rb
 delete mode 100644 spec/lib/geozone_stats_spec.rb

diff --git a/app/lib/geozone_stats.rb b/app/lib/geozone_stats.rb
deleted file mode 100644
index 6b7971610..000000000
--- a/app/lib/geozone_stats.rb
+++ /dev/null
@@ -1,24 +0,0 @@
-class GeozoneStats
-  attr_reader :geozone, :participants
-
-  def initialize(geozone, participants)
-    @geozone = geozone
-    @participants = participants
-  end
-
-  def geozone_participants
-    participants.where(geozone: geozone)
-  end
-
-  def name
-    geozone.name
-  end
-
-  def count
-    geozone_participants.count
-  end
-
-  def percentage
-    PercentageCalculator.calculate(count, participants.count)
-  end
-end
diff --git a/app/lib/percentage_calculator.rb b/app/lib/percentage_calculator.rb
deleted file mode 100644
index bb8f3d355..000000000
--- a/app/lib/percentage_calculator.rb
+++ /dev/null
@@ -1,7 +0,0 @@
-module PercentageCalculator
-  def self.calculate(fraction, total)
-    return 0.0 if total.zero?
-
-    (fraction * 100.0 / total).round(3)
-  end
-end
diff --git a/app/models/concerns/statisticable.rb b/app/models/concerns/statisticable.rb
index 53533b4ed..690cdecf5 100644
--- a/app/models/concerns/statisticable.rb
+++ b/app/models/concerns/statisticable.rb
@@ -114,19 +114,23 @@ module Statisticable
   end
 
   def participants_by_geozone
-    geozone_stats.to_h do |stats|
+    geozones.to_h do |geozone|
+      count = participants.where(geozone: geozone).count
+
       [
-        stats.name,
+        geozone.name,
         {
-          count: stats.count,
-          percentage: stats.percentage
+          count: count,
+          percentage: calculate_percentage(count, total_participants)
         }
       ]
     end
   end
 
   def calculate_percentage(fraction, total)
-    PercentageCalculator.calculate(fraction, total)
+    return 0.0 if total.zero?
+
+    (fraction * 100.0 / total).round(3)
   end
 
   def version
@@ -178,10 +182,6 @@ module Statisticable
       Geozone.order("name")
     end
 
-    def geozone_stats
-      geozones.map { |geozone| GeozoneStats.new(geozone, participants) }
-    end
-
     def range_description(start, finish)
       if finish > 200
         I18n.t("stats.age_more_than", start: start)
diff --git a/spec/lib/geozone_stats_spec.rb b/spec/lib/geozone_stats_spec.rb
deleted file mode 100644
index 570315406..000000000
--- a/spec/lib/geozone_stats_spec.rb
+++ /dev/null
@@ -1,44 +0,0 @@
-require "rails_helper"
-
-describe GeozoneStats do
-  let(:winterfell) { create(:geozone, name: "Winterfell") }
-  let(:riverlands) { create(:geozone, name: "Riverlands") }
-
-  describe "#name" do
-    let(:stats) { GeozoneStats.new(winterfell, []) }
-
-    it "returns the geozone name" do
-      expect(stats.name).to eq "Winterfell"
-    end
-  end
-
-  describe "#count" do
-    before do
-      2.times { create(:user, geozone: winterfell) }
-      1.times { create(:user, geozone: riverlands) }
-    end
-
-    let(:winterfell_stats) { GeozoneStats.new(winterfell, User.all) }
-    let(:riverlands_stats) { GeozoneStats.new(riverlands, User.all) }
-
-    it "counts participants from the geozone" do
-      expect(winterfell_stats.count).to eq 2
-      expect(riverlands_stats.count).to eq 1
-    end
-  end
-
-  describe "#percentage" do
-    before do
-      2.times { create(:user, geozone: winterfell) }
-      1.times { create(:user, geozone: riverlands) }
-    end
-
-    let(:winterfell_stats) { GeozoneStats.new(winterfell, User.all) }
-    let(:riverlands_stats) { GeozoneStats.new(riverlands, User.all) }
-
-    it "calculates percentage relative to the amount of participants" do
-      expect(winterfell_stats.percentage).to eq 66.667
-      expect(riverlands_stats.percentage).to eq 33.333
-    end
-  end
-end

From 6f0c27c0fb90a2d8e0a570102c66c207075c9fbb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Mon, 8 Apr 2024 19:47:28 +0200
Subject: [PATCH 2/9] Remove unused code in statisticable concern

This code isn't used since commit e3063cd24f.
---
 app/models/concerns/statisticable.rb | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/app/models/concerns/statisticable.rb b/app/models/concerns/statisticable.rb
index 690cdecf5..c3ee15139 100644
--- a/app/models/concerns/statisticable.rb
+++ b/app/models/concerns/statisticable.rb
@@ -174,10 +174,6 @@ module Statisticable
        [90, 300]]
     end
 
-    def participants_between_ages(from, to)
-      participants.between_ages(from, to)
-    end
-
     def geozones
       Geozone.order("name")
     end

From 80dcbfc23c218366678f0e87e4bf3b6e5bed6312 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Sun, 7 Apr 2024 19:17:44 +0200
Subject: [PATCH 3/9] Improve performance generating stats

Debugging shows that the bottleneck in the stats calculation is the
number of times we're querying the users table using the same array of
IDs in the `where` condition but each time combined with other
conditions.

So we're inserting the results of querying the users table with the
array of IDs in a temporary table and using this temporary table for the
other calculations. When querying this temporary table, there's no need
to filter for IDs anymore.

For budget stats, the `generate` method is now about 10-20 times faster
for a budget with 20,000 participants. For budgets with only a few dozen
participants, there's no significant difference in performance.

I thought about modifying the `participants` method and use the
temporary table there. The problem, however, is that in this case it
isn't clear when to drop the temporary table, and we could end up with
thousands of temporary tables in the database if we don't do it right.
Creating and dropping the temporary table in the same transaction, on
the other hand, guarantees that won't be the case.

Note there's no risk of duplicate tables since they're created and
dropped inside a transaction, so we're always using the same table name
for the same resource. We're adding a test that fails with a
`PG::DuplicateTable: ERROR:  relation "participants__1"` error if we
don't use a transaction.
---
 app/models/concerns/statisticable.rb | 39 +++++++++++++++++++++++++++-
 spec/models/statisticable_spec.rb    | 29 ++++++++++++++++++++-
 2 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/app/models/concerns/statisticable.rb b/app/models/concerns/statisticable.rb
index c3ee15139..826d7dc68 100644
--- a/app/models/concerns/statisticable.rb
+++ b/app/models/concerns/statisticable.rb
@@ -47,7 +47,23 @@ module Statisticable
   end
 
   def generate
-    stats_methods.each { |stat_name| send(stat_name) }
+    User.transaction do
+      create_participants_table
+
+      begin
+        define_singleton_method :participants do
+          participants_class.all
+        end
+
+        stats_methods.each { |stat_name| send(stat_name) }
+      ensure
+        define_singleton_method :participants do
+          participants_from_original_table
+        end
+      end
+
+      drop_participants_table
+    end
   end
 
   def stats_methods
@@ -77,6 +93,7 @@ module Statisticable
   def participants
     @participants ||= User.unscoped.where(id: participant_ids)
   end
+  alias_method :participants_from_original_table, :participants
 
   def total_male_participants
     participants.male.count
@@ -151,6 +168,26 @@ module Statisticable
       participations.map { |participation| self.class.send("#{participation}_methods") }.flatten
     end
 
+    def create_participants_table
+      User.connection.create_table(
+        participants_table_name,
+        temporary: true,
+        as: participants_from_original_table.to_sql
+      )
+    end
+
+    def drop_participants_table
+      User.connection.drop_table(participants_table_name, if_exists: true, temporary: true)
+    end
+
+    def participants_table_name
+      @participants_table_name ||= "participants_#{resource.class.table_name}_#{resource.id}"
+    end
+
+    def participants_class
+      @participants_class ||= Class.new(User).tap { |klass| klass.table_name = participants_table_name }
+    end
+
     def total_participants_with_gender
       @total_participants_with_gender ||= participants.where.not(gender: nil).distinct.count
     end
diff --git a/spec/models/statisticable_spec.rb b/spec/models/statisticable_spec.rb
index 9768b39ee..f1817fbbd 100644
--- a/spec/models/statisticable_spec.rb
+++ b/spec/models/statisticable_spec.rb
@@ -8,6 +8,11 @@ describe Statisticable do
       def participants
         User.all
       end
+      alias_method :participants_from_original_table, :participants
+
+      def total_participants
+        User.count
+      end
 
       def participation_date
         Time.current
@@ -17,7 +22,7 @@ describe Statisticable do
     stub_const("DummyStats", dummy_stats)
   end
 
-  let(:stats) { DummyStats.new(nil) }
+  let(:stats) { DummyStats.new(double(id: 1, class: double(table_name: ""))) }
 
   describe "#gender?" do
     context "No participants" do
@@ -194,4 +199,26 @@ describe Statisticable do
       end
     end
   end
+
+  describe "#generate" do
+    it "drops the temporary table after finishing" do
+      stats.generate
+
+      expect { stats.send(:participants_class).first }.to raise_exception(ActiveRecord::StatementInvalid)
+    end
+
+    it "can be executed twice without errors" do
+      stats.generate
+
+      expect { stats.generate }.not_to raise_exception
+    end
+
+    it "can be executed twice in parallel since it uses a transaction" do
+      other_stats = DummyStats.new(stats.resource)
+
+      [stats, other_stats].map do |stat|
+        Thread.new { stat.generate }
+      end.each(&:join)
+    end
+  end
 end

From 62cd4c8d7b70685a10e459178c4c062404bf25f6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Tue, 9 Apr 2024 15:53:57 +0200
Subject: [PATCH 4/9] Add indices to stats temporary tables

Since we're doing many queries to get stats for each age group and each
geozone, testing shows these indices make stats calculation about 25%
faster on processes with 100,000 participants.
---
 app/models/concerns/statisticable.rb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/models/concerns/statisticable.rb b/app/models/concerns/statisticable.rb
index 826d7dc68..9d47720ee 100644
--- a/app/models/concerns/statisticable.rb
+++ b/app/models/concerns/statisticable.rb
@@ -174,6 +174,8 @@ module Statisticable
         temporary: true,
         as: participants_from_original_table.to_sql
       )
+      User.connection.add_index participants_table_name, :date_of_birth
+      User.connection.add_index participants_table_name, :geozone_id
     end
 
     def drop_participants_table

From f30fa29994e0b650e86f6a2d4a0216d82d95cb36 Mon Sep 17 00:00:00 2001
From: Julian Herrero <microweb10@gmail.com>
Date: Thu, 28 Mar 2024 13:45:06 +0100
Subject: [PATCH 5/9] Allow running specs scenarios using the Rails cache

Cache is by default disabled on every non-production environment
---
 spec/spec_helper.rb | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index a82d425e6..15c49772e 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -193,6 +193,11 @@ RSpec.configure do |config|
     savon.unmock!
   end
 
+  config.before(:each, :with_cache) do
+    allow(Rails).to receive(:cache).and_return(ActiveSupport::Cache.lookup_store(:memory_store))
+    Rails.cache.clear
+  end
+
   # Allows RSpec to persist some state between runs in order to support
   # the `--only-failures` and `--next-failure` CLI options.
   config.example_status_persistence_file_path = "spec/examples.txt"

From 653848fc4e9a6f466a8418ffcc3de34f6b93786e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Mon, 8 Apr 2024 14:53:13 +0200
Subject: [PATCH 6/9] Extract method to get the stats key in stats

This way we remove a bit of duplication and it'll be easier to change
the `stats_cache` method.
---
 app/models/budget/stats.rb           | 4 ++--
 app/models/concerns/statisticable.rb | 4 ++++
 app/models/poll/stats.rb             | 4 ++--
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/app/models/budget/stats.rb b/app/models/budget/stats.rb
index 4d62cd6de..04cc6d480 100644
--- a/app/models/budget/stats.rb
+++ b/app/models/budget/stats.rb
@@ -192,7 +192,7 @@ class Budget::Stats
 
     stats_cache(*stats_methods)
 
-    def stats_cache(key, &)
-      Rails.cache.fetch("budgets_stats/#{budget.id}/#{phases.join}/#{key}/#{version}", &)
+    def full_cache_key_for(key)
+      "budgets_stats/#{budget.id}/#{phases.join}/#{key}/#{version}"
     end
 end
diff --git a/app/models/concerns/statisticable.rb b/app/models/concerns/statisticable.rb
index 9d47720ee..8e4ea424c 100644
--- a/app/models/concerns/statisticable.rb
+++ b/app/models/concerns/statisticable.rb
@@ -224,4 +224,8 @@ module Statisticable
         I18n.t("stats.age_range", start: start, finish: finish)
       end
     end
+
+    def stats_cache(key, &)
+      Rails.cache.fetch(full_cache_key_for(key), &)
+    end
 end
diff --git a/app/models/poll/stats.rb b/app/models/poll/stats.rb
index c449bf17d..0aeb93109 100644
--- a/app/models/poll/stats.rb
+++ b/app/models/poll/stats.rb
@@ -121,7 +121,7 @@ class Poll::Stats
 
     stats_cache(*stats_methods)
 
-    def stats_cache(key, &)
-      Rails.cache.fetch("polls_stats/#{poll.id}/#{key}/#{version}", &)
+    def full_cache_key_for(key)
+      "polls_stats/#{poll.id}/#{key}/#{version}"
     end
 end

From a5646fcdb3416dd7f343169f011705e097316270 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Thu, 18 Apr 2024 21:11:54 +0200
Subject: [PATCH 7/9] Remove Cron job to generate stats

Since now generating stats (assuming the results aren't in the cache)
only takes a few seconds even when there are a hundred thousand
participants, as opposed to the several minutes it took to generate them
when we introduced the Cron job, we can simply generate the stats during
the first request to the stats page.

Note that, in order to avoid creating a temporary table when the stats
are cached, we're making sure we only create this table when we need to.
Otherwise, we could spend up to 1 second on every request to the stats
page creating a table that isn't going to be used.

Also note we're using an instance variable to check whether we're
creating a table; I tried to use `table_exists?`, but it didn't work. I
wonder whether `table_exists?` doesn't detect temporary tables.
---
 app/controllers/budgets/stats_controller.rb |  2 +-
 app/controllers/polls_controller.rb         |  2 +-
 app/models/concerns/statisticable.rb        |  9 +++++++--
 config/schedule.rb                          |  4 ----
 lib/tasks/stats.rake                        | 22 ---------------------
 5 files changed, 9 insertions(+), 30 deletions(-)

diff --git a/app/controllers/budgets/stats_controller.rb b/app/controllers/budgets/stats_controller.rb
index 33480238d..353ff113b 100644
--- a/app/controllers/budgets/stats_controller.rb
+++ b/app/controllers/budgets/stats_controller.rb
@@ -8,7 +8,7 @@ module Budgets
 
     def show
       authorize! :read_stats, @budget
-      @stats = Budget::Stats.new(@budget)
+      @stats = Budget::Stats.new(@budget).tap(&:generate)
       @headings = @budget.headings.sort_by_name
     end
 
diff --git a/app/controllers/polls_controller.rb b/app/controllers/polls_controller.rb
index ecb8ffadb..80b72309b 100644
--- a/app/controllers/polls_controller.rb
+++ b/app/controllers/polls_controller.rb
@@ -23,7 +23,7 @@ class PollsController < ApplicationController
   end
 
   def stats
-    @stats = Poll::Stats.new(@poll)
+    @stats = Poll::Stats.new(@poll).tap(&:generate)
   end
 
   def results
diff --git a/app/models/concerns/statisticable.rb b/app/models/concerns/statisticable.rb
index 8e4ea424c..d644f0dec 100644
--- a/app/models/concerns/statisticable.rb
+++ b/app/models/concerns/statisticable.rb
@@ -48,10 +48,9 @@ module Statisticable
 
   def generate
     User.transaction do
-      create_participants_table
-
       begin
         define_singleton_method :participants do
+          create_participants_table unless participants_table_created?
           participants_class.all
         end
 
@@ -176,10 +175,12 @@ module Statisticable
       )
       User.connection.add_index participants_table_name, :date_of_birth
       User.connection.add_index participants_table_name, :geozone_id
+      @participants_table_created = true
     end
 
     def drop_participants_table
       User.connection.drop_table(participants_table_name, if_exists: true, temporary: true)
+      @participants_table_created = false
     end
 
     def participants_table_name
@@ -190,6 +191,10 @@ module Statisticable
       @participants_class ||= Class.new(User).tap { |klass| klass.table_name = participants_table_name }
     end
 
+    def participants_table_created?
+      @participants_table_created.present?
+    end
+
     def total_participants_with_gender
       @total_participants_with_gender ||= participants.where.not(gender: nil).distinct.count
     end
diff --git a/config/schedule.rb b/config/schedule.rb
index b0755dbaf..fcfd626e3 100644
--- a/config/schedule.rb
+++ b/config/schedule.rb
@@ -27,10 +27,6 @@ every 1.day, at: "5:00 am" do
   rake "-s sitemap:refresh:no_ping"
 end
 
-every 2.hours do
-  rake "-s stats:generate"
-end
-
 every 1.day, at: "1:00 am", roles: [:cron] do
   rake "files:remove_old_cached_attachments"
 end
diff --git a/lib/tasks/stats.rake b/lib/tasks/stats.rake
index b7d397301..526309823 100644
--- a/lib/tasks/stats.rake
+++ b/lib/tasks/stats.rake
@@ -1,23 +1,4 @@
 namespace :stats do
-  desc "Generates stats which are not cached yet"
-  task generate: :environment do
-    ApplicationLogger.new.info "Updating budget and poll stats"
-
-    Tenant.run_on_each do
-      admin_ability = Ability.new(Administrator.first.user)
-
-      Budget.accessible_by(admin_ability, :read_stats).find_each do |budget|
-        Budget::Stats.new(budget).generate
-        print "."
-      end
-
-      Poll.accessible_by(admin_ability, :stats).find_each do |poll|
-        Poll::Stats.new(poll).generate
-        print "."
-      end
-    end
-  end
-
   desc "Expires stats cache"
   task expire_cache: :environment do
     Tenant.run_on_each do
@@ -26,7 +7,4 @@ namespace :stats do
       end
     end
   end
-
-  desc "Deletes stats cache and generates it again"
-  task regenerate: [:expire_cache, :generate]
 end

From a4461a1a56332c3e1d97eb3310dc635ec29f8a78 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Thu, 18 Apr 2024 22:16:26 +0200
Subject: [PATCH 8/9] Expire the stats cache once per day

When we first started caching the stats, generating them was a process
that took several minutes, so we never expired the cache.

However, there have been cases where we run into issues where the stats
shown on the screen were outdated. That's why we introduced a task to
manually expire the cache.

But now, generating the stats only takes a few seconds, so we can
automatically expire them every day, remove all the logic needed to
manually expire them, and get rid of most of the issues related to the
cache being outdated.

We're expiring them every day because it's the same day we were doing in
public stats (which we removed in commit 631b48f58), only we're using
`expires_at:` to set the expiration time, in order to simplify the code.

Note that, in the test, we're using `travel_to(time)` so the test passes
even when it starts an instant before midnight. We aren't using
`:with_frozen_time` because, in similar cases (although not in this
case, but I'm not sure whether that's intentional), `travel_to` shows
this error:

> Calling `travel_to` with a block, when we have previously already made
> a call to `travel_to`, can lead to confusing time stubbing.
---
 app/models/budget.rb                          |  1 -
 app/models/budget/stats.rb                    |  2 +-
 app/models/concerns/statisticable.rb          |  6 +---
 app/models/concerns/stats_versionable.rb      | 11 -------
 app/models/poll.rb                            |  1 -
 app/models/poll/stats.rb                      |  2 +-
 app/models/stats_version.rb                   |  5 ----
 .../20240408154814_drop_stats_versions.rb     | 12 ++++++++
 db/schema.rb                                  |  8 -----
 lib/tasks/stats.rake                          | 10 -------
 spec/models/poll/stats_spec.rb                | 28 -----------------
 spec/models/statisticable_spec.rb             | 30 +++++++++++++++++++
 spec/models/stats_version_spec.rb             | 13 --------
 13 files changed, 45 insertions(+), 84 deletions(-)
 delete mode 100644 app/models/concerns/stats_versionable.rb
 delete mode 100644 app/models/stats_version.rb
 create mode 100644 db/migrate/20240408154814_drop_stats_versions.rb
 delete mode 100644 lib/tasks/stats.rake
 delete mode 100644 spec/models/stats_version_spec.rb

diff --git a/app/models/budget.rb b/app/models/budget.rb
index 4fc250bf4..319f02d4b 100644
--- a/app/models/budget.rb
+++ b/app/models/budget.rb
@@ -1,7 +1,6 @@
 class Budget < ApplicationRecord
   include Measurable
   include Sluggable
-  include StatsVersionable
   include Reportable
   include Imageable
 
diff --git a/app/models/budget/stats.rb b/app/models/budget/stats.rb
index 04cc6d480..d03e4edd3 100644
--- a/app/models/budget/stats.rb
+++ b/app/models/budget/stats.rb
@@ -193,6 +193,6 @@ class Budget::Stats
     stats_cache(*stats_methods)
 
     def full_cache_key_for(key)
-      "budgets_stats/#{budget.id}/#{phases.join}/#{key}/#{version}"
+      "budgets_stats/#{budget.id}/#{phases.join}/#{key}"
     end
 end
diff --git a/app/models/concerns/statisticable.rb b/app/models/concerns/statisticable.rb
index d644f0dec..cb1fae680 100644
--- a/app/models/concerns/statisticable.rb
+++ b/app/models/concerns/statisticable.rb
@@ -149,10 +149,6 @@ module Statisticable
     (fraction * 100.0 / total).round(3)
   end
 
-  def version
-    "v#{resource.find_or_create_stats_version.updated_at.to_i}"
-  end
-
   def advanced?
     resource.advanced_stats_enabled?
   end
@@ -231,6 +227,6 @@ module Statisticable
     end
 
     def stats_cache(key, &)
-      Rails.cache.fetch(full_cache_key_for(key), &)
+      Rails.cache.fetch(full_cache_key_for(key), expires_at: Date.current.end_of_day, &)
     end
 end
diff --git a/app/models/concerns/stats_versionable.rb b/app/models/concerns/stats_versionable.rb
deleted file mode 100644
index 9e9138ecc..000000000
--- a/app/models/concerns/stats_versionable.rb
+++ /dev/null
@@ -1,11 +0,0 @@
-module StatsVersionable
-  extend ActiveSupport::Concern
-
-  included do
-    has_one :stats_version, as: :process, inverse_of: :process
-  end
-
-  def find_or_create_stats_version
-    stats_version || create_stats_version
-  end
-end
diff --git a/app/models/poll.rb b/app/models/poll.rb
index 3d587b59f..4021fcf1c 100644
--- a/app/models/poll.rb
+++ b/app/models/poll.rb
@@ -7,7 +7,6 @@ class Poll < ApplicationRecord
   include Notifiable
   include Searchable
   include Sluggable
-  include StatsVersionable
   include Reportable
   include SDG::Relatable
 
diff --git a/app/models/poll/stats.rb b/app/models/poll/stats.rb
index 0aeb93109..8a0b1562b 100644
--- a/app/models/poll/stats.rb
+++ b/app/models/poll/stats.rb
@@ -122,6 +122,6 @@ class Poll::Stats
     stats_cache(*stats_methods)
 
     def full_cache_key_for(key)
-      "polls_stats/#{poll.id}/#{key}/#{version}"
+      "polls_stats/#{poll.id}/#{key}"
     end
 end
diff --git a/app/models/stats_version.rb b/app/models/stats_version.rb
deleted file mode 100644
index f47ddea44..000000000
--- a/app/models/stats_version.rb
+++ /dev/null
@@ -1,5 +0,0 @@
-class StatsVersion < ApplicationRecord
-  validates :process, presence: true
-
-  belongs_to :process, polymorphic: true
-end
diff --git a/db/migrate/20240408154814_drop_stats_versions.rb b/db/migrate/20240408154814_drop_stats_versions.rb
new file mode 100644
index 000000000..a88c1d879
--- /dev/null
+++ b/db/migrate/20240408154814_drop_stats_versions.rb
@@ -0,0 +1,12 @@
+class DropStatsVersions < ActiveRecord::Migration[7.0]
+  def change
+    drop_table :stats_versions, id: :serial do |t|
+      t.string :process_type
+      t.integer :process_id
+      t.datetime :created_at, precision: nil, null: false
+      t.datetime :updated_at, precision: nil, null: false
+
+      t.index ["process_type", "process_id"], name: "index_stats_versions_on_process_type_and_process_id"
+    end
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 0b5dfa0d5..d330136cc 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -1515,14 +1515,6 @@ ActiveRecord::Schema[7.0].define(version: 2024_04_24_013913) do
     t.string "locale"
   end
 
-  create_table "stats_versions", id: :serial, force: :cascade do |t|
-    t.string "process_type"
-    t.integer "process_id"
-    t.datetime "created_at", precision: nil, null: false
-    t.datetime "updated_at", precision: nil, null: false
-    t.index ["process_type", "process_id"], name: "index_stats_versions_on_process_type_and_process_id"
-  end
-
   create_table "taggings", id: :serial, force: :cascade do |t|
     t.integer "tag_id"
     t.string "taggable_type"
diff --git a/lib/tasks/stats.rake b/lib/tasks/stats.rake
deleted file mode 100644
index 526309823..000000000
--- a/lib/tasks/stats.rake
+++ /dev/null
@@ -1,10 +0,0 @@
-namespace :stats do
-  desc "Expires stats cache"
-  task expire_cache: :environment do
-    Tenant.run_on_each do
-      [Budget, Poll].each do |model_class|
-        model_class.find_each { |record| record.find_or_create_stats_version.touch }
-      end
-    end
-  end
-end
diff --git a/spec/models/poll/stats_spec.rb b/spec/models/poll/stats_spec.rb
index ddb95a361..9bf5f56c2 100644
--- a/spec/models/poll/stats_spec.rb
+++ b/spec/models/poll/stats_spec.rb
@@ -299,32 +299,4 @@ describe Poll::Stats do
       end
     end
   end
-
-  describe "#version", :with_frozen_time do
-    context "record with no stats" do
-      it "returns a string based on the current time" do
-        expect(stats.version).to eq "v#{Time.current.to_i}"
-      end
-
-      it "doesn't overwrite the timestamp when called multiple times" do
-        time = Time.current
-
-        expect(stats.version).to eq "v#{time.to_i}"
-
-        unfreeze_time
-
-        travel_to 2.seconds.from_now do
-          expect(stats.version).to eq "v#{time.to_i}"
-        end
-      end
-    end
-
-    context "record with stats" do
-      before { poll.create_stats_version(updated_at: 1.day.ago) }
-
-      it "returns the version of the existing stats" do
-        expect(stats.version).to eq "v#{1.day.ago.to_i}"
-      end
-    end
-  end
 end
diff --git a/spec/models/statisticable_spec.rb b/spec/models/statisticable_spec.rb
index f1817fbbd..e1705d86d 100644
--- a/spec/models/statisticable_spec.rb
+++ b/spec/models/statisticable_spec.rb
@@ -4,6 +4,12 @@ describe Statisticable do
   before do
     dummy_stats = Class.new do
       include Statisticable
+      attr_accessor :total
+      stats_cache :total
+
+      def full_cache_key_for(key)
+        "dummy_stats/#{object_id}/#{key}"
+      end
 
       def participants
         User.all
@@ -221,4 +227,28 @@ describe Statisticable do
       end.each(&:join)
     end
   end
+
+  describe "cache" do
+    it "expires the cache at the end of the day", :with_cache do
+      time = Time.current
+
+      travel_to(time) do
+        stats.total = 6
+
+        expect(stats.total).to eq 6
+
+        stats.total = 7
+
+        expect(stats.total).to eq 6
+      end
+
+      travel_to(time.end_of_day) do
+        expect(stats.total).to eq 6
+      end
+
+      travel_to(time.end_of_day + 1.second) do
+        expect(stats.total).to eq 7
+      end
+    end
+  end
 end
diff --git a/spec/models/stats_version_spec.rb b/spec/models/stats_version_spec.rb
deleted file mode 100644
index eb875ed09..000000000
--- a/spec/models/stats_version_spec.rb
+++ /dev/null
@@ -1,13 +0,0 @@
-require "rails_helper"
-
-describe StatsVersion do
-  describe "validations" do
-    it "is valid with a process" do
-      expect(StatsVersion.new(process: Budget.new)).to be_valid
-    end
-
-    it "is not valid without a process" do
-      expect(StatsVersion.new(process: nil)).not_to be_valid
-    end
-  end
-end

From 16f844595dd87be12f522857de8bcb66147c7a98 Mon Sep 17 00:00:00 2001
From: Julian Herrero <microweb10@gmail.com>
Date: Sat, 30 Mar 2024 12:02:11 +0100
Subject: [PATCH 9/9] Don't use the cache in admin budget stats
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In commit e51e03446, we started using the same code to show stats in the
public area and in the admin area. However, in doing so we introduced a
bug, since stats in the public area are only shown after a certain part
of the process has finished, meaning the stats appearing on the page
never change (in theory), so it's perfectly fine to cache them. However,
in the admin area stats can be accessed while the process is still
ongoing, so caching the stats will lead to the wrong results being
displayed.

We've thought about expiring the cache when new supports or ballot lines
are added; however, that means the methods calculating the stats for the
supporting phase would expire when supports are added/removed but the
methods calculating the stats for the voting phase would expire when
ballot lines are added/removed. It gets even more complex because the
`headings` method calculates stats for both the supporting and the
voting phases.

So, since loading stats in the admin section is fast even without the
cache because they only load very basic statistics, we're taking the
simple approach of disabling the cache in this case, so everything works
the same way it did before commit e51e03446.

Co-authored-by: Javi Martín <javim@elretirao.net>
---
 .../admin/stats/budget_balloting_component.rb |  2 +-
 .../stats/budget_supporting_component.rb      |  2 +-
 app/models/concerns/statisticable.rb          | 13 +++--
 spec/models/statisticable_spec.rb             | 22 ++++++--
 spec/system/admin/stats_spec.rb               | 53 +++++++++++++++++++
 5 files changed, 83 insertions(+), 9 deletions(-)

diff --git a/app/components/admin/stats/budget_balloting_component.rb b/app/components/admin/stats/budget_balloting_component.rb
index 9b9065c44..12addf185 100644
--- a/app/components/admin/stats/budget_balloting_component.rb
+++ b/app/components/admin/stats/budget_balloting_component.rb
@@ -8,7 +8,7 @@ class Admin::Stats::BudgetBallotingComponent < ApplicationComponent
   private
 
     def stats
-      @stats ||= Budget::Stats.new(budget)
+      @stats ||= Budget::Stats.new(budget, cache: false)
     end
 
     def headings_stats
diff --git a/app/components/admin/stats/budget_supporting_component.rb b/app/components/admin/stats/budget_supporting_component.rb
index 5d7b9cea2..0fdeb7b20 100644
--- a/app/components/admin/stats/budget_supporting_component.rb
+++ b/app/components/admin/stats/budget_supporting_component.rb
@@ -8,7 +8,7 @@ class Admin::Stats::BudgetSupportingComponent < ApplicationComponent
   private
 
     def stats
-      @stats ||= Budget::Stats.new(budget)
+      @stats ||= Budget::Stats.new(budget, cache: false)
     end
 
     def headings_stats
diff --git a/app/models/concerns/statisticable.rb b/app/models/concerns/statisticable.rb
index cb1fae680..f8b13766a 100644
--- a/app/models/concerns/statisticable.rb
+++ b/app/models/concerns/statisticable.rb
@@ -3,7 +3,7 @@ module Statisticable
   PARTICIPATIONS = %w[gender age geozone].freeze
 
   included do
-    attr_reader :resource
+    attr_reader :resource, :cache
   end
 
   class_methods do
@@ -42,8 +42,9 @@ module Statisticable
     end
   end
 
-  def initialize(resource)
+  def initialize(resource, cache: true)
     @resource = resource
+    @cache = cache
   end
 
   def generate
@@ -226,7 +227,11 @@ module Statisticable
       end
     end
 
-    def stats_cache(key, &)
-      Rails.cache.fetch(full_cache_key_for(key), expires_at: Date.current.end_of_day, &)
+    def stats_cache(key, &block)
+      if cache
+        Rails.cache.fetch(full_cache_key_for(key), expires_at: Date.current.end_of_day, &block)
+      else
+        block.call
+      end
     end
 end
diff --git a/spec/models/statisticable_spec.rb b/spec/models/statisticable_spec.rb
index e1705d86d..fa3301610 100644
--- a/spec/models/statisticable_spec.rb
+++ b/spec/models/statisticable_spec.rb
@@ -28,7 +28,8 @@ describe Statisticable do
     stub_const("DummyStats", dummy_stats)
   end
 
-  let(:stats) { DummyStats.new(double(id: 1, class: double(table_name: ""))) }
+  let(:resource) { double(id: 1, class: double(table_name: "")) }
+  let(:stats) { DummyStats.new(resource) }
 
   describe "#gender?" do
     context "No participants" do
@@ -228,8 +229,8 @@ describe Statisticable do
     end
   end
 
-  describe "cache" do
-    it "expires the cache at the end of the day", :with_cache do
+  describe "cache", :with_cache do
+    it "expires the cache at the end of the day by default" do
       time = Time.current
 
       travel_to(time) do
@@ -250,5 +251,20 @@ describe Statisticable do
         expect(stats.total).to eq 7
       end
     end
+
+    it "does not use the cache with cache: false" do
+      stats = DummyStats.new(resource, cache: false)
+      time = Time.current
+
+      travel_to(time) do
+        stats.total = 6
+
+        expect(stats.total).to eq 6
+
+        stats.total = 7
+
+        expect(stats.total).to eq 7
+      end
+    end
   end
 end
diff --git a/spec/system/admin/stats_spec.rb b/spec/system/admin/stats_spec.rb
index 88c77cf2d..2ea281266 100644
--- a/spec/system/admin/stats_spec.rb
+++ b/spec/system/admin/stats_spec.rb
@@ -99,6 +99,33 @@ describe "Stats", :admin do
         expect(page).to have_link "Go back", count: 1
       end
 
+      scenario "Don't use the cache for supports", :with_cache do
+        budget.update!(phase: :selecting)
+        investment = create(:budget_investment, heading: heading_all_city)
+        create(:user, :level_two, votables: [investment])
+
+        supporter = create(:user, :level_two)
+
+        visit budget_supporting_admin_stats_path(budget_id: budget)
+
+        expect(page).to have_content "VOTES\n1"
+        expect(page).to have_content "PARTICIPANTS\n1"
+
+        in_browser(:supporter) do
+          login_as(supporter)
+
+          visit budget_investment_path(budget, investment)
+          click_button "Support"
+
+          expect(page).to have_button "Remove your support"
+        end
+
+        refresh
+
+        expect(page).to have_content "VOTES\n2"
+        expect(page).to have_content "PARTICIPANTS\n2"
+      end
+
       scenario "hide final voting link" do
         visit admin_stats_path
         click_link "Participatory Budgets"
@@ -135,6 +162,32 @@ describe "Stats", :admin do
         expect(page).to have_content "VOTES\n3"
         expect(page).to have_content "PARTICIPANTS\n2"
       end
+
+      scenario "Don't use the cache for votes", :with_cache do
+        budget.update!(phase: :balloting)
+        create(:user, ballot_lines: [investment])
+
+        balloter = create(:user, :level_two)
+
+        visit budget_balloting_admin_stats_path(budget_id: budget.id)
+
+        expect(page).to have_content "VOTES\n1"
+        expect(page).to have_content "PARTICIPANTS\n1"
+
+        in_browser(:balloter) do
+          login_as(balloter)
+
+          visit budget_investment_path(budget, investment)
+          click_button "Vote"
+
+          expect(page).to have_button "Remove vote"
+        end
+
+        refresh
+
+        expect(page).to have_content "VOTES\n2"
+        expect(page).to have_content "PARTICIPANTS\n2"
+      end
     end
   end