Fix migration login route output and add tests #595
Conversation
joshcanhelp
changed the title
Codecov Report
@@ Coverage Diff @@
## master #595 +/- ##
===========================================
+ Coverage 31.03% 32.4% +1.37%
- Complexity 1306 1309 +3
===========================================
Files 54 54
Lines 4183 4187 +4
===========================================
+ Hits 1298 1357 +59
+ Misses 2885 2830 -55
Continue to review full report at Codecov.
|
| @@ -22,11 +37,6 @@ public function setup_rewrites( $force_ws = false ) { | |||
|
|
|||
| add_rewrite_rule( '^auth0', 'index.php?auth0=1', 'top' ); | |||
| add_rewrite_rule( '^\.well-known/oauth2-client-configuration', 'index.php?a0_action=oauth2-config', 'top' ); | |||
|
|
|||
| // if ( $force_ws || $this->a0_options->get( 'migration_ws' ) ) { | |||
There was a problem hiding this comment.
@cocojoe - It's been commented out for a while, no need to keep in in there.
| if ( ! empty( $page ) ) { | ||
| switch ( $page ) { | ||
| case 'oauth2-config': | ||
| $this->oauth2_config(); | ||
| exit; | ||
| case 'migration-ws-login': | ||
| $this->migration_ws_login(); | ||
| exit; | ||
| $output = $this->migration_ws_login(); |
There was a problem hiding this comment.
What's going on here that you buffer it and then either echo it or return it? Only appears to be one case that needs to do this? Is the intention to have more since you broke out the if (output) logic later on vs handling in the case?
There was a problem hiding this comment.
All these routes handle execution and, in some cases echo out a string, some just return. I'm combining the behavior into one flow so JSON is echoed and the process exits at the same time. In case the methods need to return instead of echoing, I added that query_var (mostly used for testing at this point but could be helpful in a custom API route).
There was a problem hiding this comment.
I still think the code could be tidier for this single use case. However, makes no real odds in terms of execution.
There was a problem hiding this comment.
All of these routes will do the same, just wanted to keep PRs a little more streamlined 👍
| if ( ! empty( $page ) ) { | ||
| switch ( $page ) { | ||
| case 'oauth2-config': | ||
| $this->oauth2_config(); | ||
| exit; | ||
| case 'migration-ws-login': | ||
| $this->migration_ws_login(); | ||
| exit; | ||
| $output = $this->migration_ws_login(); |
There was a problem hiding this comment.
I still think the code could be tidier for this single use case. However, makes no real odds in terms of execution.
joshcanhelp
force-pushed
the
fix-migration-error-output
branch
from
441a624 to
f2e75c2
Compare
Changes
WP_Auth0_Options_Generic::reset()to reset all options back to defaultsWP_Auth0_Routesfor theWP_Auth0_Ip_CheckclassTesting
Checklist
composer pre-commit