Change stats layout #3512
Merged
Change stats layout #3512
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
houndci-bot
reviewed
May 20, 2019
| percentage_participants_all_phase: participants_percent(heading_totals, groups_totals, :total_participants_all_phase), | ||
| percentage_district_population_all_phase: population_percent(population, heading_totals[:total_participants_all_phase]) | ||
| percentage_participants_every_phase: participants_percent(heading_totals, groups_totals, :total_participants_every_phase), | ||
| percentage_district_population_every_phase: population_percent(population, heading_totals[:total_participants_every_phase]) |
There was a problem hiding this comment.
Metrics/LineLength: Line is too long. [131/100] (https://github.com/bbatsov/ruby-style-guide#80-character-limits)
| @@ -122,8 +151,8 @@ def calculate_heading_stats_with_totals(heading_totals, groups_totals, populatio | |||
| percentage_district_population_support_phase: population_percent(population, heading_totals[:total_participants_support_phase]), | |||
| percentage_participants_vote_phase: participants_percent(heading_totals, groups_totals, :total_participants_vote_phase), | |||
| percentage_district_population_vote_phase: population_percent(population, heading_totals[:total_participants_vote_phase]), | |||
| percentage_participants_all_phase: participants_percent(heading_totals, groups_totals, :total_participants_all_phase), | |||
| percentage_district_population_all_phase: population_percent(population, heading_totals[:total_participants_all_phase]) | |||
| percentage_participants_every_phase: participants_percent(heading_totals, groups_totals, :total_participants_every_phase), | |||
There was a problem hiding this comment.
Metrics/LineLength: Line is too long. [130/100] (https://github.com/bbatsov/ruby-style-guide#80-character-limits)
| groups[:total][:total_participants_every_phase] = groups.collect {|_k, v| v[:total_participants_every_phase]}.sum | ||
|
|
||
| budget.headings.each do |heading| | ||
| groups[heading.id].merge!(calculate_heading_stats_with_totals(groups[heading.id], groups[:total], heading.population)) |
There was a problem hiding this comment.
Metrics/LineLength: Line is too long. [124/100] (https://github.com/bbatsov/ruby-style-guide#80-character-limits)
| groups[:total][:total_investments_count] = groups.collect {|_k, v| v[:total_investments_count]}.sum | ||
| groups[:total][:total_participants_support_phase] = groups.collect {|_k, v| v[:total_participants_support_phase]}.sum | ||
| groups[:total][:total_participants_vote_phase] = groups.collect {|_k, v| v[:total_participants_vote_phase]}.sum | ||
| groups[:total][:total_participants_every_phase] = groups.collect {|_k, v| v[:total_participants_every_phase]}.sum |
There was a problem hiding this comment.
Metrics/LineLength: Line is too long. [117/100] (https://github.com/bbatsov/ruby-style-guide#80-character-limits)
| groups[:total] = Hash.new(0) | ||
| groups[:total][:total_investments_count] = groups.collect {|_k, v| v[:total_investments_count]}.sum | ||
| groups[:total][:total_participants_support_phase] = groups.collect {|_k, v| v[:total_participants_support_phase]}.sum | ||
| groups[:total][:total_participants_vote_phase] = groups.collect {|_k, v| v[:total_participants_vote_phase]}.sum |
There was a problem hiding this comment.
Metrics/LineLength: Line is too long. [115/100] (https://github.com/bbatsov/ruby-style-guide#80-character-limits)
| budget.ballots.pluck(:ballot_lines_count).inject(0) { |sum, x| sum + x } | ||
| groups[:total] = Hash.new(0) | ||
| groups[:total][:total_investments_count] = groups.collect {|_k, v| v[:total_investments_count]}.sum | ||
| groups[:total][:total_participants_support_phase] = groups.collect {|_k, v| v[:total_participants_support_phase]}.sum |
There was a problem hiding this comment.
Metrics/LineLength: Line is too long. [121/100] (https://github.com/bbatsov/ruby-style-guide#80-character-limits)
| def total_votes | ||
| budget.ballots.pluck(:ballot_lines_count).inject(0) { |sum, x| sum + x } | ||
| groups[:total] = Hash.new(0) | ||
| groups[:total][:total_investments_count] = groups.collect {|_k, v| v[:total_investments_count]}.sum |
There was a problem hiding this comment.
Metrics/LineLength: Line is too long. [103/100] (https://github.com/bbatsov/ruby-style-guide#80-character-limits)
javierm
force-pushed
the
backport-gender_participants
branch
from
15e4d27
to
f754740
Compare
javierm
force-pushed
the
backport-stats
branch
2 times, most recently
from
361e2ac
to
5b737ed
Compare
decabeza
approved these changes
May 21, 2019
javierm
force-pushed
the
backport-gender_participants
branch
from
f754740
to
d2f4ef2
Compare
javierm
force-pushed
the
backport-gender_participants
branch
from
d2f4ef2
to
e4a032e
Compare
We would now like to differenciate between 70-year-old people and 90-year-old people.
This way we can easily see the h3 tag's parent is the h2 tag.
We try to make the method return data which is easier to handle in the view.
The rest of the `before` block still uses instance variables, but at least the rest of the file doesn't use instance variables anymore.
So related methods are on the same line.
Naming it "Dropdown" was misleading.
Turbolinks doesn't get on well with Foundation's Sticky, and so we need to manually trigger the event on Turbolinks' `page:load`.
The code is easier to read now, it returns the same results it used to return, and performance-wise it's probably the same thing, but if it's not, we'll trust Rails will do optimizations that we don't when we manually pluck the IDs.
We currently don't store geozone population.
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. Note we keep the method `participants_by_geozone` to return a hash because we're caching the stats and storing GeozoneStats objects would need a lot more memory and we would get an error.
If there's demographic data for all participants, it doesn't make sense to show the message. We're using translations instead of an `if` in the view because the text is also different when there's only one participant. In some languages the text might also be different depending on how many people with no demographic data participated. Another possibility would be to use an `if` in the view so we don't display an empty paragraph when the cont is zero, and then using translation for `one` and `other`. I haven't gone that way because I thought the logic would be more complex and the benefits wouldn't be that great.
As the Rails guides say: > All scope methods will return an ActiveRecord::Relation object That means `find_by_kind` will return a relation when nothing is found; the expected behaviour is to return `nil`, like all `find_by` methods do. Using scopes also means strange things happen when we try to chain scopes like `phases.published.drafting`. With scopes, the `drafting` part would be ignored and all published phases would be returned.
This implementation is a bit more robust because we don't have to change any of the "or_later?" methods if we add or remove a new phase. We could also use metaprogramming to reduce code duplication in these methods. So far, I've decided to keep the code simple since the duplication seems reasonable.
We're going to use it so we know if a budget has finished its support phase.
So these styles are available in CONSUL. Note we're not including these styles inside `.participation-stats` because this class is used in Plaza de España's statistics.
"All phase" doesn't sound right in English, and we're going to refactor the code related to the phases.
This way we can show statistics for the supports phase before the vote phase is over.
This way we recalculate all data including the participants in the phase which has just finished.
This way we can share the `participants` method between budget and poll stats.
If users participated and were hidden after participating, we should still count them in the participants stats. In the tests, we set users' `hidden_at` attribute before they vote. Although in real life they would vote first and then they would be hidden, I've written the tests like this for the sake of simplicity.
We need a way to manually expire the cache for a budget or poll without expiring the cache of every budget or poll. Using the `updated_at` column would be dangerous because most of the times we update a budget or a poll, we don't need to regenerate their stats. We've considered adding a `stats_updated_at` column to each of these tables. However, in that case we would also need to add a similar column in the future to every process type whose stats we want to generate.
These methods are only used while stats are being generated; once stats are generated, they aren't used anymore. So there's no need to store them using the Dalli cache. Furthermore, there are polls (and even budgets) with hundreds of thousands of participants. Calculating stats for them takes a very long time because we can't store all those records in the Dalli cache. However, since these records aren't used once the stats are generated, we can store them in an instance variable while we generate the stats, speeding up the process.
We're generating stats every 2 hours because it's less than the time it will take to generate stats for every process. Once stats are generated, this task should take less than a second. The regenerate task has been added so we can manually execute it.
Using SQL's `select` instead of converting the records to a ruby array increases performance dramatically when there are thousands of records. For a poll with 200000 voters, calculating stats took more than 7 minutes, and now it takes less than 2 minutes.
Methods defined inside "included" cannot be called using `super` from a class including the module.
Due to technical issues, sometimes users voted in booths and their vote couldn't be added to the database. So we're including them in the users with no demographic data.
This was referenced May 23, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
References
Objectives
Visual Changes
Before these changes:
After these changes:
Release notes