skipping existing thumbs (even when it shouldn't)

[nosilver4u]

I was testing some things to make sure EWWW IO was working properly with the new version. I'm doing just one attachment at a time for testing, and I unchecked the "skip regenerating..." box to make RT recreate ALL the thumbs, but it isn't. I thought EWWW might have been messing with something, so I disabled it, and the issue persists.
I checked the timestamps on the files before and after, expecting them to all be updated (except the full-size image), but only the new/missing images have recent timestamps (the rest are from Nov 2).
Am I misunderstanding what that option does, or is that an actual bug?

[Viper007Bond]

If you have that unchecked, it should create new thumbnails for all currently registered thumbnail sizes. It won't generate new files for any sizes that are no longer registered even if the old thumbnail files still exist.

I even wrote a unit test to explicitly test to make sure it was working as expected:

https://github.com/Viper007Bond/regenerate-thumbnails/blob/v3.0.1/tests/test-regenerator.php#L242

There could of course be a bug in my test, but as far as I know it works.

[idofri]

I think @nosilver4u is right.

The "Skip regenerating existing..." setting doesn't work when regenerating thumbs for single attachments.
The REST route callback method is unsuccessfully trying to get request params:
https://github.com/Viper007Bond/regenerate-thumbnails/blob/2b537fb9cf6948526406ec96de31315f4ce832ae/includes/class-regeneratethumbnails-rest-controller.php#L182-L185
but the regenerate() method always falls-back to it's defaults.

#60 should fix that I reckon.

[nosilver4u]

Yeah, I provided a pretty detailed breakdown in Slack on Friday, which confirmed the bug. Your observation is half the problem, the other half is that the REST API is not converting a string 'false' to a boolean false, and in PHP, the string 'false' evaluates to a boolean true.

[idofri]

@nosilver4u could you elaborate on this subject?
in what situation exactly the string 'false' is being sent?

[nosilver4u]

I tested it when regenerating thumbs for a single attachment. I don't know if the JS involved is submitting it as a string, or if PHP is failing to convert the POST variable, or if the fault lies with the REST API, since it doesn't run parse_request() like a normal front-end request would.
At any rate, it comes into the regenerate_item() method as a string, coming from the $request->get_param() calls. I tested it with is_string() and is_bool() and found it was a string every time I tested it.

My suspicion is that this is a flaw in the REST API, or above that at the JS level, but I didn't dig that far, as I had already spent several hours debugging it.

[idofri]

In what part of the procedure have you tested it?

Have A look at the declaration of route's request args:
https://github.com/Viper007Bond/regenerate-thumbnails/blob/2b537fb9cf6948526406ec96de31315f4ce832ae/includes/class-regeneratethumbnails-rest-controller.php#L36-L40

It's being stated that these args are booleans, And later being enforced at:

if ( 'boolean' === $args['type'] && ! rest_is_boolean( $value ) ) {
	/* translators: 1: parameter, 2: type name */
	return new WP_Error( 'rest_invalid_param', sprintf( __( '%1$s is not of type %2$s.' ), $value, 'boolean' ) );
}

https://github.com/WordPress/WordPress/blob/master/wp-includes/rest-api.php#L1137-L1140

if ( 'boolean' === $args['type'] ) {
	return rest_sanitize_boolean( $value );
}

https://github.com/WordPress/WordPress/blob/master/wp-includes/rest-api.php#L1267-L1269

[nosilver4u]

I checked it in the actual regenerate method, but looking at how the args are defined (as you pointed out), I expect it will be resolved by your pull request.
I had gone about a fix a little differently, which left the args in a sub-array of the 'regeneration_args' index, so the definitions (which you pointed out) were still not lining up with the actual POST args. Not being familiar with the REST API myself, I hadn't even realized that you COULD specify the type of the args like that, so all I knew was that with my "fix", they were still coming in as strings.
If I have time later today, I'll certainly double-check your fix on my own system as well.

[nosilver4u]

confirmed the fix from @1do

[Viper007Bond]

I misunderstood the problem before, thinking it was a backend issue and not a UI issue.

Another reason I need to get around to writing end-to-end tests so that I can test the UI too.