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 Management API Guardian enrollments endpoint #182

Merged
merged 5 commits into from
Aug 14, 2019
Merged

Add Management API Guardian enrollments endpoint #182

merged 5 commits into from
Aug 14, 2019

Conversation

tomgi
Copy link
Contributor

@tomgi tomgi commented Jul 23, 2019

Changes

Add endpoint for Guardian enrollments API

References

https://auth0.com/docs/api/management/v2#!/Guardian/get_factors
https://auth0.com/docs/api/management/v2#!/Guardian/get_enrollments_by_id
https://auth0.com/docs/api/management/v2#!/Guardian/delete_enrollments_by_id

Testing

  • This change adds unit test coverage
  • This change adds integration test coverage
  • This change has been tested on the latest version of Ruby

Checklist

@tomgi tomgi requested a review from a team July 23, 2019 06:42
Copy link
Contributor

@lbalmaceda lbalmaceda left a comment

Choose a reason for hiding this comment

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

@tomgi Thanks for the contribution. I have some doubts regarding the introduced aliases. Not the naming but the use of them, asked Josh about his opinion. May I ask why you left out the remaining Guardian endpoints? e.g. templates, provider configuration, the PUT of a factor, tickets, etc. Cheers

lib/auth0/api/v2/guardian.rb Show resolved Hide resolved
spec/lib/auth0/api/v2/guardian_spec.rb Outdated Show resolved Hide resolved
@tomgi
Copy link
Contributor Author

tomgi commented Jul 24, 2019

Thanks for feedback @lbalmaceda

To be honest I left out the remaining Guardian endpoints for a purely pragmatic reason - I didn't need them in my project that uses this gem 😅

But that's a fair point, since I'm adding some of the Guardian endpoints anyway, I might as well spend some time to add all of them - I will try to do that later today.

@tomgi
Copy link
Contributor Author

tomgi commented Jul 25, 2019

Hey @lbalmaceda, I've added all the remaining Guardian endpoints

Copy link
Contributor

@joshcanhelp joshcanhelp left a comment

Choose a reason for hiding this comment

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

Thanks for the PR here @tomgi! A few issues below, minor change, and a question.

lib/auth0/api/v2/guardian.rb Outdated Show resolved Hide resolved
lib/auth0/api/v2/guardian.rb Show resolved Hide resolved
lib/auth0/api/v2/guardian.rb Outdated Show resolved Hide resolved
lib/auth0/api/v2/guardian.rb Outdated Show resolved Hide resolved
lib/auth0/api/v2/guardian.rb Outdated Show resolved Hide resolved
lib/auth0/api/v2/guardian.rb Outdated Show resolved Hide resolved
lib/auth0/api/v2/guardian.rb Outdated Show resolved Hide resolved
@joshcanhelp joshcanhelp modified the milestone: v4.8.0 Jul 29, 2019
@tomgi
Copy link
Contributor Author

tomgi commented Aug 13, 2019

Thanks @joshcanhelp, I've applied your suggestions.

@joshcanhelp joshcanhelp added this to the v4.9.0 milestone Aug 14, 2019
@joshcanhelp joshcanhelp merged commit 4e03ea7 into auth0:master Aug 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants