dominic-ks / bdvs-password-reset

WordPress - Allow users to reset their password using a random code via the REST API
GNU General Public License v3.0
9 stars 3 forks source link

Task/refactoring reset password route #10

Open Spiker1992 opened 3 years ago

Spiker1992 commented 3 years ago

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

Spiker1992 commented 3 years ago

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?

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.

Spiker1992 commented 3 years ago

@dominic-ks I pushed changes, should be fine now.

dominic-ks commented 3 years ago

@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 /bdpwr/v1/reset-password I get a fatal error: Uncaught TypeError: Argument 1 passed to BDPWR_Reset_Password_Action::handle() must be of the type array, object given ?

Spiker1992 commented 3 years ago

@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, https://github.com/dominic-ks/bdvs-password-reset/pull/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

dominic-ks commented 3 years ago

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.

Spiker1992 commented 3 years ago

@dominic-ks Ok. I will add a branch to test this PR using PHPUnit and then patch issues in here.

dominic-ks commented 3 years ago

@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.

Spiker1992 commented 3 years ago

@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 - https://github.com/Spiker1992/bdvs-password-reset/commit/67ea200a9d8cfc92f6dc26a7bc8bde989a78717b. 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 :(

Spiker1992 commented 3 years ago

@dominic-ks, forgot to ask, which PHP version are you supporting?

Spiker1992 commented 3 years ago

@dominic-ks found codesniffer repo for this - https://github.com/WordPress/WordPress-Coding-Standards

Spiker1992 commented 3 years ago

@dominic-ks I have tested this endpoint and it should now work.