Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change stats layout #1951

Merged
merged 84 commits into from
Apr 11, 2019
Merged

Change stats layout #1951

merged 84 commits into from
Apr 11, 2019

Conversation

javierm
Copy link

@javierm javierm commented Apr 4, 2019

References

This pull request includes the changes in these pull requests:

Objectives

  • Include stats by gender, age and geozone for both polls and budgets
  • Unify the code and the interface of budget and poll stats
  • Use an improved layout for stats

Visual Changes

Before these changes:

Generic and advanced budget stats are shown together

After these changes:

Gender and age stats are displayed first, and there's a menu to navigate through the stats

Advanced stats are displayed after generic stats

Does this PR need a Backport to CONSUL?

Yes. When backporting, also backport the part of #1975 related to stats.

Release notes

⚠️ If you'd like to generate stats for your existing polls and budgets, execute:

RAILS_ENV=production bin/rake stats:generate

end
second_total = Poll::Answer.where(answer: "En blanco", question: poll.questions.second).pluck(:author_id).count
second_total -= (Poll::Answer.where(answer: poll.questions.first.question_answers.where(given_order: 1).first.title, question: poll.questions.first).pluck(:author_id) & Poll::Answer.where(answer: "En blanco", question: poll.questions.second).pluck(:author_id)).uniq.count
second_total -= (Poll::Answer.where(answer: poll.questions.first.question_answers.where(given_order: 2).first.title, question: poll.questions.first).pluck(:author_id) & Poll::Answer.where(answer: "En blanco", question: poll.questions.second).pluck(:author_id)).uniq.count

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Metrics/LineLength: Line is too long. [275/100] (https://github.com/bbatsov/ruby-style-guide#80-character-limits)

0
end
second_total = Poll::Answer.where(answer: "En blanco", question: poll.questions.second).pluck(:author_id).count
second_total -= (Poll::Answer.where(answer: poll.questions.first.question_answers.where(given_order: 1).first.title, question: poll.questions.first).pluck(:author_id) & Poll::Answer.where(answer: "En blanco", question: poll.questions.second).pluck(:author_id)).uniq.count

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Metrics/LineLength: Line is too long. [275/100] (https://github.com/bbatsov/ruby-style-guide#80-character-limits)

def total_web_null
0
end
second_total = Poll::Answer.where(answer: "En blanco", question: poll.questions.second).pluck(:author_id).count

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Metrics/LineLength: Line is too long. [116/100] (https://github.com/bbatsov/ruby-style-guide#80-character-limits)

double_white = (Poll::Answer.where(answer: "En blanco", question: poll.questions.first).pluck(:author_id) & Poll::Answer.where(answer: "En blanco", question: poll.questions.second).pluck(:author_id)).uniq.count
first_total = Poll::Answer.where(answer: "En blanco", question: poll.questions.first).pluck(:author_id).count
first_total -= (Poll::Answer.where(answer: "En blanco", question: poll.questions.first).pluck(:author_id) & Poll::Answer.where(answer: poll.questions.second.question_answers.where(given_order: 1).first.title, question: poll.questions.second).pluck(:author_id)).uniq.count
first_total -= (Poll::Answer.where(answer: "En blanco", question: poll.questions.first).pluck(:author_id) & Poll::Answer.where(answer: poll.questions.second.question_answers.where(given_order: 2).first.title, question: poll.questions.second).pluck(:author_id)).uniq.count

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Metrics/LineLength: Line is too long. [275/100] (https://github.com/bbatsov/ruby-style-guide#80-character-limits)

return 0 unless poll.questions.second.present?
double_white = (Poll::Answer.where(answer: "En blanco", question: poll.questions.first).pluck(:author_id) & Poll::Answer.where(answer: "En blanco", question: poll.questions.second).pluck(:author_id)).uniq.count
first_total = Poll::Answer.where(answer: "En blanco", question: poll.questions.first).pluck(:author_id).count
first_total -= (Poll::Answer.where(answer: "En blanco", question: poll.questions.first).pluck(:author_id) & Poll::Answer.where(answer: poll.questions.second.question_answers.where(given_order: 1).first.title, question: poll.questions.second).pluck(:author_id)).uniq.count

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Metrics/LineLength: Line is too long. [275/100] (https://github.com/bbatsov/ruby-style-guide#80-character-limits)

User.where(id: (authors + voters + balloters + poll_ballot_voters).uniq.compact)
end
groups[:total][:percentage_participants_support_phase] = groups.collect {|_k, v| v[:percentage_participants_support_phase]}.sum
groups[:total][:percentage_participants_vote_phase] = groups.collect {|_k, v| v[:percentage_participants_vote_phase]}.sum

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Metrics/LineLength: Line is too long. [125/100] (https://github.com/bbatsov/ruby-style-guide#80-character-limits)

def participants
User.where(id: (authors + voters + balloters + poll_ballot_voters).uniq.compact)
end
groups[:total][:percentage_participants_support_phase] = groups.collect {|_k, v| v[:percentage_participants_support_phase]}.sum

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Metrics/LineLength: Line is too long. [131/100] (https://github.com/bbatsov/ruby-style-guide#80-character-limits)

def total_supports
supports(budget).count
budget.headings.each do |heading|
groups[heading.id].merge!(calculate_heading_stats_with_totals(groups[heading.id], groups[:total], heading.population))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Metrics/LineLength: Line is too long. [124/100] (https://github.com/bbatsov/ruby-style-guide#80-character-limits)

app/models/budget/stats.rb Outdated Show resolved Hide resolved
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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Metrics/LineLength: Line is too long. [115/100] (https://github.com/bbatsov/ruby-style-guide#80-character-limits)

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])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Metrics/LineLength: Line is too long. [130/100] (https://github.com/bbatsov/ruby-style-guide#80-character-limits)

budget.investments.unfeasible.count
groups[:total][:percentage_participants_support_phase] = groups.collect {|_k, v| v[:percentage_participants_support_phase]}.sum
groups[:total][:percentage_participants_vote_phase] = groups.collect {|_k, v| v[:percentage_participants_vote_phase]}.sum
groups[:total][:percentage_participants_every_phase] = groups.collect {|_k, v| v[:percentage_participants_every_phase]}.sum

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Metrics/LineLength: Line is too long. [127/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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Metrics/LineLength: Line is too long. [117/100] (https://github.com/bbatsov/ruby-style-guide#80-character-limits)

budget.investments.selected.count
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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Metrics/LineLength: Line is too long. [121/100] (https://github.com/bbatsov/ruby-style-guide#80-character-limits)

def total_selected_investments
budget.investments.selected.count
groups[:total] = Hash.new(0)
groups[:total][:total_investments_count] = groups.collect {|_k, v| v[:total_investments_count]}.sum

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Metrics/LineLength: Line is too long. [103/100] (https://github.com/bbatsov/ruby-style-guide#80-character-limits)

@javierm javierm changed the title [WIP] Change stats layout Change stats layout Apr 10, 2019
end

def balloters
budget.ballots.where('ballot_lines_count > ?', 0).pluck(:user_id)
@balloters ||= budget.ballots.where("ballot_lines_count > ?", 0).distinct.pluck(:user_id).compact

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Metrics/LineLength: Line is too long. [103/100] (https://github.com/bbatsov/ruby-style-guide#80-character-limits)

javierm and others added 19 commits April 11, 2019 16:54
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`.
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.
@javierm
Copy link
Author

javierm commented Apr 11, 2019

Travis failure was related to issue consul#3325 and not to this pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants