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

Add Geos to Report #49

Merged
merged 2 commits into from
Nov 28, 2018
Merged

Add Geos to Report #49

merged 2 commits into from
Nov 28, 2018

Conversation

aaronapayne
Copy link

Adds geos to the report object. This is supported per the checkr docs, but isn't currently supported in this gem.

@aaronapayne
Copy link
Author

aaronapayne commented Nov 27, 2018

@erdey when I tested this locally without any default, geos returned null if the report did not have any geos.

I added a default value for the result in a new commit. This is what is returned now when I make the calls if there are no geos:
"geos": {"object":"list","data":[]},
this is what it looks like if there are geos:
"geos": {"object":"list","data":[{"id":"1ab3c...2d88f"}]},

does this seem like the correct behavior? I didn't see a pattern for testing this in report_test.rb

@erdey
Copy link

erdey commented Nov 27, 2018

@aaronapayne I think empty array for the data is a good default. I also don't see any examples of testing similar behavior, so I'll defer to the primary maintainers on whether we need to test it.

@nothingisfunny
Copy link
Contributor

nothingisfunny commented Nov 28, 2018

@erdey @aaronapayne {} and [] as a default value produce the same result, a collection with data pointing to an empty array:

#<Checkr::APIList:0x3fc39da33374> JSON: { "object": "list", "data": [ ] }

I tested these changes locally and it seems to be working fine. ✨

Copy link
Contributor

@nothingisfunny nothingisfunny left a comment

Choose a reason for hiding this comment

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

🌎

@nothingisfunny nothingisfunny merged commit c71d988 into checkr:master Nov 28, 2018
nothingisfunny added a commit that referenced this pull request Nov 29, 2018
@nothingisfunny
Copy link
Contributor

@aaronapayne Thank you for this contribution! 🙌🏻

@jperichon
Copy link
Member

@aaronapayne we bumped to 1.5.3, thank you so much! 👌

@aaronapayne
Copy link
Author

Thanks all! I appreciate your help on this! 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants