Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

getEntityRecords returns empty list for custom endpoint #61348

Closed
MatzeKitt opened this issue May 3, 2024 · 16 comments
Closed

getEntityRecords returns empty list for custom endpoint #61348

MatzeKitt opened this issue May 3, 2024 · 16 comments
Labels
[Package] Core data /packages/core-data [Type] Bug An existing feature does not function as intended

Comments

@MatzeKitt
Copy link
Contributor

MatzeKitt commented May 3, 2024

Description

I have a custom API endpoint to request the user roles of a user, which returns basically this:

$user = wp_get_current_user();
return rest_ensure_response( $user->roles );

This endpoint is accessible via /namespace/v1/roles and works fine.

Then, I use this endpoint in a PluginSidebar like so:

const SidebarContent = compose( [
	withDispatch( ( dispatch ) => {
		// allow using a custom REST API
		dispatch( 'core' ).addEntities( [
			{
				name: 'user_roles',
				kind: 'namespace',
				baseURL: '/namespace/v1/roles',
			},
		] );
		
		return null;
	} ),
] )( ( props ) => {
	const data = useSelect( ( select ) => {
		return {
			// request the REST API
			roles: select( 'core' ).getEntityRecords( 'namespace', 'user_roles', { per_page: 100 } ),
		}
	} );
	console.log({data});
	
	return null;
} )

registerPlugin(
	'my-sidebar',
	<div>
		<SidebarContent />
	</div>
);

The data.roles is just an empty array despite the API endpoint returns the correct data as array (e.g. ["subscriber", "administrator"]), as also seen in the network tab in the developer tools.

Step-by-step reproduction instructions

  1. Create custom API endpoint
  2. Create plugin sidebar from above
  3. Check the output of data.roles, which is empty

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@MatzeKitt MatzeKitt added the [Type] Bug An existing feature does not function as intended label May 3, 2024
@Mamaduka Mamaduka added [Package] Core data /packages/core-data Needs Testing Needs further testing to be confirmed. labels May 3, 2024
@Mamaduka
Copy link
Member

I haven't started debugging this yet, but I would guess that the returned response "shape" is the cause, which is usually an array of objects with a unique key.

The settings endpoint might be similar to yours; it exposes a single entity for a site and doesn't support "plural" selectors.

// Returns null.
wp.data.select( 'core' ).getEntityRecords( 'root', 'site' );

// Returns settings object.
wp.data.select( 'core' ).getEntityRecord( 'root', 'site' );

Can you test if wp.data.select( 'core' ).getEntityRecord returns values properly?

@MatzeKitt
Copy link
Contributor Author

MatzeKitt commented May 13, 2024

Using getEntityRecord, I just get a single role returned, even though the user has multiple roles.

Changing the API output to an array of objects and use getEntityRecords again also returns an empty array:

[{
    "role": "editor"
}, {
    "role": "administrator"
}]

@MatzeKitt
Copy link
Contributor Author

Okay, after some more investigation, it seems there was a breaking change and getEntityRecords now explicitly requires having an id property for each value in the returned objects. This works:

[{
    "role": "editor",
    "id": 1
}, {
    "role": "administrator",
    "id": 2
}]

@Mamaduka
Copy link
Member

Thanks for the additional details, @MatzeKitt!

Can you share an example of the endpoint code you're testing with? I have something locally, but I want to be as close to the source as possible.

The entities have always required a primary key; that's how they're stored. However, this is less documented due to the entities' semi-private API.

@MatzeKitt
Copy link
Contributor Author

MatzeKitt commented May 14, 2024

The API consists of these files:
https://gist.github.com/MatzeKitt/e657d1a28a4291ddc48dd394df76b7de
https://gist.github.com/MatzeKitt/fe0befa4b4ba5d73247e4d21ef79e3ab

This is the current working example with ID.

The original has a slightly different get_value method in the User_Api class:

/**
 * Get user information.
 *
 * @param	string				$option The current object name
 * @param	\WP_REST_Request	$request The current request
 * @return	mixed|void|\WP_Error|\WP_HTTP_Response|\WP_REST_Response The API response
 */
public function get_value( string $option, WP_REST_Request $request ) {
	$user = wp_get_current_user();
	
	switch ( $request->get_param( 'get' ) ) {
		case 'current_roles':
			if ( empty( $user->roles ) ) {
				return rest_ensure_response( [ 'no_roles' ] );
			}
			
			// make super admin always to administrator
			if ( is_super_admin() ) {
				return rest_ensure_response( array_merge( $user->roles, [ 'administrator' ] ) );
			}
			
			return rest_ensure_response( $user->roles );
		case 'restricted_roles':
			return rest_ensure_response( Heimdall::get_instance()->get_restricted_roles( 'name' ) );
		case 'roles':
			return rest_ensure_response( Heimdall::get_instance()->get_roles( 'name' ) );
	}
}

The actual request is done via select( 'core' ).getEntityRecords( 'rh', 'user/v1/current_roles', { per_page: 100 } ).

Thank you for investigating!

The entities have always required a primary key; that's how they're stored. However, this is less documented due to the entities' semi-private API.

Interesting, since it worked before 6.5 quite fine without an ID whatsoever. It seems something changed there. Since it’s semi-private, what is the intented public API for such a function? A custom store with apiFetch?

@Mamaduka
Copy link
Member

I call everything that's not documented "semi-private" :)

But I agree that if it was working previously without an ID, and unless that was a bug, we'll have to restore the previous behavior.

cc @jsnajdr, @youknowriad

@youknowriad
Copy link
Contributor

Yeah, entities were always meant to have an "ID", I wonder how it was working before and what was considered "primary key" internally for this use-case.

@jsnajdr
Copy link
Member

jsnajdr commented May 14, 2024

The items returned by the API endpoint are expected to be objects. The code cannot handle strings. Mainly because every item needs to have an unique "key". You can register a key field name when adding the entity:

addEntities( [
  {
    name: 'user_roles',
    kind: 'namespace',
    baseURL: '/namespace/v1/roles',
    key: 'role', // <<=== this is new
  },
] )

Then the version of the endpoint that returns [ { role: 'editor' } ] should start working. If the entity key is not specified, it defaults to id.

If we wanted to support the [ 'editor', 'administrator' ] response, we'd have to support function keys like key: item => item or some special form key: '__self'. Both say that the item itself is the unique key.

I believe it was always like this, this bug report is not a regression. And @MatzeKitt doesn't say anywhere that it is.

@Mamaduka
Copy link
Member

@jsnajdr, the latest comment from @MatzeKitt mentions that this was working without a key pre-WP 6.5.

My understanding is the same. Entity record lists need an array of objects with a unique key.

@MatzeKitt
Copy link
Contributor Author

MatzeKitt commented May 14, 2024

Yeah, it definitely worked before. But if it was never intended to work that way, I’m totally fine with it not being changed again. I now know what I did wrong and could fix it on my end. 🙂

Also thank you @jsnajdr for the key property, I will try it.

@jsnajdr
Copy link
Member

jsnajdr commented May 14, 2024

OK, I'll try to find what changed.

@jsnajdr
Copy link
Member

jsnajdr commented May 14, 2024

One of the PRs that affect this is #55832 by @ntsekouras. In getQueriedItemsUncached, it adds a new condition to ignore items where itemId is undefined. That's why the resulting array is empty.

But there must be also something else. If all the items have an undefined key, they will overwrite each other in the state that stores the items data. Then the expected result is [ 'administrator', 'administrator' ], as the administrator item overwrites the editor item with the same key.

@Mamaduka Mamaduka removed the Needs Testing Needs further testing to be confirmed. label May 14, 2024
@ntsekouras
Copy link
Contributor

Caught up with the issue. Is there something actionable here? My understanding is that having an id is required for the whole system to work properly.

I'm not sure how my previous PR (extra check) could have caused this though, as @jsnajdr explains above..

@MatzeKitt
Copy link
Contributor Author

I close it since even though it worked before for me, I now adjusted my code accordingly and since it previously was used in the wrong way by myself, any deeper investigation would be waste of time.

@ntsekouras
Copy link
Contributor

Thank you @MatzeKitt for reporting the issue though!

@jsnajdr
Copy link
Member

jsnajdr commented May 24, 2024

Is there something actionable here?

One actionable thing to do is to figure out why it used to work before, and when exactly it stopped working. As far as I know, entities were always indexed by a key, so it's a mystery how an items array without keys could ever work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Core data /packages/core-data [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

5 participants