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

Hxl stats view 145 147 #180

Merged
merged 26 commits into from
Aug 28, 2017
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions app/controllers/hxlstats_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
class HxlstatsController < ApplicationController
def show
@results = HxlStatsView.results_by_emotional_state

respond_to do |format|
format.html
format.json { send_data @results, filename: "hxlstats-#{Date.today}.json" }
end
end
end
109 changes: 109 additions & 0 deletions app/models/hxl_stats_view.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
HXLSTATS_COLUMN_HEADERS =
Copy link
Member

Choose a reason for hiding this comment

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

Any reason line 1 - 66 are outside of the class? Let's pop them back in. The constants (in this case HXLSTATS_COLUMN_HEADERS and HXL_STATS_TAGS) usually would live right at the top of the class (but inside it), and the methods get_counts_by_gender and get_counts_by_age can be private methods within the class. I.e. stick them under the keyword private.

["Emotional State",
"Stage of journey",
"Country drawn in",
"Total children affected",
"Children who identify as female",
"Children who identify as male",
"Children who identify as neither female nor male",
"Children between the ages of 5-12",
"Children between the ages of 13-18",
"Children under 5 years old",
"Older than 18 years old"].freeze

HXL_STATS_TAGS =
["#impact+indicator+code",
"#affected+children",
"#country+code",
"#affected+children+total",
"#affected+children+female",
"#affected+children+male",
"#affected+children+other_gender",
"#affected+children+age_5_12",
"#affected+children+age_13_18",
"#affected+children+age_under5",
"#affected+children+age_18plus"].freeze

def get_counts_by_gender(hxlstatsmajorgroup)
# Group by gender and aggregate
gender_totals = Hash.new(0)
@hxlstatsgroupsgender = hxlstatsmajorgroup.group_by(&:gender) # shorthand for { |hxlstat| hxlstat.gender }
Copy link
Member

Choose a reason for hiding this comment

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

No need for an instance variable here as it will only be used locally within the method. So @hxlstatsgroupsgender -> hxlstatsgroupsgender

@hxlstatsgroupsgender.each do |k_gender, v_gender|
# puts " cycle genders"
# puts " %s " % k_gender
# puts " %s " % v_gender
gender_totals[k_gender] = v_gender.count
end
gender_totals # hash of key gender to v counts
end

# rubocop:disable MethodLength
def get_counts_by_age(hxlstatsmajorgroup)
# Processing Age aggregations
@hxlstatsgroupsage = hxlstatsmajorgroup.group_by(&:age) # shorthand for { |hxlstat| hxlstat.age }
Copy link
Member

Choose a reason for hiding this comment

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

No need for an instance variable here as it will only be used locally within the method. So @hxlstatsgroupsage -> hxlstatsgroupsage

# gender *and* age inclusiveness
younger_than_five = 0
five_twelve_total = 0
thirteen_eighteen_total = 0
older_total = 0

@hxlstatsgroupsage.each do |k_age, _v_age|
# k_age is age, _v_age is array of hxlstatsmatview
# puts " cycle ages"
# puts " %s" % k_age
# puts " %s" % _v_age.count
if k_age < 5
younger_than_five += 1
elsif k_age >= 5 && k_age < 13
five_twelve_total += 1
elsif k_age >= 13 && k_age <= 18
thirteen_eighteen_total += 1
else
older_total += 1
end
end
[younger_than_five, five_twelve_total, thirteen_eighteen_total, older_total]
end

class HxlStatsView < ActiveRecord::Base
# View aggregates data in a multi-nested-group by for the HXL proxy/endpoint
#
self.table_name = 'hxl_stats_view'
def readonly?
true
end

@gender_lookup = { 'male' => 0,
Copy link
Member

Choose a reason for hiding this comment

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

We already conveniently have the gender mapping available as an enum in the Drawing model. We can reference that in the hxl_stats_counts method rather than create a new hash here to avoid repetition.

I.e.

Drawing.genders

# Response => {"not_specified"=>0, "female"=>1, "male"=>2, "other"=>3}

Drawing.genders["female"]

# Response => 1

What value does hxl_stats_counts return for drawings with 'other' i.e. 3? We might need to update the logic so anything with 0 or 3 falls into 'other' for now.

'female' => 1,
'other_gender' => 2 }

def self.hxl_stats_counts
results = []
@hxlstats = HxlStatsView.all
Copy link
Member

Choose a reason for hiding this comment

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

Same re replacing instance variable with local variable - @hxlstats -> hxlstats


# First group by major categories, before aggregating independently for gender and age and recombining
@hxlstatsgroups = @hxlstats.group_by { |hxlstat| [hxlstat.mood_rating, hxlstat.stage_of_journey, hxlstat.country] }
Copy link
Member

Choose a reason for hiding this comment

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

Same re replacing instance variable with local variable - @hxlstatsgroups -> hxlstatsgroups

@hxlstatsgroups.each do |keys, hxlstatsmajorgroup| # keys = mood_rating, stage_of_j, country
gender_totals = get_counts_by_gender(hxlstatsmajorgroup)
younger_than_five, five_twelve_total, thirteen_eighteen_total, older_total = get_counts_by_age(hxlstatsmajorgroup)
results.append([*keys,
hxlstatsmajorgroup.count,
gender_totals[@gender_lookup['female']], # 1
gender_totals[@gender_lookup['male']], # 0
gender_totals[@gender_lookup['other_gender']], # 2
five_twelve_total,
thirteen_eighteen_total,
younger_than_five,
older_total])
end
results
end

def self.results_by_emotional_state
@results = []
Copy link
Member

Choose a reason for hiding this comment

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

Same re replacing instance variable with local variable - @results -> results.

@results = @results.append(HXLSTATS_COLUMN_HEADERS)
@results = @results.append(HXL_STATS_TAGS)
@results = @results.concat(hxl_stats_counts)
@results
end
end
2 changes: 2 additions & 0 deletions app/views/hxlstats/new.html.haml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
%h1 Hxlstats#new
Copy link
Member

Choose a reason for hiding this comment

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

We can remove this too as we don't have a new route set up anymore.

%p Find me in app/views/hxlstats/new.html.haml
9 changes: 9 additions & 0 deletions app/views/hxlstats/show.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<pre>
[
<% @results.each do | line | %>
<%= line %>
<% end %>
]
</pre>


2 changes: 2 additions & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,7 @@

resources :drawings, :organisations

get 'hxlstats', to: 'hxlstats#show'

root 'drawings#index'
end
24 changes: 24 additions & 0 deletions db/migrate/20170601162837_create_hxl_stats_view.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
class CreateHxlStatsView < ActiveRecord::Migration
def up
execute <<-SQL
Copy link
Member

Choose a reason for hiding this comment

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

Run locally, some rubocop bits:

db/migrate/20170601162837_create_hxl_stats_view.rb:2:9: C: Trailing whitespace detected.
  def up
        ^
db/migrate/20170601162837_create_hxl_stats_view.rb:5:33: C: Trailing whitespace detected.
      SELECT d.id AS drawing_id,
                                ^
db/migrate/20170601162837_create_hxl_stats_view.rb:8:29: C: Trailing whitespace detected.
             o.id AS org_id,
                            ^^
db/migrate/20170601162837_create_hxl_stats_view.rb:11:29: C: Trailing whitespace detected.
                    users u,
                            ^
db/migrate/20170601162837_create_hxl_stats_view.rb:24:1: C: 1 trailing blank lines detected.

CREATE VIEW hxl_stats_view AS
SELECT d.id AS drawing_id,
d.*,
u.country as user_country,
o.id AS org_id,
o.name AS org_name
FROM drawings d,
users u,
organisations o
WHERE d.user_id = u.id AND
u.organisation_id = o.id
SQL
end

def down
execute <<-SQL
DROP VIEW IF EXISTS hxl_stats_view
SQL
end
end

2 changes: 1 addition & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 20170303152046) do
ActiveRecord::Schema.define(version: 20170601162837) do

# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
Expand Down
10 changes: 10 additions & 0 deletions spec/controllers/hxlstats_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
require 'rails_helper'

RSpec.describe HxlstatsController, type: :controller do
describe "GET #show" do
it "returns http success" do
get :show
expect(response).to have_http_status(:success)
end
end
end