-
Notifications
You must be signed in to change notification settings - Fork 40
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
Support for new end points. #10
Conversation
New 3 methods: `verification_start`, `verification_check` and `info` were added to support authy's phone verification and info services. Usage: authy.phones().verification_start({ via: "sms", country_code: "1", phone_number: "111-111-1111" } ); authy.phones().verification_check({ country_code: "1", phone_number: "111-111-1111", verification_code: "0000" } ); authy.phones().info({ country_code: "1", phone_number: "111-111-1111" } );
A new method `_request` was created to handle GET and POST requests. This allows us to remove repeated code and improve readability.
User status returns the status of a user given the id. A new method `user_status` along with tests were added to support this end point.
Documentation for the new methods were added: - start phone verification - check phone verification - phone info - user status
Given the fact that @jveldridge has been a major contributor in the past and I believe is using this in production I would love their input on this as well. |
Also wanted to say thank you for the PR, I'll try and review as soon as I can. :) |
user_status(id, callback); | ||
|
||
```javascript | ||
authy.user_satus('1337', function (err, res) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/satus/status
I don't see the new endpoints documented at docs.authy.com--do you know if they will be added there? |
@jveldridge they are. There is a menu ("Jump to...") in the top right corner. We should make it more visible though. |
}); | ||
}; | ||
|
||
switch(type) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should an if statement instead of a switch, there are only 2 cases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or just add the method correct method to the options for the request module and forego the flow control statements completely
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, that was thinking in possible changes in the future, like adding other methods (put, delete).
Phone verification methods were modified to accept individual parameters instead of an options object.
I've pushed a new commit. I decided to remove the |
Hi @evilpacket, do you have plans to merge this PR? |
He is out ATM, I can talk to him on monday, if he hasn't merged/published it over the weekend |
Sorry for the delay. If Daniel has merge button permission he can do so otherwise I'll do so after I'm not crazy busy. Hopefully this weekend.
|
Support for new end points.
This pull request contains 3 "big" changes:
Please find below more details.
Start phone verification (POST /protected/json/phones/verification/start)
Check phone verification (GET /protected/json/phones/verification/check)
Phone information (GET /protected/json/phones/info)
User status (GET /protected/json/users/:user_id/status)
The phone related methods were encapsulated in the Authy.phone() method. For instance, to call the Start Phone Verification method you need to do: