[post-excerpt][15.2.3] Fix the "is rest api?" condition in the post-excerpt PHP package entry point

[fullofcaffeine]

What?

Fixes a faulty condition in the post-excerpt package entry point PHP.

Why?

The condition was wrong, it should check if the current user is admin or the current request is an API request, the API part was faulty and hence not evaluating to true when it should have, causing some tests to fail (and possibly other bugs).

How?

Change the condition expression from is_admin() || defined('REST_REQUEST') || 'REST_REQUEST' to is_admin() || defined('REST_REQUEST') && REST_REQUEST. The expression after the first or should check if the REST_REQUEST constant is defined and short-circuit if it doesn't, and if it is, then return its truth value. That effectively evaluates to true if the current request is an API request. That's what this fix does.

[tyxla]

Nice catch 😅 🚀

[skorasaurus]

Thanks for the catch; there's another pending PR (#48598) that was resolving a regression introduced #48403 where the post_excerpt filter was ignored.

could you address @tmanhollan 's questions at https://github.com/WordPress/gutenberg/pull/48598/files/b1ff4c7d3ffff5708b449caf38f1320d814b6ef6..21367410f67674fd5ecfe9d0280c42c62d81daf4#issuecomment-1449086819
whether the is_admin check still necessary and if the proper scope is used?

[skorasaurus]

This PR does fix #48403 for me but @jeffreyvr - could you confirm that it is fixed for you by adding replacing the end of /build/block-library/blocks/post-excerpt.php with the following ?

if ( is_admin() ||
	defined( 'REST_REQUEST' ) && REST_REQUEST ) {
	add_filter(
		'excerpt_length',
		function() {
			return 100;
		},
		PHP_INT_MAX
	);
}

but there are a couple other related issues that the approach at #48403 resolves and this PR does not:

• in the editor and front-end of the website, the post excerpts block's content length is limited to the upper bound of the excerpt_length filter. For example, if you have excerpt_length filter set to 55 but in the block editor, set the block's excerpt to 100; the excerpt will only display 55 words in the editor... if you change block's excerpt to 25 in the editor; the block will correctly display 25 words.

[tmanhollan]

Thanks @fullofcaffeine and @skorasaurus. This does resolve the problem of the post_excerpt filter being ignored, but it causes the post excerpts block in the editor to be limited to the length defined by the post_excerpt filter (or the default of 55). This is the same problem I had with my first revision in #48598.

As far as I can tell, that's because the excerpts aren't loaded by the admin page that displays the editor (/wp/wp-admin/post.php?post=1234&action=edit), but rather by a separate request to /wp-json/wp/v2/pages?context=edit&offset=0&order=desc&orderby=date&per_page=10&_locale=user. It seems that in that context, is_admin() returns false. And REST_REQUEST is set inside the function rest_api_loaded(), which is not called until the parse_request action fires. So it is not set yet when the condition is evaluated outside the filter's callback. Moving it inside the callback solves that, but you need to return the initial value in the case where the condition is false.

[fullofcaffeine]

Hi folks! 👋🏻 If #48403 is a better solution, then I'm happy to accept it and ship a patch release, I don't have any strong opinions as I don't know much about the code around it. We needed a fix as some tests for our custom WP install were failing because of the faulty condition.

[fullofcaffeine]

I'll be shipping 15.2.4 with this fix for now, but if #48403 is merged, let me know and I'll cherry-pick into the 15.2 and 15.3 branches today.

[fullofcaffeine]

could you address @tmanhollan 's questions at https://github.com/WordPress/gutenberg/pull/48598/files/b1ff4c7d3ffff5708b449caf38f1320d814b6ef6..21367410f67674fd5ecfe9d0280c42c62d81daf4#issuecomment-1449086819
whether the is_admin check still necessary and if the proper scope is used?

@skorasaurus Wish I could help here, but unfortunately, I don't know :/

[mcsf]

Even if this condition is no longer causing those false positives for REST_REQUEST, this is still a very concerning piece of code. This effectively always overrides excerpt_length regardless of its source. See my comment here.