Task/refactoring reset password route #10
Conversation
Added an action class that processes send request code Added repository of messages Added error message fectory that returns WP_ERROR class Added class that returns default response structure
There was a problem hiding this comment.
OK great, mostly looks fine. Couple of minor points on naming etc. and there's one type error there.
So I guess then we're really moving everything out of the API requests themselves and putting all the functionality into different classes for ease of testing?
| @@ -0,0 +1,9 @@ | |||
| <?php | |||
|
|
|||
| class Error_Message_Registry | |||
There was a problem hiding this comment.
Can class names be prepended with BDPWR_ please?
| @@ -0,0 +1,12 @@ | |||
| <?php | |||
|
|
|||
| class Response_Repository { | |||
There was a problem hiding this comment.
Can class names be prepended with BDPWR_ please?
Yes, and reusability. The reason why this refactoring mostly grabbed my attention is because this repo already has a code to perform a password reset. The new feature only needs to have support for one extra field. So, by adding a base action class, all I need to do, other than adding a route entry, is decorate the base class with support for that new field. |
- Appended BDPWR_ to class names - Fixed issue with data passed into handle() method for Reset_Password_Action within password reset route
|
@dominic-ks I pushed changes, should be fine now. |
|
@Spiker1992 Hey sorry for the delay, busy week. Have you tested this code OK? I've loaded up this pull request and when I call |
|
@dominic-ks I haven't tested it... I have to say, I have no idea how to test it. I would like to add PHPUnit first, #12, and then add a test for this endpoint before dealing with this PR. I think it would take 10-20 minutes to set up an initial test for this endpoint, which should be quicker than me figuring out how to test it manually :S |
|
OK well I think then I'm going to work on adding some more contributing docs to this repo with info on that then, I'll let you know when that's done. |
|
@dominic-ks Ok. I will add a branch to test this PR using PHPUnit and then patch issues in here. |
|
@Spiker1992 Hey, FYI - I've added a CONTRIBUTING.md file to the project which I think will help. Let me know if you get any questions. |
|
@dominic-ks ok, great, thanks. Btw, I have added an initial test set up for endpoint tests and added a success test for all the endpoints - Spiker1992@67ea200. I can create a PR from that commit into my PR for units test if you up for it? Btw, do you know of any WP auto formatting packages for Visual Code? Given that WP requires PHP 7.4, it would have been nice for the WP team to update their coding standards to reflect new functionality :( |
|
@dominic-ks, forgot to ask, which PHP version are you supporting? |
|
@dominic-ks found codesniffer repo for this - https://github.com/WordPress/WordPress-Coding-Standards |
…on class Issue was due to the call back function receiving `WP_REST_Request` class.
|
@dominic-ks I have tested this endpoint and it should now work. |
Added an action class that processes send request code
Added repository of messages
Added error message fectory that returns WP_ERROR class
Added class that returns default response structure