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

implemented /api/v2/grants endpoint of auth0 management api #74

Merged
merged 2 commits into from
Sep 6, 2017

Conversation

neshanjo
Copy link
Contributor

@neshanjo neshanjo commented Sep 1, 2017

The Grants endpoint of the management API was missing. Since I need it in one of my projects, I decided to directly add it to the official library. Also added unit tests.

Somehow, the git diff here at github marks some changes as a complete remove and add. However, in .gitignore, I only added a NetBeans IDE ignore pattern at the end and in ManagementAPI.java, only lines 118-125 were added.

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.

Thanks for the contribution. 🎉 This is a relatively new API and I need to check with the other team the usage. The GET looks ok but the DELETE I'm still trying to figure out how it works (which params are required, etc). I'll come back to this in a few days.
Regarding the diff my bet is that it's because of the line endings. Please check this and this articles to fix them and add the files again.
Cheers 👍

@neshanjo
Copy link
Contributor Author

neshanjo commented Sep 4, 2017

I fixed the line endings and did some further investigation of the delete endpoint, i.e., I tried it out by calling the management API. It is valid to omit either the user id or the grant id. If you omit both, you get

{
    "statusCode": 400,
    "error": "Bad Request",
    "message": "Missing required field: user_id",
    "errorCode": "invalid_body"
}

My code now reflects this behaviour.

Cheers,
Johannes

@lbalmaceda
Copy link
Contributor

It looks a lot cleaner now! 👍 I checked with the team and they told me only one parameter is required at the same time, either user_id (query) or grant_id (path). Because of this, I would have 2 separate methods:

  • delete(String grantId) which sets the grant_id in the path
  • deleteAll(String userId) which sets the user_id in the query
    What do you think?

@neshanjo
Copy link
Contributor Author

neshanjo commented Sep 5, 2017

👍 That's a good idea, and much more friendly for the API consumer!

@lbalmaceda
Copy link
Contributor

Cool! Whenever you can feel free to make the changes and ping me so I merge it.
Cheers

@neshanjo
Copy link
Contributor Author

neshanjo commented Sep 6, 2017

I've made the changes.

@neshanjo
Copy link
Contributor Author

neshanjo commented Sep 6, 2017

Sorry, I've just discovered a copy/paste error (GrantsEntityTest is referencing MGMT_CLIENT_GRANTS_LIST). I will fix this. Please do not merge yet!

@neshanjo
Copy link
Contributor Author

neshanjo commented Sep 6, 2017

It is fixed now. Ready to merge (from my side).

@lbalmaceda lbalmaceda added this to the v1-Next milestone Sep 6, 2017
@lbalmaceda lbalmaceda merged commit 8e4e563 into auth0:master Sep 6, 2017
@neshanjo neshanjo deleted the manage-grants branch September 8, 2017 13:16
@lbalmaceda lbalmaceda modified the milestones: v1-Next, 1.3.0 Sep 8, 2017
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.

2 participants