Add WP API endpoint to generate tokens#28
Conversation
This reverts commit 4314151.
danielbachhuber
left a comment
There was a problem hiding this comment.
Great start! Left some comments to be discussed.
|
@gdespoulain Are you ready for a final review here? |
|
@danielbachhuber let's go, do you have something you still see as a blocker or are we good to go? :) |
| 'methods' => WP_REST_Server::CREATABLE, | ||
| 'callback' => 'one_time_login_api_request', | ||
| 'args' => array( | ||
| 'user' => array(), |
There was a problem hiding this comment.
Is user a required argument? It seems nothing would happen if it's not provided:
There was a problem hiding this comment.
Yes, in the WP CLI command it seemed to be a required argument as well so I kept it that way
There was a problem hiding this comment.
Ok. Can you add 'required' => true to the argument definition, and remove the isset( $parameters['user'] ) check in the callback?
There was a problem hiding this comment.
When you don't set 'required' => false, the default is always true, that's why it's not there (it's not necessary).
Concerning the isset: I find it better to check the existence of a value before using it, as a principle, just to prevent ending up in a fatal error in the case something unexpected happens
There was a problem hiding this comment.
When you don't set
'required' => false, the default is always true, that's why it's not there (it's not necessary).
Let's add it, just for the sake of clarity.
Concerning the isset: I find it better to check the existence of a value before using it, as a principle, just to prevent ending up in a fatal error in the case something unexpected happens
I think we'd want a fatal error in this case, to indicate something is broken (vs. an empty response which makes it less obvious that something is broken).
|
@gdespoulain Just one last question, I think. |
|
Thanks for your work on this, @gdespoulain ! I've just tagged the v0.4.0 release. |
|
@danielbachhuber Thank you too! :) Nice to have worked on this plugin! :) |
Hi!
Here is a proposition for an API endpoint if you like :)
Changes:
Have fun!