Fix Migration Token Generation; Add JSON Content-Type header #617
Conversation
| * | ||
| * @return mixed | ||
| */ | ||
| public function add_json_header( array $headers ) { |
There was a problem hiding this comment.
Needs to be public to work with add_filter() call
| @@ -612,6 +612,7 @@ public function migration_ws_validation( array $old_options, array $input ) { | |||
| return $input; | |||
| } | |||
|
|
|||
| $input['migration_token'] = $this->options->get( 'migration_token' ); | |||
There was a problem hiding this comment.
This setting is not typical in that it does not accept an input. The token is generated and then remains in the database (or is received from a constant).
There was a problem hiding this comment.
what if the value is missing/null? When generated I guess this is not the case. But when taking it from a constant value, are you checking this is not null in the "constants reader" logic?
There was a problem hiding this comment.
If the value is empty/null, then a token gets generated.
The constant loading populates $this->options->get() so that call catches both.
| @@ -1,6 +1,6 @@ | |||
| <?php | |||
There was a problem hiding this comment.
Renaming test suite
| @@ -98,10 +100,27 @@ public function custom_requests( $wp, $return = false ) { | |||
| return $output; | |||
| } | |||
|
|
|||
| if ( $json_header ) { | |||
There was a problem hiding this comment.
is this OK here or should it go before the if ($return) above ?? Maybe the send_headers() function might need to be called as well and you missed that (I don't really know)
There was a problem hiding this comment.
No need for a header if we're just spitting back the data rather than outputting.
| @@ -612,6 +612,7 @@ public function migration_ws_validation( array $old_options, array $input ) { | |||
| return $input; | |||
| } | |||
|
|
|||
| $input['migration_token'] = $this->options->get( 'migration_token' ); | |||
There was a problem hiding this comment.
what if the value is missing/null? When generated I guess this is not the case. But when taking it from a constant value, are you checking this is not null in the "constants reader" logic?
joshcanhelp
force-pushed
the
fix-migration-token-reset-on-save
branch
from
ef5fd12 to
e152006
Compare
Codecov Report
@@ Coverage Diff @@
## master #617 +/- ##
============================================
+ Coverage 36.4% 36.42% +0.01%
- Complexity 1264 1266 +2
============================================
Files 51 51
Lines 3920 3929 +9
============================================
+ Hits 1427 1431 +4
- Misses 2493 2498 +5
Continue to review full report at Codecov.
|
Changes
Content-Type: application/jsonheader to endpoints that output JSONNote: We added an internal task to add a control to regenerate this token in wp-admin.
Testing
exits, making testing impossible)Checklist