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.
It is way more efficient because we're caching the result of that
method, and this way we only store each voter once in the cache. We were
storing many voters several times and then we were filtering them with
`uniq`.
So if we don't have information regarding gender, age or geozone, stats
regarding those topics will not be shown.
Note we're using `spec/models/statisticable_spec.rb` because having the
same file in `spec/models/concerns` caused the tests to be executed
twice.
Also note the implementation behind the `gender?`, `age?` and `geozone?`
methods is a bit primitive. We might need to make it more robust in the
future.
It will make it far easier to call other methods on the stats object,
and we're already caching the methods.
We had to remove the view fragment caching because the stats object
isn't as easy to cache. The good thing about it is the view will
automatically be updated when we change logic regarding which stats to
show, and the methods taking long to execute are cached in the model.
For now we think showing them would be showing too much data and it
would be a bit confusing.
I've been tempted to just remove the view and keep the methods in the
model in case they're used by other institutions using CONSUL. However,
it's probably better to wait until we're asked to re-implement them, and
in the meantime we don't maintain code nobody uses. The code wasn't that
great to start with (I know it because I wrote it).
We were expecting `balloters` to include `poll_ballot_voters` (that's
why we're substracting them to calculate web participants), but reality
has proven `poll_ballot_voters` aren't included in `balloters`.
We didn't use metaprogramming from the start because the
`null_percentage_web` method had a particular behaviour.
However, the behaviour (due to a typo) didn't really matter because
there are no null web votes, and so the `null_percentage_web` is always
zero.
While we already had "one test to rule all stats", testing each method
individually makes reading, adding and changing tests easier.
Note we need to make all methods being tested public. We could also test
them using methods like `stats.generate[:total_valid_votes]` instead of
`stats.total_valid_votes`, but then the tests would be more difficult to
read.