search

shernshiou / node-uber

Uber API nodejs wrapper
MIT License
188 stars 66 forks source link

Need support for revoke #61

Closed rcurrier666 closed 7 years ago

rcurrier666 commented 7 years ago

Need supports for the OAuth revoke call to revoke a bearer token. I'm calling the API endpoint directly for now, but it would be nice to use this package to do it. If there is anyone else interested in this I could take a stab at implementing it as it is really straight-forward, just call the revoke endpoint with client-id, client-secret, and token. I've never worked on a NPM so not entirely sure how to go about building one, but Google is my friend and it can't be that hard.

dawsbot commented 7 years ago

This quote makes me so happy.

not entirely sure how to go about building one, but Google is my friend and it can't be that hard.

Seems like an awesome opportunity to have you take a stab at it. Let us know if it's not working out and we (@agraebe) can step in a bit to help you @rcurrier666.

agraebe commented 7 years ago

@rcurrier666 It'd be great if you could take a stab! We're here to help in case something is unclear :)

rcurrier666 commented 7 years ago

I have working code but not sure about the error handling (my JavaScript-fu is still weak in that area). Would one of you fine fellows offer to do a code review? Its only 26 lines in Uber.js. Still have to update the README.

agraebe commented 7 years ago

of course! just send over a PR and I'l take a look!

rcurrier666 commented 7 years ago

Pull request created. If I did it wrong let me know, its my first time so be gentle (or not). I saw I have 4 lint warnings. I've fixed them in my branch, but not sure how to get them into the pull request.

rcurrier666 commented 7 years ago

Finally had some time to get back to this. I fixed my git branch (still learning git), renamed revoke to revokeToken for clarity and added tests for revokeToken. Uber.js is failing code coverage. Line 183 is mine, the others are not. I have no idea how to test the "happy path" since revokeToken doesn't return anything (revoke is happy if any string is passed in). If someone knows how to test line 183 (return;) let me know and I'll fix the tests. This should be ready for code review and merging now.

shernshiou commented 7 years ago

@rcurrier666 I will check on that in a few days.

agraebe commented 7 years ago

solved with #64