Only prepare links if requested on index.

[spacedmonkey]

Trac ticket: https://core.trac.wordpress.org/ticket/57902

---

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

[spacedmonkey]

CC @dlh01

[kadamwhite]

Tests fail.

I'm interested in the fact that this patch seeks to prevent the links being added (which is convenient, since we already have access to the current $request in this code), whereas #4483 comes at it by trying to avoid generating the links. Do we need some code on both sides?

If we can get the tests passing on this patch, I'm 👍 on at minimum not adding them when we can avoid it, regardless of whether we also pursue something more like the other patch later on.

[spacedmonkey]

@kadamwhite Sorry for pushing up unit test fails. Should be fixed now.

The idea of this ticket is two fold.

1. Respect _fields=links and not return that in the response.
2. No prepare the links in the first place, saving database / cache lookups.

See b7bae69 for referennce, where we did this for other endpoints.

[dlh01]

Looking good to me. Thanks for picking it up!

[mukeshpanchal27]

Thanks @spacedmonkey for the PR, Look good to me left minot nitpick

[swissspidy]

Isn't this a breaking change? If someone used ?_fields=xyz before, they got the links for free. Now they have to use ?_fields=xyz,links to get the same behavior.

This was already a problem in 6.1 when this was added to all the other REST controllers.

I'd be wary of making such a breaking change that late in the cycle.

[costdev]

Thanks for the PR @spacedmonkey! I've left some test-related feedback in this review.

[costdev]

Suggested change

	*
	* @throws Exception

This shouldn't throw an exception.

[spacedmonkey]

@costdev This comes from upstream method. See

wordpress-develop/tests/phpunit/includes/abstract-testcase.php

Lines 1151 to 1171 in 252a2c1

	public static function text_array_to_dataprovider( $input ) {
	$data = array();
	foreach ( $input as $value ) {
	if ( ! is_string( $value ) ) {
	throw new Exception(
	'All values in the input array should be text strings. Fix the input data.'
	);
	}
	if ( isset( $data[ $value ] ) ) {
	throw new Exception(
	"Attempting to add a duplicate data set for value $value to the data provider. Fix the input data."
	);
	}
	$data[ $value ] = array( $value );
	}
	return $data;
	}

[spacedmonkey]

Isn't this a breaking change? If someone used ?_fields=xyz before, they got the links for free. Now they have to use ?_fields=xyz,links to get the same behavior.

This was already a problem in 6.1 when this was added to all the other REST controllers.

I'd be wary of making such a breaking change that late in the cycle.

@swissspidy
The fact that links are returned even when they are not requested, is a bug. You do not request this data and is generated and returned anyway. If you don't do anything, just hit the index, you still will get links. The only change is behaviour here, is if you request /wp-json?_fields=name, links will not be returned. By why would expect the links to be there, you didn't request them.

[swissspidy]

The only change is behaviour here, is if you request /wp-json?_fields=name, links will not be returned. By why would expect the links to be there, you didn't request them.

Because that was the existing behavior ever since WordPress 4.7.0. It just worked. This bug fix will break this behavior, simple as that. Even if it's not documented and not intended.
If you remember, we had the exact same problem with 6.1.0 when this change broke REST API usage in the Web Stories plugin. But that ship has sailed now.

Don't get me wrong, I don't have a problem with making this change, as it was already changed for all the other controllers and the index controller isn't used as much. I just like to point out that now is very late to do this change. I suggest doing this early in the 6.4.0 cycle instead.

[costdev]

From a code perspective, this looks good. However, I'm mindful of @swissspidy's comment about landing this early in a cycle. Pascal, we're 3 weeks from 6.4 Beta 1 - Is there still time to consider this one for commit 6.4 in your opinion?

[spacedmonkey]

Committed https://core.trac.wordpress.org/changeset/56566