consul/nairobi
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #5456 from consuldemocracy/budgets_stats_cache

Browse Source 
Don't use the cache in admin budget stats
This commit is contained in:
Javi Martín
2024-05-20 16:39:05 +02:00
committed by GitHub
parent fe415d8367 16f844595d
commit bb9e47579e
23 changed files with 216 additions and 206 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
app/components/admin/stats/budget_balloting_component.rb
View File

@@ -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

2
app/components/admin/stats/budget_supporting_component.rb
View File

@@ -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

2
app/controllers/budgets/stats_controller.rb
View File

@@ -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

2
app/controllers/polls_controller.rb
View File

@@ -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

24
app/lib/geozone_stats.rb
View File

@@ -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

7
app/lib/percentage_calculator.rb
View File

@@ -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

1
app/models/budget.rb
View File

@@ -1,7 +1,6 @@
class Budget < ApplicationRecord
include Measurable
include Sluggable
include StatsVersionable
include Reportable
include Imageable

4
app/models/budget/stats.rb
View File

@@ -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}"
end
end

83
app/models/concerns/statisticable.rb
View File

@@ -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,12 +42,28 @@ module Statisticable
end
end
def initialize(resource)
def initialize(resource, cache: true)
@resource = resource
@cache = cache
end
def generate
stats_methods.each { |stat_name| send(stat_name) }
User.transaction do
begin
define_singleton_method :participants do
create_participants_table unless participants_table_created?
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
@@ -114,23 +131,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)
end
return 0.0 if total.zero?
def version
"v#{resource.find_or_create_stats_version.updated_at.to_i}"
(fraction * 100.0 / total).round(3)
end
def advanced?
@@ -147,6 +164,34 @@ 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
)
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
@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 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
@@ -170,18 +215,10 @@ module Statisticable
[90, 300]]
end
def participants_between_ages(from, to)
participants.between_ages(from, to)
end
def geozones
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)
@@ -189,4 +226,12 @@ module Statisticable
I18n.t("stats.age_range", start: start, finish: finish)
end
end
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

11
app/models/concerns/stats_versionable.rb
View File

@@ -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

1
app/models/poll.rb
View File

@@ -7,7 +7,6 @@ class Poll < ApplicationRecord
include Notifiable
include Searchable
include Sluggable
include StatsVersionable
include Reportable
include SDG::Relatable

4
app/models/poll/stats.rb
View File

@@ -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}"
end
end

5
app/models/stats_version.rb
View File

@@ -1,5 +0,0 @@
class StatsVersion < ApplicationRecord
validates :process, presence: true
belongs_to :process, polymorphic: true
end

4
config/schedule.rb
View File

@@ -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

12
db/migrate/20240408154814_drop_stats_versions.rb Normal file
View File

@@ -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

8
db/schema.rb
View File

@@ -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"

32
lib/tasks/stats.rake
View File

@@ -1,32 +0,0 @@
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
[Budget, Poll].each do |model_class|
model_class.find_each { |record| record.find_or_create_stats_version.touch }
end
end
end
desc "Deletes stats cache and generates it again"
task regenerate: [:expire_cache, :generate]
end

44
spec/lib/geozone_stats_spec.rb
View File

@@ -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

28
spec/models/poll/stats_spec.rb
View File

@@ -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

75
spec/models/statisticable_spec.rb
View File

@@ -4,10 +4,21 @@ 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
end
alias_method :participants_from_original_table, :participants
def total_participants
User.count
end
def participation_date
Time.current
@@ -17,7 +28,8 @@ describe Statisticable do
stub_const("DummyStats", dummy_stats)
end
let(:stats) { DummyStats.new(nil) }
let(:resource) { double(id: 1, class: double(table_name: "")) }
let(:stats) { DummyStats.new(resource) }
describe "#gender?" do
context "No participants" do
@@ -194,4 +206,65 @@ 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
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
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
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

13
spec/models/stats_version_spec.rb
View File

@@ -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

5
spec/spec_helper.rb
View File

@@ -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"

53
spec/system/admin/stats_spec.rb
View File

@@ -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