Notes: Fix 'children' embedding via REST API #72561

Mamaduka · 2025-10-22T06:35:51Z

What?

Requires #71728.
Related #71668 (comment).

PR overrides the children embeddable link for notes to allow embedding their replies (children). The current link returns no results because it uses the default type and status collection params, instead of inheriting from the current query.

An alternative option is to add a new replies link just for the note comment type and not modify the existing one, but considering we're already patching responses for the Notes, I don't think this is a breaking change.

Why?

See #71668 (comment).

Testing Instructions

Comment out following filter - add_action( 'comments_clauses', 'exclude_block_comments_from_admin', 10, 2 );.
Create a post.
Add blocks.
Add notes and their replies to these blocks.
Run JS test snippet.

Test JS snippet

function testNotesEmbedQuery( ) {
    const postId = wp.data.select( 'core/editor' ).getCurrentPostId();

    const queryArgs = {
		post: postId,
		type: 'note',
		status: 'all',
		parent: 0,
		per_page: 100,
		context: 'edit',
		_embed: 'children',
	};

	return wp.data.select( 'core' ).getEntityRecords( 'root', 'comment', queryArgs );
}

console.log( testNotesEmbedQuery() );

Testing Instructions for Keyboard

Same.

Screeenshot

Mamaduka · 2025-10-22T07:13:32Z

lib/compat/wordpress-6.9/class-gutenberg-rest-comment-controller-6-9.php

+			$args = array(
+				'parent' => $comment->comment_ID,
+				'type'   => $comment->comment_type,
+				'status' => 'all',


Currently, embedded replies are missing a couple of properties, such as content.raw and meta.

I know that links can pass their own context, but I haven't seen it used in practice by core. Is this something we should avoid?

P.S. What's the reason for disallowing embed context for specific properties, but allowing it in view context? Wasn't able to find anything in the docs.

such as content.raw

We wouldn't include content.raw, because it's an edit only property. The embed should be a subset of view.

I know that links can pass their own context, but I haven't seen it used in practice by core. Is this something we should avoid?

Yeah, I wouldn't want us to introduce that to Core, because that's really something that the client should be deciding on.

I wouldn't be opposed to something like ?_embed=children.view perhaps? Maybe that's too cheeky of an API.

What's the reason for disallowing embed context for specific properties, but allowing it in view context?

Ultimately, it's a judgement call about payload size/cost and importance. The embed context is often thought of as being used in a context (ha) where you are displaying a small snippet of a resource, and then would often link to a view where you can see everything about a resource.

I think we can still make it work without content.raw. We could refetch the note that the user is editing with proper context, or just use content.rendered. The form currently lacks RichText capabilities.

The meta values are used for display, so we'll need to fetch them. It would have been nice if each meta property could define its context, if needed.

Ultimately, it's a judgement call about payload size/cost and importance. The embed context is often thought of as being used in a context (ha) where you are displaying a small snippet of a resource, and then would often link to a view where you can see everything about a resource.

Thanks for the explanation. The logic now makes sense.

It would have been nice if each meta property could define its context, if needed.

They can in their show_in_rest schema definition. I've always used it to add an edit context and hide the value. I think we could safely extend the context for meta to include embed as a value, and then automatically apply a default context of view,edit for each meta key.

Really? I tried setting a custom context for these meta fields, but it wasn't available in the embed response.

The meta fields object has its context set to view,edit, and I don't think you can override it without changing the REST API code.

https://github.com/WordPress/wordpress-develop/blob/bd797e0358dc1f8b216f48dec1b31ef4ef24028d/src/wp-includes/rest-api/fields/class-wp-rest-meta-fields.php#L528

Right, at the moment, it only works to constrain it further. So I'm suggesting we add embed to the context list for meta. And when build up the properties schema here we add a default context of view,edit. That way, any existing meta keys continue to not show up in an embed context.

Got it. However, that's probably something for the next major release. But I'll create a ticket and patch on Trac.

Do you think that the current children link fix makes sense to include with the current release, even if we don't end up using embedded values?

Yep, I think so. It looks like a valid bug fix to me.

github-actions · 2025-10-23T09:39:04Z

Flaky tests detected in 72c6616.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/18743459196
📝 Reported issues:

[Flaky Test] ensures List View global shortcut works properly #51533 in /test/e2e/specs/editor/various/list-view.spec.js

github-actions · 2025-10-27T14:34:19Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: Mamaduka <[email protected]>
Co-authored-by: TimothyBJacobs <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Mamaduka · 2025-10-27T14:48:16Z

I'm going to merge this after all checks pass. @adamsilverstein is working on backports.

jeffpaul · 2025-10-28T14:11:11Z

@Mamaduka Does this need the 6.9 backport label or expected to have this pulled into the wordpress-develop work that @adamsilverstein is working on?

Mamaduka · 2025-10-28T14:24:54Z

@jeffpaul, this should be part of WordPress/wordpress-develop#10405 (comment).

adamsilverstein · 2025-10-28T18:59:13Z

@jeffpaul - I am pulling this in manually.

Mamaduka self-assigned this Oct 22, 2025

Mamaduka added [Type] Bug An existing feature does not function as intended REST API Interaction Related to REST API [Feature] Notes Phase 3 of the Gutenberg roadmap around block commenting labels Oct 22, 2025

Mamaduka force-pushed the try/notes-endpoint-embed-children branch from 05c6482 to e213526 Compare October 22, 2025 06:38

Mamaduka commented Oct 22, 2025

View reviewed changes

Mamaduka requested review from TimothyBJacobs, adamsilverstein and t-hamano October 22, 2025 07:13

Mamaduka force-pushed the try/notes-endpoint-embed-children branch from e213526 to 72c6616 Compare October 23, 2025 09:11

This was referenced Oct 26, 2025

Ensure comments REST controller returns note type children WordPress/wordpress-develop#10420

Closed

Try/notes staggered load - load parents first #72693

Closed

Mamaduka marked this pull request as ready for review October 27, 2025 14:34

Mamaduka requested a review from spacedmonkey as a code owner October 27, 2025 14:34

Mamaduka added 2 commits October 27, 2025 18:37

Notes: Fix 'children' embedding via REST API

b0d6b34

Add backport changelog entry

b8a7e37

Mamaduka force-pushed the try/notes-endpoint-embed-children branch from 72c6616 to b8a7e37 Compare October 27, 2025 14:38

Mamaduka mentioned this pull request Oct 28, 2025

WP_Comment_Query: exclude the 'note' type, unless 'all' or 'note' type explicitly requested WordPress/wordpress-develop#10405

Closed

Mamaduka merged commit c7a37db into trunk Oct 28, 2025
34 checks passed

Mamaduka deleted the try/notes-endpoint-embed-children branch October 28, 2025 04:22

github-actions bot mentioned this pull request Oct 28, 2025

PHP backports for WP 6.9 #72379

Closed

github-actions bot added this to the Gutenberg 22.0 milestone Oct 28, 2025

Notes: Fix 'children' embedding via REST API #72561

Notes: Fix 'children' embedding via REST API #72561

Uh oh!

Conversation

Mamaduka commented Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What?

Why?

Testing Instructions

Testing Instructions for Keyboard

Screeenshot

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Oct 23, 2025

Uh oh!

github-actions bot commented Oct 27, 2025

Uh oh!

Mamaduka commented Oct 27, 2025

Uh oh!

Uh oh!

jeffpaul commented Oct 28, 2025

Uh oh!

Mamaduka commented Oct 28, 2025

Uh oh!

adamsilverstein commented Oct 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Mamaduka commented Oct 22, 2025 •

edited

Loading