From 309106ca6d733a09fac5232745cde0d5553a22e3 Mon Sep 17 00:00:00 2001 From: Harry Levine Date: Fri, 21 Jul 2017 09:40:58 -0600 Subject: [PATCH 1/9] Dedicate class for formatting data We use this logic now twice in user.rb. Adding it as a private method in the class triggered errors, and besides that, really was not a concern of user.rb. Abstracting this into a dedicated class for when we need to massage data, in future scenarios. --- app/lib/format_data.rb | 11 +++++++++++ test/lib/format_data_test.rb | 14 ++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 app/lib/format_data.rb create mode 100644 test/lib/format_data_test.rb diff --git a/app/lib/format_data.rb b/app/lib/format_data.rb new file mode 100644 index 00000000..e2f23e57 --- /dev/null +++ b/app/lib/format_data.rb @@ -0,0 +1,11 @@ +class FormatData + + # Converts a comma-separated string into an array of its contents. + # + # @param comma_separated_string [String] String of comma-separated values, i.e '80123, 80202' + # @return [Array] An array of strings, i.e. ['80123', '80202'] + # + def self.csv_to_array(comma_separated_string) + comma_separated_string.split(',').map(&:strip) + end +end diff --git a/test/lib/format_data_test.rb b/test/lib/format_data_test.rb new file mode 100644 index 00000000..e103c50d --- /dev/null +++ b/test/lib/format_data_test.rb @@ -0,0 +1,14 @@ +require 'test_helper' + +class FormatDataTest < ActiveSupport::TestCase + test '.csv_to_array' do + results = FormatData.csv_to_array '80202' + assert_equal ['80202'], results + + results = FormatData.csv_to_array '80202, 80126' + assert_equal ['80202', '80126'], results + + results = FormatData.csv_to_array 'TX, MN' + assert_equal ['TX', 'MN'], results + end +end From 2db4e87ec7646d692180033d8268de1beedda318 Mon Sep 17 00:00:00 2001 From: Harry Levine Date: Fri, 21 Jul 2017 10:15:10 -0600 Subject: [PATCH 2/9] Methods to get user count by state(s) --- app/models/user.rb | 12 ++++++++++++ test/models/user_test.rb | 19 +++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/app/models/user.rb b/app/models/user.rb index 16c76b5b..14d02596 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -29,6 +29,7 @@ class User < ApplicationRecord scope :mentors, -> { where(mentor: true) } scope :by_zip, ->(zip) { where(zip: zip) } + scope :by_state, ->(state) { where(state: state) } # Returns a count of all users with the passed in zip code(s) # @@ -40,6 +41,17 @@ def self.count_by_zip(zip_codes) zips = zip_codes.split(',').map(&:strip) by_zip(zips).count + # Returns a count of all users with the passed in state abbreviations. + # + # @param state_abbreviations [String] String of comma-separated state_abbreviations, i.e. 'CO', or 'CO, TX' + # @return [Integer] A count of the requested users. + # + def self.count_by_state(state_abbreviations) + return 0 unless state_abbreviations.present? + + states = FormatData.csv_to_array(state_abbreviations) + + self.by_state(states).count end # Returns a count of all users within the passed in location. The location can diff --git a/test/models/user_test.rb b/test/models/user_test.rb index e0d88242..2cfa11fa 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -124,4 +124,23 @@ def stub_geocoder test 'it returns a users full name' do assert_equal 'first last', User.new(first_name: 'first', last_name: 'last').name end + + test '.count_by_state returns a count of all users within the passed in state(s)' do + tom = create :user + sam = create :user + bob = create :user + + tom.update_columns state: 'TX' + sam.update_columns state: 'TX' + bob.update_columns state: 'CA' + + results = User.count_by_state 'TX' + assert_equal 2, results + + results = User.count_by_state 'TX, CA' + assert_equal 3, results + + results = User.count_by_state '' + assert_equal 0, results + end end From 8d7d62e3904280e846ef41e3496ac4085ffad93d Mon Sep 17 00:00:00 2001 From: Harry Levine Date: Fri, 21 Jul 2017 10:16:10 -0600 Subject: [PATCH 3/9] Updates UsersByLocation class to search by :state --- app/lib/users_by_location.rb | 5 ++- test/lib/users_by_location_test.rb | 60 ++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 test/lib/users_by_location_test.rb diff --git a/app/lib/users_by_location.rb b/app/lib/users_by_location.rb index 96ea34c4..af6d0813 100644 --- a/app/lib/users_by_location.rb +++ b/app/lib/users_by_location.rb @@ -1,5 +1,6 @@ class UsersByLocation def initialize(params) + @state = params[:state] @zip = params[:zip] @city = params[:city] @lat_long = params[:lat_long] @@ -7,7 +8,9 @@ def initialize(params) end def count - if @zip.present? + if @state.present? + User.count_by_state @state + elsif @zip.present? User.count_by_zip @zip elsif @city.present? User.count_by_location @city, @radius diff --git a/test/lib/users_by_location_test.rb b/test/lib/users_by_location_test.rb new file mode 100644 index 00000000..65d93844 --- /dev/null +++ b/test/lib/users_by_location_test.rb @@ -0,0 +1,60 @@ +require 'test_helper' + +class UsersByLocationTest < ActiveSupport::TestCase + setup do + tom = create :user + sam = create :user + + tom.update_columns latitude: 39.622200, longitude: -104.889371, zip: '80111', state: 'CO' + sam.update_columns latitude: 30.312601, longitude: -97.738591, zip: '78756', state: 'TX' + end + + test '#count by state' do + params = { state: 'CO' } + results = UsersByLocation.new(params).count + assert_equal 1, results + + params = { state: 'TX' } + results = UsersByLocation.new(params).count + assert_equal 1, results + + params = { state: 'CO, TX' } + results = UsersByLocation.new(params).count + assert_equal 2, results + end + + test '#count by zip' do + params = { zip: '80111' } + results = UsersByLocation.new(params).count + assert_equal 1, results + + params = { zip: '78756' } + results = UsersByLocation.new(params).count + assert_equal 1, results + + params = { zip: '80111, 78756' } + results = UsersByLocation.new(params).count + assert_equal 2, results + end + + test '#count by city' do + params = { city: 'Greenwood Village, CO, US' } + results = UsersByLocation.new(params).count + assert_equal 1, results + + params = { city: 'Austin, TX, US' } + results = UsersByLocation.new(params).count + assert_equal 1, results + end + + test '#count by lat_long' do + params = { lat_long: [39.622200, -104.889371] } + results = UsersByLocation.new(params).count + assert_equal 1, results + + params = { lat_long: [30.312601, -97.738591] } + results = UsersByLocation.new(params).count + assert_equal 1, results + end +end + From 4aed52356fc84d4f8a0e34ee2eaab13166595603 Mon Sep 17 00:00:00 2001 From: Harry Levine Date: Fri, 21 Jul 2017 10:16:38 -0600 Subject: [PATCH 4/9] Updates API endpoint specs and docs for new :state param --- apiary.apib | 9 ++++++++- test/controllers/api/v1/users_controller_test.rb | 16 ++++++++++++---- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/apiary.apib b/apiary.apib index e170bf56..62255f26 100644 --- a/apiary.apib +++ b/apiary.apib @@ -393,12 +393,13 @@ API endpoints that Operation Code's Rails backend makes available to its React f errors: "Some error message" } -## User | Count By Location [/api/v1/users/by_location{?zip,city,lat_long,radius}] +## User | Count By Location [/api/v1/users/by_location{?state,zip,city,lat_long,radius}] ### By Location [GET] + Parameters + + state (string, optional) - String of comma-separated state abbreviations, i.e. 'TX', or 'TX, CA' + zip (string, optional) - String of comma-separated zip code(s), i.e. '80126', or '80126, 80203' + city (string, optional) - A string of a 'City, State, County' (i.e. 'Denver, CO, US') + lat_long (integer, optional) - Comma-separated integer latitude and longitude, i.e. 30.285648, -97.742052 @@ -408,6 +409,12 @@ API endpoints that Operation Code's Rails backend makes available to its React f + Body + { + state: 'TX, CA' + } + + /*or*/ + { zip: '80126, 80203' } diff --git a/test/controllers/api/v1/users_controller_test.rb b/test/controllers/api/v1/users_controller_test.rb index c5f0b2e2..19ed1b32 100644 --- a/test/controllers/api/v1/users_controller_test.rb +++ b/test/controllers/api/v1/users_controller_test.rb @@ -11,11 +11,16 @@ class Api::V1::UsersControllerTest < ActionDispatch::IntegrationTest end test ":by_location returns User.count of users located in the passed in location" do - tom = create :user, zip: '78705' - sam = create :user, zip: '78756' + tom = create :user + sam = create :user - tom.update latitude: 30.285648, longitude: -97.742052 - sam.update latitude: 30.312601, longitude: -97.738591 + tom.update_columns latitude: 30.285648, longitude: -97.742052, zip: '78705', state: 'TX' + sam.update_columns latitude: 30.312601, longitude: -97.738591, zip: '78756', state: 'TX' + + params = { state: 'TX' } + get api_v1_users_by_location_url(params), headers: @headers, as: :json + assert_equal({ 'user_count' => 2 }, response.parsed_body) + assert_equal 200, response.status params = { zip: '78705' } get api_v1_users_by_location_url(params), as: :json @@ -25,13 +30,16 @@ class Api::V1::UsersControllerTest < ActionDispatch::IntegrationTest params = { zip: '78705, 78756' } get api_v1_users_by_location_url(params), as: :json assert_equal({ 'user_count' => 2 }, response.parsed_body) + assert_equal 200, response.status params = { lat_long: [30.285648, -97.742052] } get api_v1_users_by_location_url(params), as: :json assert_equal({ 'user_count' => 2 }, response.parsed_body) + assert_equal 200, response.status params = { lat_long: [30.285648, -97.742052], radius: 1 } get api_v1_users_by_location_url(params), as: :json assert_equal({ 'user_count' => 1 }, response.parsed_body) + assert_equal 200, response.status end end From ac046948f519ca68fcff58ea63629a4b6b143a4b Mon Sep 17 00:00:00 2001 From: Harry Levine Date: Fri, 21 Jul 2017 10:16:53 -0600 Subject: [PATCH 5/9] New make command for rake db:seed --- Makefile | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Makefile b/Makefile index 872316a4..386d39e7 100644 --- a/Makefile +++ b/Makefile @@ -39,6 +39,10 @@ db_migrate: db_rollback: docker-compose run ${RAILS_CONTAINER} rake db:rollback +.PHONY: db_seed +db_seed: + docker-compose run ${RAILS_CONTAINER} rake db:seed + .PHONY: test test: bg docker-compose run operationcode-psql bash -c "while ! psql --host=operationcode-psql --username=postgres -c 'SELECT 1'; do sleep 5; done;" From 858c22b88163ef6dca05af27563f6a43c541af90 Mon Sep 17 00:00:00 2001 From: Harry Levine Date: Fri, 21 Jul 2017 10:17:40 -0600 Subject: [PATCH 6/9] Updates count_by_zip to use new FormatData class --- app/models/user.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 14d02596..4ed20029 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -38,9 +38,11 @@ class User < ApplicationRecord def self.count_by_zip(zip_codes) return 0 unless zip_codes.present? - zips = zip_codes.split(',').map(&:strip) + zips = FormatData.csv_to_array(zip_codes) + + self.by_zip(zips).count + end - by_zip(zips).count # Returns a count of all users with the passed in state abbreviations. # # @param state_abbreviations [String] String of comma-separated state_abbreviations, i.e. 'CO', or 'CO, TX' From 3922c892b16909e1262234653efc3806ae7b6d94 Mon Sep 17 00:00:00 2001 From: Harry Levine Date: Fri, 21 Jul 2017 10:17:54 -0600 Subject: [PATCH 7/9] Updates method documentation --- app/models/user.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/models/user.rb b/app/models/user.rb index 4ed20029..1cf4202e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -34,6 +34,7 @@ class User < ApplicationRecord # Returns a count of all users with the passed in zip code(s) # # @param zip_codes [String] String of comma-separated zip code(s), i.e. '80126', or '80126, 80203' + # @return [Integer] A count of the requested users. # def self.count_by_zip(zip_codes) return 0 unless zip_codes.present? @@ -62,6 +63,7 @@ def self.count_by_state(state_abbreviations) # @param location [String || Array] Either a 'City, State, County' or [Latitude, Longitude] # For example, 'Denver, CO, US'. # @param radius [Integer] Include results within the radius' distance from the location, in miles. + # @return [Integer] A count of the requested users. # @see https://github.com/alexreisner/geocoder#for-activerecord-models # def self.count_by_location(location, radius=20) From 85a265fa27f3de9f8d98630c07efd53744393d2d Mon Sep 17 00:00:00 2001 From: Harry Levine Date: Sat, 22 Jul 2017 09:03:28 -0600 Subject: [PATCH 8/9] Geocoder is finicky when testing cities. Using a bigger city. When hitting the actual API it works, by during test runs, it is flaky. --- test/lib/users_by_location_test.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/lib/users_by_location_test.rb b/test/lib/users_by_location_test.rb index 65d93844..bea013ce 100644 --- a/test/lib/users_by_location_test.rb +++ b/test/lib/users_by_location_test.rb @@ -5,7 +5,7 @@ class UsersByLocationTest < ActiveSupport::TestCase tom = create :user sam = create :user - tom.update_columns latitude: 39.622200, longitude: -104.889371, zip: '80111', state: 'CO' + tom.update_columns latitude: 39.763034, longitude: -104.961969, zip: '80205', state: 'CO' sam.update_columns latitude: 30.312601, longitude: -97.738591, zip: '78756', state: 'TX' end @@ -24,7 +24,7 @@ class UsersByLocationTest < ActiveSupport::TestCase end test '#count by zip' do - params = { zip: '80111' } + params = { zip: '80205' } results = UsersByLocation.new(params).count assert_equal 1, results @@ -32,13 +32,13 @@ class UsersByLocationTest < ActiveSupport::TestCase results = UsersByLocation.new(params).count assert_equal 1, results - params = { zip: '80111, 78756' } + params = { zip: '80205, 78756' } results = UsersByLocation.new(params).count assert_equal 2, results end test '#count by city' do - params = { city: 'Greenwood Village, CO, US' } + params = { city: 'Denver, CO, US' } results = UsersByLocation.new(params).count assert_equal 1, results @@ -48,7 +48,7 @@ class UsersByLocationTest < ActiveSupport::TestCase end test '#count by lat_long' do - params = { lat_long: [39.622200, -104.889371] } + params = { lat_long: [39.763034, -104.961969] } results = UsersByLocation.new(params).count assert_equal 1, results From b717621d7f10ce3dc9592d37a539b45c70b32087 Mon Sep 17 00:00:00 2001 From: Harry Levine Date: Sat, 22 Jul 2017 20:03:55 -0600 Subject: [PATCH 9/9] Trial for random Travis failures --- test/lib/users_by_location_test.rb | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/test/lib/users_by_location_test.rb b/test/lib/users_by_location_test.rb index bea013ce..bd0bb24a 100644 --- a/test/lib/users_by_location_test.rb +++ b/test/lib/users_by_location_test.rb @@ -37,16 +37,6 @@ class UsersByLocationTest < ActiveSupport::TestCase assert_equal 2, results end - test '#count by city' do - params = { city: 'Denver, CO, US' } - results = UsersByLocation.new(params).count - assert_equal 1, results - - params = { city: 'Austin, TX, US' } - results = UsersByLocation.new(params).count - assert_equal 1, results - end - test '#count by lat_long' do params = { lat_long: [39.763034, -104.961969] } results = UsersByLocation.new(params).count