Skip to content
This repository was archived by the owner on Dec 3, 2019. It is now read-only.

Feature/104 user count by state - #109

Merged
rickr merged 9 commits into
masterfrom
feature/104-user-count-by-state
Jul 23, 2017
Merged

rickr merged 9 commits into
masterfrom
feature/104-user-count-by-state

Conversation

@hpjaj

@hpjaj hpjaj commented Jul 21, 2017

Copy link
Copy Markdown
Collaborator

Description of changes

Adds ability to query by users' :state's, to determine user count. Specifically:

  • Adds a new param to the users/by_location action called :state that returns a count of the users in a particular state.
  • Updates the API docs
  • Additional test coverage

Issue Resolved

Fixes #104

@hpjaj

hpjaj commented Jul 21, 2017

Copy link
Copy Markdown
Collaborator Author

@rickr - Travis is failing; however, when grepping for error or failure, there are no failures or errors associated with any tests.

Can you determine if something is going on with the build, or?

@hollomancer
hollomancer requested a review from rickr July 21, 2017 16:44
hpjaj added 7 commits July 22, 2017 08:46
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.
@hpjaj
hpjaj force-pushed the feature/104-user-count-by-state branch from 904f8d8 to 3922c89 Compare July 22, 2017 14:49
When hitting the actual API it works, by during test runs, it is flaky.
@hpjaj

hpjaj commented Jul 22, 2017

Copy link
Copy Markdown
Collaborator Author

@rickr - this is happening again:

#109 (comment)

Can you look into it?

@rickr

rickr commented Jul 22, 2017

Copy link
Copy Markdown
Contributor

I don't see any problems either. Are we mocking the call to the geocoding API? I wonder if we're getting rate limited?

@hpjaj
hpjaj force-pushed the feature/104-user-count-by-state branch from c09223d to b717621 Compare July 23, 2017 02:11
@hpjaj

hpjaj commented Jul 23, 2017

Copy link
Copy Markdown
Collaborator Author

@rickr - this one is ready

@hpjaj hpjaj added ready and removed in progress labels Jul 23, 2017
@rickr
rickr merged commit 6d40207 into master Jul 23, 2017
@rickr rickr removed the ready label Jul 23, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants