From 793bfed37298fdb96048efa53e7c9396c511fb67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 18 Mar 2019 18:07:09 +0100 Subject: [PATCH] Display only existing stats 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. --- app/models/concerns/statisticable.rb | 57 ++++++- app/views/budgets/stats/show.html.erb | 2 +- app/views/polls/stats.html.erb | 2 +- app/views/shared/stats/_age.html.erb | 28 ++++ app/views/shared/stats/_gender.html.erb | 15 ++ app/views/shared/stats/_geozone.html.erb | 23 +++ app/views/shared/stats/_links.html.erb | 15 +- .../shared/stats/_participation.html.erb | 72 +-------- spec/models/statisticable_spec.rb | 148 ++++++++++++++++++ 9 files changed, 277 insertions(+), 85 deletions(-) create mode 100644 app/views/shared/stats/_age.html.erb create mode 100644 app/views/shared/stats/_gender.html.erb create mode 100644 app/views/shared/stats/_geozone.html.erb create mode 100644 spec/models/statisticable_spec.rb diff --git a/app/models/concerns/statisticable.rb b/app/models/concerns/statisticable.rb index 411bec21c..5c6289ed1 100644 --- a/app/models/concerns/statisticable.rb +++ b/app/models/concerns/statisticable.rb @@ -1,5 +1,6 @@ module Statisticable extend ActiveSupport::Concern + PARTICIPATIONS = %w[gender age geozone] included do attr_reader :resource @@ -9,7 +10,27 @@ module Statisticable end def generate - self.class.stats_methods.each { |stat_name| send(stat_name) } + stats_methods.each { |stat_name| send(stat_name) } + end + + def stats_methods + base_stats_methods + participation_methods + end + + def participations + PARTICIPATIONS.select { |participation| send("#{participation}?") } + end + + def gender? + participants.male.any? || participants.female.any? + end + + def age? + participants.between_ages(age_groups.flatten.min, age_groups.flatten.max).any? + end + + def geozone? + participants.where(geozone: geozones).any? end def total_male_participants @@ -66,6 +87,14 @@ module Statisticable private + def base_stats_methods + self.class.base_stats_methods + end + + def participation_methods + participations.map { |participation| self.class.send("#{participation}_methods") }.flatten + end + def total_participants_with_gender participants.where.not(gender: nil).distinct.count end @@ -115,10 +144,28 @@ module Statisticable class_methods do def stats_methods - %i[total_participants - total_male_participants total_female_participants total_unknown_gender_or_age - male_percentage female_percentage - participants_by_age participants_by_geozone] + base_stats_methods + gender_methods + age_methods + geozone_methods + end + + def base_stats_methods + %i[total_participants participations] + participation_check_methods + end + + def participation_check_methods + PARTICIPATIONS.map { |participation| :"#{participation}?" } + end + + def gender_methods + %i[total_male_participants total_female_participants total_unknown_gender_or_age + male_percentage female_percentage] + end + + def age_methods + [:participants_by_age] + end + + def geozone_methods + [:participants_by_geozone] end def stats_cache(*method_names) diff --git a/app/views/budgets/stats/show.html.erb b/app/views/budgets/stats/show.html.erb index 7edf44342..5ce48bd88 100644 --- a/app/views/budgets/stats/show.html.erb +++ b/app/views/budgets/stats/show.html.erb @@ -40,7 +40,7 @@
- <%= render "shared/stats/links" %> + <%= render "shared/stats/links", stats: @stats %>

<%= link_to t("stats.advanced"), "#advanced_statistics" %>

    diff --git a/app/views/polls/stats.html.erb b/app/views/polls/stats.html.erb index 55d103c97..da2a8351c 100644 --- a/app/views/polls/stats.html.erb +++ b/app/views/polls/stats.html.erb @@ -7,7 +7,7 @@
    - <%= render "shared/stats/links" %> + <%= render "shared/stats/links", stats: @stats %>

    <%= link_to t("stats.advanced"), "#advanced_statistics" %>

      diff --git a/app/views/shared/stats/_age.html.erb b/app/views/shared/stats/_age.html.erb new file mode 100644 index 000000000..5114c8b8c --- /dev/null +++ b/app/views/shared/stats/_age.html.erb @@ -0,0 +1,28 @@ +
      +

      <%= t("stats.by_age") %>

      + + + + + + + + + + + <% stats.participants_by_age.values.each do |group| %> + + + + + <% end %> + +
      <%= t("stats.age") %><%= t("stats.total") %>
      <%= group[:range] %> + + <%= "#{group[:count]} (#{number_to_stats_percentage(group[:percentage])})" %> + +
      + +
      +
      +
      diff --git a/app/views/shared/stats/_gender.html.erb b/app/views/shared/stats/_gender.html.erb new file mode 100644 index 000000000..7c34acce7 --- /dev/null +++ b/app/views/shared/stats/_gender.html.erb @@ -0,0 +1,15 @@ +
      +

      <%= t("stats.by_gender") %>

      + + <%= number_with_info_tags( + stats.total_male_participants, + t("stats.men_percentage", percentage: number_to_stats_percentage(stats.male_percentage)), + html_class: "participants male" + ) %> + + <%= number_with_info_tags( + stats.total_female_participants, + t("stats.women_percentage", percentage: number_to_stats_percentage(stats.female_percentage)), + html_class: "participants female" + ) %> +
      diff --git a/app/views/shared/stats/_geozone.html.erb b/app/views/shared/stats/_geozone.html.erb new file mode 100644 index 000000000..683ae951e --- /dev/null +++ b/app/views/shared/stats/_geozone.html.erb @@ -0,0 +1,23 @@ +
      +

      <%= t("stats.by_geozone") %>

      + + + + + + + + + + + + <% stats.participants_by_geozone.each do |geozone, participants| %> + + + + + + <% end %> + +
      <%= t("stats.geozone") %><%= t("stats.total") %><%= t("stats.geozone_participation") %>
      <%= geozone %><%= "#{participants[:total][:count]} (#{number_to_stats_percentage(participants[:total][:percentage])})" %><%= number_to_stats_percentage(participants[:percentage]) %>
      +
      diff --git a/app/views/shared/stats/_links.html.erb b/app/views/shared/stats/_links.html.erb index 581cce4b2..8e869c45a 100644 --- a/app/views/shared/stats/_links.html.erb +++ b/app/views/shared/stats/_links.html.erb @@ -3,13 +3,10 @@
    • <%= link_to t("stats.total_participants"), "#total_participants" %>
      • -
    • - <%= link_to t("stats.by_gender"), "#participants_by_gender" %> -
      • -
    • - <%= link_to t("stats.by_age"), "#participants_by_age" %> -
      • -
    • - <%= link_to t("stats.by_geozone"), "#participants_by_geozone" %> -
      • + + <% stats.participations.each do |stat| %> +
    • + <%= link_to t("stats.by_#{stat}"), "#participants_by_#{stat}" %> +
      • + <% end %>
    diff --git a/app/views/shared/stats/_participation.html.erb b/app/views/shared/stats/_participation.html.erb index 0a4bd2f06..d15123dc2 100644 --- a/app/views/shared/stats/_participation.html.erb +++ b/app/views/shared/stats/_participation.html.erb @@ -11,73 +11,7 @@ ) %>
    -
    -

    <%= t("stats.by_gender") %>

    - - <%= number_with_info_tags( - stats.total_male_participants, - t("stats.men_percentage", percentage: number_to_stats_percentage(stats.male_percentage)), - html_class: "participants male" - ) %> - - <%= number_with_info_tags( - stats.total_female_participants, - t("stats.women_percentage", percentage: number_to_stats_percentage(stats.female_percentage)), - html_class: "participants female" - ) %> -
    - -
    -

    <%= t("stats.by_age") %>

    - - - - - - - - - - - <% stats.participants_by_age.values.each do |group| %> - - - - - <% end %> - -
    <%= t("stats.age") %><%= t("stats.total") %>
    <%= group[:range] %> - - <%= "#{group[:count]} (#{number_to_stats_percentage(group[:percentage])})" %> - -
    - - -
    -
    -
    - -
    -

    <%= t("stats.by_geozone") %>

    - - - - - - - - - - - - <% stats.participants_by_geozone.each do |geozone, participants| %> - - - - - - <% end %> - -
    <%= t("stats.geozone") %><%= t("stats.total") %><%= t("stats.geozone_participation") %>
    <%= geozone %><%= "#{participants[:total][:count]} (#{number_to_stats_percentage(participants[:total][:percentage])})" %><%= number_to_stats_percentage(participants[:percentage]) %>
    -
    + <% stats.participations.each do |participation| %> + <%= render "shared/stats/#{participation}", stats: stats %> + <% end %>
    diff --git a/spec/models/statisticable_spec.rb b/spec/models/statisticable_spec.rb new file mode 100644 index 000000000..9794ef7bc --- /dev/null +++ b/spec/models/statisticable_spec.rb @@ -0,0 +1,148 @@ +require "rails_helper" + +describe Statisticable do + class DummyStats + include Statisticable + + def participants + User.all + end + end + + let(:stats) { DummyStats.new(nil) } + + describe "#gender?" do + context "No participants" do + it "is false" do + expect(stats.gender?).to be false + end + end + + context "All participants have no defined gender" do + before { create(:user, gender: nil) } + + it "is false" do + expect(stats.gender?).to be false + end + end + + context "There's a male participant" do + before { create(:user, gender: "male") } + + it "is true" do + expect(stats.gender?).to be true + end + end + + context "There's a female participant" do + before { create(:user, gender: "female") } + + it "is true" do + expect(stats.gender?).to be true + end + end + end + + describe "#age?" do + context "No participants" do + it "is false" do + expect(stats.age?).to be false + end + end + + context "All participants have no defined age" do + before { create(:user, date_of_birth: nil) } + + it "is false" do + expect(stats.age?).to be false + end + end + + context "All participants have impossible ages" do + before do + create(:user, date_of_birth: 3.seconds.ago) + create(:user, date_of_birth: 3000.years.ago) + end + + it "is false" do + expect(stats.age?).to be false + end + end + + context "There's a participant with a defined age" do + before { create(:user, date_of_birth: 30.years.ago) } + + it "is true" do + expect(stats.age?).to be true + end + end + end + + describe "#geozone?" do + context "No participants" do + it "is false" do + expect(stats.geozone?).to be false + end + end + + context "All participants have no defined geozone" do + before { create(:user, geozone: nil) } + + it "is false" do + expect(stats.geozone?).to be false + end + end + + context "There's a participant with a defined geozone" do + before { create(:user, geozone: create(:geozone)) } + + it "is true" do + expect(stats.geozone?).to be true + end + end + end + + describe "#stats_methods" do + it "includes total participants" do + expect(stats.stats_methods).to include(:total_participants) + end + + context "no gender stats" do + before { allow(stats).to receive(:gender?).and_return(false) } + + it "doesn't include gender methods" do + expect(stats.stats_methods).not_to include(:total_male_participants) + end + end + + context "no age stats" do + before { allow(stats).to receive(:age?).and_return(false) } + + it "doesn't include age methods" do + expect(stats.stats_methods).not_to include(:participants_by_age) + end + end + + context "no geozone stats" do + before { allow(stats).to receive(:geozone?).and_return(false) } + + it "doesn't include age methods" do + expect(stats.stats_methods).not_to include(:participants_by_geozone) + end + end + + context "all gender, age and geozone stats" do + before do + allow(stats).to receive(:gender?).and_return(true) + allow(stats).to receive(:age?).and_return(true) + allow(stats).to receive(:geozone?).and_return(true) + end + + it "includes all stats methods" do + expect(stats.stats_methods).to include(:total_male_participants) + expect(stats.stats_methods).to include(:participants_by_age) + expect(stats.stats_methods).to include(:participants_by_geozone) + end + end + end +end