Skip to content

Commit

Permalink
Generate stats on every request without caching them
Browse files Browse the repository at this point in the history
In commit e51e034, 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.

But, since now we're using a temporary table to calculate the stats,
generating them might take between 1 and 2 seconds even in processes
with a hundred thousand participants, meaning we can disable the cache
and generate the stats on every request instead. It might not scale so
well when there are millions of participants; we might re-enable the
cache or find a different way to optimize the stats calculations if we
ever get to that point.

Note that, since we're only using the temporary table inside the
`generate` method, we need to cache the results obtained inside this
method so they're available elsewhere. We're using instance variables
for this purpose. This means the code doesn't improve much with this
change, but at least we don't have to worry about when we should expire
the cache.

Since loading stats in the admin section is fast even without the cache
because they only load very basic statistics, we're not even generating
them first in this case, so everything works the same way it did before
commit e51e034.

Co-authored-by: Javi Martín <javim@elretirao.net>
  • Loading branch information
microweb10 and javierm committed Apr 18, 2024
1 parent fe7f61b commit 5d84035
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 10 deletions.
4 changes: 0 additions & 4 deletions app/models/budget/stats.rb
Expand Up @@ -179,8 +179,4 @@ def supports(supportable)
end

stats_cache(*stats_methods)

def full_cache_key_for(key)
"budgets_stats/#{budget.id}/#{phases.join}/#{key}"
end
end
10 changes: 8 additions & 2 deletions app/models/concerns/statisticable.rb
Expand Up @@ -222,7 +222,13 @@ def range_description(start, finish)
end
end

def stats_cache(key, &)
Rails.cache.fetch(full_cache_key_for(key), expires_in: 1.hour, &)
def stats_cache(key, &block)
instance_variable_name = key.to_s.gsub("?", "_question")

unless instance_variable_get("@#{instance_variable_name}")
instance_variable_set("@#{instance_variable_name}", block.call)
end

instance_variable_get("@#{instance_variable_name}")
end
end
4 changes: 0 additions & 4 deletions app/models/poll/stats.rb
Expand Up @@ -116,8 +116,4 @@ def total_unregistered_booth
end

stats_cache(*stats_methods)

def full_cache_key_for(key)
"polls_stats/#{poll.id}/#{key}"
end
end
53 changes: 53 additions & 0 deletions spec/system/admin/stats_spec.rb
Expand Up @@ -111,6 +111,33 @@
expect(page).to have_link "Go back", count: 1
end

scenario "Clear cache after new 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"
Expand Down Expand Up @@ -147,6 +174,32 @@
expect(page).to have_content "VOTES\n3"
expect(page).to have_content "PARTICIPANTS\n2"
end

scenario "Clear cache after new 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

Expand Down

0 comments on commit 5d84035

Please sign in to comment.