From f9baca59e0c722b8d85ee5abd03ecbe7f17c89ec Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Fri, 17 Jul 2026 11:11:50 -0700 Subject: [PATCH 01/23] Suggest mode 2/7: suggestion storage, REST controller, and provider Adds the comment-meta storage backbone: _wp_suggestion payload meta and sanitization (64KB cap, KSES on block snapshots), the 7.1 REST comment controller subclass scoped to suggestion lifecycle updates, the comment-meta SuggestionsProvider (create/update/delete, apply/reject for attribute and structural operations, schema versioning + migration), the pending-suggestion overlay store with its debounced write queue, and the auto-save loop that persists overlay entries as note comments. Inline (marker-based) operations are added by a later layer. Nothing captures suggestions yet - that starts in the next layer. --- .../wordpress-7.1/block-suggestions.php | 202 ++++ ...-gutenberg-rest-comment-controller-7-1.php | 301 +++++ lib/load.php | 5 + .../editor/src/components/provider/index.js | 62 +- .../components/suggestion-mode/auto-save.js | 290 +++++ .../src/components/suggestion-mode/index.js | 19 + .../suggestion-mode/overlay-context.js | 613 ++++++++++ .../components/suggestion-mode/provider.js | 1033 +++++++++++++++++ .../suggestion-mode/suggestion-write-queue.js | 63 + .../suggestion-mode/test/auto-save.js | 500 ++++++++ .../suggestion-mode/test/overlay-context.js | 328 ++++++ .../suggestion-mode/test/provider.js | 827 +++++++++++++ .../test/suggestion-write-queue.js | 98 ++ ...est-comments-controller-gutenberg-test.php | 699 ++++++++++- tools/eslint/suppressions.json | 10 + 15 files changed, 5028 insertions(+), 22 deletions(-) create mode 100644 lib/compat/wordpress-7.1/block-suggestions.php create mode 100644 lib/compat/wordpress-7.1/class-gutenberg-rest-comment-controller-7-1.php create mode 100644 packages/editor/src/components/suggestion-mode/auto-save.js create mode 100644 packages/editor/src/components/suggestion-mode/index.js create mode 100644 packages/editor/src/components/suggestion-mode/overlay-context.js create mode 100644 packages/editor/src/components/suggestion-mode/provider.js create mode 100644 packages/editor/src/components/suggestion-mode/suggestion-write-queue.js create mode 100644 packages/editor/src/components/suggestion-mode/test/auto-save.js create mode 100644 packages/editor/src/components/suggestion-mode/test/overlay-context.js create mode 100644 packages/editor/src/components/suggestion-mode/test/provider.js create mode 100644 packages/editor/src/components/suggestion-mode/test/suggestion-write-queue.js diff --git a/lib/compat/wordpress-7.1/block-suggestions.php b/lib/compat/wordpress-7.1/block-suggestions.php new file mode 100644 index 00000000000000..ab51db331f8c89 --- /dev/null +++ b/lib/compat/wordpress-7.1/block-suggestions.php @@ -0,0 +1,202 @@ + $inner_block ) { + if ( is_array( $inner_block ) ) { + $block['innerBlocks'][ $index ] = gutenberg_kses_suggestion_block_snapshot( $inner_block ); + } + } + } + return $block; +} + +/** + * Sanitizes a `_wp_suggestion` payload to match what the writing user could + * publish directly in post content. + * + * The suggestion payload is applied verbatim to block attributes when a + * reviewer accepts it. Without write-time filtering, a low-capability + * suggester could smuggle markup (script tags, event handlers) that a + * reviewer with `unfiltered_html` would then persist under their own KSES + * exemption. To close that hole while keeping parity with regular editing: + * + * - Users with `unfiltered_html` store the payload as-is — the same + * freedom they already have in post content. + * - Everyone else has `wp_kses_post()` applied to the string values that + * get APPLIED to content on accept/reject: `after`, `afterHTML`, + * `beforeHTML`, and the serialized block snapshot in `block`. + * + * `before` is intentionally NOT filtered: it is only compared against live + * content for conflict detection, never applied. Filtering it would produce + * false staleness warnings whenever the real content contains markup that + * KSES would strip. + * + * Note: apply-time sanitization scope is still under discussion; this + * write-time capability-matched filter is the baseline. + * + * @param string $value Raw JSON payload. + * @return string Sanitized JSON payload, or '' when the payload is invalid. + */ +function gutenberg_sanitize_suggestion_payload( $value ) { + if ( current_user_can( 'unfiltered_html' ) ) { + return $value; + } + + $decoded = json_decode( $value, true ); + // The REST controller rejects invalid-JSON payloads with a 400 before + // this callback runs; treat any non-REST garbage the same way the size + // cap does — reject rather than store something the client can't parse. + if ( ! is_array( $decoded ) ) { + return ''; + } + + if ( isset( $decoded['operations'] ) && is_array( $decoded['operations'] ) ) { + foreach ( $decoded['operations'] as $index => $operation ) { + if ( ! is_array( $operation ) ) { + continue; + } + foreach ( array( 'after', 'afterHTML', 'beforeHTML' ) as $key ) { + if ( isset( $operation[ $key ] ) && is_string( $operation[ $key ] ) ) { + $operation[ $key ] = wp_kses_post( $operation[ $key ] ); + } + } + if ( isset( $operation['block'] ) && is_array( $operation['block'] ) ) { + $operation['block'] = gutenberg_kses_suggestion_block_snapshot( $operation['block'] ); + } + $decoded['operations'][ $index ] = $operation; + } + } + + $encoded = wp_json_encode( $decoded ); + return false === $encoded ? '' : $encoded; +} + +/** + * Registers the comment meta used by suggested edits. + * + * Notes ship in WordPress 6.9 core, which registers the base note meta + * (`_wp_note_status`). Suggestions are a Gutenberg 7.1 feature layered on top, + * so the suggestion-specific meta is registered here: + * + * - `_wp_suggestion` — proposed edit, JSON payload. Presence of this + * meta is what makes a note a suggestion. + * - `_wp_suggestion_status` — suggestion lifecycle (`pending` / `applied` + * / `rejected`). Set on apply or reject so the + * comment thread persists as evidence even after + * the suggestion is resolved. + * + * The suggestion is stored as comment meta rather than `comment_content` so a + * note can carry both a discussion (content) and a proposed edit (meta), and so + * per-meta `auth_callback`/`sanitize_callback` give strict per-field control + * independent of comment-text moderation. Size validation is strict: oversized + * payloads are rejected rather than truncated, since truncating JSON corrupts + * the payload. + */ +function gutenberg_register_suggestion_meta() { + $max_suggestion_payload_bytes = GUTENBERG_SUGGESTION_PAYLOAD_MAX_BYTES; + + register_meta( + 'comment', + '_wp_suggestion', + array( + 'type' => 'string', + 'description' => __( 'Suggested edit payload (JSON).', 'gutenberg' ), + 'single' => true, + 'show_in_rest' => array( + 'schema' => array( + 'type' => 'string', + 'maxLength' => $max_suggestion_payload_bytes, + ), + ), + 'sanitize_callback' => function ( $value ) use ( $max_suggestion_payload_bytes ) { + if ( ! is_string( $value ) ) { + return ''; + } + // Reject rather than truncate. Truncating mid-string produces + // invalid JSON; `parseSuggestionPayload` would then return + // null and the suggestion would silently disappear. + if ( strlen( $value ) > $max_suggestion_payload_bytes ) { + return ''; + } + // Capability-matched KSES filtering; runs as the writing user + // (the suggester) on create/update. + return gutenberg_sanitize_suggestion_payload( $value ); + }, + 'auth_callback' => function ( $allowed, $meta_key, $object_id ) { + // During comment creation the comment does not yet exist, so + // `object_id` is 0. Defer to the comment controller's own + // create permission — if the request can create the + // comment at all, it can set the suggestion meta on it. + if ( ! $object_id ) { + return current_user_can( 'edit_posts' ); + } + $comment = get_comment( $object_id ); + if ( $comment && 'note' === $comment->comment_type ) { + return current_user_can( 'edit_post', $comment->comment_post_ID ); + } + return current_user_can( 'edit_comment', $object_id ); + }, + ) + ); + + register_meta( + 'comment', + '_wp_suggestion_status', + array( + 'type' => 'string', + 'description' => __( 'Suggestion lifecycle status.', 'gutenberg' ), + 'single' => true, + 'show_in_rest' => array( + 'schema' => array( + 'type' => 'string', + 'enum' => array( 'pending', 'applied', 'rejected' ), + ), + ), + 'auth_callback' => function ( $allowed, $meta_key, $object_id ) { + $comment = get_comment( $object_id ); + if ( $comment && 'note' === $comment->comment_type ) { + return current_user_can( 'edit_post', $comment->comment_post_ID ); + } + return current_user_can( 'edit_comment', $object_id ); + }, + ) + ); +} +add_action( 'init', 'gutenberg_register_suggestion_meta' ); diff --git a/lib/compat/wordpress-7.1/class-gutenberg-rest-comment-controller-7-1.php b/lib/compat/wordpress-7.1/class-gutenberg-rest-comment-controller-7-1.php new file mode 100644 index 00000000000000..6e525ad0404b09 --- /dev/null +++ b/lib/compat/wordpress-7.1/class-gutenberg-rest-comment-controller-7-1.php @@ -0,0 +1,301 @@ + GUTENBERG_SUGGESTION_PAYLOAD_MAX_BYTES ) { + return new WP_Error( + 'rest_suggestion_too_large', + sprintf( + /* translators: %d: maximum allowed byte length. */ + __( 'Suggestion payload exceeds the %d-byte limit.', 'gutenberg' ), + GUTENBERG_SUGGESTION_PAYLOAD_MAX_BYTES + ), + array( 'status' => 413 ) + ); + } + // An empty string is the documented "no suggestion" value; anything + // else must decode to a JSON object carrying the payload fields. + if ( '' !== $value ) { + $decoded = json_decode( $value, true ); + if ( ! is_array( $decoded ) ) { + return new WP_Error( + 'rest_suggestion_invalid_json', + __( 'Suggestion payload must be a valid JSON object.', 'gutenberg' ), + array( 'status' => 400 ) + ); + } + } + return true; + } + + /** + * Determines whether a note-update request touches only the fields used + * by the suggestion apply/reject lifecycle. + * + * Allowed fields: + * - `status` (limited to `approved` or `hold`) + * - `meta._wp_suggestion_status` + * + * Any other field present in the request disqualifies it from the + * `edit_post` shortcut, forcing it through core's `edit_comment` check + * instead. Core's `update_item` reads `$request['content']` etc. from + * the MERGED param view (JSON > POST > GET > URL), so the inspected + * key set is built from every client-supplied channel — a field + * smuggled in as a query parameter (`?content=rewritten`) alongside a + * lifecycle-only body cannot slip past the allowlist. Server-injected + * schema defaults (`post`, `parent`) are excluded: the client didn't + * send them and they don't rewrite anything. + * + * Note this method is a scope limiter, not an authorization boundary: + * see `update_item_permissions_check` for what the shortcut does and + * does not protect against. + * + * @param WP_REST_Request $request Full details about the request. + * @return bool + */ + private static function is_suggestion_lifecycle_update( $request ) { + // Union of every client-supplied parameter channel, mirroring the + // precedence core uses when reading `$request[ $key ]`. + $params = array(); + foreach ( array( + $request->get_url_params(), + $request->get_query_params(), + $request->get_body_params(), + $request->get_json_params(), + ) as $channel ) { + if ( is_array( $channel ) ) { + $params = array_merge( $params, $channel ); + } + } + if ( empty( $params ) ) { + return false; + } + + /* + * Keys that are always present or can't write comment fields: + * `id` comes from the route; the underscore-prefixed keys are + * REST meta-parameters (`_locale` is added by api-fetch on every + * editor request); `context` shapes the response, not the write. + */ + $ignored_keys = array( + 'id', + 'context', + '_locale', + '_fields', + '_embed', + '_envelope', + '_jsonp', + '_method', + ); + + $allowed_keys = array( 'status', 'meta' ); + $inspected = array_diff( array_keys( $params ), $ignored_keys ); + if ( empty( $inspected ) ) { + return false; + } + foreach ( $inspected as $key ) { + if ( ! in_array( $key, $allowed_keys, true ) ) { + return false; + } + } + + if ( + isset( $params['status'] ) && + ! in_array( $params['status'], array( 'approved', 'hold' ), true ) + ) { + return false; + } + + if ( isset( $params['meta'] ) ) { + if ( ! is_array( $params['meta'] ) ) { + return false; + } + $allowed_meta = array( '_wp_suggestion_status' ); + foreach ( array_keys( $params['meta'] ) as $meta_key ) { + if ( ! in_array( $meta_key, $allowed_meta, true ) ) { + return false; + } + } + } + + return true; + } + + /** + * Checks if a given request has access to update a comment. + * + * Extends core's check so that users who can `edit_post` on the parent + * post are also allowed to update note-type comments for + * suggestion-lifecycle fields (status and `_wp_suggestion_status` + * meta). This unblocks the suggestion workflow where a post editor + * applies or rejects a suggestion authored by someone else. + * + * Scope note: the lifecycle allowlist limits what THIS shortcut + * grants; it does not (and cannot) prevent note rewrites in general. + * Requests that touch other fields fall through to core's + * `edit_comment` check, and core's `map_meta_cap` resolves + * `edit_comment` to `edit_post` on the comment's parent post - so a + * post editor already holds full edit permission over notes on their + * posts through the core fallback. + * + * @param WP_REST_Request $request Full details about the request. + * @return true|WP_Error True if the request has access, WP_Error otherwise. + */ + public function update_item_permissions_check( $request ) { + $comment = $this->get_comment( $request['id'] ); + if ( is_wp_error( $comment ) ) { + return $comment; + } + + // For note comments, allow users who can edit the parent post to + // update suggestion-lifecycle fields only. + if ( + 'note' === $comment->comment_type && + self::is_suggestion_lifecycle_update( $request ) + ) { + $post = get_post( $comment->comment_post_ID ); + if ( $post && current_user_can( 'edit_post', $post->ID ) ) { + return true; + } + } + + // Fall back to core's default check (moderate_comments or edit_comment). + return parent::update_item_permissions_check( $request ); + } + + /** + * Prepares a single comment for create or update. + * + * Wraps core's preparation with two suggestion-specific concerns: + * + * - Rejects oversized `_wp_suggestion` payloads with a clean 413, and + * payloads that aren't valid JSON objects with a 400, before any + * storage happens (both create and update call this and return + * its WP_Error). + * - Surfaces the `_wp_suggestion` payload in the prepared `meta` so the + * content-allowed check can recognize a payload-only note, mirroring + * how core copies `_wp_note_status` for the same check. + * + * @param WP_REST_Request $request Request object. + * @return array|WP_Error Prepared comment, or WP_Error. + */ + protected function prepare_item_for_database( $request ) { + $payload_check = self::validate_suggestion_payload( $request ); + if ( is_wp_error( $payload_check ) ) { + return $payload_check; + } + + $prepared_comment = parent::prepare_item_for_database( $request ); + if ( is_wp_error( $prepared_comment ) ) { + return $prepared_comment; + } + + if ( isset( $request['meta']['_wp_suggestion'] ) ) { + if ( ! isset( $prepared_comment['meta'] ) || ! is_array( $prepared_comment['meta'] ) ) { + $prepared_comment['meta'] = array(); + } + $prepared_comment['meta']['_wp_suggestion'] = $request['meta']['_wp_suggestion']; + } + + return $prepared_comment; + } + + /** + * Allows a note comment to have empty content when it carries a + * suggestion payload. + * + * A pure suggestion (a proposed edit with no discussion text) has empty + * `comment_content`; core would otherwise reject it. Everything else + * defers to core's check. + * + * @param array $prepared_comment Prepared comment data. + * @return bool + */ + protected function check_is_comment_content_allowed( $prepared_comment ) { + if ( + isset( $prepared_comment['comment_type'] ) && + 'note' === $prepared_comment['comment_type'] && + ! empty( $prepared_comment['meta']['_wp_suggestion'] ) + ) { + return true; + } + + return parent::check_is_comment_content_allowed( $prepared_comment ); + } + } +} + +add_action( + 'rest_api_init', + function () { + // Register after core's default comments controller (priority 10) so the + // note-aware /wp/v2/comments routes are overridden with the + // suggestion-aware subclass. + $controller = new Gutenberg_REST_Comment_Controller_7_1(); + $controller->register_routes(); + }, + 11 +); diff --git a/lib/load.php b/lib/load.php index 8c1dc53d4fb9bc..a43a45d7847f22 100644 --- a/lib/load.php +++ b/lib/load.php @@ -79,6 +79,11 @@ function gutenberg_is_experiment_enabled( $name ) { require __DIR__ . '/compat/wordpress-7.1/block-bindings.php'; require __DIR__ . '/compat/wordpress-7.1/query-block.php'; require __DIR__ . '/compat/wordpress-7.1/block-comments.php'; + // The suggestion meta registration and REST controller are always loaded: + // both are inert without suggestion data, which only the experiment-gated + // editor UI creates. The user-facing feature is gated in JS. + require __DIR__ . '/compat/wordpress-7.1/block-suggestions.php'; + require __DIR__ . '/compat/wordpress-7.1/class-gutenberg-rest-comment-controller-7-1.php'; // Plugin specific code. require_once __DIR__ . '/class-wp-rest-global-styles-controller-gutenberg.php'; diff --git a/packages/editor/src/components/provider/index.js b/packages/editor/src/components/provider/index.js index 1558d5c70bb68b..3733200a76a555 100644 --- a/packages/editor/src/components/provider/index.js +++ b/packages/editor/src/components/provider/index.js @@ -2,6 +2,7 @@ * WordPress dependencies */ import { + Fragment, useCallback, useEffect, useLayoutEffect, @@ -47,10 +48,24 @@ import PatternRenameModal from '../pattern-rename-modal'; import PatternDuplicateModal from '../pattern-duplicate-modal'; import TemplatePartMenuItems from '../template-part-menu-items'; import MediaEditorModalMount from '../media/media-editor-modal'; +import { + SuggestionOverlayProvider, + SuggestionAutoSave, + isSuggestionModeEnabled, +} from '../suggestion-mode'; const { ExperimentalBlockEditorProvider } = unlock( blockEditorPrivateApis ); const { PatternsMenuItems } = unlock( editPatternsPrivateApis ); +/* + * With the experiment off the overlay context (and its block-tree + * subscriptions) never mounts; consumers fall back to the context default, + * which is inert. + */ +const MaybeSuggestionOverlayProvider = isSuggestionModeEnabled() + ? SuggestionOverlayProvider + : Fragment; + const noop = () => {}; /** @@ -462,27 +477,32 @@ export const ExperimentalEditorProvider = withRegistryProvider( settings={ blockEditorSettings } useSubRegistry={ false } > - { children } - { ! settings.isPreviewMode && ( - <> - - - { mode === 'template-locked' && ( - - ) } - { type === 'wp_navigation' && ( - - ) } - - - - - - - - - - ) } + + { children } + { ! settings.isPreviewMode && ( + <> + + + { mode === 'template-locked' && ( + + ) } + { type === 'wp_navigation' && ( + + ) } + + + + + + + + { isSuggestionModeEnabled() && ( + + ) } + + + ) } + diff --git a/packages/editor/src/components/suggestion-mode/auto-save.js b/packages/editor/src/components/suggestion-mode/auto-save.js new file mode 100644 index 00000000000000..65d3d57c4cc307 --- /dev/null +++ b/packages/editor/src/components/suggestion-mode/auto-save.js @@ -0,0 +1,290 @@ +/** + * Background auto-save for Suggest mode. + * + * Replaces the explicit "Submit suggestion" button (`commit-bar.js` in earlier + * phases) with a debounced background save so a suggester sees their pending + * change persist on its own after a short pause in typing — the same model + * Google Docs uses for Suggesting mode. + * + * Behavior summary: + * - **Debounce**: per-block timer of `AUTOSAVE_DEBOUNCE_MS` (1500 ms). + * Each new edit on a block clears that block's timer and starts a new + * one; saves only fire during idle windows so a user typing through a + * paragraph generates one save, not one per keystroke. + * - **Per-block queue**: each `clientId` has a sequential promise chain + * (`queuesRef`). Saves on the same block are linked end-to-end so a + * slow network call doesn't race with a follow-up save and produce + * duplicate POSTs or out-of-order writes. Different blocks have + * independent queues and run concurrently. + * - **Create vs update vs delete**: a fresh overlay creates a new note; + * subsequent edits update the same note's `_wp_suggestion` meta; an + * overlay reverted back to baseline (user undid their suggestion) + * trashes the note. + * - **Collaboration**: the linked comment can be resolved by another peer + * mid-session (their accept/reject flips its `status`). Before each + * update we re-read the comment via core-data; if the linkage is stale + * we orphan it and create a fresh note. PR #75147 widened + * `metadata.noteId` to an array so multiple notes can coexist on a block. + * + * Refs are used heavily because: + * - The provider callbacks (`createSuggestion`, `updateSuggestion`, + * `deleteSuggestion`) are recreated whenever `postModified` changes, + * but in-flight saves always need the latest reference. + * - The save functions run inside a `setTimeout` callback that doesn't + * re-render, so reading the latest entries / callbacks via refs avoids + * stale-closure bugs without resubscribing on every overlay change. + */ +/** + * WordPress dependencies + */ +import { useRegistry, useSelect } from '@wordpress/data'; +import { store as coreStore } from '@wordpress/core-data'; +import { useCallback, useEffect, useRef } from '@wordpress/element'; + +/** + * Internal dependencies + */ +import { useSuggestionOverlay } from './overlay-context'; +import { operationsFromOverlay, useSuggestionsProvider } from './provider'; +import { EDITOR_STORE_NAME, SUGGEST_INTENT } from './constants'; +import { unlock } from '../../lock-unlock'; + +const AUTOSAVE_DEBOUNCE_MS = 1500; + +/** + * Deterministic fingerprint of a list of operations so we can detect whether + * the overlay has changed relative to what we last synced without comparing + * deep object trees on every render. + * + * @param {Array} operations Operations to fingerprint. + * @return {string} Stable serialization. + */ +export function fingerprintOperations( operations ) { + try { + return JSON.stringify( operations ); + } catch { + return ''; + } +} + +/** + * Derive the operation list a given overlay entry should persist. + * + * Structural entries (block-remove, block-insert-after, block-move) carry + * a single pre-built op in `entry.structuralOp`; the interceptor wrote it + * after detecting the corresponding tree mutation. Attribute-set entries + * derive their ops from the baseline-vs-overlay diff. An entry can have + * both — a user can edit attributes on a block that was suggested for + * removal — in which case the structural op leads and any attribute ops + * follow. + * + * @param {Object} entry Overlay entry. + * @return {Array} Ops describing the entry's pending suggestion. + */ +export function operationsForEntry( entry ) { + const ops = []; + if ( entry.structuralOp ) { + ops.push( entry.structuralOp ); + } + const attrOps = operationsFromOverlay( + entry.baselineAttributes, + entry.overlayAttributes + ); + for ( const op of attrOps ) { + ops.push( op ); + } + return ops; +} + +/** + * Invisible component that auto-commits pending overlay edits to the server + * as note comments. Replaces the manual "Submit suggestion" button — in + * Suggest mode each block's pending changes are persisted after a short + * idle window, and subsequent edits update the same note rather than + * spawning a new one. + * + * @return {null} Renders nothing. + */ +export default function SuggestionAutoSave() { + const { entries, setCommentId, setSyncedOpsKey } = useSuggestionOverlay(); + const { createSuggestion, updateSuggestion, deleteSuggestion } = + useSuggestionsProvider(); + const registry = useRegistry(); + + const isSuggestMode = useSelect( + ( select ) => + // `getEditorIntent` is private while Suggest mode is experimental. + unlock( select( EDITOR_STORE_NAME ) ).getEditorIntent() === + SUGGEST_INTENT, + [] + ); + + // Refs are read from inside async callbacks so a save always operates on + // the latest overlay state, not the values captured when the timer was + // scheduled. This avoids stale-closure pitfalls (e.g. acting on a null + // commentId after the previous save just set one). + const entriesRef = useRef( entries ); + entriesRef.current = entries; + + // Provider callbacks are captured in refs for the same reason: they + // change reference whenever `postModified` updates, but the in-flight + // queue should always call the latest version. + const createRef = useRef( createSuggestion ); + createRef.current = createSuggestion; + const updateRef = useRef( updateSuggestion ); + updateRef.current = updateSuggestion; + const deleteRef = useRef( deleteSuggestion ); + deleteRef.current = deleteSuggestion; + const setCommentIdRef = useRef( setCommentId ); + setCommentIdRef.current = setCommentId; + const setSyncedOpsKeyRef = useRef( setSyncedOpsKey ); + setSyncedOpsKeyRef.current = setSyncedOpsKey; + + // Per-clientId debounce timer. + const timersRef = useRef( new Map() ); + // Per-clientId promise chain. New saves are enqueued onto the existing + // chain so saves on the same block always run sequentially — no races, + // no duplicate POSTs, and no dropped work when the user keeps typing + // during a slow network call. + const queuesRef = useRef( new Map() ); + // Synchronous mirror of each block's last-known comment id. `setCommentId` + // updates React state, which only reaches `entriesRef` on the next render + // commit; a save queued immediately after a `create` resolves would run + // before that commit and read a stale `entry.commentId` of null, POSTing a + // duplicate note. This ref is written the instant a create resolves (and on + // every rotation/clear), so the queued save sees the fresh id without waiting + // for React. A `null` value is a deliberate "known to have no note" marker, + // distinct from "no entry yet" (fall back to `entry.commentId`). + const commentIdsRef = useRef( new Map() ); + const writeCommentId = useCallback( ( clientId, id ) => { + commentIdsRef.current.set( clientId, id ); + setCommentIdRef.current( clientId, id ); + }, [] ); + + const syncOnce = useCallback( + async ( clientId ) => { + const entry = entriesRef.current[ clientId ]; + if ( ! entry ) { + return; + } + const operations = operationsForEntry( entry ); + const fingerprint = fingerprintOperations( operations ); + if ( fingerprint === entry.syncedOpsKey ) { + return; + } + + // The overlay's `commentId` reference can outlive the note it + // points at: another collaborator may have accepted or rejected + // the suggestion mid-session, flipping the comment's status from + // `hold` to `approved`. Updating that comment would clobber its + // payload (and the resolved status header) with the user's new, + // unrelated edit. Treat a resolved link as if there were none so + // the next save creates a fresh note that coexists with the + // resolved one — this only works because PR #75147 lets a block + // hold multiple note ids in `metadata.noteId`. + // Prefer the synchronous mirror over `entry.commentId`: it reflects + // a create that resolved after this entry snapshot was taken but + // before React re-rendered, which is exactly the create->update + // window a duplicate note would slip through. + let commentId = commentIdsRef.current.has( clientId ) + ? commentIdsRef.current.get( clientId ) + : entry.commentId; + if ( commentId ) { + const linkedComment = registry + .select( coreStore ) + .getEntityRecord( 'root', 'comment', commentId ); + if ( linkedComment && linkedComment.status !== 'hold' ) { + commentId = null; + writeCommentId( clientId, null ); + } + } + + try { + if ( operations.length === 0 ) { + if ( commentId ) { + await deleteRef.current( { commentId } ); + writeCommentId( clientId, null ); + } + } else if ( commentId ) { + await updateRef.current( { + commentId, + blockName: entry.blockName, + operations, + } ); + } else { + const saved = await createRef.current( { + clientId, + blockName: entry.blockName, + operations, + } ); + if ( saved?.id ) { + writeCommentId( clientId, saved.id ); + } + } + setSyncedOpsKeyRef.current( clientId, fingerprint ); + } catch { + // Error notice is surfaced inside the provider. The next overlay + // change will re-enqueue a sync, so transient failures recover + // on their own. + } + }, + [ registry, writeCommentId ] + ); + + const enqueueSync = useCallback( + ( clientId ) => { + const queues = queuesRef.current; + const previous = queues.get( clientId ) ?? Promise.resolve(); + const next = previous + .catch( () => {} ) + .then( () => syncOnce( clientId ) ); + queues.set( clientId, next ); + next.finally( () => { + if ( queues.get( clientId ) === next ) { + queues.delete( clientId ); + } + } ); + }, + [ syncOnce ] + ); + + useEffect( () => { + if ( ! isSuggestMode ) { + return undefined; + } + + const timers = timersRef.current; + + for ( const [ clientId, entry ] of Object.entries( entries ) ) { + const operations = operationsForEntry( entry ); + const fingerprint = fingerprintOperations( operations ); + if ( fingerprint === entry.syncedOpsKey ) { + continue; + } + + if ( timers.has( clientId ) ) { + clearTimeout( timers.get( clientId ) ); + } + const timer = setTimeout( () => { + timers.delete( clientId ); + enqueueSync( clientId ); + }, AUTOSAVE_DEBOUNCE_MS ); + timers.set( clientId, timer ); + } + + return undefined; + }, [ isSuggestMode, entries, enqueueSync ] ); + + // Clear all pending timers on unmount. + useEffect( () => { + const timers = timersRef.current; + return () => { + for ( const timer of timers.values() ) { + clearTimeout( timer ); + } + timers.clear(); + }; + }, [] ); + + return null; +} diff --git a/packages/editor/src/components/suggestion-mode/index.js b/packages/editor/src/components/suggestion-mode/index.js new file mode 100644 index 00000000000000..784c6891318434 --- /dev/null +++ b/packages/editor/src/components/suggestion-mode/index.js @@ -0,0 +1,19 @@ +export { isSuggestionModeEnabled, useCanSuggest } from './gate'; +export { + SuggestionOverlayProvider, + useSuggestionOverlay, + overlayReducer, +} from './overlay-context'; +export { default as SuggestionAutoSave } from './auto-save'; +export { + useSuggestionsProvider, + operationsFromOverlay, + applyOperations, + hasAttributeConflict, + parseSuggestionPayload, + payloadByteLength, + findStructuralOp, + clearSuggestionMarkerAttributes, + PAYLOAD_MAX_BYTES, + SCHEMA_VERSION, +} from './provider'; diff --git a/packages/editor/src/components/suggestion-mode/overlay-context.js b/packages/editor/src/components/suggestion-mode/overlay-context.js new file mode 100644 index 00000000000000..686f258a71a9a2 --- /dev/null +++ b/packages/editor/src/components/suggestion-mode/overlay-context.js @@ -0,0 +1,613 @@ +/** + * In-memory overlay system for Suggest mode. + * + * The overlay holds user edits made while the editor is in `suggest` intent + * without ever writing them through to the block-editor store. Each entry is + * keyed by `clientId` and carries: + * - `baselineAttributes` — captured on first edit, used by + * `operationsFromOverlay` (provider.js) to build the persisted suggestion. + * - `overlayAttributes` — pending user changes; merged into the rendered + * attributes by `withSuggestionOverlay` so the user sees their edit, but + * never stored. + * + * Why an overlay rather than a draft post / branch? + * - The post stays at its real baseline so autosave, undo/redo, and + * real-time collaboration sync see only persisted state. + * - Multiple editors can suggest concurrently without conflicting writes. + * - Suggestions stay immutable until explicitly committed (`createSuggestion`), + * so a half-typed edit never leaks into the post. + * + * Overlay entry lifecycle: + * 1. `captureBaseline` — fired on first `setAttributes` (HOC) or on the first + * detected store-level mutation (store-interceptor). + * 2. `setOverlayAttributes` — accumulated by the wrapped `setAttributes` or + * by interceptor diffs. + * 3. `clearOverlay` / `PRUNE_ORPHANS` — entries are dropped when the + * suggestion is committed, rejected, or the underlying block is deleted. + * + * The orphan prune runs whenever the live block tree shrinks; it skips when + * the block-editor store isn't registered (tests, standalone consumers). + */ +/** + * WordPress dependencies + */ +import { + createContext, + useCallback, + useContext, + useEffect, + useMemo, + useReducer, + useRef, +} from '@wordpress/element'; +import { useRegistry, useSelect } from '@wordpress/data'; + +/** + * Internal dependencies + */ +import { createSuggestionWriteQueue } from './suggestion-write-queue'; + +// Referenced by name to keep the provider runnable in tests and standalone +// contexts where the block-editor store isn't registered. Orphan cleanup is +// skipped in those environments. +const BLOCK_EDITOR_STORE_NAME = 'core/block-editor'; + +/* + * Monotonic sequence shared by every capture path so the undo guard can order + * an overlay-held attribute suggestion against marker/structural captures + * (which live on the real undo stack). Module-scoped: the ordering only needs + * to be consistent within a session, not persisted. + */ +let captureSequence = 0; +const nextCaptureSeq = () => ++captureSequence; + +/* + * How long an armed undo/redo adoption token stays valid. The token is armed + * synchronously when undo/redo dispatches, but the block-editor tree only + * reflects the entity change after React re-renders and the block-sync effect + * runs — an async gap the interceptor can't observe directly. The expiry + * bounds the window so a stale token (an undo that ended up changing nothing + * block-related) can't swallow a later genuine edit. + */ +const UNDO_ADOPTION_TTL_MS = 1000; + +/** + * @typedef {Object} OverlayEntry + * @property {string} blockName The block name at the time the + * overlay was opened. + * @property {Object} baselineAttributes The attributes captured when + * Suggest mode first began editing + * this block. + * @property {Object} overlayAttributes Pending attribute changes that + * have not yet been committed. + */ + +/** + * @typedef {Object} OverlayContextValue + * @property {Object.} entries Per-clientId entries. + * @property {Function} captureBaseline Store a baseline for a + * block if one isn't set. + * @property {Function} setOverlayAttributes Merge overlay attributes + * onto an entry. + * @property {Function} clearOverlay Remove the entry. + * @property {Function} hasOverlay Check if an entry has any + * overlay attributes. + */ + +const EMPTY_ENTRIES = Object.freeze( {} ); + +const OverlayContext = createContext( { + entries: EMPTY_ENTRIES, + captureBaseline: () => {}, + setOverlayAttributes: () => {}, + clearOverlay: () => {}, + setCommentId: () => {}, + setSyncedOpsKey: () => {}, + setStructuralOp: () => {}, + hasOverlay: () => false, + requestInterceptorBypass: () => {}, + consumeInterceptorBypass: () => false, + registerFormatHandler: () => () => {}, + requestFormatSuggestion: () => false, + registerContentHandler: () => () => {}, + requestContentSuggestion: () => false, + // Standalone default (no provider mounted): run the task immediately. + enqueueSuggestionWrite: ( clientId, task ) => task(), + markDeferredInsertion: () => {}, + unmarkDeferredInsertion: () => {}, + isDeferredInsertion: () => false, + clearDeferredInsertions: () => {}, + getLastContentCaptureSeq: () => 0, + armUndoRedoAdoption: () => {}, + consumeUndoRedoAdoption: () => false, +} ); + +/** + * Reducer managing the map of pending block overlays. + * + * @param {Object} state Current state. + * @param {Object} action Action. + * @return {Object} Next state. + */ +export function overlayReducer( state, action ) { + switch ( action.type ) { + case 'CAPTURE_BASELINE': { + if ( state[ action.clientId ] ) { + return state; + } + return { + ...state, + [ action.clientId ]: { + blockName: action.blockName, + baselineAttributes: action.attributes, + overlayAttributes: {}, + commentId: null, + syncedOpsKey: null, + }, + }; + } + case 'SET_OVERLAY_ATTRIBUTES': { + const entry = state[ action.clientId ]; + if ( ! entry ) { + return state; + } + return { + ...state, + [ action.clientId ]: { + ...entry, + overlayAttributes: { + ...entry.overlayAttributes, + ...action.attributes, + }, + // Capture-order stamp read by the undo guard to find the + // most recently edited attribute suggestion. Kept from + // the previous state when the action carries no sequence + // (e.g. reducer unit tests). + lastEditSeq: action.seq ?? entry.lastEditSeq, + }, + }; + } + case 'CLEAR_OVERLAY': { + if ( ! state[ action.clientId ] ) { + return state; + } + const { [ action.clientId ]: _removed, ...rest } = state; + return rest; + } + case 'SET_COMMENT_ID': { + const entry = state[ action.clientId ]; + if ( ! entry ) { + return state; + } + return { + ...state, + [ action.clientId ]: { + ...entry, + commentId: action.commentId, + }, + }; + } + case 'SET_SYNCED_OPS_KEY': { + const entry = state[ action.clientId ]; + if ( ! entry ) { + return state; + } + return { + ...state, + [ action.clientId ]: { + ...entry, + syncedOpsKey: action.syncedOpsKey, + }, + }; + } + case 'SET_STRUCTURAL_OP': { + // Structural ops (block-remove, block-insert-after, block-move) + // don't have a baseline-vs-overlay attribute diff; the operation + // itself describes the change. Auto-save reads `structuralOp` + // straight through. Replaces any existing op for the same block + // — only one structural marker can be pending at a time. + const existing = state[ action.clientId ]; + return { + ...state, + [ action.clientId ]: { + blockName: action.blockName, + baselineAttributes: existing?.baselineAttributes ?? {}, + overlayAttributes: existing?.overlayAttributes ?? {}, + commentId: existing?.commentId ?? null, + syncedOpsKey: existing?.syncedOpsKey ?? null, + lastEditSeq: existing?.lastEditSeq, + structuralOp: action.op, + // Capture-order stamp read by the undo guard to find the + // most recently captured structural suggestion. + structuralOpSeq: action.seq ?? existing?.structuralOpSeq, + }, + }; + } + case 'PRUNE_ORPHANS': { + // Action carries a serializable array; the reducer materializes a + // Set internally for the lookup. Keeps actions Redux-DevTools- + // friendly (Sets aren't serializable for time-travel). + const liveIds = Array.isArray( action.liveClientIds ) + ? new Set( action.liveClientIds ) + : action.liveClientIds; + const keys = Object.keys( state ); + let changed = false; + const next = {}; + for ( const key of keys ) { + if ( liveIds.has( key ) ) { + next[ key ] = state[ key ]; + } else { + changed = true; + } + } + return changed ? next : state; + } + default: + return state; + } +} + +/** + * Provider exposing the suggestion overlay to descendant blocks. + * + * The overlay is intentionally in-memory only. It stores pending attribute + * changes per `clientId` so a block can render the user's in-progress + * suggestion without mutating the real block-editor state. + * + * @param {{ children: React.ReactNode }} props + */ +export function SuggestionOverlayProvider( { children } ) { + const [ entries, dispatch ] = useReducer( overlayReducer, EMPTY_ENTRIES ); + const registry = useRegistry(); + + const captureBaseline = useCallback( + ( clientId, blockName, attributes ) => + dispatch( { + type: 'CAPTURE_BASELINE', + clientId, + blockName, + attributes, + } ), + [] + ); + + const setOverlayAttributes = useCallback( + ( clientId, attributes ) => + dispatch( { + type: 'SET_OVERLAY_ATTRIBUTES', + clientId, + attributes, + seq: nextCaptureSeq(), + } ), + [] + ); + + const clearOverlay = useCallback( + ( clientId ) => dispatch( { type: 'CLEAR_OVERLAY', clientId } ), + [] + ); + + const setCommentId = useCallback( + ( clientId, commentId ) => + dispatch( { type: 'SET_COMMENT_ID', clientId, commentId } ), + [] + ); + + const setSyncedOpsKey = useCallback( + ( clientId, syncedOpsKey ) => + dispatch( { type: 'SET_SYNCED_OPS_KEY', clientId, syncedOpsKey } ), + [] + ); + + /* + * Sequence stamp of the most recent capture that lives on the real undo + * stack (an inline marker write or a structural marker). The undo guard + * compares it against overlay entries' `lastEditSeq` to decide whether + * Ctrl+Z should cancel a pending attribute suggestion or perform a normal + * undo. A ref because it is written from `registry.subscribe` and event + * handlers, and read synchronously inside the wrapped undo dispatch. + */ + const lastContentCaptureSeqRef = useRef( 0 ); + + const getLastContentCaptureSeq = useCallback( + () => lastContentCaptureSeqRef.current, + [] + ); + + const setStructuralOp = useCallback( ( clientId, blockName, op ) => { + dispatch( { + type: 'SET_STRUCTURAL_OP', + clientId, + blockName, + op, + seq: nextCaptureSeq(), + } ); + }, [] ); + + const hasEntries = Object.keys( entries ).length > 0; + + const hasOverlay = useCallback( + ( clientId ) => { + const entry = entries[ clientId ]; + return ( + !! entry && Object.keys( entry.overlayAttributes ).length > 0 + ); + }, + [ entries ] + ); + + // Tracks clientIds whose next block-attribute mutation should bypass the + // store interceptor. The accept-suggestion flow uses this to land applied + // attributes on the live block — without it, the interceptor would treat + // the apply as just another user edit and revert it into the overlay. + // A ref-set rather than reducer state because the value is consumed + // inside `registry.subscribe` (which doesn't react to React state) and + // must clear synchronously when the dispatch is processed. + const bypassClientIdsRef = useRef( new Set() ); + + const requestInterceptorBypass = useCallback( ( clientId ) => { + if ( clientId ) { + bypassClientIdsRef.current.add( clientId ); + /* + * Every inline marker write (addition/deletion/format keyboards, + * content reconciler) requests a bypass first, so this doubles as + * the "an inline capture happened" stamp for the undo guard. + * Apply/reject flows bump it too, which is harmless — ordering + * only matters relative to pending attribute and structural + * captures still held by the overlay. + */ + lastContentCaptureSeqRef.current = nextCaptureSeq(); + } + }, [] ); + + const consumeInterceptorBypass = useCallback( ( clientId ) => { + const set = bypassClientIdsRef.current; + if ( ! set.has( clientId ) ) { + return false; + } + set.delete( clientId ); + return true; + }, [] ); + + // Single slot for the format-suggestion handler. The per-block overlay HOC + // only *detects* a formatting-only edit (cheap, no store access); the + // actual note creation + marker write lives in one mounted component + // (`SuggestionFormatKeyboard`) that registers its handler here. Keeping the + // heavy `useSuggestionsProvider` out of every block's render is why this is + // a singleton rather than a per-block hook. A ref (not state) so + // registering doesn't re-render every subscribed block. + const formatHandlerRef = useRef( null ); + + const registerFormatHandler = useCallback( ( handler ) => { + formatHandlerRef.current = handler; + return () => { + if ( formatHandlerRef.current === handler ) { + formatHandlerRef.current = null; + } + }; + }, [] ); + + const requestFormatSuggestion = useCallback( ( request ) => { + const handler = formatHandlerRef.current; + if ( ! handler ) { + return false; + } + /* + * The handler returns a synchronous verdict: `false` means it cannot + * process this request, and the caller must let the edit fall through + * to the overlay path rather than swallow it. Anything else — + * including a promise from an async handler — counts as accepted. + */ + return handler( request ) !== false; + }, [] ); + + // Single slot for the content-reconciliation handler, the twin of the format + // handler above for text edits that reach the block as a whole new `content` + // value rather than a `beforeinput` the keyboards intercept (a committed IME + // composition, autocorrect, a drag-drop, a multi-line paste). The per-block + // HOC runs the cheap diff and hands a ready marker plan here; this single + // mounted component owns note creation and the marker write. + const contentHandlerRef = useRef( null ); + + const registerContentHandler = useCallback( ( handler ) => { + contentHandlerRef.current = handler; + return () => { + if ( contentHandlerRef.current === handler ) { + contentHandlerRef.current = null; + } + }; + }, [] ); + + const requestContentSuggestion = useCallback( ( request ) => { + const handler = contentHandlerRef.current; + if ( ! handler ) { + return false; + } + // Same synchronous-verdict contract as `requestFormatSuggestion`. + return handler( request ) !== false; + }, [] ); + + /* + * One write queue per editor, shared by the content reconciler and the + * format keyboard so their note-then-marker flights serialize per block + * instead of interleaving (each component keeping its own in-flight guard + * previously let one of each race on the same block). A ref because the + * queue is imperative state consumed outside React's render cycle. + */ + const writeQueueRef = useRef( null ); + if ( writeQueueRef.current === null ) { + writeQueueRef.current = createSuggestionWriteQueue(); + } + const enqueueSuggestionWrite = useCallback( + ( clientId, task ) => writeQueueRef.current.enqueue( clientId, task ), + [] + ); + + // Tracks new blocks whose registration as an insertion suggestion the + // store interceptor has DEFERRED: an unmodified default block inserted in + // Suggest mode (clicking the appender) is not a suggestion until the user + // puts something into it. The overlay HOC and the inline suggestion + // keyboards consult this set so the first edit inside such a block falls + // through to the real attributes — letting the interceptor register the + // whole block as a single `block-insert-after` suggestion — instead of + // opening a separate inline/overlay suggestion next to the insertion. + // A ref-set for the same reason as the bypass set above: it is written + // from inside `registry.subscribe` and read synchronously during event + // handling, neither of which can wait on React state. + const deferredInsertionsRef = useRef( new Set() ); + + const markDeferredInsertion = useCallback( ( clientId ) => { + if ( clientId ) { + deferredInsertionsRef.current.add( clientId ); + } + }, [] ); + + const unmarkDeferredInsertion = useCallback( ( clientId ) => { + deferredInsertionsRef.current.delete( clientId ); + }, [] ); + + const isDeferredInsertion = useCallback( + ( clientId ) => deferredInsertionsRef.current.has( clientId ), + [] + ); + + // Reset when a Suggest session starts: a block deferred in a previous + // session is seeded into the interceptor's snapshot like any other + // pre-existing block, so a stale entry would wrongly write edits through. + const clearDeferredInsertions = useCallback( () => { + deferredInsertionsRef.current.clear(); + }, [] ); + + /* + * Undo/redo adoption tokens. The undo guard arms one token per undo/redo + * dispatch; the store interceptor consumes a token when the resulting + * block-editor change lands, and adopts that change as the new capture + * baseline instead of treating it as a fresh user edit (which would + * re-capture the undo as a brand-new suggestion). Tokens expire (see + * UNDO_ADOPTION_TTL_MS) because the block sync happens a React commit + * after the dispatch and an undo may turn out to touch nothing + * block-related. A counter-of-expiries rather than a boolean so two quick + * undo presses arm two adoptions. + */ + const undoAdoptionExpiriesRef = useRef( [] ); + + const armUndoRedoAdoption = useCallback( () => { + undoAdoptionExpiriesRef.current.push( + Date.now() + UNDO_ADOPTION_TTL_MS + ); + }, [] ); + + const consumeUndoRedoAdoption = useCallback( () => { + const expiries = undoAdoptionExpiriesRef.current; + const now = Date.now(); + while ( expiries.length > 0 && expiries[ 0 ] <= now ) { + expiries.shift(); + } + if ( expiries.length === 0 ) { + return false; + } + expiries.shift(); + return true; + }, [] ); + + // Prune overlay entries whose block was removed from the editor. This + // prevents stale baselines from persisting after a block is deleted. + // The block-count subscription only runs when there are entries to + // prune; in Edit / View intent (no entries) there's no point watching + // the block tree at all. + const blockCount = useSelect( + ( select ) => { + if ( ! hasEntries ) { + return 0; + } + const blockEditor = select( BLOCK_EDITOR_STORE_NAME ); + return blockEditor?.getClientIdsWithDescendants?.().length ?? 0; + }, + [ hasEntries ] + ); + useEffect( () => { + if ( ! hasEntries ) { + return; + } + const getLive = registry.select( + BLOCK_EDITOR_STORE_NAME + )?.getClientIdsWithDescendants; + if ( ! getLive ) { + return; + } + const live = getLive(); + const liveSet = new Set( live ); + const hasOrphan = Object.keys( entries ).some( + ( key ) => ! liveSet.has( key ) + ); + if ( hasOrphan ) { + dispatch( { type: 'PRUNE_ORPHANS', liveClientIds: live } ); + } + }, [ hasEntries, blockCount, entries, registry ] ); + + const value = useMemo( + () => ( { + entries, + captureBaseline, + setOverlayAttributes, + clearOverlay, + setCommentId, + setSyncedOpsKey, + setStructuralOp, + hasOverlay, + requestInterceptorBypass, + consumeInterceptorBypass, + registerFormatHandler, + requestFormatSuggestion, + registerContentHandler, + requestContentSuggestion, + enqueueSuggestionWrite, + markDeferredInsertion, + unmarkDeferredInsertion, + isDeferredInsertion, + clearDeferredInsertions, + getLastContentCaptureSeq, + armUndoRedoAdoption, + consumeUndoRedoAdoption, + } ), + [ + entries, + captureBaseline, + setOverlayAttributes, + clearOverlay, + setCommentId, + setSyncedOpsKey, + setStructuralOp, + hasOverlay, + requestInterceptorBypass, + consumeInterceptorBypass, + registerFormatHandler, + requestFormatSuggestion, + registerContentHandler, + requestContentSuggestion, + enqueueSuggestionWrite, + markDeferredInsertion, + unmarkDeferredInsertion, + isDeferredInsertion, + clearDeferredInsertions, + getLastContentCaptureSeq, + armUndoRedoAdoption, + consumeUndoRedoAdoption, + ] + ); + + return ( + + { children } + + ); +} + +/** + * Hook returning the suggestion overlay API. + * + * @return {OverlayContextValue} Overlay API. + */ +export function useSuggestionOverlay() { + return useContext( OverlayContext ); +} diff --git a/packages/editor/src/components/suggestion-mode/provider.js b/packages/editor/src/components/suggestion-mode/provider.js new file mode 100644 index 00000000000000..8b8d41db95f54f --- /dev/null +++ b/packages/editor/src/components/suggestion-mode/provider.js @@ -0,0 +1,1033 @@ +/** + * WordPress dependencies + */ +import { useCallback, useMemo } from '@wordpress/element'; +import { useDispatch, useRegistry, useSelect } from '@wordpress/data'; +import { store as coreStore } from '@wordpress/core-data'; +import { store as blockEditorStore } from '@wordpress/block-editor'; +import { store as interfaceStore } from '@wordpress/interface'; +import { store as noticesStore } from '@wordpress/notices'; +import { __ } from '@wordpress/i18n'; + +/** + * Internal dependencies + */ +import { EDITOR_STORE_NAME } from './constants'; +import { useSuggestionOverlay } from './overlay-context'; +import { + addNoteIdToMetadata, + getNoteIdsFromMetadata, +} from '../collab-sidebar/utils'; +import { ALL_NOTES_SIDEBAR, SIDEBARS } from '../collab-sidebar/constants'; + +/** + * @typedef {Object} SuggestionOperation + * @property {'attribute-set'|'inline-suggestion'|'block-insert-after'|'block-remove'|'block-move'} type + * Operation type. `attribute-set` and `inline-suggestion` ship in + * Phase 2; the structural variants ship in Phase 6 (issue #77434). + * @property {string} [attribute] The attribute being changed (`attribute-set`) or + * carrying the marker (`inline-suggestion`). + * @property {'del'|'add'|'format'} [suggestionType] Inline marker kind (`inline-suggestion` only): `del` + * wraps existing text proposed for removal, `add` wraps proposed + * new text, `format` wraps a run whose formatting changed (text + * unchanged). + * @property {string} [beforeHTML] Original run HTML captured for a `format` suggestion, so a + * reject can restore the pre-suggestion formatting. + * @property {string} [afterHTML] Proposed run HTML for a `format` suggestion, used to + * summarize which formats changed. + * @property {*} [before] The baseline value (`attribute-set`). + * @property {*} [after] The proposed value (`attribute-set`). + */ + +/** + * @typedef {Object} SuggestionPayload + * @property {number} schemaVersion Payload schema version. + * @property {string} blockName Block name at capture time. + * @property {string|null} baseRevision Post `modified_gmt` at + * capture, used by Phase 3 to + * detect stale suggestions. + * @property {SuggestionOperation[]} operations Ordered operations. + */ + +/** + * Suggestion payload schema version. v1 emitted only `attribute-set` + * operations; v2 reserves the structural op types (`block-insert-after`, + * `block-remove`, `block-move`) tracked in issue #77434. + * + * Reader rule: + * parsed < SCHEMA_VERSION → migrate forward, then apply. + * parsed === SCHEMA_VERSION → apply as-is. + * parsed > SCHEMA_VERSION → refuse (newer-editor notice; offer Reject only). + * + * Bumping this constant requires a corresponding migration step in + * `parseSuggestionPayload`. + */ +const SCHEMA_VERSION = 2; + +/** + * Maximum byte length of a serialized suggestion payload. Mirrors + * `GUTENBERG_SUGGESTION_PAYLOAD_MAX_BYTES` in + * `lib/compat/wordpress-7.1/block-suggestions.php`. The client checks before + * submitting so a doomed request never leaves the browser; the REST + * controller is the authoritative gate. + */ +const PAYLOAD_MAX_BYTES = 65536; + +/** + * Byte length of a serialized payload, measured the way PHP `strlen()` + * counts (UTF-8 bytes, not chars). + * + * @param {SuggestionPayload} payload + * @return {number} UTF-8 byte length of the serialized JSON. + */ +function payloadByteLength( payload ) { + const serialized = JSON.stringify( payload ); + if ( typeof TextEncoder !== 'undefined' ) { + return new TextEncoder().encode( serialized ).length; + } + // Conservative upper bound: 4 bytes per UTF-16 code unit covers all + // possible UTF-8 expansions. Used only in test/JSDOM environments + // without TextEncoder. + return serialized.length * 4; +} + +/** + * Build attribute-set operations by diffing an overlay entry against its + * captured baseline. Attributes whose value differs are emitted; unchanged + * or absent keys are skipped. + * + * @param {Object} baselineAttributes Attributes captured on first edit. + * @param {Object} overlayAttributes Pending attribute changes. + * @return {SuggestionOperation[]} Operations describing the suggestion. + */ +export function operationsFromOverlay( baselineAttributes, overlayAttributes ) { + const operations = []; + for ( const [ attribute, after ] of Object.entries( + overlayAttributes || {} + ) ) { + const before = baselineAttributes?.[ attribute ]; + if ( ! isAttributeEqual( before, after ) ) { + operations.push( { + type: 'attribute-set', + attribute, + before: before ?? null, + after, + } ); + } + } + return operations; +} + +/** + * Structural equality for attribute values. Handles primitives, arrays, and + * plain objects with arbitrary key order. + * + * `JSON.stringify` is order-sensitive ({a:1,b:2} ≠ {b:2,a:1}), so a stringify- + * based compare produces spurious "changed" detections when block code re- + * emits a `style` object with reordered keys. The recursive walk avoids that. + * + * @param {*} a First value. + * @param {*} b Second value. + * @return {boolean} True when the values are structurally equal. + */ +function isAttributeEqual( a, b ) { + if ( a === b ) { + return true; + } + if ( a === null || a === undefined || b === null || b === undefined ) { + return false; + } + // One side is a primitive (typically a string from a JSON-deserialized + // suggestion payload) and the other is a wrapper object (typically a + // `RichTextData` instance from the live block-editor store). Compare + // their string representations so the same logical content reads as + // equal across the serialization boundary — otherwise `hasAttributeConflict` + // flags every content suggestion as stale and the apply flow short- + // circuits to a never-visible "stale" dialog. + const aIsObject = typeof a === 'object'; + const bIsObject = typeof b === 'object'; + if ( aIsObject !== bIsObject ) { + return String( a ) === String( b ); + } + if ( ! aIsObject ) { + return false; + } + const aIsArray = Array.isArray( a ); + const bIsArray = Array.isArray( b ); + if ( aIsArray !== bIsArray ) { + return false; + } + if ( aIsArray ) { + if ( a.length !== b.length ) { + return false; + } + for ( let i = 0; i < a.length; i++ ) { + if ( ! isAttributeEqual( a[ i ], b[ i ] ) ) { + return false; + } + } + return true; + } + const aKeys = Object.keys( a ); + const bKeys = Object.keys( b ); + if ( aKeys.length !== bKeys.length ) { + return false; + } + // Wrapper objects like `RichTextData` hold their content in private + // class fields, so `Object.keys()` returns an empty array for any two + // instances regardless of the text they wrap. Fall back to a string + // compare so two wrappers with different content don't look equal. + if ( aKeys.length === 0 ) { + return String( a ) === String( b ); + } + for ( const key of aKeys ) { + if ( ! Object.prototype.hasOwnProperty.call( b, key ) ) { + return false; + } + if ( ! isAttributeEqual( a[ key ], b[ key ] ) ) { + return false; + } + } + return true; +} + +/** + * Operation types that mutate the block tree's structure rather than a + * single block's attributes. These flow through a different apply/reject + * path than `attribute-set`: Apply dispatches the corresponding block- + * editor action (`removeBlock`, `insertBlock`, `moveBlockToPosition`), + * Reject just clears the `metadata.suggestion` marker. + */ +const STRUCTURAL_OP_TYPES = new Set( [ + 'block-remove', + 'block-insert-after', + 'block-move', +] ); + +/** + * Locate the structural operation in a suggestion payload. v2 payloads carry + * at most one structural op per suggestion (the auto-save loop persists each + * structural mutation as its own note); attribute-set ops can ride along + * inside the same payload but the structural op leads. + * + * @param {SuggestionOperation[]} operations Payload operations. + * @return {SuggestionOperation|null} Structural op, or null when none. + */ +export function findStructuralOp( operations ) { + if ( ! Array.isArray( operations ) ) { + return null; + } + for ( const op of operations ) { + if ( op && STRUCTURAL_OP_TYPES.has( op.type ) ) { + return op; + } + } + return null; +} + +/** + * Build attributes that clear the `metadata.suggestion` marker on a block + * while preserving every other metadata field. Used by Apply (after the + * mutation lands) and by Reject (to drop the pending state). + * + * @param {Object} currentAttributes Block's current attributes. + * @return {Object} Partial attributes payload safe for `updateBlockAttributes`. + */ +export function clearSuggestionMarkerAttributes( currentAttributes ) { + const meta = currentAttributes?.metadata; + if ( ! meta || meta.suggestion === undefined ) { + return null; + } + const { suggestion: _drop, ...rest } = meta; + return { metadata: rest }; +} + +/** + * Apply a suggestion payload's operations to a block's current attributes + * to produce the new attributes. Pure function — no side effects. + * + * @param {Object} currentAttributes Block's current attributes. + * @param {SuggestionOperation[]} operations Operations from the payload. + * @return {Object} Merged attributes with suggestions applied. + */ +export function applyOperations( currentAttributes, operations ) { + const result = { ...currentAttributes }; + for ( const op of operations ) { + if ( op.type === 'attribute-set' ) { + result[ op.attribute ] = op.after; + } + } + return result; +} + +/** + * Report whether applying the suggestion's operations over the block's + * current attributes would overwrite concurrent changes made by someone + * else. A suggestion is considered conflicting only when the baseline + * captured at suggest-time differs from the attribute's current value — + * simply reopening the post after any auto-save doesn't qualify. + * + * @param {Object} currentAttributes Block's current attributes. + * @param {SuggestionOperation[]} operations Operations from the payload. + * @return {boolean} True if at least one targeted attribute has diverged. + */ +export function hasAttributeConflict( currentAttributes, operations ) { + if ( ! Array.isArray( operations ) ) { + return false; + } + // Inserted blocks have no pre-existing attributes — the overlay's + // baseline for a `block-insert-after` entry is `{}`, so every + // attribute-set op rides on `before: null`. Comparing that against the + // live (already-typed-into) block's attributes always reads as + // divergence, which falsely fires the staleness prompt on apply. The + // attribute-set ops describe the inserted block's content, not an + // overwrite of pre-existing data, so there is nothing to conflict with. + if ( findStructuralOp( operations )?.type === 'block-insert-after' ) { + return false; + } + for ( const op of operations ) { + if ( op.type !== 'attribute-set' ) { + continue; + } + if ( + ! isAttributeEqual( + op.before ?? null, + currentAttributes?.[ op.attribute ] ?? null + ) + ) { + return true; + } + } + return false; +} + +/** + * Migrate a payload emitted by an older `SCHEMA_VERSION` up to the current + * shape. v1 → v2 is a pure additive change (structural op types reserved but + * v1 payloads never used them), so the migration just stamps the version + * field forward — no shape rewriting is needed. + * + * Add a new `case` per future bump; never remove old cases, since the + * comment-meta store may contain payloads written by every prior version. + * + * @param {Object} parsed Parsed JSON payload of a known older version. + * @return {Object} Payload upgraded to the current schema. + */ +function migrateSuggestionPayload( parsed ) { + let next = parsed; + if ( next.schemaVersion === 1 ) { + next = { ...next, schemaVersion: 2 }; + } + return next; +} + +/** + * Parse a `_wp_suggestion` meta value into a typed payload. Refuses payloads + * written by a newer editor (`schemaVersion > SCHEMA_VERSION`) so a partial + * apply can't drop op types this consumer doesn't understand. Migrates + * older payloads forward to the current shape. + * + * @param {string|undefined} raw The raw JSON string from comment meta. + * @return {SuggestionPayload|null} Parsed payload, or null when the input is + * malformed or the payload was written by a newer editor. + */ +export function parseSuggestionPayload( raw ) { + if ( ! raw ) { + return null; + } + let parsed; + try { + parsed = JSON.parse( raw ); + } catch { + return null; + } + if ( + typeof parsed !== 'object' || + parsed === null || + ! Array.isArray( parsed.operations ) + ) { + return null; + } + // Pre-versioned payloads (schemaVersion missing) are treated as v1 — the + // only writer that emitted them was the v1 implementation. + const version = + typeof parsed.schemaVersion === 'number' ? parsed.schemaVersion : 1; + if ( version > SCHEMA_VERSION ) { + return null; + } + if ( version < SCHEMA_VERSION ) { + return migrateSuggestionPayload( { + ...parsed, + schemaVersion: version, + } ); + } + return parsed; +} + +/* + * Comment ids with an apply/reject decision currently in flight. Deciding a + * suggestion mutates block content (clears markers) BEFORE the comment's + * lifecycle status lands on the server, so the note garbage collector (see + * suggestion-note-gc.js) would briefly observe "marker gone, note still + * pending" and trash a note that was just resolved. Module-scoped because + * `useSuggestionsProvider` is instantiated once per consumer and the guard + * must be shared across all of them. + */ +const decisionsInFlight = new Set(); + +/** + * Whether an apply/reject decision for the given comment is in flight. + * + * @param {number|string} commentId Comment id to check. + * @return {boolean} True while a decision is being processed. + */ +export function isSuggestionDecisionInFlight( commentId ) { + return decisionsInFlight.has( String( commentId ) ); +} + +/** + * Wrap a decision callback (apply/reject) so its comment id is registered as + * in flight for the duration of the call. + * + * @param {Function} decide Decision callback taking `{ commentId, ... }`. + * @return {Function} Wrapped callback. + */ +function withDecisionInFlight( decide ) { + return async ( args ) => { + const key = String( args?.commentId ); + decisionsInFlight.add( key ); + try { + return await decide( args ); + } finally { + decisionsInFlight.delete( key ); + } + }; +} + +/** + * Comment-meta backed suggestions provider. The provider shape is stable so + * a future Yjs-backed provider can swap in without touching the UI. + * + * Storage: a `note` comment with the suggestion payload serialized to + * the `_wp_suggestion` comment meta. Linkage to a block reuses the existing + * `metadata.noteId` block attribute. + * + * @return {{ + * createSuggestion: Function, + * applySuggestion: Function, + * rejectSuggestion: Function, + * }} Suggestions API. + */ +export function useSuggestionsProvider() { + const { postId, postModified } = useSelect( ( select ) => { + const editor = select( EDITOR_STORE_NAME ); + const id = editor?.getCurrentPostId?.() ?? null; + const postType = editor?.getCurrentPostType?.() ?? null; + const record = + id && postType + ? select( coreStore ).getEditedEntityRecord( + 'postType', + postType, + id + ) + : null; + return { + postId: id, + postModified: record?.modified_gmt ?? null, + }; + }, [] ); + + const { saveEntityRecord } = useDispatch( coreStore ); + const { createNotice } = useDispatch( noticesStore ); + const { enableComplementaryArea } = useDispatch( interfaceStore ); + const { getActiveComplementaryArea } = useSelect( interfaceStore ); + const { + updateBlockAttributes, + removeBlock, + moveBlockToPosition, + __unstableMarkNextChangeAsNotPersistent: markNextChangeAsNotPersistent, + } = useDispatch( blockEditorStore ); + const { + getBlockAttributes: selectBlockAttributes, + getBlockRootClientId: selectBlockRootClientId, + getClientIdsWithDescendants: selectClientIdsWithDescendants, + } = useSelect( blockEditorStore ); + const { requestInterceptorBypass, clearOverlay } = useSuggestionOverlay(); + const registry = useRegistry(); + + const createSuggestion = useCallback( + async ( { clientId, blockName, operations } ) => { + if ( ! postId ) { + throw new Error( 'No post id available for suggestion.' ); + } + if ( ! operations || operations.length === 0 ) { + return null; + } + + const payload = /** @type {SuggestionPayload} */ ( { + schemaVersion: SCHEMA_VERSION, + blockName, + baseRevision: postModified, + operations, + } ); + + if ( payloadByteLength( payload ) > PAYLOAD_MAX_BYTES ) { + const error = new Error( + __( 'Suggestion is too large to save.' ) + ); + createNotice( 'error', error.message, { + type: 'snackbar', + isDismissible: true, + } ); + throw error; + } + + try { + const savedRecord = await saveEntityRecord( + 'root', + 'comment', + { + post: postId, + content: '', + status: 'hold', + type: 'note', + parent: 0, + meta: { + _wp_suggestion: JSON.stringify( payload ), + }, + }, + { throwOnError: true } + ); + + if ( savedRecord?.id ) { + // Append to the noteId array so a fresh suggestion on a + // block whose previous note(s) have been applied or + // rejected coexists with them rather than overwriting + // the link. Other metadata fields like bindings and name + // are preserved by `addNoteIdToMetadata`. + const existingMeta = + selectBlockAttributes( clientId )?.metadata ?? {}; + /* + * The linkage is system bookkeeping, not a user edit: it + * must never be captured by undo history. Left + * persistent, the first Ctrl+Z after making a suggestion + * pops this write instead of the suggestion, leaving the + * marker in place. + */ + markNextChangeAsNotPersistent?.( { history: 'ignore' } ); + updateBlockAttributes( clientId, { + metadata: addNoteIdToMetadata( + existingMeta, + savedRecord.id + ), + } ); + + // Surface the new note: when a non-notes sidebar (e.g. + // post or block settings) is open, switch it to the + // notes sidebar so the suggestion is immediately + // visible. A closed sidebar stays closed. + const activeArea = getActiveComplementaryArea( 'core' ); + if ( activeArea && ! SIDEBARS.includes( activeArea ) ) { + enableComplementaryArea( 'core', ALL_NOTES_SIDEBAR ); + } + } + + return savedRecord; + } catch ( error ) { + createNotice( + 'error', + error?.message || __( 'Unable to submit suggestion.' ), + { type: 'snackbar', isDismissible: true } + ); + throw error; + } + }, + [ + postId, + postModified, + saveEntityRecord, + updateBlockAttributes, + markNextChangeAsNotPersistent, + selectBlockAttributes, + createNotice, + getActiveComplementaryArea, + enableComplementaryArea, + ] + ); + + /** + * Update an existing suggestion's payload (auto-save path). Replaces + * the `_wp_suggestion` meta on the comment without changing its author, + * status, or thread identity, so the user sees a single note + * accumulating edits rather than a new note per save burst. + * + * @param {Object} args Update arguments. + * @param {number|string} args.commentId Comment id of the + * existing suggestion. + * @param {string} args.blockName Block name (recorded + * on the payload). + * @param {SuggestionOperation[]} args.operations Latest operations. + * @return {Promise} The saved comment record. + */ + const updateSuggestion = useCallback( + async ( { commentId, blockName, operations } ) => { + if ( ! commentId ) { + throw new Error( 'No comment id for suggestion update.' ); + } + + const payload = /** @type {SuggestionPayload} */ ( { + schemaVersion: SCHEMA_VERSION, + blockName, + baseRevision: postModified, + operations, + } ); + + if ( payloadByteLength( payload ) > PAYLOAD_MAX_BYTES ) { + const error = new Error( + __( 'Suggestion is too large to save.' ) + ); + createNotice( 'error', error.message, { + type: 'snackbar', + isDismissible: true, + } ); + throw error; + } + + try { + return await saveEntityRecord( + 'root', + 'comment', + { + id: commentId, + meta: { + _wp_suggestion: JSON.stringify( payload ), + }, + }, + { throwOnError: true } + ); + } catch ( error ) { + createNotice( + 'error', + error?.message || __( 'Unable to update suggestion.' ), + { type: 'snackbar', isDismissible: true } + ); + throw error; + } + }, + [ postModified, saveEntityRecord, createNotice ] + ); + + /** + * Delete a suggestion. The auto-saver calls this when the overlay is + * fully reverted to baseline — the user retracted their edit, so the + * note no longer carries a meaningful suggestion. + * + * @param {Object} args Delete arguments. + * @param {number|string} args.commentId Comment id to trash. + * @return {Promise} + */ + const deleteSuggestion = useCallback( + async ( { commentId } ) => { + if ( ! commentId ) { + return; + } + try { + await saveEntityRecord( + 'root', + 'comment', + { id: commentId, status: 'trash' }, + { throwOnError: true } + ); + } catch ( error ) { + createNotice( + 'error', + error?.message || __( 'Unable to remove suggestion.' ), + { type: 'snackbar', isDismissible: true } + ); + throw error; + } + }, + [ saveEntityRecord, createNotice ] + ); + + /** + * Apply a suggestion to the live block, then persist the lifecycle + * status to the comment meta. On a server failure the block is rolled + * back so the UI is never left in a half-applied state. + * + * @param {Object} args Apply arguments. + * @param {number|string} args.commentId Comment id holding the + * suggestion (`_wp_suggestion` + * meta). + * @param {string} args.clientId Block client id of the apply + * target. May be undefined if + * the acting user opened the + * post fresh and the metadata + * linkage was never persisted — + * the apply path then scans the + * live tree by `metadata.noteId`. + * @param {SuggestionPayload} args.payload Parsed payload (from + * `parseSuggestionPayload`). + * @return {Promise} + */ + const applySuggestion = useCallback( + async ( { commentId, clientId, payload } ) => { + if ( ! payload || ! Array.isArray( payload.operations ) ) { + createNotice( 'error', __( 'Invalid suggestion payload.' ), { + type: 'snackbar', + isDismissible: true, + } ); + return; + } + + // `thread.blockClientId` is derived by matching `metadata.noteId` + // on blocks currently in the editor. If the Suggest author never + // auto-saved the post after the comment was created — or the + // author reloaded before the save landed — the metadata linkage + // won't exist yet and the caller will pass `clientId: undefined`. + // Fall back to scanning the live block tree for a block whose + // `metadata.noteId` includes the comment id (the field is an + // array post-#75147 to support multiple notes per block, so use + // the shared normalization helper instead of strict equality). + let targetClientId = clientId; + if ( ! targetClientId ) { + const liveIds = selectClientIdsWithDescendants?.() ?? []; + const commentIdKey = String( commentId ); + for ( const id of liveIds ) { + const ids = getNoteIdsFromMetadata( + selectBlockAttributes( id )?.metadata + ); + if ( ids.some( ( n ) => String( n ) === commentIdKey ) ) { + targetClientId = id; + break; + } + } + } + + if ( ! targetClientId ) { + createNotice( + 'error', + __( + 'Could not find the block this suggestion applies to.' + ), + { type: 'snackbar', isDismissible: true } + ); + return; + } + + // Structural ops (block-remove, block-insert-after; block-move + // ships in a follow-up) can't ride the updateBlockAttributes + // path: their apply mutates the tree rather than a single + // block's attributes. Branch out, run the matching block- + // editor action, and short-circuit before the attribute-set + // rollback machinery below. + const structuralOp = findStructuralOp( payload.operations ); + if ( structuralOp ) { + try { + if ( structuralOp.type === 'block-remove' ) { + // Bypass twice: the marker-clear dispatch lands + // first (so the live block ends without the + // pending-remove flag should the removeBlock fail), + // then the actual removal. + const clearAttrs = clearSuggestionMarkerAttributes( + selectBlockAttributes( targetClientId ) + ); + if ( clearAttrs ) { + requestInterceptorBypass( targetClientId ); + updateBlockAttributes( targetClientId, clearAttrs ); + } + requestInterceptorBypass( targetClientId ); + clearOverlay( targetClientId ); + removeBlock( targetClientId ); + } else if ( + structuralOp.type === 'block-insert-after' || + structuralOp.type === 'block-move' + ) { + // The block is already at its proposed location + // (the user inserted or moved it during Suggest + // mode); apply commits the captured edits onto the + // live block AND clears the pending marker so the + // block loses its dimmed/outlined treatment. + // + // Attribute-set ops in the same payload represent + // edits the user made between the structural + // change and auto-save. They never reach the live + // block on the suggester's side — the interceptor + // reverts them into the overlay — so collaborators + // (and the suggester after a reload) see the live + // block in the captured shape (typically empty + // content for a fresh paragraph). Apply must + // materialize those edits on the live block, + // otherwise the inserted/moved block ends up in + // the wrong shape after acceptance. + const currentAttributes = + selectBlockAttributes( targetClientId ); + const withOpsApplied = applyOperations( + currentAttributes, + payload.operations + ); + const markerCleared = + clearSuggestionMarkerAttributes( withOpsApplied ); + const finalAttributes = markerCleared + ? { ...withOpsApplied, ...markerCleared } + : withOpsApplied; + requestInterceptorBypass( targetClientId ); + updateBlockAttributes( + targetClientId, + finalAttributes + ); + clearOverlay( targetClientId ); + } + + await saveEntityRecord( + 'root', + 'comment', + { + id: commentId, + status: 'approved', + meta: { _wp_suggestion_status: 'applied' }, + }, + { throwOnError: true } + ); + + createNotice( 'snackbar', __( 'Suggestion applied.' ), { + type: 'snackbar', + isDismissible: true, + } ); + } catch ( error ) { + createNotice( + 'error', + error?.message || + __( 'Failed to save suggestion status.' ), + { type: 'snackbar', isDismissible: true } + ); + } + return; + } + + const currentAttributes = selectBlockAttributes( targetClientId ); + const newAttributes = applyOperations( + currentAttributes, + payload.operations + ); + + // Build a rollback payload that covers exactly the keys this + // apply touched. `updateBlockAttributes` is a partial merge — + // passing `currentAttributes` alone would leave keys that the + // apply newly added stuck on the block (set to their `after` + // value), since they have no entry in the original attributes + // to override them. Listing each touched key with its original + // value (or `undefined` when the key was added by this apply) + // restores the block cleanly. + const rollbackPayload = {}; + for ( const op of payload.operations ) { + if ( op.type !== 'attribute-set' ) { + continue; + } + rollbackPayload[ op.attribute ] = + Object.prototype.hasOwnProperty.call( + currentAttributes ?? {}, + op.attribute + ) + ? currentAttributes[ op.attribute ] + : undefined; + } + + try { + // Bypass the suggest-mode interceptor for this dispatch so + // the applied attributes actually land on the live block + // instead of being reverted into the overlay. Clearing the + // overlay entry resets the per-block suggestion tracking, + // so any subsequent user edit captures a fresh baseline + // from the post-apply attributes. Outside Suggest mode the + // interceptor isn't running and these calls are no-ops. + requestInterceptorBypass( targetClientId ); + clearOverlay( targetClientId ); + updateBlockAttributes( targetClientId, newAttributes ); + + await saveEntityRecord( + 'root', + 'comment', + { + id: commentId, + status: 'approved', + meta: { _wp_suggestion_status: 'applied' }, + }, + { throwOnError: true } + ); + + createNotice( 'snackbar', __( 'Suggestion applied.' ), { + type: 'snackbar', + isDismissible: true, + } ); + } catch ( error ) { + // Roll back the block change so the UI isn't left in a + // half-applied state if the server rejected the update. + requestInterceptorBypass( targetClientId ); + updateBlockAttributes( targetClientId, rollbackPayload ); + createNotice( + 'error', + error?.message || __( 'Failed to save suggestion status.' ), + { type: 'snackbar', isDismissible: true } + ); + } + }, + [ + saveEntityRecord, + updateBlockAttributes, + removeBlock, + selectBlockAttributes, + selectClientIdsWithDescendants, + createNotice, + requestInterceptorBypass, + clearOverlay, + ] + ); + + /** + * Reject a suggestion by setting the comment's lifecycle status. The + * comment itself stays as a thread (status `approved`) so the + * conversation persists as evidence that the suggestion was reviewed. + * For structural suggestions (e.g. `block-remove`), also clears the + * `metadata.suggestion` marker on the live block so the dimmed/struck + * visual treatment goes away. + * + * @param {Object} args Reject arguments. + * @param {number|string} args.commentId Comment id of the rejected + * suggestion. + * @param {string} [args.clientId] Target block clientId, if + * known. + * @param {SuggestionPayload} [args.payload] Parsed suggestion payload — + * inspected to detect a + * structural op so the marker + * can be cleared on the live + * block. + * @return {Promise} + */ + const rejectSuggestion = useCallback( + async ( { commentId, clientId, payload } ) => { + // Reject behavior depends on the structural op type: + // - block-remove: drop the marker (block stays). + // - block-insert-after: dispatch removeBlock to undo the + // suggested insertion. The marker on the live block goes + // away with the block itself. + // - block-move: clear the marker, then dispatch + // moveBlockToPosition to put the block back at its + // pre-move parent + index. + // - attribute-set (no structural op): no live-block change. + const structuralOp = findStructuralOp( payload?.operations ); + if ( structuralOp && clientId ) { + if ( structuralOp.type === 'block-insert-after' ) { + requestInterceptorBypass( clientId ); + clearOverlay( clientId ); + removeBlock( clientId ); + } else if ( structuralOp.type === 'block-move' ) { + const clearAttrs = clearSuggestionMarkerAttributes( + selectBlockAttributes( clientId ) + ); + requestInterceptorBypass( clientId ); + clearOverlay( clientId ); + /* + * Batch the marker-clear and the restoring move into ONE + * store update. The interceptor recognizes a reject + * landing by their combination — a block that moved in + * the same tick its pending-move marker disappeared — + * and adopts it instead of re-capturing the restore as + * a fresh move suggestion (which is what happens when + * the two dispatches fire the subscriber separately and + * the reviewer is in Suggesting intent). This is also + * the shape a remote reject arrives in through sync. + */ + registry.batch( () => { + if ( clearAttrs ) { + updateBlockAttributes( clientId, clearAttrs ); + } + moveBlockToPosition( + clientId, + /* + * `fromRootClientId` must be the block's CURRENT + * parent: after a cross-parent move the block lives + * in the destination parent, and the reducer looks + * the block up there. Passing the original parent + * for both roots made cross-parent rejects silently + * no-op. `moveBlockToPosition` expects '' (not null) + * for the root. + */ + selectBlockRootClientId( clientId ) ?? '', + structuralOp.fromParentClientId ?? '', + structuralOp.fromIndex ?? 0 + ); + } ); + } else { + const clearAttrs = clearSuggestionMarkerAttributes( + selectBlockAttributes( clientId ) + ); + if ( clearAttrs ) { + requestInterceptorBypass( clientId ); + updateBlockAttributes( clientId, clearAttrs ); + } + clearOverlay( clientId ); + } + } + + try { + await saveEntityRecord( + 'root', + 'comment', + { + id: commentId, + status: 'approved', + meta: { _wp_suggestion_status: 'rejected' }, + }, + { throwOnError: true } + ); + + createNotice( 'snackbar', __( 'Suggestion rejected.' ), { + type: 'snackbar', + isDismissible: true, + } ); + } catch ( error ) { + createNotice( + 'error', + error?.message || __( 'Failed to reject suggestion.' ), + { type: 'snackbar', isDismissible: true } + ); + } + }, + [ + saveEntityRecord, + createNotice, + selectBlockAttributes, + selectBlockRootClientId, + updateBlockAttributes, + removeBlock, + moveBlockToPosition, + requestInterceptorBypass, + clearOverlay, + registry, + ] + ); + + // Decisions are wrapped so the note garbage collector can distinguish a + // marker deliberately cleared by apply/reject from one withdrawn by the + // user (undo, deleting the marked text). Wrapped here — not per-callback — + // so every consumer of the provider gets the guard. + const applySuggestionGuarded = useMemo( + () => withDecisionInFlight( applySuggestion ), + [ applySuggestion ] + ); + const rejectSuggestionGuarded = useMemo( + () => withDecisionInFlight( rejectSuggestion ), + [ rejectSuggestion ] + ); + + return { + createSuggestion, + updateSuggestion, + deleteSuggestion, + applySuggestion: applySuggestionGuarded, + rejectSuggestion: rejectSuggestionGuarded, + }; +} + +export { SCHEMA_VERSION, PAYLOAD_MAX_BYTES, payloadByteLength }; diff --git a/packages/editor/src/components/suggestion-mode/suggestion-write-queue.js b/packages/editor/src/components/suggestion-mode/suggestion-write-queue.js new file mode 100644 index 00000000000000..b81c97ba687d55 --- /dev/null +++ b/packages/editor/src/components/suggestion-mode/suggestion-write-queue.js @@ -0,0 +1,63 @@ +/** + * Per-block serial queue for inline suggestion writes. + * + * The content reconciler (`suggestion-content-reconciler.js`) and the format + * keyboard (`suggestion-format-keyboard.js`) share an async shape: open one or + * more suggestion notes over REST, then write markers into the block's + * `content`. Two of those flights interleaving on the same block clobber each + * other — each writes content computed from its own pre-flight snapshot — and + * the two components used to keep separate in-flight guards, so one of each + * could always be in flight on one block simultaneously. This queue is the + * single shared ordering point: tasks for the same block run strictly one + * after another (each re-validates against live content when its turn comes), + * while tasks for different blocks stay independent. + */ + +/** + * Create a write queue keyed by block client id. + * + * @return {{enqueue: Function, hasPending: Function}} Queue API. + */ +export function createSuggestionWriteQueue() { + const chains = new Map(); + + return { + /** + * Run `task` after every previously enqueued task for the same + * block has settled. A rejected task never poisons later tasks — + * the stored chain always resolves. + * + * @param {string} clientId Block client id the write targets. + * @param {Function} task Async task to run. + * @return {Promise<*>} The task's own settlement (observable by the + * caller, including rejection). + */ + enqueue( clientId, task ) { + const previous = chains.get( clientId ) ?? Promise.resolve(); + const run = previous.then( () => task() ); + const settled = run.then( + () => {}, + () => {} + ); + chains.set( clientId, settled ); + settled.then( () => { + // Drop the chain entry once it drains so the map doesn't + // grow with every block ever edited. + if ( chains.get( clientId ) === settled ) { + chains.delete( clientId ); + } + } ); + return run; + }, + + /** + * Whether a task is queued or in flight for the block. + * + * @param {string} clientId Block client id. + * @return {boolean} True when the block has pending writes. + */ + hasPending( clientId ) { + return chains.has( clientId ); + }, + }; +} diff --git a/packages/editor/src/components/suggestion-mode/test/auto-save.js b/packages/editor/src/components/suggestion-mode/test/auto-save.js new file mode 100644 index 00000000000000..03334347885013 --- /dev/null +++ b/packages/editor/src/components/suggestion-mode/test/auto-save.js @@ -0,0 +1,500 @@ +/** + * External dependencies + */ +import { render, act } from '@testing-library/react'; + +/** + * WordPress dependencies + */ +import { createRegistry, RegistryProvider } from '@wordpress/data'; +import { store as coreStore } from '@wordpress/core-data'; +import { store as noticesStore } from '@wordpress/notices'; + +/** + * Internal dependencies + */ +import SuggestionAutoSave, { operationsForEntry } from '../auto-save'; +import { + SuggestionOverlayProvider, + useSuggestionOverlay, +} from '../overlay-context'; +import { store as editorStore } from '../../../store'; +import { unlock } from '../../../lock-unlock'; + +// jest.mock factories may only reference variables prefixed with `mock`. +const mockCreateSuggestion = jest.fn(); +const mockUpdateSuggestion = jest.fn(); +const mockDeleteSuggestion = jest.fn(); + +jest.mock( '../provider', () => { + const actual = jest.requireActual( '../provider' ); + return { + ...actual, + useSuggestionsProvider: () => ( { + createSuggestion: mockCreateSuggestion, + updateSuggestion: mockUpdateSuggestion, + deleteSuggestion: mockDeleteSuggestion, + } ), + }; +} ); + +const createSuggestion = mockCreateSuggestion; +const updateSuggestion = mockUpdateSuggestion; +const deleteSuggestion = mockDeleteSuggestion; + +beforeEach( () => { + createSuggestion.mockReset(); + updateSuggestion.mockReset(); + deleteSuggestion.mockReset(); + jest.useFakeTimers(); +} ); + +afterEach( () => { + jest.useRealTimers(); +} ); + +function renderInSuggestMode( ui ) { + const registry = createRegistry(); + registry.register( noticesStore ); + registry.register( coreStore ); + registry.register( editorStore ); + unlock( registry.dispatch( editorStore ) ).setEditorIntent( 'suggest' ); + + const wrapper = ( { children } ) => ( + + { children } + + ); + + return { registry, ...render( ui, { wrapper } ) }; +} + +// Seed a comment record so `getEntityRecord( 'root', 'comment', id )` resolves +// without an HTTP fetch — mirrors what `useNoteThreads`'s entity query would +// have populated by the time a suggestion is in flight. +function seedComment( registry, comment ) { + registry + .dispatch( coreStore ) + .receiveEntityRecords( 'root', 'comment', [ comment ] ); +} + +// Test harness exposes the overlay API via a render-prop ref so tests can +// drive the reducer directly. +let overlayHandle; +function CaptureOverlay() { + overlayHandle = useSuggestionOverlay(); + return null; +} + +async function flushPromises() { + await act( async () => { + // Resolve any pending microtasks queued by setTimeout's await chain. + await Promise.resolve(); + } ); +} + +describe( 'SuggestionAutoSave', () => { + it( 'POSTs a new suggestion after the debounce window', async () => { + createSuggestion.mockResolvedValue( { id: 42 } ); + + renderInSuggestMode( + <> + + + + ); + + act( () => { + overlayHandle.captureBaseline( 'a', 'core/paragraph', { + content: 'Hi', + } ); + overlayHandle.setOverlayAttributes( 'a', { content: 'Hello' } ); + } ); + + // Before the debounce window: no POST. + expect( createSuggestion ).not.toHaveBeenCalled(); + + await act( async () => { + jest.advanceTimersByTime( 1500 ); + } ); + await flushPromises(); + await flushPromises(); + + expect( createSuggestion ).toHaveBeenCalledTimes( 1 ); + expect( createSuggestion ).toHaveBeenCalledWith( + expect.objectContaining( { + clientId: 'a', + blockName: 'core/paragraph', + operations: expect.arrayContaining( [ + expect.objectContaining( { attribute: 'content' } ), + ] ), + } ) + ); + } ); + + it( 'updates the same comment on subsequent edits', async () => { + createSuggestion.mockResolvedValue( { id: 42 } ); + updateSuggestion.mockResolvedValue( { id: 42 } ); + + renderInSuggestMode( + <> + + + + ); + + act( () => { + overlayHandle.captureBaseline( 'a', 'core/paragraph', { + content: 'Hi', + } ); + overlayHandle.setOverlayAttributes( 'a', { content: 'Hello' } ); + } ); + + await act( async () => { + jest.advanceTimersByTime( 1500 ); + } ); + await flushPromises(); + await flushPromises(); + + expect( createSuggestion ).toHaveBeenCalledTimes( 1 ); + + // User keeps typing. + act( () => { + overlayHandle.setOverlayAttributes( 'a', { + content: 'Hello world', + } ); + } ); + + await act( async () => { + jest.advanceTimersByTime( 1500 ); + } ); + await flushPromises(); + await flushPromises(); + + expect( updateSuggestion ).toHaveBeenCalledTimes( 1 ); + expect( updateSuggestion ).toHaveBeenCalledWith( + expect.objectContaining( { + commentId: 42, + operations: expect.arrayContaining( [ + expect.objectContaining( { + attribute: 'content', + after: 'Hello world', + } ), + ] ), + } ) + ); + } ); + + it( 'deletes the comment when the overlay returns to baseline', async () => { + createSuggestion.mockResolvedValue( { id: 42 } ); + deleteSuggestion.mockResolvedValue( undefined ); + + renderInSuggestMode( + <> + + + + ); + + act( () => { + overlayHandle.captureBaseline( 'a', 'core/paragraph', { + content: 'Hi', + } ); + overlayHandle.setOverlayAttributes( 'a', { content: 'Hello' } ); + } ); + + await act( async () => { + jest.advanceTimersByTime( 1500 ); + } ); + await flushPromises(); + await flushPromises(); + + // User reverts the overlay back to the baseline. + act( () => { + overlayHandle.setOverlayAttributes( 'a', { content: 'Hi' } ); + } ); + + await act( async () => { + jest.advanceTimersByTime( 1500 ); + } ); + await flushPromises(); + await flushPromises(); + + expect( deleteSuggestion ).toHaveBeenCalledTimes( 1 ); + expect( deleteSuggestion ).toHaveBeenCalledWith( { commentId: 42 } ); + } ); + + it( 'does not duplicate work when the user keeps typing during an in-flight save', async () => { + // `createSuggestion` resolves only when we explicitly let it. + let resolveCreate; + createSuggestion.mockImplementation( + () => + new Promise( ( resolve ) => { + resolveCreate = resolve; + } ) + ); + updateSuggestion.mockResolvedValue( { id: 42 } ); + + renderInSuggestMode( + <> + + + + ); + + act( () => { + overlayHandle.captureBaseline( 'a', 'core/paragraph', { + content: 'Hi', + } ); + overlayHandle.setOverlayAttributes( 'a', { content: 'Hello' } ); + } ); + + await act( async () => { + jest.advanceTimersByTime( 1500 ); + } ); + await flushPromises(); + + // Save_A is in flight. User keeps typing. + act( () => { + overlayHandle.setOverlayAttributes( 'a', { + content: 'Hello world', + } ); + } ); + + await act( async () => { + jest.advanceTimersByTime( 1500 ); + } ); + await flushPromises(); + + // We must not have issued a duplicate create — the second sync is + // queued behind the in-flight create. + expect( createSuggestion ).toHaveBeenCalledTimes( 1 ); + expect( updateSuggestion ).toHaveBeenCalledTimes( 0 ); + + // Let save_A complete; the queued sync_B should now run as an update + // on the just-issued comment id. + await act( async () => { + resolveCreate( { id: 42 } ); + } ); + await flushPromises(); + await flushPromises(); + await flushPromises(); + + expect( updateSuggestion ).toHaveBeenCalledTimes( 1 ); + expect( updateSuggestion ).toHaveBeenCalledWith( + expect.objectContaining( { + commentId: 42, + operations: expect.arrayContaining( [ + expect.objectContaining( { + after: 'Hello world', + } ), + ] ), + } ) + ); + } ); + + it( 'creates a fresh suggestion when the linked note has been resolved', async () => { + // First create resolves; second create resolves with a different id so + // we can assert the overlay's commentId rotated. + createSuggestion + .mockResolvedValueOnce( { id: 42 } ) + .mockResolvedValueOnce( { id: 43 } ); + updateSuggestion.mockResolvedValue( { id: 42 } ); + + const { registry } = renderInSuggestMode( + <> + + + + ); + + // User A's first edit: bold suggestion. Auto-save creates note 42. + act( () => { + overlayHandle.captureBaseline( 'a', 'core/paragraph', { + content: 'Hi', + } ); + overlayHandle.setOverlayAttributes( 'a', { content: 'Hello' } ); + } ); + + await act( async () => { + jest.advanceTimersByTime( 1500 ); + } ); + await flushPromises(); + await flushPromises(); + + expect( createSuggestion ).toHaveBeenCalledTimes( 1 ); + + // User B accepts note 42 — server flips status to 'approved'. Seed + // the resolved comment in the registry so the next sync sees it. + seedComment( registry, { id: 42, status: 'approved' } ); + + // User A keeps editing the same block (different attribute change). + act( () => { + overlayHandle.setOverlayAttributes( 'a', { + content: 'Hello world', + } ); + } ); + + await act( async () => { + jest.advanceTimersByTime( 1500 ); + } ); + await flushPromises(); + await flushPromises(); + + // The new edit must NOT update the resolved note 42 — it must spawn + // a fresh note that coexists with the resolved one. + expect( updateSuggestion ).not.toHaveBeenCalled(); + expect( createSuggestion ).toHaveBeenCalledTimes( 2 ); + expect( createSuggestion ).toHaveBeenLastCalledWith( + expect.objectContaining( { + clientId: 'a', + operations: expect.arrayContaining( [ + expect.objectContaining( { + attribute: 'content', + after: 'Hello world', + } ), + ] ), + } ) + ); + } ); + + it( 'continues to update the linked note while it is still pending', async () => { + createSuggestion.mockResolvedValue( { id: 42 } ); + updateSuggestion.mockResolvedValue( { id: 42 } ); + + const { registry } = renderInSuggestMode( + <> + + + + ); + + act( () => { + overlayHandle.captureBaseline( 'a', 'core/paragraph', { + content: 'Hi', + } ); + overlayHandle.setOverlayAttributes( 'a', { content: 'Hello' } ); + } ); + + await act( async () => { + jest.advanceTimersByTime( 1500 ); + } ); + await flushPromises(); + await flushPromises(); + + // Note exists in the cache but is still pending — same as the + // real-world case where the comments query has run but no one has + // resolved the note yet. + seedComment( registry, { id: 42, status: 'hold' } ); + + act( () => { + overlayHandle.setOverlayAttributes( 'a', { + content: 'Hello world', + } ); + } ); + + await act( async () => { + jest.advanceTimersByTime( 1500 ); + } ); + await flushPromises(); + await flushPromises(); + + expect( createSuggestion ).toHaveBeenCalledTimes( 1 ); + expect( updateSuggestion ).toHaveBeenCalledTimes( 1 ); + expect( updateSuggestion ).toHaveBeenCalledWith( + expect.objectContaining( { commentId: 42 } ) + ); + } ); + + it( 'does nothing when the editor is not in Suggest intent', async () => { + const registry = createRegistry(); + registry.register( noticesStore ); + registry.register( editorStore ); + unlock( registry.dispatch( editorStore ) ).setEditorIntent( 'edit' ); + + const wrapper = ( { children } ) => ( + + + { children } + + + ); + + render( + <> + + + , + { wrapper } + ); + + act( () => { + overlayHandle.captureBaseline( 'a', 'core/paragraph', { + content: 'Hi', + } ); + overlayHandle.setOverlayAttributes( 'a', { content: 'Hello' } ); + } ); + + await act( async () => { + jest.advanceTimersByTime( 5000 ); + } ); + await flushPromises(); + + expect( createSuggestion ).not.toHaveBeenCalled(); + } ); +} ); + +describe( 'operationsForEntry', () => { + it( 'derives attribute-set ops from baseline + overlay when no structural op is set', () => { + expect( + operationsForEntry( { + blockName: 'core/paragraph', + baselineAttributes: { content: 'a' }, + overlayAttributes: { content: 'b' }, + } ) + ).toEqual( [ + { + type: 'attribute-set', + attribute: 'content', + before: 'a', + after: 'b', + }, + ] ); + } ); + + it( 'returns the structural op as a single-element array when present', () => { + const op = { + type: 'block-remove', + clientId: 'x', + blockName: 'core/paragraph', + }; + expect( + operationsForEntry( { + blockName: 'core/paragraph', + baselineAttributes: {}, + overlayAttributes: {}, + structuralOp: op, + } ) + ).toEqual( [ op ] ); + } ); + + it( 'emits structural op first then attribute-set ops when both are present', () => { + const op = { type: 'block-remove', clientId: 'x' }; + expect( + operationsForEntry( { + blockName: 'core/paragraph', + baselineAttributes: { content: 'a' }, + overlayAttributes: { content: 'b' }, + structuralOp: op, + } ) + ).toEqual( [ + op, + { + type: 'attribute-set', + attribute: 'content', + before: 'a', + after: 'b', + }, + ] ); + } ); +} ); diff --git a/packages/editor/src/components/suggestion-mode/test/overlay-context.js b/packages/editor/src/components/suggestion-mode/test/overlay-context.js new file mode 100644 index 00000000000000..3819f3e1f9e3e6 --- /dev/null +++ b/packages/editor/src/components/suggestion-mode/test/overlay-context.js @@ -0,0 +1,328 @@ +/** + * Internal dependencies + */ +import { overlayReducer } from '../overlay-context'; + +describe( 'overlayReducer', () => { + const CLIENT_ID = 'abc-123'; + const INITIAL = Object.freeze( {} ); + + it( 'captures a baseline once per client id', () => { + const afterFirst = overlayReducer( INITIAL, { + type: 'CAPTURE_BASELINE', + clientId: CLIENT_ID, + blockName: 'core/paragraph', + attributes: { content: 'Hello' }, + } ); + expect( afterFirst[ CLIENT_ID ] ).toEqual( { + blockName: 'core/paragraph', + baselineAttributes: { content: 'Hello' }, + overlayAttributes: {}, + commentId: null, + syncedOpsKey: null, + } ); + + const afterSecond = overlayReducer( afterFirst, { + type: 'CAPTURE_BASELINE', + clientId: CLIENT_ID, + blockName: 'core/paragraph', + attributes: { content: 'CHANGED' }, + } ); + expect( afterSecond ).toBe( afterFirst ); + } ); + + it( 'merges overlay attributes over an existing entry', () => { + const withBaseline = overlayReducer( INITIAL, { + type: 'CAPTURE_BASELINE', + clientId: CLIENT_ID, + blockName: 'core/paragraph', + attributes: { content: 'A', level: 2 }, + } ); + const withOverlay = overlayReducer( withBaseline, { + type: 'SET_OVERLAY_ATTRIBUTES', + clientId: CLIENT_ID, + attributes: { content: 'B' }, + } ); + expect( withOverlay[ CLIENT_ID ].overlayAttributes ).toEqual( { + content: 'B', + } ); + expect( withOverlay[ CLIENT_ID ].baselineAttributes ).toEqual( { + content: 'A', + level: 2, + } ); + + const updated = overlayReducer( withOverlay, { + type: 'SET_OVERLAY_ATTRIBUTES', + clientId: CLIENT_ID, + attributes: { level: 3 }, + } ); + expect( updated[ CLIENT_ID ].overlayAttributes ).toEqual( { + content: 'B', + level: 3, + } ); + } ); + + it( 'ignores overlay writes without a captured baseline', () => { + const next = overlayReducer( INITIAL, { + type: 'SET_OVERLAY_ATTRIBUTES', + clientId: CLIENT_ID, + attributes: { content: 'Nope' }, + } ); + expect( next ).toBe( INITIAL ); + } ); + + it( 'removes an entry on clear', () => { + const withEntry = overlayReducer( INITIAL, { + type: 'CAPTURE_BASELINE', + clientId: CLIENT_ID, + blockName: 'core/paragraph', + attributes: {}, + } ); + const cleared = overlayReducer( withEntry, { + type: 'CLEAR_OVERLAY', + clientId: CLIENT_ID, + } ); + expect( cleared ).toEqual( {} ); + expect( cleared ).not.toBe( withEntry ); + } ); + + it( 'returns the same reference for unknown actions', () => { + const next = overlayReducer( INITIAL, { type: 'UNKNOWN' } ); + expect( next ).toBe( INITIAL ); + } ); + + it( 'stamps structural ops with the capture sequence and keeps attribute stamps', () => { + const withBaseline = overlayReducer( INITIAL, { + type: 'CAPTURE_BASELINE', + clientId: CLIENT_ID, + blockName: 'core/paragraph', + attributes: { content: 'A' }, + } ); + const withAttr = overlayReducer( withBaseline, { + type: 'SET_OVERLAY_ATTRIBUTES', + clientId: CLIENT_ID, + attributes: { content: 'B' }, + seq: 3, + } ); + const withOp = overlayReducer( withAttr, { + type: 'SET_STRUCTURAL_OP', + clientId: CLIENT_ID, + blockName: 'core/paragraph', + op: { type: 'block-remove' }, + seq: 5, + } ); + expect( withOp[ CLIENT_ID ].structuralOpSeq ).toBe( 5 ); + // The attribute stamp survives the structural rewrite of the entry. + expect( withOp[ CLIENT_ID ].lastEditSeq ).toBe( 3 ); + } ); + + it( 'stamps overlay writes with the capture sequence and keeps the last one', () => { + const withBaseline = overlayReducer( INITIAL, { + type: 'CAPTURE_BASELINE', + clientId: CLIENT_ID, + blockName: 'core/heading', + attributes: { level: 2 }, + } ); + const first = overlayReducer( withBaseline, { + type: 'SET_OVERLAY_ATTRIBUTES', + clientId: CLIENT_ID, + attributes: { level: 3 }, + seq: 7, + } ); + expect( first[ CLIENT_ID ].lastEditSeq ).toBe( 7 ); + + // A later write without a sequence (reducer-level consumers) keeps + // the previous stamp instead of clearing it. + const second = overlayReducer( first, { + type: 'SET_OVERLAY_ATTRIBUTES', + clientId: CLIENT_ID, + attributes: { level: 4 }, + } ); + expect( second[ CLIENT_ID ].lastEditSeq ).toBe( 7 ); + + const third = overlayReducer( second, { + type: 'SET_OVERLAY_ATTRIBUTES', + clientId: CLIENT_ID, + attributes: { level: 5 }, + seq: 9, + } ); + expect( third[ CLIENT_ID ].lastEditSeq ).toBe( 9 ); + } ); + + it( 'prunes entries whose clientId is no longer live', () => { + const state = { + 'alive-1': { + blockName: 'core/paragraph', + baselineAttributes: {}, + overlayAttributes: { content: 'hi' }, + }, + 'orphan-1': { + blockName: 'core/paragraph', + baselineAttributes: {}, + overlayAttributes: { content: 'gone' }, + }, + }; + const next = overlayReducer( state, { + type: 'PRUNE_ORPHANS', + liveClientIds: new Set( [ 'alive-1' ] ), + } ); + expect( Object.keys( next ) ).toEqual( [ 'alive-1' ] ); + } ); + + it( 'stores a comment id on the entry', () => { + const withEntry = overlayReducer( INITIAL, { + type: 'CAPTURE_BASELINE', + clientId: CLIENT_ID, + blockName: 'core/paragraph', + attributes: {}, + } ); + const withCommentId = overlayReducer( withEntry, { + type: 'SET_COMMENT_ID', + clientId: CLIENT_ID, + commentId: 42, + } ); + expect( withCommentId[ CLIENT_ID ].commentId ).toBe( 42 ); + // Clearing the id is permitted. + const cleared = overlayReducer( withCommentId, { + type: 'SET_COMMENT_ID', + clientId: CLIENT_ID, + commentId: null, + } ); + expect( cleared[ CLIENT_ID ].commentId ).toBeNull(); + } ); + + it( 'stores a synced operations fingerprint on the entry', () => { + const withEntry = overlayReducer( INITIAL, { + type: 'CAPTURE_BASELINE', + clientId: CLIENT_ID, + blockName: 'core/paragraph', + attributes: {}, + } ); + const synced = overlayReducer( withEntry, { + type: 'SET_SYNCED_OPS_KEY', + clientId: CLIENT_ID, + syncedOpsKey: 'fingerprint-1', + } ); + expect( synced[ CLIENT_ID ].syncedOpsKey ).toBe( 'fingerprint-1' ); + } ); + + it( 'ignores SET_COMMENT_ID without a captured baseline', () => { + const next = overlayReducer( INITIAL, { + type: 'SET_COMMENT_ID', + clientId: CLIENT_ID, + commentId: 1, + } ); + expect( next ).toBe( INITIAL ); + } ); + + it( 'returns the same reference when no orphans are present', () => { + const state = { + 'alive-1': { + blockName: 'core/paragraph', + baselineAttributes: {}, + overlayAttributes: {}, + }, + }; + const next = overlayReducer( state, { + type: 'PRUNE_ORPHANS', + liveClientIds: new Set( [ 'alive-1', 'other' ] ), + } ); + expect( next ).toBe( state ); + } ); + + it( 'isolates overlays between multiple blocks', () => { + // Two blocks both get baselines and overlays; each is tracked + // independently. + let state = overlayReducer( INITIAL, { + type: 'CAPTURE_BASELINE', + clientId: 'block-a', + blockName: 'core/paragraph', + attributes: { content: 'A-original' }, + } ); + state = overlayReducer( state, { + type: 'CAPTURE_BASELINE', + clientId: 'block-b', + blockName: 'core/heading', + attributes: { content: 'B-original', level: 2 }, + } ); + state = overlayReducer( state, { + type: 'SET_OVERLAY_ATTRIBUTES', + clientId: 'block-a', + attributes: { content: 'A-proposed' }, + } ); + state = overlayReducer( state, { + type: 'SET_OVERLAY_ATTRIBUTES', + clientId: 'block-b', + attributes: { level: 3 }, + } ); + + expect( state[ 'block-a' ].overlayAttributes ).toEqual( { + content: 'A-proposed', + } ); + expect( state[ 'block-b' ].overlayAttributes ).toEqual( { level: 3 } ); + expect( state[ 'block-b' ].baselineAttributes ).toEqual( { + content: 'B-original', + level: 2, + } ); + + // Clearing one doesn't affect the other. + const afterClear = overlayReducer( state, { + type: 'CLEAR_OVERLAY', + clientId: 'block-a', + } ); + expect( afterClear[ 'block-a' ] ).toBeUndefined(); + expect( afterClear[ 'block-b' ] ).toEqual( state[ 'block-b' ] ); + } ); + + it( 'creates an entry on SET_STRUCTURAL_OP when none existed', () => { + const op = { + type: 'block-remove', + clientId: CLIENT_ID, + blockName: 'core/paragraph', + }; + const next = overlayReducer( INITIAL, { + type: 'SET_STRUCTURAL_OP', + clientId: CLIENT_ID, + blockName: 'core/paragraph', + op, + } ); + expect( next[ CLIENT_ID ] ).toMatchObject( { + blockName: 'core/paragraph', + structuralOp: op, + baselineAttributes: {}, + overlayAttributes: {}, + commentId: null, + syncedOpsKey: null, + } ); + } ); + + it( 'preserves attribute-overlay state when adding a structural op to an existing entry', () => { + let state = overlayReducer( INITIAL, { + type: 'CAPTURE_BASELINE', + clientId: CLIENT_ID, + blockName: 'core/paragraph', + attributes: { content: 'baseline' }, + } ); + state = overlayReducer( state, { + type: 'SET_OVERLAY_ATTRIBUTES', + clientId: CLIENT_ID, + attributes: { content: 'edited' }, + } ); + state = overlayReducer( state, { + type: 'SET_STRUCTURAL_OP', + clientId: CLIENT_ID, + blockName: 'core/paragraph', + op: { type: 'block-remove', clientId: CLIENT_ID }, + } ); + expect( state[ CLIENT_ID ].baselineAttributes ).toEqual( { + content: 'baseline', + } ); + expect( state[ CLIENT_ID ].overlayAttributes ).toEqual( { + content: 'edited', + } ); + expect( state[ CLIENT_ID ].structuralOp ).toEqual( { + type: 'block-remove', + clientId: CLIENT_ID, + } ); + } ); +} ); diff --git a/packages/editor/src/components/suggestion-mode/test/provider.js b/packages/editor/src/components/suggestion-mode/test/provider.js new file mode 100644 index 00000000000000..e389f2993bcf17 --- /dev/null +++ b/packages/editor/src/components/suggestion-mode/test/provider.js @@ -0,0 +1,827 @@ +/** + * External dependencies + */ +import { render, act } from '@testing-library/react'; + +/** + * WordPress dependencies + */ +import { + createRegistry, + createReduxStore, + RegistryProvider, +} from '@wordpress/data'; +import { store as blockEditorStore } from '@wordpress/block-editor'; +import { store as noticesStore } from '@wordpress/notices'; +import { + createBlock, + registerBlockType, + unregisterBlockType, + getBlockTypes, +} from '@wordpress/blocks'; + +/** + * Internal dependencies + */ +import { + operationsFromOverlay, + applyOperations, + hasAttributeConflict, + parseSuggestionPayload, + payloadByteLength, + PAYLOAD_MAX_BYTES, + findStructuralOp, + clearSuggestionMarkerAttributes, + useSuggestionsProvider, +} from '../provider'; + +describe( 'operationsFromOverlay', () => { + it( 'emits one attribute-set op per changed key', () => { + const ops = operationsFromOverlay( + { content: 'Hello', level: 2 }, + { content: 'Hi', level: 3 } + ); + expect( ops ).toEqual( [ + { + type: 'attribute-set', + attribute: 'content', + before: 'Hello', + after: 'Hi', + }, + { + type: 'attribute-set', + attribute: 'level', + before: 2, + after: 3, + }, + ] ); + } ); + + it( 'skips attributes that equal their baseline', () => { + const ops = operationsFromOverlay( + { content: 'Same', level: 2 }, + { content: 'Same', level: 3 } + ); + expect( ops ).toEqual( [ + { + type: 'attribute-set', + attribute: 'level', + before: 2, + after: 3, + }, + ] ); + } ); + + it( 'deep-compares object-valued attributes', () => { + const ops = operationsFromOverlay( + { style: { typography: { fontSize: '16px' } } }, + { style: { typography: { fontSize: '16px' } } } + ); + expect( ops ).toEqual( [] ); + } ); + + it( 'is insensitive to key order in object-valued attributes', () => { + // `style` re-emitted with reordered keys must not appear as a + // changed attribute. A naive JSON.stringify compare would flag it. + const ops = operationsFromOverlay( + { style: { typography: { fontSize: '16px' }, color: 'red' } }, + { style: { color: 'red', typography: { fontSize: '16px' } } } + ); + expect( ops ).toEqual( [] ); + } ); + + it( 'compares arrays element-wise', () => { + expect( + operationsFromOverlay( + { classes: [ 'a', 'b' ] }, + { classes: [ 'a', 'b' ] } + ) + ).toEqual( [] ); + const ops = operationsFromOverlay( + { classes: [ 'a', 'b' ] }, + { classes: [ 'b', 'a' ] } + ); + expect( ops ).toHaveLength( 1 ); + expect( ops[ 0 ].attribute ).toBe( 'classes' ); + } ); + + it( 'captures a null baseline when the attribute is new', () => { + const ops = operationsFromOverlay( {}, { url: 'https://x.test' } ); + expect( ops ).toEqual( [ + { + type: 'attribute-set', + attribute: 'url', + before: null, + after: 'https://x.test', + }, + ] ); + } ); + + it( 'returns an empty array for an empty overlay', () => { + expect( operationsFromOverlay( { a: 1 }, {} ) ).toEqual( [] ); + expect( operationsFromOverlay( { a: 1 }, null ) ).toEqual( [] ); + } ); +} ); + +describe( 'applyOperations', () => { + it( 'applies attribute-set operations to produce new attributes', () => { + const result = applyOperations( + { content: 'Hello', level: 2, align: 'left' }, + [ + { + type: 'attribute-set', + attribute: 'content', + before: 'Hello', + after: 'Hi', + }, + { + type: 'attribute-set', + attribute: 'level', + before: 2, + after: 3, + }, + ] + ); + expect( result ).toEqual( { + content: 'Hi', + level: 3, + align: 'left', + } ); + } ); + + it( 'returns a copy even when operations are empty', () => { + const attrs = { content: 'Same' }; + const result = applyOperations( attrs, [] ); + expect( result ).toEqual( attrs ); + expect( result ).not.toBe( attrs ); + } ); + + it( 'composes with clearSuggestionMarkerAttributes to produce the inserted-block apply payload', () => { + // When applying a block-insert-after suggestion the live block on + // the accepter's side is in its captured-at-insertion shape — the + // suggester's interceptor diverted any subsequent typing into the + // overlay rather than committing it. Apply must (1) materialize + // those overlay edits as attribute-set ops, AND (2) clear the + // pending-insert marker. Combining `applyOperations` with + // `clearSuggestionMarkerAttributes` yields the merged payload the + // provider hands to `updateBlockAttributes`. + const liveAttributes = { + content: '', + metadata: { + noteId: [ 42 ], + suggestion: { type: 'pending-insert' }, + }, + }; + const operations = [ + { + type: 'block-insert-after', + clientId: 'abc', + blockName: 'core/paragraph', + anchorClientId: null, + parentClientId: null, + block: { name: 'core/paragraph', attributes: { content: '' } }, + }, + { + type: 'attribute-set', + attribute: 'content', + before: '', + after: 'Hello world', + }, + ]; + + const withOpsApplied = applyOperations( liveAttributes, operations ); + const markerCleared = clearSuggestionMarkerAttributes( withOpsApplied ); + const finalAttributes = markerCleared + ? { ...withOpsApplied, ...markerCleared } + : withOpsApplied; + + // The typed content is committed to the live block... + expect( finalAttributes.content ).toBe( 'Hello world' ); + // ...and the pending-insert marker is gone, while noteId + // (system metadata) is preserved. + expect( finalAttributes.metadata ).toEqual( { noteId: [ 42 ] } ); + expect( finalAttributes.metadata.suggestion ).toBeUndefined(); + } ); +} ); + +describe( 'payloadByteLength', () => { + it( 'measures ASCII payload byte length', () => { + // {"a":"hello"} is 13 bytes. + expect( payloadByteLength( { a: 'hello' } ) ).toBe( 13 ); + } ); + + it( 'counts multi-byte characters by UTF-8 byte length', () => { + // {"a":"€"} = 8 ASCII bytes + 3 bytes for the euro sign = 11. + expect( payloadByteLength( { a: '€' } ) ).toBe( 11 ); + } ); + + it( 'exposes a numeric size cap', () => { + expect( PAYLOAD_MAX_BYTES ).toBeGreaterThan( 0 ); + expect( typeof PAYLOAD_MAX_BYTES ).toBe( 'number' ); + } ); +} ); + +describe( 'hasAttributeConflict', () => { + const CONTENT_OP = { + type: 'attribute-set', + attribute: 'content', + before: 'Hello', + after: 'Hi', + }; + + it( 'returns false when the targeted attribute still matches the baseline', () => { + expect( + hasAttributeConflict( { content: 'Hello', level: 2 }, [ + CONTENT_OP, + ] ) + ).toBe( false ); + } ); + + it( 'returns true when the targeted attribute has diverged', () => { + expect( + hasAttributeConflict( { content: 'Hola' }, [ CONTENT_OP ] ) + ).toBe( true ); + } ); + + it( 'ignores unrelated attribute changes on the block', () => { + // Post modified bumps often because an unrelated attribute (or another + // block entirely) changed — those should never count as a conflict + // for this suggestion. + expect( + hasAttributeConflict( + { content: 'Hello', level: 3, align: 'center' }, + [ CONTENT_OP ] + ) + ).toBe( false ); + } ); + + it( 'deep-compares object-valued attributes', () => { + const op = { + type: 'attribute-set', + attribute: 'style', + before: { typography: { fontSize: '16px' } }, + after: { typography: { fontSize: '20px' } }, + }; + expect( + hasAttributeConflict( + { style: { typography: { fontSize: '16px' } } }, + [ op ] + ) + ).toBe( false ); + expect( + hasAttributeConflict( + { style: { typography: { fontSize: '18px' } } }, + [ op ] + ) + ).toBe( true ); + } ); + + it( 'treats a null baseline as equal to a missing current attribute', () => { + const op = { + type: 'attribute-set', + attribute: 'url', + before: null, + after: 'https://x.test', + }; + expect( hasAttributeConflict( {}, [ op ] ) ).toBe( false ); + expect( + hasAttributeConflict( { url: 'https://other.test' }, [ op ] ) + ).toBe( true ); + } ); + + it( 'returns false for malformed input', () => { + expect( hasAttributeConflict( {}, undefined ) ).toBe( false ); + expect( hasAttributeConflict( {}, [] ) ).toBe( false ); + } ); + + it( 'compares string baselines against wrapper-object live values via toString', () => { + // Regression: rich-text attributes are stored on a block as + // `RichTextData` instances but serialize into the suggestion payload + // as plain strings. Without a string-vs-wrapper fallback, + // `hasAttributeConflict` flagged every content suggestion as stale + // because `typeof string` !== `typeof object`, which short-circuited + // the apply flow into a never-visible "Apply anyway" dialog. + const wrapper = { + toString() { + return 'Hello'; + }, + }; + const wrapperOther = { + toString() { + return 'Hola'; + }, + }; + expect( + hasAttributeConflict( { content: wrapper }, [ CONTENT_OP ] ) + ).toBe( false ); + expect( + hasAttributeConflict( { content: wrapperOther }, [ CONTENT_OP ] ) + ).toBe( true ); + } ); + + it( 'returns false for a block-insert-after payload even when attribute-set ops appear divergent', () => { + // The overlay baseline for an inserted block is `{}` — every + // attribute-set op the auto-save loop persists carries + // `before: null` regardless of what the user typed. Comparing + // that against the live (already-typed-into) block on the + // accepting client must not fire the staleness prompt. + const operations = [ + { + type: 'block-insert-after', + clientId: 'inserted', + blockName: 'core/paragraph', + anchorClientId: 'anchor', + parentClientId: null, + block: { + name: 'core/paragraph', + attributes: { content: 'Hi' }, + innerBlocks: [], + }, + }, + { + type: 'attribute-set', + attribute: 'content', + before: null, + after: 'Hi', + }, + ]; + expect( hasAttributeConflict( { content: 'Hi' }, operations ) ).toBe( + false + ); + } ); +} ); + +describe( 'parseSuggestionPayload', () => { + it( 'parses a valid current-version JSON payload', () => { + const raw = JSON.stringify( { + schemaVersion: 2, + blockName: 'core/paragraph', + baseRevision: '2026-04-15T00:00:00', + operations: [ + { + type: 'attribute-set', + attribute: 'content', + before: 'a', + after: 'b', + }, + ], + } ); + const result = parseSuggestionPayload( raw ); + expect( result ).not.toBeNull(); + expect( result.schemaVersion ).toBe( 2 ); + expect( result.operations ).toHaveLength( 1 ); + expect( result.blockName ).toBe( 'core/paragraph' ); + } ); + + it( 'migrates a v1 payload forward to the current version', () => { + const raw = JSON.stringify( { + schemaVersion: 1, + blockName: 'core/paragraph', + baseRevision: null, + operations: [ + { + type: 'attribute-set', + attribute: 'content', + before: 'a', + after: 'b', + }, + ], + } ); + const result = parseSuggestionPayload( raw ); + expect( result ).not.toBeNull(); + expect( result.schemaVersion ).toBe( 2 ); + expect( result.operations ).toHaveLength( 1 ); + } ); + + it( 'treats a missing schemaVersion as v1 and migrates it', () => { + const raw = JSON.stringify( { + blockName: 'core/paragraph', + baseRevision: null, + operations: [ + { + type: 'attribute-set', + attribute: 'content', + before: 'a', + after: 'b', + }, + ], + } ); + const result = parseSuggestionPayload( raw ); + expect( result ).not.toBeNull(); + expect( result.schemaVersion ).toBe( 2 ); + } ); + + it( 'refuses a payload from a newer editor', () => { + const raw = JSON.stringify( { + schemaVersion: 99, + blockName: 'core/paragraph', + baseRevision: null, + operations: [ + { + type: 'block-rotate', + clientId: 'x', + }, + ], + } ); + expect( parseSuggestionPayload( raw ) ).toBeNull(); + } ); + + it( 'returns null for missing, empty, or invalid input', () => { + expect( parseSuggestionPayload( undefined ) ).toBeNull(); + expect( parseSuggestionPayload( '' ) ).toBeNull(); + expect( parseSuggestionPayload( 'not json' ) ).toBeNull(); + expect( parseSuggestionPayload( '42' ) ).toBeNull(); + expect( + parseSuggestionPayload( JSON.stringify( { noOps: true } ) ) + ).toBeNull(); + } ); +} ); + +describe( 'findStructuralOp', () => { + it( 'returns null for a payload of attribute-set ops only', () => { + expect( + findStructuralOp( [ + { type: 'attribute-set', attribute: 'content', after: 'x' }, + ] ) + ).toBeNull(); + } ); + + it( 'returns the block-remove op when present', () => { + const op = { + type: 'block-remove', + clientId: 'abc', + blockName: 'core/paragraph', + }; + expect( + findStructuralOp( [ + { type: 'attribute-set', attribute: 'x', after: 1 }, + op, + ] ) + ).toBe( op ); + } ); + + it( 'recognizes block-insert-after and block-move op types', () => { + expect( + findStructuralOp( [ { type: 'block-insert-after' } ] )?.type + ).toBe( 'block-insert-after' ); + expect( findStructuralOp( [ { type: 'block-move' } ] )?.type ).toBe( + 'block-move' + ); + } ); + + it( 'returns null for non-array input', () => { + expect( findStructuralOp( null ) ).toBeNull(); + expect( findStructuralOp( undefined ) ).toBeNull(); + } ); +} ); + +describe( 'clearSuggestionMarkerAttributes', () => { + it( 'returns null when there is no marker to clear', () => { + expect( clearSuggestionMarkerAttributes( {} ) ).toBeNull(); + expect( + clearSuggestionMarkerAttributes( { metadata: { noteId: 1 } } ) + ).toBeNull(); + } ); + + it( 'strips the suggestion field while preserving other metadata', () => { + expect( + clearSuggestionMarkerAttributes( { + content: 'hi', + metadata: { noteId: 7, suggestion: { type: 'pending-remove' } }, + } ) + ).toEqual( { metadata: { noteId: 7 } } ); + } ); +} ); + +/* + * Minimal `core/interface` stub tracking only the active complementary + * area — the piece of state `createSuggestion` reads and writes when + * surfacing a new note. Registered under the production store name so + * the provider's descriptor-based lookups resolve to it. + */ +function createStubInterfaceStore() { + return createReduxStore( 'core/interface', { + reducer: ( state = { activeArea: null }, action ) => + action.type === 'ENABLE_AREA' ? { activeArea: action.area } : state, + actions: { + enableComplementaryArea: ( scope, area ) => ( { + type: 'ENABLE_AREA', + area, + } ), + }, + selectors: { + getActiveComplementaryArea: ( state ) => state.activeArea, + }, + } ); +} + +describe( 'rejectSuggestion (block-move)', () => { + const PARAGRAPH = 'core/test-move-paragraph'; + const GROUP = 'core/test-move-group'; + + beforeAll( () => { + registerBlockType( PARAGRAPH, { + apiVersion: 3, + attributes: { + content: { type: 'string', default: '' }, + metadata: { type: 'object' }, + }, + save: () => null, + category: 'text', + title: 'Test Move Paragraph', + } ); + registerBlockType( GROUP, { + apiVersion: 3, + attributes: { + metadata: { type: 'object' }, + }, + save: () => null, + category: 'design', + title: 'Test Move Group', + } ); + } ); + + afterAll( () => { + getBlockTypes().forEach( ( block ) => + unregisterBlockType( block.name ) + ); + } ); + + /* + * The provider persists lifecycle updates through core-data's + * `saveEntityRecord`. A stub registered under the same store name keeps + * the reject flow synchronous and network-free — the assertions here are + * about the block tree, not the REST round-trip. + */ + function createStubCoreStore() { + return createReduxStore( 'core', { + reducer: ( state = {} ) => state, + actions: { + saveEntityRecord: () => ( { type: 'SAVE_ENTITY_RECORD' } ), + }, + selectors: { + getEditedEntityRecord: () => null, + getEntityRecord: () => null, + getCurrentUser: () => null, + }, + } ); + } + + function setup( initialBlocks ) { + const registry = createRegistry(); + registry.register( noticesStore ); + registry.register( blockEditorStore ); + registry.register( createStubCoreStore() ); + registry.register( createStubInterfaceStore() ); + registry.dispatch( blockEditorStore ).resetBlocks( initialBlocks ); + + let providerHandle; + function CaptureProvider() { + providerHandle = useSuggestionsProvider(); + return null; + } + + render( + + + + ); + + return { registry, getProvider: () => providerHandle }; + } + + function movePayload( structuralOp ) { + return { + schemaVersion: 2, + blockName: PARAGRAPH, + baseRevision: null, + operations: [ structuralOp ], + }; + } + + it( 'restores a cross-parent move back to the original parent', async () => { + // Current state: the block was suggested-moved from the root (index + // 0) INTO the group. Reject must restore it to the root. + const moved = createBlock( PARAGRAPH, { + content: 'Moved', + metadata: { suggestion: { type: 'pending-move' } }, + } ); + const sibling = createBlock( PARAGRAPH, { content: 'Sibling' } ); + const group = createBlock( GROUP, {}, [ + createBlock( PARAGRAPH, { content: 'Child' } ), + moved, + ] ); + + const { registry, getProvider } = setup( [ sibling, group ] ); + + await act( async () => { + await getProvider().rejectSuggestion( { + commentId: 1, + clientId: moved.clientId, + payload: movePayload( { + type: 'block-move', + clientId: moved.clientId, + blockName: PARAGRAPH, + fromParentClientId: null, + fromIndex: 0, + toParentClientId: group.clientId, + } ), + } ); + } ); + + const blockEditor = registry.select( blockEditorStore ); + // The block is back at the ROOT (its original parent), not stuck + // inside the group. Before the fix, `moveBlockToPosition` received + // the original parent as both from- and to-root, missed the block in + // the destination parent, and silently left the tree unchanged. + expect( blockEditor.getBlockRootClientId( moved.clientId ) || '' ).toBe( + '' + ); + expect( blockEditor.getBlockIndex( moved.clientId ) ).toBe( 0 ); + // The pending-move marker is cleared. + expect( + blockEditor.getBlockAttributes( moved.clientId )?.metadata + ?.suggestion + ).toBeUndefined(); + } ); + + it( 'restores a same-parent move back to its original index', async () => { + const a = createBlock( PARAGRAPH, { content: 'A' } ); + const moved = createBlock( PARAGRAPH, { + content: 'Moved', + metadata: { suggestion: { type: 'pending-move' } }, + } ); + const b = createBlock( PARAGRAPH, { content: 'B' } ); + + // Current order: [A, B, Moved] — the block was suggested-moved from + // index 0 to the end of the root. + const { registry, getProvider } = setup( [ a, b, moved ] ); + + await act( async () => { + await getProvider().rejectSuggestion( { + commentId: 2, + clientId: moved.clientId, + payload: movePayload( { + type: 'block-move', + clientId: moved.clientId, + blockName: PARAGRAPH, + fromParentClientId: null, + fromIndex: 0, + toParentClientId: null, + } ), + } ); + } ); + + const blockEditor = registry.select( blockEditorStore ); + expect( blockEditor.getBlockIndex( moved.clientId ) ).toBe( 0 ); + expect( blockEditor.getBlockRootClientId( moved.clientId ) || '' ).toBe( + '' + ); + } ); +} ); + +describe( 'createSuggestion (notes sidebar switch)', () => { + const PARAGRAPH = 'core/test-sidebar-paragraph'; + const ALL_NOTES_SIDEBAR = 'edit-post/collab-history-sidebar'; + const FLOATING_NOTES_SIDEBAR = 'edit-post/collab-sidebar'; + + beforeAll( () => { + registerBlockType( PARAGRAPH, { + apiVersion: 3, + attributes: { + content: { type: 'string', default: '' }, + metadata: { type: 'object' }, + }, + save: () => null, + category: 'text', + title: 'Test Sidebar Paragraph', + } ); + } ); + + afterAll( () => { + getBlockTypes().forEach( ( block ) => + unregisterBlockType( block.name ) + ); + } ); + + /* + * Store stubs registered under the production store names so the + * provider's descriptor-based lookups resolve to them. `saveEntityRecord` + * returns a record with an id so the post-save branch (metadata linkage + * and the sidebar switch) runs. + */ + function createStubCoreStore() { + return createReduxStore( 'core', { + reducer: ( state = {} ) => state, + actions: { + saveEntityRecord: () => () => ( { id: 123 } ), + }, + selectors: { + getEditedEntityRecord: () => null, + getEntityRecord: () => null, + getCurrentUser: () => null, + }, + } ); + } + + function createStubEditorStore() { + return createReduxStore( 'core/editor', { + reducer: ( state = {} ) => state, + selectors: { + getCurrentPostId: () => 42, + getCurrentPostType: () => 'post', + }, + } ); + } + + function setup( { activeArea } ) { + const registry = createRegistry(); + registry.register( noticesStore ); + registry.register( blockEditorStore ); + registry.register( createStubCoreStore() ); + registry.register( createStubEditorStore() ); + const interfaceStub = createStubInterfaceStore(); + registry.register( interfaceStub ); + if ( activeArea ) { + registry + .dispatch( interfaceStub ) + .enableComplementaryArea( 'core', activeArea ); + } + + const block = createBlock( PARAGRAPH, { content: 'Hello' } ); + registry.dispatch( blockEditorStore ).resetBlocks( [ block ] ); + + let providerHandle; + function CaptureProvider() { + providerHandle = useSuggestionsProvider(); + return null; + } + + render( + + + + ); + + return { registry, block, getProvider: () => providerHandle }; + } + + async function createAttributeSuggestion( getProvider, block ) { + await act( async () => { + await getProvider().createSuggestion( { + clientId: block.clientId, + blockName: PARAGRAPH, + operations: [ + { + type: 'attribute-set', + attribute: 'content', + value: 'Hello world', + baseline: 'Hello', + }, + ], + } ); + } ); + } + + it( 'switches an open non-notes sidebar to the All notes sidebar', async () => { + const { registry, block, getProvider } = setup( { + activeArea: 'edit-post/document', + } ); + + await createAttributeSuggestion( getProvider, block ); + + expect( + registry + .select( 'core/interface' ) + .getActiveComplementaryArea( 'core' ) + ).toBe( ALL_NOTES_SIDEBAR ); + } ); + + it( 'leaves a closed sidebar closed', async () => { + const { registry, block, getProvider } = setup( { + activeArea: null, + } ); + + await createAttributeSuggestion( getProvider, block ); + + expect( + registry + .select( 'core/interface' ) + .getActiveComplementaryArea( 'core' ) + ).toBe( null ); + } ); + + it( 'leaves an already-open notes sidebar in place', async () => { + const { registry, block, getProvider } = setup( { + activeArea: FLOATING_NOTES_SIDEBAR, + } ); + + await createAttributeSuggestion( getProvider, block ); + + expect( + registry + .select( 'core/interface' ) + .getActiveComplementaryArea( 'core' ) + ).toBe( FLOATING_NOTES_SIDEBAR ); + } ); +} ); diff --git a/packages/editor/src/components/suggestion-mode/test/suggestion-write-queue.js b/packages/editor/src/components/suggestion-mode/test/suggestion-write-queue.js new file mode 100644 index 00000000000000..0b89ecd9edb318 --- /dev/null +++ b/packages/editor/src/components/suggestion-mode/test/suggestion-write-queue.js @@ -0,0 +1,98 @@ +/** + * Internal dependencies + */ +import { createSuggestionWriteQueue } from '../suggestion-write-queue'; + +/** Create a promise whose resolution the test controls. */ +function deferred() { + let resolve; + let reject; + const promise = new Promise( ( res, rej ) => { + resolve = res; + reject = rej; + } ); + return { promise, resolve, reject }; +} + +async function flushMicrotasks() { + // A few turns so chained `.then`s inside the queue settle. + for ( let i = 0; i < 5; i++ ) { + await Promise.resolve(); + } +} + +describe( 'createSuggestionWriteQueue', () => { + it( 'runs tasks for the same block strictly one after another', async () => { + const queue = createSuggestionWriteQueue(); + const first = deferred(); + const order = []; + + queue.enqueue( 'a', async () => { + order.push( 'first:start' ); + await first.promise; + order.push( 'first:end' ); + } ); + queue.enqueue( 'a', async () => { + order.push( 'second:start' ); + } ); + await flushMicrotasks(); + + // The second task must not start while the first is in flight. + expect( order ).toEqual( [ 'first:start' ] ); + + first.resolve(); + await flushMicrotasks(); + expect( order ).toEqual( [ + 'first:start', + 'first:end', + 'second:start', + ] ); + } ); + + it( 'lets tasks for different blocks run independently', async () => { + const queue = createSuggestionWriteQueue(); + const blockedForever = deferred(); + const order = []; + + queue.enqueue( 'a', async () => { + await blockedForever.promise; + } ); + queue.enqueue( 'b', async () => { + order.push( 'b:ran' ); + } ); + await flushMicrotasks(); + + expect( order ).toEqual( [ 'b:ran' ] ); + } ); + + it( 'does not let a rejected task poison later tasks on the block', async () => { + const queue = createSuggestionWriteQueue(); + const order = []; + + const failing = queue.enqueue( 'a', async () => { + throw new Error( 'boom' ); + } ); + // The caller can still observe the failure. + await expect( failing ).rejects.toThrow( 'boom' ); + + queue.enqueue( 'a', async () => { + order.push( 'after-failure' ); + } ); + await flushMicrotasks(); + expect( order ).toEqual( [ 'after-failure' ] ); + } ); + + it( 'reports and clears pending state per block', async () => { + const queue = createSuggestionWriteQueue(); + const gate = deferred(); + + expect( queue.hasPending( 'a' ) ).toBe( false ); + queue.enqueue( 'a', () => gate.promise ); + expect( queue.hasPending( 'a' ) ).toBe( true ); + expect( queue.hasPending( 'b' ) ).toBe( false ); + + gate.resolve(); + await flushMicrotasks(); + expect( queue.hasPending( 'a' ) ).toBe( false ); + } ); +} ); diff --git a/phpunit/experimental/class-wp-rest-comments-controller-gutenberg-test.php b/phpunit/experimental/class-wp-rest-comments-controller-gutenberg-test.php index 02d2ea8ed39a56..d3c578a2836e1c 100644 --- a/phpunit/experimental/class-wp-rest-comments-controller-gutenberg-test.php +++ b/phpunit/experimental/class-wp-rest-comments-controller-gutenberg-test.php @@ -1,6 +1,28 @@ array( 'reopen' ), ); } + + /** + * Test that a suggestion payload can be stored and retrieved via meta. + */ + public function test_create_note_with_suggestion_meta() { + wp_set_current_user( self::$editor_id ); + $post_id = self::factory()->post->create( array( 'post_author' => self::$editor_id ) ); + + $payload = wp_json_encode( + array( + 'schemaVersion' => 1, + 'blockName' => 'core/paragraph', + 'baseRevision' => '2026-04-15T00:00:00', + 'operations' => array( + array( + 'type' => 'attribute-set', + 'attribute' => 'content', + 'before' => 'Hello', + 'after' => 'Hello world', + ), + ), + ) + ); + + $params = array( + 'post' => $post_id, + 'content' => '', + 'type' => 'note', + 'author' => self::$editor_id, + 'meta' => array( + '_wp_suggestion' => $payload, + ), + ); + + $request = new WP_REST_Request( 'POST', '/wp/v2/comments' ); + $request->add_header( 'Content-Type', 'application/json' ); + $request->set_body( wp_json_encode( $params ) ); + + $response = rest_get_server()->dispatch( $request ); + $this->assertSame( 201, $response->get_status() ); + + $data = $response->get_data(); + $comment_id = $data['id'] ?? null; + $this->assertIsInt( $comment_id ); + + // Bypass REST schema variability across WP versions and check the + // stored meta directly. The sanitize_callback is in scope of this + // test; the REST layer assembles schemas at runtime in a way that + // isn't always available in the experimental phpunit harness. + $stored = get_comment_meta( $comment_id, '_wp_suggestion', true ); + $this->assertNotEmpty( $stored, 'Suggestion meta should round-trip into storage.' ); + $decoded = json_decode( $stored, true ); + $this->assertSame( 'core/paragraph', $decoded['blockName'] ?? null ); + $this->assertSame( 1, $decoded['schemaVersion'] ?? null ); + $this->assertCount( 1, $decoded['operations'] ?? array() ); + } + + /** + * Test that a user without `unfiltered_html` has script markup stripped + * from the applied fields of a suggestion payload at write time. The + * `after` value is what a reviewer's accept writes into block attributes, + * so it must be limited to what the suggester could publish directly. + */ + public function test_suggestion_payload_is_ksesed_for_user_without_unfiltered_html() { + wp_set_current_user( self::$author_id ); + $post_id = self::factory()->post->create( array( 'post_author' => self::$author_id ) ); + + $payload = wp_json_encode( + array( + 'schemaVersion' => 2, + 'blockName' => 'core/paragraph', + 'baseRevision' => null, + 'operations' => array( + array( + 'type' => 'attribute-set', + 'attribute' => 'content', + 'before' => 'Hello', + 'after' => 'Hello world', + ), + ), + ) + ); + + $request = new WP_REST_Request( 'POST', '/wp/v2/comments' ); + $request->add_header( 'Content-Type', 'application/json' ); + $request->set_body( + wp_json_encode( + array( + 'post' => $post_id, + 'content' => '', + 'type' => 'note', + 'author' => self::$author_id, + 'meta' => array( + '_wp_suggestion' => $payload, + ), + ) + ) + ); + + $response = rest_get_server()->dispatch( $request ); + $this->assertSame( 201, $response->get_status() ); + + $data = $response->get_data(); + $stored = get_comment_meta( $data['id'], '_wp_suggestion', true ); + $decoded = json_decode( $stored, true ); + $after = $decoded['operations'][0]['after'] ?? ''; + + $this->assertStringNotContainsString( '', + ), + ), + ) + ); + + $request = new WP_REST_Request( 'POST', '/wp/v2/comments' ); + $request->add_header( 'Content-Type', 'application/json' ); + $request->set_body( + wp_json_encode( + array( + 'post' => $post_id, + 'content' => '', + 'type' => 'note', + 'author' => self::$admin_id, + 'meta' => array( + '_wp_suggestion' => $payload, + ), + ) + ) + ); + + $response = rest_get_server()->dispatch( $request ); + $this->assertSame( 201, $response->get_status() ); + + $data = $response->get_data(); + $stored = get_comment_meta( $data['id'], '_wp_suggestion', true ); + $decoded = json_decode( $stored, true ); + + $this->assertStringContainsString( + ' content'; + $payload = wp_json_encode( + array( + 'schemaVersion' => 2, + 'blockName' => 'core/paragraph', + 'baseRevision' => null, + 'operations' => array( + array( + 'type' => 'attribute-set', + 'attribute' => 'content', + 'before' => $before_value, + 'after' => 'Plain replacement', + ), + ), + ) + ); + + $request = new WP_REST_Request( 'POST', '/wp/v2/comments' ); + $request->add_header( 'Content-Type', 'application/json' ); + $request->set_body( + wp_json_encode( + array( + 'post' => $post_id, + 'content' => '', + 'type' => 'note', + 'author' => self::$author_id, + 'meta' => array( + '_wp_suggestion' => $payload, + ), + ) + ) + ); + + $response = rest_get_server()->dispatch( $request ); + $this->assertSame( 201, $response->get_status() ); + + $data = $response->get_data(); + $stored = get_comment_meta( $data['id'], '_wp_suggestion', true ); + $decoded = json_decode( $stored, true ); + + $this->assertSame( + $before_value, + $decoded['operations'][0]['before'] ?? null, + 'The `before` baseline must be stored verbatim for conflict detection.' + ); + } + + /** + * Test that a payload that isn't valid JSON is rejected with a 400 rather + * than stored as garbage the client would silently null out. + */ + public function test_create_rejects_invalid_json_suggestion_payload() { + wp_set_current_user( self::$editor_id ); + $post_id = self::factory()->post->create( array( 'post_author' => self::$editor_id ) ); + + $request = new WP_REST_Request( 'POST', '/wp/v2/comments' ); + $request->add_header( 'Content-Type', 'application/json' ); + $request->set_body( + wp_json_encode( + array( + 'post' => $post_id, + 'content' => '', + 'type' => 'note', + 'meta' => array( + '_wp_suggestion' => 'this is {not valid json', + ), + ) + ) + ); + + $response = rest_get_server()->dispatch( $request ); + $this->assertErrorResponse( 'rest_suggestion_invalid_json', $response, 400 ); + } + + /** + * Test that an editor can update a note they did not author (edit_post check). + */ + public function test_editor_can_update_note_on_own_post() { + wp_set_current_user( self::$admin_id ); + $post_id = self::factory()->post->create( array( 'post_author' => self::$editor_id ) ); + + // Admin creates a note on editor's post. + $comment_id = self::factory()->comment->create( + array( + 'comment_post_ID' => $post_id, + 'comment_type' => 'note', + 'comment_approved' => 1, + 'user_id' => self::$admin_id, + 'comment_content' => 'suggestion note', + ) + ); + + // Editor (post author) updates the note they did not author. + wp_set_current_user( self::$editor_id ); + + $request = new WP_REST_Request( 'PUT', '/wp/v2/comments/' . $comment_id ); + $request->add_header( 'Content-Type', 'application/json' ); + $request->set_body( + wp_json_encode( + array( + 'status' => 'approved', + 'meta' => array( + '_wp_suggestion_status' => 'applied', + ), + ) + ) + ); + + $response = rest_get_server()->dispatch( $request ); + // The suggestion-lifecycle override passed because the editor owns + // the parent post; the update succeeds with a 200 status. + $this->assertSame( 200, $response->get_status() ); + } + + /** + * Test that a subscriber cannot update a note on someone else's post. + */ + public function test_subscriber_cannot_update_note() { + wp_set_current_user( self::$editor_id ); + $post_id = self::factory()->post->create( array( 'post_author' => self::$editor_id ) ); + + $comment_id = self::factory()->comment->create( + array( + 'comment_post_ID' => $post_id, + 'comment_type' => 'note', + 'user_id' => self::$editor_id, + 'comment_content' => 'a suggestion', + ) + ); + + // Subscriber tries to update the note. + wp_set_current_user( self::$subscriber_id ); + + $request = new WP_REST_Request( 'PUT', '/wp/v2/comments/' . $comment_id ); + $request->add_header( 'Content-Type', 'application/json' ); + $request->set_body( + wp_json_encode( + array( + 'meta' => array( + '_wp_suggestion_status' => 'rejected', + ), + ) + ) + ); + + $response = rest_get_server()->dispatch( $request ); + $this->assertErrorResponse( 'rest_cannot_edit', $response, 403 ); + } + + /** + * Test that _wp_suggestion_status does not persist invalid enum values. + */ + public function test_suggestion_status_ignores_invalid_value() { + wp_set_current_user( self::$editor_id ); + $post_id = self::factory()->post->create( array( 'post_author' => self::$editor_id ) ); + + $comment_id = self::factory()->comment->create( + array( + 'comment_post_ID' => $post_id, + 'comment_type' => 'note', + 'comment_approved' => 1, + 'user_id' => self::$editor_id, + ) + ); + + // First set a valid value. + update_comment_meta( $comment_id, '_wp_suggestion_status', 'pending' ); + + $request = new WP_REST_Request( 'PUT', '/wp/v2/comments/' . $comment_id ); + $request->add_header( 'Content-Type', 'application/json' ); + $request->set_body( + wp_json_encode( + array( + 'meta' => array( + '_wp_suggestion_status' => 'invalid_value', + ), + ) + ) + ); + + rest_get_server()->dispatch( $request ); + // Even if the request succeeds, the invalid value should not + // overwrite the existing valid value. + $stored = get_comment_meta( $comment_id, '_wp_suggestion_status', true ); + $this->assertSame( 'pending', $stored ); + } + + /** + * Test that a subscriber cannot apply a suggestion even if the request + * only touches the suggestion-lifecycle fields. + */ + public function test_subscriber_cannot_apply_suggestion() { + wp_set_current_user( self::$editor_id ); + $post_id = self::factory()->post->create( array( 'post_author' => self::$editor_id ) ); + + $comment_id = self::factory()->comment->create( + array( + 'comment_post_ID' => $post_id, + 'comment_type' => 'note', + 'comment_approved' => 1, + 'user_id' => self::$editor_id, + 'comment_content' => '', + ) + ); + + wp_set_current_user( self::$subscriber_id ); + + $request = new WP_REST_Request( 'PUT', '/wp/v2/comments/' . $comment_id ); + $request->add_header( 'Content-Type', 'application/json' ); + $request->set_body( + wp_json_encode( + array( + 'status' => 'approved', + 'meta' => array( + '_wp_suggestion_status' => 'applied', + ), + ) + ) + ); + + $response = rest_get_server()->dispatch( $request ); + $this->assertErrorResponse( 'rest_cannot_edit', $response, 403 ); + } + + /** + * Test that creating a note with an oversized suggestion payload is + * rejected with a clear 413 error rather than silently truncated. + */ + public function test_create_rejects_oversized_suggestion_payload() { + wp_set_current_user( self::$editor_id ); + $post_id = self::factory()->post->create( array( 'post_author' => self::$editor_id ) ); + + $oversized = str_repeat( 'a', GUTENBERG_SUGGESTION_PAYLOAD_MAX_BYTES + 1 ); + + $request = new WP_REST_Request( 'POST', '/wp/v2/comments' ); + $request->add_header( 'Content-Type', 'application/json' ); + $request->set_body( + wp_json_encode( + array( + 'post' => $post_id, + 'content' => '', + 'type' => 'note', + 'meta' => array( + '_wp_suggestion' => $oversized, + ), + ) + ) + ); + + $response = rest_get_server()->dispatch( $request ); + $this->assertErrorResponse( 'rest_suggestion_too_large', $response, 413 ); + } + + /** + * Test that updating a note with an oversized suggestion payload is + * rejected with a clear 413 error. + */ + public function test_update_rejects_oversized_suggestion_payload() { + wp_set_current_user( self::$editor_id ); + $post_id = self::factory()->post->create( array( 'post_author' => self::$editor_id ) ); + + $comment_id = self::factory()->comment->create( + array( + 'comment_post_ID' => $post_id, + 'comment_type' => 'note', + 'comment_approved' => 1, + 'user_id' => self::$editor_id, + 'comment_content' => 'a suggestion', + ) + ); + + $oversized = str_repeat( 'a', GUTENBERG_SUGGESTION_PAYLOAD_MAX_BYTES + 1 ); + + $request = new WP_REST_Request( 'PUT', '/wp/v2/comments/' . $comment_id ); + $request->add_header( 'Content-Type', 'application/json' ); + $request->set_body( + wp_json_encode( + array( + 'meta' => array( + '_wp_suggestion' => $oversized, + ), + ) + ) + ); + + $response = rest_get_server()->dispatch( $request ); + $this->assertErrorResponse( 'rest_suggestion_too_large', $response, 413 ); + } + + /** + * Test that the sanitize_callback rejects rather than truncates an + * oversized payload reaching the meta layer through a non-REST path. + * Truncating mid-string would corrupt the JSON. + */ + public function test_sanitize_callback_rejects_oversized_value() { + $post_id = self::factory()->post->create(); + $comment_id = self::factory()->comment->create( + array( + 'comment_post_ID' => $post_id, + 'comment_type' => 'note', + ) + ); + + $oversized = str_repeat( 'a', GUTENBERG_SUGGESTION_PAYLOAD_MAX_BYTES + 1 ); + update_comment_meta( $comment_id, '_wp_suggestion', $oversized ); + + $stored = get_comment_meta( $comment_id, '_wp_suggestion', true ); + $this->assertSame( '', $stored, 'Oversized payload should be rejected, not truncated.' ); + } + + /** + * Test that `is_suggestion_lifecycle_update` correctly rejects + * request bodies that touch fields outside the suggestion-lifecycle + * allowlist. We assert against the private helper via a request + * probe rather than through the full REST dispatch because actual + * permission behavior for `edit_comment` on a foreign note on a + * post the current user authored is governed by core's + * `map_meta_cap` for `edit_comment` (which delegates to `edit_post` + * on the comment's parent post) — outside the scope of this override. + */ + public function test_lifecycle_update_rejects_non_allowlisted_fields() { + $cases = array( + 'content field blocks shortcut' => array( + 'body' => array( + 'status' => 'approved', + 'content' => 'rewritten', + ), + 'expected' => false, + ), + 'only id/status/meta passes shortcut' => array( + 'body' => array( + 'status' => 'approved', + 'meta' => array( + '_wp_suggestion_status' => 'applied', + ), + ), + 'expected' => true, + ), + 'non-approved status blocks shortcut' => array( + 'body' => array( + 'status' => 'spam', + ), + 'expected' => false, + ), + 'non-allowlisted meta blocks shortcut' => array( + 'body' => array( + 'meta' => array( + '_wp_note_status' => 'resolved', + ), + ), + 'expected' => false, + ), + ); + + foreach ( $cases as $label => $case ) { + $request = new WP_REST_Request( 'PUT', '/wp/v2/comments/1' ); + $request->add_header( 'Content-Type', 'application/json' ); + $request->set_body( wp_json_encode( $case['body'] ) ); + + $reflection = new ReflectionMethod( + 'Gutenberg_REST_Comment_Controller_7_1', + 'is_suggestion_lifecycle_update' + ); + if ( PHP_VERSION_ID < 80100 ) { + $reflection->setAccessible( true ); + } + + $this->assertSame( + $case['expected'], + $reflection->invoke( null, $request ), + "Lifecycle shortcut expectation mismatched for: {$label}" + ); + } + } + + /** + * Test that a field smuggled in as a QUERY parameter alongside a + * lifecycle-only body does not take the lifecycle shortcut. Core's + * `update_item` reads `$request['content']` from the merged param view + * (JSON > POST > GET > URL), so `PUT /wp/v2/comments/?content=x` + * with a lifecycle-only JSON body would rewrite the note while a + * body-only allowlist still classified it as a lifecycle update. + */ + public function test_lifecycle_update_rejects_query_param_content_rewrite() { + $request = new WP_REST_Request( 'PUT', '/wp/v2/comments/1' ); + $request->add_header( 'Content-Type', 'application/json' ); + $request->set_query_params( array( 'content' => 'rewritten via query param' ) ); + $request->set_body( + wp_json_encode( + array( + 'status' => 'approved', + 'meta' => array( + '_wp_suggestion_status' => 'applied', + ), + ) + ) + ); + + $reflection = new ReflectionMethod( + 'Gutenberg_REST_Comment_Controller_7_1', + 'is_suggestion_lifecycle_update' + ); + if ( PHP_VERSION_ID < 80100 ) { + $reflection->setAccessible( true ); + } + $this->assertFalse( + $reflection->invoke( null, $request ), + 'A content rewrite via query parameter must not take the lifecycle shortcut.' + ); + } + + /** + * Test that REST meta-parameters that ride on every editor request + * (api-fetch appends `_locale=user`) do not disqualify an otherwise + * lifecycle-only update from the shortcut. + */ + public function test_lifecycle_update_ignores_rest_meta_query_params() { + $request = new WP_REST_Request( 'PUT', '/wp/v2/comments/1' ); + $request->add_header( 'Content-Type', 'application/json' ); + $request->set_query_params( array( '_locale' => 'user' ) ); + $request->set_body( + wp_json_encode( + array( + 'status' => 'approved', + 'meta' => array( + '_wp_suggestion_status' => 'applied', + ), + ) + ) + ); + + $reflection = new ReflectionMethod( + 'Gutenberg_REST_Comment_Controller_7_1', + 'is_suggestion_lifecycle_update' + ); + if ( PHP_VERSION_ID < 80100 ) { + $reflection->setAccessible( true ); + } + $this->assertTrue( $reflection->invoke( null, $request ) ); + } + + /** + * Test that the lifecycle helper also accepts form-encoded request + * bodies, not only JSON. Custom integrations may issue updates with + * `application/x-www-form-urlencoded` and should benefit from the + * same `edit_post` shortcut as the JSON path. + */ + public function test_lifecycle_update_accepts_form_encoded_bodies() { + $request = new WP_REST_Request( 'PUT', '/wp/v2/comments/1' ); + $request->add_header( 'Content-Type', 'application/x-www-form-urlencoded' ); + $request->set_body_params( + array( + 'status' => 'approved', + 'meta' => array( + '_wp_suggestion_status' => 'applied', + ), + ) + ); + + $reflection = new ReflectionMethod( + 'Gutenberg_REST_Comment_Controller_7_1', + 'is_suggestion_lifecycle_update' + ); + if ( PHP_VERSION_ID < 80100 ) { + $reflection->setAccessible( true ); + } + $this->assertTrue( $reflection->invoke( null, $request ) ); + } } diff --git a/tools/eslint/suppressions.json b/tools/eslint/suppressions.json index f15a15e87c5276..368a340090bfe3 100644 --- a/tools/eslint/suppressions.json +++ b/tools/eslint/suppressions.json @@ -1297,6 +1297,16 @@ "count": 2 } }, + "packages/editor/src/components/suggestion-mode/auto-save.js": { + "react-hooks/refs": { + "count": 6 + } + }, + "packages/editor/src/components/suggestion-mode/test/auto-save.js": { + "react-hooks/globals": { + "count": 1 + } + }, "packages/editor/src/components/sync-connection-error-modal/index.tsx": { "@wordpress/use-recommended-components": { "count": 2 From 93e5cadd582c4ec9fa226031a034f2aeab025c0a Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Wed, 12 Aug 2026 11:29:39 -0700 Subject: [PATCH 02/23] Drop dependency-group comment blocks in suggestion mode data files Trunk switched the @wordpress/dependency-group rule to 'never' mode, so the import group headers in these files now fail lint. --- .../src/components/suggestion-mode/auto-save.js | 7 ------- .../src/components/suggestion-mode/overlay-context.js | 7 ------- .../editor/src/components/suggestion-mode/provider.js | 7 ------- .../src/components/suggestion-mode/test/auto-save.js | 11 ----------- .../suggestion-mode/test/overlay-context.js | 3 --- .../src/components/suggestion-mode/test/provider.js | 11 ----------- .../suggestion-mode/test/suggestion-write-queue.js | 3 --- 7 files changed, 49 deletions(-) diff --git a/packages/editor/src/components/suggestion-mode/auto-save.js b/packages/editor/src/components/suggestion-mode/auto-save.js index 65d3d57c4cc307..1eb805fe84d0b6 100644 --- a/packages/editor/src/components/suggestion-mode/auto-save.js +++ b/packages/editor/src/components/suggestion-mode/auto-save.js @@ -34,16 +34,9 @@ * re-render, so reading the latest entries / callbacks via refs avoids * stale-closure bugs without resubscribing on every overlay change. */ -/** - * WordPress dependencies - */ import { useRegistry, useSelect } from '@wordpress/data'; import { store as coreStore } from '@wordpress/core-data'; import { useCallback, useEffect, useRef } from '@wordpress/element'; - -/** - * Internal dependencies - */ import { useSuggestionOverlay } from './overlay-context'; import { operationsFromOverlay, useSuggestionsProvider } from './provider'; import { EDITOR_STORE_NAME, SUGGEST_INTENT } from './constants'; diff --git a/packages/editor/src/components/suggestion-mode/overlay-context.js b/packages/editor/src/components/suggestion-mode/overlay-context.js index 686f258a71a9a2..75dca7185d8fb5 100644 --- a/packages/editor/src/components/suggestion-mode/overlay-context.js +++ b/packages/editor/src/components/suggestion-mode/overlay-context.js @@ -28,9 +28,6 @@ * The orphan prune runs whenever the live block tree shrinks; it skips when * the block-editor store isn't registered (tests, standalone consumers). */ -/** - * WordPress dependencies - */ import { createContext, useCallback, @@ -41,10 +38,6 @@ import { useRef, } from '@wordpress/element'; import { useRegistry, useSelect } from '@wordpress/data'; - -/** - * Internal dependencies - */ import { createSuggestionWriteQueue } from './suggestion-write-queue'; // Referenced by name to keep the provider runnable in tests and standalone diff --git a/packages/editor/src/components/suggestion-mode/provider.js b/packages/editor/src/components/suggestion-mode/provider.js index 8b8d41db95f54f..4eeb494c26283d 100644 --- a/packages/editor/src/components/suggestion-mode/provider.js +++ b/packages/editor/src/components/suggestion-mode/provider.js @@ -1,6 +1,3 @@ -/** - * WordPress dependencies - */ import { useCallback, useMemo } from '@wordpress/element'; import { useDispatch, useRegistry, useSelect } from '@wordpress/data'; import { store as coreStore } from '@wordpress/core-data'; @@ -8,10 +5,6 @@ import { store as blockEditorStore } from '@wordpress/block-editor'; import { store as interfaceStore } from '@wordpress/interface'; import { store as noticesStore } from '@wordpress/notices'; import { __ } from '@wordpress/i18n'; - -/** - * Internal dependencies - */ import { EDITOR_STORE_NAME } from './constants'; import { useSuggestionOverlay } from './overlay-context'; import { diff --git a/packages/editor/src/components/suggestion-mode/test/auto-save.js b/packages/editor/src/components/suggestion-mode/test/auto-save.js index 03334347885013..04168eff237dc7 100644 --- a/packages/editor/src/components/suggestion-mode/test/auto-save.js +++ b/packages/editor/src/components/suggestion-mode/test/auto-save.js @@ -1,18 +1,7 @@ -/** - * External dependencies - */ import { render, act } from '@testing-library/react'; - -/** - * WordPress dependencies - */ import { createRegistry, RegistryProvider } from '@wordpress/data'; import { store as coreStore } from '@wordpress/core-data'; import { store as noticesStore } from '@wordpress/notices'; - -/** - * Internal dependencies - */ import SuggestionAutoSave, { operationsForEntry } from '../auto-save'; import { SuggestionOverlayProvider, diff --git a/packages/editor/src/components/suggestion-mode/test/overlay-context.js b/packages/editor/src/components/suggestion-mode/test/overlay-context.js index 3819f3e1f9e3e6..e267c8c243cc48 100644 --- a/packages/editor/src/components/suggestion-mode/test/overlay-context.js +++ b/packages/editor/src/components/suggestion-mode/test/overlay-context.js @@ -1,6 +1,3 @@ -/** - * Internal dependencies - */ import { overlayReducer } from '../overlay-context'; describe( 'overlayReducer', () => { diff --git a/packages/editor/src/components/suggestion-mode/test/provider.js b/packages/editor/src/components/suggestion-mode/test/provider.js index e389f2993bcf17..d55b7769447b8e 100644 --- a/packages/editor/src/components/suggestion-mode/test/provider.js +++ b/packages/editor/src/components/suggestion-mode/test/provider.js @@ -1,11 +1,4 @@ -/** - * External dependencies - */ import { render, act } from '@testing-library/react'; - -/** - * WordPress dependencies - */ import { createRegistry, createReduxStore, @@ -19,10 +12,6 @@ import { unregisterBlockType, getBlockTypes, } from '@wordpress/blocks'; - -/** - * Internal dependencies - */ import { operationsFromOverlay, applyOperations, diff --git a/packages/editor/src/components/suggestion-mode/test/suggestion-write-queue.js b/packages/editor/src/components/suggestion-mode/test/suggestion-write-queue.js index 0b89ecd9edb318..e2ce30370db2a1 100644 --- a/packages/editor/src/components/suggestion-mode/test/suggestion-write-queue.js +++ b/packages/editor/src/components/suggestion-mode/test/suggestion-write-queue.js @@ -1,6 +1,3 @@ -/** - * Internal dependencies - */ import { createSuggestionWriteQueue } from '../suggestion-write-queue'; /** Create a promise whose resolution the test controls. */ From 568e8d60f754be211d042488a614a1d645c4cf7b Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Fri, 21 Aug 2026 17:32:25 -0700 Subject: [PATCH 03/23] Suggest mode: cancel pending auto-saves when leaving Suggest intent The auto-save scheduler never cleared its debounce timers when the intent changed. Because the component is mounted on the experiment flag rather than the intent, switching from Suggest to Edit or View left any armed timer running, and it went on to POST a note for edits the user had just walked away from - visible to collaborators as a deliberate suggestion. Cancelling is not lossy: the overlay entry keeps its unsynced fingerprint, so returning to Suggest reschedules the save. --- .../components/suggestion-mode/auto-save.js | 17 ++++++- .../suggestion-mode/test/auto-save.js | 50 +++++++++++++++++++ 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/packages/editor/src/components/suggestion-mode/auto-save.js b/packages/editor/src/components/suggestion-mode/auto-save.js index 1eb805fe84d0b6..c7362569e2b550 100644 --- a/packages/editor/src/components/suggestion-mode/auto-save.js +++ b/packages/editor/src/components/suggestion-mode/auto-save.js @@ -242,12 +242,25 @@ export default function SuggestionAutoSave() { ); useEffect( () => { + const timers = timersRef.current; + + /* + * Leaving Suggest mode cancels every pending debounce. The component + * stays mounted across intent changes (it is gated on the experiment + * flag, not the intent), so without this a timer scheduled moments + * before the switch still fires and POSTs a note for an edit the + * user walked away from. Cancelling is not lossy: the overlay entry + * keeps its unsynced fingerprint, so re-entering Suggest re-runs this + * effect and reschedules the save. + */ if ( ! isSuggestMode ) { + for ( const timer of timers.values() ) { + clearTimeout( timer ); + } + timers.clear(); return undefined; } - const timers = timersRef.current; - for ( const [ clientId, entry ] of Object.entries( entries ) ) { const operations = operationsForEntry( entry ); const fingerprint = fingerprintOperations( operations ); diff --git a/packages/editor/src/components/suggestion-mode/test/auto-save.js b/packages/editor/src/components/suggestion-mode/test/auto-save.js index 04168eff237dc7..fc0ec8d210775f 100644 --- a/packages/editor/src/components/suggestion-mode/test/auto-save.js +++ b/packages/editor/src/components/suggestion-mode/test/auto-save.js @@ -431,6 +431,56 @@ describe( 'SuggestionAutoSave', () => { expect( createSuggestion ).not.toHaveBeenCalled(); } ); + + it( 'drops a pending save when the user leaves Suggest intent, and resumes it on return', async () => { + createSuggestion.mockResolvedValue( { id: 42 } ); + + const { registry } = renderInSuggestMode( + <> + + + + ); + + act( () => { + overlayHandle.captureBaseline( 'a', 'core/paragraph', { + content: 'Hi', + } ); + overlayHandle.setOverlayAttributes( 'a', { content: 'Hello' } ); + } ); + + // Leave Suggest mode mid-debounce. + await act( async () => { + jest.advanceTimersByTime( 500 ); + } ); + act( () => { + unlock( registry.dispatch( editorStore ) ).setEditorIntent( + 'edit' + ); + } ); + + await act( async () => { + jest.advanceTimersByTime( 5000 ); + } ); + await flushPromises(); + + expect( createSuggestion ).not.toHaveBeenCalled(); + + // Returning to Suggest reschedules the still-unsynced entry. + act( () => { + unlock( registry.dispatch( editorStore ) ).setEditorIntent( + 'suggest' + ); + } ); + + await act( async () => { + jest.advanceTimersByTime( 1500 ); + } ); + await flushPromises(); + await flushPromises(); + + expect( createSuggestion ).toHaveBeenCalledTimes( 1 ); + } ); } ); describe( 'operationsForEntry', () => { From ff96c0f4ff076daf189a6ac228b6754246bfe7a0 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Fri, 21 Aug 2026 17:32:35 -0700 Subject: [PATCH 04/23] Suggest mode: persist a structural decision before mutating the block tree Applying or rejecting a block-remove, block-insert-after, or block-move suggestion dispatched the tree mutation first and only surfaced an error notice if the save failed. Nothing rolled the tree back, so a failed save left the editor diverged from the server: the block was gone (or moved) while the note still read as pending, and for block-remove the marker went with the block, leaving no way to act on the note again. The attribute-set path solves this by snapshotting attributes and restoring them. A structural rollback cannot be that faithful - position, nested children, selection, and the overlay entry would all need restoring - so await the lifecycle write first instead. The mutation only runs once the decision is persisted, which is the same invariant for less machinery. --- .../components/suggestion-mode/provider.js | 148 +++++++++-------- .../suggestion-mode/test/provider.js | 153 ++++++++++++++++++ 2 files changed, 237 insertions(+), 64 deletions(-) diff --git a/packages/editor/src/components/suggestion-mode/provider.js b/packages/editor/src/components/suggestion-mode/provider.js index 4eeb494c26283d..a7b288e68a0ec8 100644 --- a/packages/editor/src/components/suggestion-mode/provider.js +++ b/packages/editor/src/components/suggestion-mode/provider.js @@ -717,6 +717,28 @@ export function useSuggestionsProvider() { const structuralOp = findStructuralOp( payload.operations ); if ( structuralOp ) { try { + /* + * Persist the decision BEFORE touching the tree. The + * attribute-set path below can mutate first and roll + * back on failure because restoring attributes is + * exact; a structural rollback is not — re-inserting a + * removed block would have to restore its position, + * nested children, selection, and overlay entry. Saving + * first costs one round-trip of latency and gives the + * same invariant for free: a failed save leaves the + * editor exactly as it was. + */ + await saveEntityRecord( + 'root', + 'comment', + { + id: commentId, + status: 'approved', + meta: { _wp_suggestion_status: 'applied' }, + }, + { throwOnError: true } + ); + if ( structuralOp.type === 'block-remove' ) { // Bypass twice: the marker-clear dispatch lands // first (so the live block ends without the @@ -772,17 +794,6 @@ export function useSuggestionsProvider() { clearOverlay( targetClientId ); } - await saveEntityRecord( - 'root', - 'comment', - { - id: commentId, - status: 'approved', - meta: { _wp_suggestion_status: 'applied' }, - }, - { throwOnError: true } - ); - createNotice( 'snackbar', __( 'Suggestion applied.' ), { type: 'snackbar', isDismissible: true, @@ -909,59 +920,6 @@ export function useSuggestionsProvider() { // pre-move parent + index. // - attribute-set (no structural op): no live-block change. const structuralOp = findStructuralOp( payload?.operations ); - if ( structuralOp && clientId ) { - if ( structuralOp.type === 'block-insert-after' ) { - requestInterceptorBypass( clientId ); - clearOverlay( clientId ); - removeBlock( clientId ); - } else if ( structuralOp.type === 'block-move' ) { - const clearAttrs = clearSuggestionMarkerAttributes( - selectBlockAttributes( clientId ) - ); - requestInterceptorBypass( clientId ); - clearOverlay( clientId ); - /* - * Batch the marker-clear and the restoring move into ONE - * store update. The interceptor recognizes a reject - * landing by their combination — a block that moved in - * the same tick its pending-move marker disappeared — - * and adopts it instead of re-capturing the restore as - * a fresh move suggestion (which is what happens when - * the two dispatches fire the subscriber separately and - * the reviewer is in Suggesting intent). This is also - * the shape a remote reject arrives in through sync. - */ - registry.batch( () => { - if ( clearAttrs ) { - updateBlockAttributes( clientId, clearAttrs ); - } - moveBlockToPosition( - clientId, - /* - * `fromRootClientId` must be the block's CURRENT - * parent: after a cross-parent move the block lives - * in the destination parent, and the reducer looks - * the block up there. Passing the original parent - * for both roots made cross-parent rejects silently - * no-op. `moveBlockToPosition` expects '' (not null) - * for the root. - */ - selectBlockRootClientId( clientId ) ?? '', - structuralOp.fromParentClientId ?? '', - structuralOp.fromIndex ?? 0 - ); - } ); - } else { - const clearAttrs = clearSuggestionMarkerAttributes( - selectBlockAttributes( clientId ) - ); - if ( clearAttrs ) { - requestInterceptorBypass( clientId ); - updateBlockAttributes( clientId, clearAttrs ); - } - clearOverlay( clientId ); - } - } try { await saveEntityRecord( @@ -975,6 +933,68 @@ export function useSuggestionsProvider() { { throwOnError: true } ); + /* + * Undo the live-block change only once the decision is + * persisted. A structural change can't be rolled back + * faithfully (position, children, selection, and the + * overlay entry would all have to be restored), so the + * tree is left untouched until the save succeeds — a + * failed reject then leaves the editor exactly as it was. + */ + if ( structuralOp && clientId ) { + if ( structuralOp.type === 'block-insert-after' ) { + requestInterceptorBypass( clientId ); + clearOverlay( clientId ); + removeBlock( clientId ); + } else if ( structuralOp.type === 'block-move' ) { + const clearAttrs = clearSuggestionMarkerAttributes( + selectBlockAttributes( clientId ) + ); + requestInterceptorBypass( clientId ); + clearOverlay( clientId ); + /* + * Batch the marker-clear and the restoring move into ONE + * store update. The interceptor recognizes a reject + * landing by their combination — a block that moved in + * the same tick its pending-move marker disappeared — + * and adopts it instead of re-capturing the restore as + * a fresh move suggestion (which is what happens when + * the two dispatches fire the subscriber separately and + * the reviewer is in Suggesting intent). This is also + * the shape a remote reject arrives in through sync. + */ + registry.batch( () => { + if ( clearAttrs ) { + updateBlockAttributes( clientId, clearAttrs ); + } + moveBlockToPosition( + clientId, + /* + * `fromRootClientId` must be the block's CURRENT + * parent: after a cross-parent move the block lives + * in the destination parent, and the reducer looks + * the block up there. Passing the original parent + * for both roots made cross-parent rejects silently + * no-op. `moveBlockToPosition` expects '' (not null) + * for the root. + */ + selectBlockRootClientId( clientId ) ?? '', + structuralOp.fromParentClientId ?? '', + structuralOp.fromIndex ?? 0 + ); + } ); + } else { + const clearAttrs = clearSuggestionMarkerAttributes( + selectBlockAttributes( clientId ) + ); + if ( clearAttrs ) { + requestInterceptorBypass( clientId ); + updateBlockAttributes( clientId, clearAttrs ); + } + clearOverlay( clientId ); + } + } + createNotice( 'snackbar', __( 'Suggestion rejected.' ), { type: 'snackbar', isDismissible: true, diff --git a/packages/editor/src/components/suggestion-mode/test/provider.js b/packages/editor/src/components/suggestion-mode/test/provider.js index d55b7769447b8e..ee52c6f0b123fa 100644 --- a/packages/editor/src/components/suggestion-mode/test/provider.js +++ b/packages/editor/src/components/suggestion-mode/test/provider.js @@ -669,6 +669,159 @@ describe( 'rejectSuggestion (block-move)', () => { } ); } ); +describe( 'decision failures leave the block tree untouched', () => { + const PARAGRAPH = 'core/test-failure-paragraph'; + + beforeAll( () => { + registerBlockType( PARAGRAPH, { + apiVersion: 3, + attributes: { + content: { type: 'string', default: '' }, + metadata: { type: 'object' }, + }, + save: () => null, + category: 'text', + title: 'Test Failure Paragraph', + } ); + } ); + + afterAll( () => { + getBlockTypes().forEach( ( block ) => + unregisterBlockType( block.name ) + ); + } ); + + // Same stub as the block-move suite, except the lifecycle write rejects + // the way a dropped connection or a concurrently trashed note would. + function createFailingCoreStore() { + return createReduxStore( 'core', { + reducer: ( state = {} ) => state, + actions: { + saveEntityRecord: () => () => { + throw new Error( 'Network error' ); + }, + }, + selectors: { + getEditedEntityRecord: () => null, + getEntityRecord: () => null, + getCurrentUser: () => null, + }, + } ); + } + + function setup( initialBlocks ) { + const registry = createRegistry(); + registry.register( noticesStore ); + registry.register( blockEditorStore ); + registry.register( createFailingCoreStore() ); + registry.register( createStubInterfaceStore() ); + registry.dispatch( blockEditorStore ).resetBlocks( initialBlocks ); + + let providerHandle; + function CaptureProvider() { + providerHandle = useSuggestionsProvider(); + return null; + } + + render( + + + + ); + + return { registry, getProvider: () => providerHandle }; + } + + it( 'keeps the block when applying a block-remove suggestion fails', async () => { + const target = createBlock( PARAGRAPH, { + content: 'Doomed', + metadata: { + noteId: [ 7 ], + suggestion: { type: 'pending-remove' }, + }, + } ); + const { registry, getProvider } = setup( [ target ] ); + + await act( async () => { + await getProvider().applySuggestion( { + commentId: 7, + clientId: target.clientId, + payload: { + schemaVersion: 2, + blockName: PARAGRAPH, + baseRevision: null, + operations: [ + { + type: 'block-remove', + clientId: target.clientId, + blockName: PARAGRAPH, + }, + ], + }, + } ); + } ); + + const blockEditor = registry.select( blockEditorStore ); + // The block survives, marker intact, so the still-pending note can + // be applied or rejected again once the server is reachable. + expect( blockEditor.getBlock( target.clientId ) ).not.toBeNull(); + expect( + blockEditor.getBlockAttributes( target.clientId )?.metadata + ?.suggestion?.type + ).toBe( 'pending-remove' ); + expect( + registry + .select( noticesStore ) + .getNotices() + .some( ( notice ) => notice.status === 'error' ) + ).toBe( true ); + } ); + + it( 'leaves a moved block in place when rejecting fails', async () => { + const a = createBlock( PARAGRAPH, { content: 'A' } ); + const moved = createBlock( PARAGRAPH, { + content: 'Moved', + metadata: { + noteId: [ 8 ], + suggestion: { type: 'pending-move' }, + }, + } ); + // Current order is [ A, Moved ]; the suggestion moved it from index 0. + const { registry, getProvider } = setup( [ a, moved ] ); + + await act( async () => { + await getProvider().rejectSuggestion( { + commentId: 8, + clientId: moved.clientId, + payload: { + schemaVersion: 2, + blockName: PARAGRAPH, + baseRevision: null, + operations: [ + { + type: 'block-move', + clientId: moved.clientId, + blockName: PARAGRAPH, + fromParentClientId: null, + fromIndex: 0, + toParentClientId: null, + }, + ], + }, + } ); + } ); + + const blockEditor = registry.select( blockEditorStore ); + // No half-rejected state: the block stays at its suggested position + // with the marker still on it. + expect( blockEditor.getBlockIndex( moved.clientId ) ).toBe( 1 ); + expect( + blockEditor.getBlockAttributes( moved.clientId )?.metadata + ?.suggestion?.type + ).toBe( 'pending-move' ); + } ); +} ); + describe( 'createSuggestion (notes sidebar switch)', () => { const PARAGRAPH = 'core/test-sidebar-paragraph'; const ALL_NOTES_SIDEBAR = 'edit-post/collab-history-sidebar'; From 7b5164631bd84f866e61733d3d13006004c9b87e Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Sun, 23 Aug 2026 07:45:49 -0700 Subject: [PATCH 05/23] Convert suggestion-mode data layer to TypeScript Rename the files this PR adds from .js to .ts/.tsx and add types so the package's strict type check covers them, per the repo convention that new files are authored in TypeScript. No behavior changes: the overlay entry, context value, reducer action, and suggestion payload shapes are now explicit interfaces, and eslint suppression paths follow the renames. --- .../{auto-save.js => auto-save.ts} | 46 ++- .../suggestion-mode/{index.js => index.ts} | 0 ...overlay-context.js => overlay-context.tsx} | 277 ++++++++++----- .../{provider.js => provider.ts} | 321 ++++++++++-------- ...ite-queue.js => suggestion-write-queue.ts} | 25 +- .../test/{auto-save.js => auto-save.tsx} | 18 +- ...{overlay-context.js => overlay-context.ts} | 8 +- .../test/{provider.js => provider.tsx} | 33 +- ...ite-queue.js => suggestion-write-queue.ts} | 10 +- tools/eslint/suppressions.json | 4 +- 10 files changed, 469 insertions(+), 273 deletions(-) rename packages/editor/src/components/suggestion-mode/{auto-save.js => auto-save.ts} (91%) rename packages/editor/src/components/suggestion-mode/{index.js => index.ts} (100%) rename packages/editor/src/components/suggestion-mode/{overlay-context.js => overlay-context.tsx} (71%) rename packages/editor/src/components/suggestion-mode/{provider.js => provider.ts} (76%) rename packages/editor/src/components/suggestion-mode/{suggestion-write-queue.js => suggestion-write-queue.ts} (72%) rename packages/editor/src/components/suggestion-mode/test/{auto-save.js => auto-save.tsx} (96%) rename packages/editor/src/components/suggestion-mode/test/{overlay-context.js => overlay-context.ts} (97%) rename packages/editor/src/components/suggestion-mode/test/{provider.js => provider.tsx} (96%) rename packages/editor/src/components/suggestion-mode/test/{suggestion-write-queue.js => suggestion-write-queue.ts} (93%) diff --git a/packages/editor/src/components/suggestion-mode/auto-save.js b/packages/editor/src/components/suggestion-mode/auto-save.ts similarity index 91% rename from packages/editor/src/components/suggestion-mode/auto-save.js rename to packages/editor/src/components/suggestion-mode/auto-save.ts index c7362569e2b550..60d92b20eaf692 100644 --- a/packages/editor/src/components/suggestion-mode/auto-save.js +++ b/packages/editor/src/components/suggestion-mode/auto-save.ts @@ -38,6 +38,7 @@ import { useRegistry, useSelect } from '@wordpress/data'; import { store as coreStore } from '@wordpress/core-data'; import { useCallback, useEffect, useRef } from '@wordpress/element'; import { useSuggestionOverlay } from './overlay-context'; +import type { OverlayEntry, SuggestionOperation } from './overlay-context'; import { operationsFromOverlay, useSuggestionsProvider } from './provider'; import { EDITOR_STORE_NAME, SUGGEST_INTENT } from './constants'; import { unlock } from '../../lock-unlock'; @@ -49,10 +50,12 @@ const AUTOSAVE_DEBOUNCE_MS = 1500; * the overlay has changed relative to what we last synced without comparing * deep object trees on every render. * - * @param {Array} operations Operations to fingerprint. - * @return {string} Stable serialization. + * @param operations Operations to fingerprint. + * @return Stable serialization. */ -export function fingerprintOperations( operations ) { +export function fingerprintOperations( + operations: SuggestionOperation[] +): string { try { return JSON.stringify( operations ); } catch { @@ -71,11 +74,13 @@ export function fingerprintOperations( operations ) { * removal — in which case the structural op leads and any attribute ops * follow. * - * @param {Object} entry Overlay entry. - * @return {Array} Ops describing the entry's pending suggestion. + * @param entry Overlay entry. + * @return Ops describing the entry's pending suggestion. */ -export function operationsForEntry( entry ) { - const ops = []; +export function operationsForEntry( + entry: OverlayEntry +): SuggestionOperation[] { + const ops: SuggestionOperation[] = []; if ( entry.structuralOp ) { ops.push( entry.structuralOp ); } @@ -96,7 +101,7 @@ export function operationsForEntry( entry ) { * idle window, and subsequent edits update the same note rather than * spawning a new one. * - * @return {null} Renders nothing. + * @return Renders nothing. */ export default function SuggestionAutoSave() { const { entries, setCommentId, setSyncedOpsKey } = useSuggestionOverlay(); @@ -134,12 +139,14 @@ export default function SuggestionAutoSave() { setSyncedOpsKeyRef.current = setSyncedOpsKey; // Per-clientId debounce timer. - const timersRef = useRef( new Map() ); + const timersRef = useRef( + new Map< string, ReturnType< typeof setTimeout > >() + ); // Per-clientId promise chain. New saves are enqueued onto the existing // chain so saves on the same block always run sequentially — no races, // no duplicate POSTs, and no dropped work when the user keeps typing // during a slow network call. - const queuesRef = useRef( new Map() ); + const queuesRef = useRef( new Map< string, Promise< void > >() ); // Synchronous mirror of each block's last-known comment id. `setCommentId` // updates React state, which only reaches `entriesRef` on the next render // commit; a save queued immediately after a `create` resolves would run @@ -148,14 +155,17 @@ export default function SuggestionAutoSave() { // every rotation/clear), so the queued save sees the fresh id without waiting // for React. A `null` value is a deliberate "known to have no note" marker, // distinct from "no entry yet" (fall back to `entry.commentId`). - const commentIdsRef = useRef( new Map() ); - const writeCommentId = useCallback( ( clientId, id ) => { - commentIdsRef.current.set( clientId, id ); - setCommentIdRef.current( clientId, id ); - }, [] ); + const commentIdsRef = useRef( new Map< string, number | null >() ); + const writeCommentId = useCallback( + ( clientId: string, id: number | null ) => { + commentIdsRef.current.set( clientId, id ); + setCommentIdRef.current( clientId, id ); + }, + [] + ); const syncOnce = useCallback( - async ( clientId ) => { + async ( clientId: string ) => { const entry = entriesRef.current[ clientId ]; if ( ! entry ) { return; @@ -183,7 +193,7 @@ export default function SuggestionAutoSave() { ? commentIdsRef.current.get( clientId ) : entry.commentId; if ( commentId ) { - const linkedComment = registry + const linkedComment: any = registry .select( coreStore ) .getEntityRecord( 'root', 'comment', commentId ); if ( linkedComment && linkedComment.status !== 'hold' ) { @@ -225,7 +235,7 @@ export default function SuggestionAutoSave() { ); const enqueueSync = useCallback( - ( clientId ) => { + ( clientId: string ) => { const queues = queuesRef.current; const previous = queues.get( clientId ) ?? Promise.resolve(); const next = previous diff --git a/packages/editor/src/components/suggestion-mode/index.js b/packages/editor/src/components/suggestion-mode/index.ts similarity index 100% rename from packages/editor/src/components/suggestion-mode/index.js rename to packages/editor/src/components/suggestion-mode/index.ts diff --git a/packages/editor/src/components/suggestion-mode/overlay-context.js b/packages/editor/src/components/suggestion-mode/overlay-context.tsx similarity index 71% rename from packages/editor/src/components/suggestion-mode/overlay-context.js rename to packages/editor/src/components/suggestion-mode/overlay-context.tsx index 75dca7185d8fb5..6a63b282e5cacb 100644 --- a/packages/editor/src/components/suggestion-mode/overlay-context.js +++ b/packages/editor/src/components/suggestion-mode/overlay-context.tsx @@ -28,6 +28,7 @@ * The orphan prune runs whenever the live block tree shrinks; it skips when * the block-editor store isn't registered (tests, standalone consumers). */ +import type { ReactNode } from 'react'; import { createContext, useCallback, @@ -39,6 +40,7 @@ import { } from '@wordpress/element'; import { useRegistry, useSelect } from '@wordpress/data'; import { createSuggestionWriteQueue } from './suggestion-write-queue'; +import type { SuggestionWriteQueue } from './suggestion-write-queue'; // Referenced by name to keep the provider runnable in tests and standalone // contexts where the block-editor store isn't registered. Orphan cleanup is @@ -65,31 +67,125 @@ const nextCaptureSeq = () => ++captureSequence; const UNDO_ADOPTION_TTL_MS = 1000; /** - * @typedef {Object} OverlayEntry - * @property {string} blockName The block name at the time the - * overlay was opened. - * @property {Object} baselineAttributes The attributes captured when - * Suggest mode first began editing - * this block. - * @property {Object} overlayAttributes Pending attribute changes that - * have not yet been committed. + * A persisted suggestion operation. Attribute ops are derived from the + * baseline-vs-overlay diff; structural ops (block-remove, block-insert-after, + * block-move) are pre-built by the store interceptor. The concrete shape + * varies per op type, so the contract stays open beyond `type`. */ +export interface SuggestionOperation { + type: string; + [ key: string ]: any; +} + +export interface OverlayEntry { + /** The block name at the time the overlay was opened. */ + blockName: string; + /** + * The attributes captured when Suggest mode first began editing this + * block. + */ + baselineAttributes: Record< string, any >; + /** Pending attribute changes that have not yet been committed. */ + overlayAttributes: Record< string, any >; + /** Note comment id the entry is synced to, when one exists. */ + commentId: number | null; + /** Fingerprint of the operations last persisted for this entry. */ + syncedOpsKey: string | null; + /** Capture-order stamp of the most recent attribute edit. */ + lastEditSeq?: number; + /** Pending structural operation, when one was captured. */ + structuralOp?: SuggestionOperation; + /** Capture-order stamp of the structural operation. */ + structuralOpSeq?: number; +} + +export type OverlayEntries = Record< string, OverlayEntry >; /** - * @typedef {Object} OverlayContextValue - * @property {Object.} entries Per-clientId entries. - * @property {Function} captureBaseline Store a baseline for a - * block if one isn't set. - * @property {Function} setOverlayAttributes Merge overlay attributes - * onto an entry. - * @property {Function} clearOverlay Remove the entry. - * @property {Function} hasOverlay Check if an entry has any - * overlay attributes. + * Handler invoked with a format/content suggestion request. Returning + * `false` means the handler cannot process the request and the edit must + * fall through to the overlay path; anything else counts as accepted. */ +type SuggestionRequestHandler = ( request: any ) => unknown; + +export interface OverlayContextValue { + entries: OverlayEntries; + captureBaseline: ( + clientId: string, + blockName: string, + attributes: Record< string, any > + ) => void; + setOverlayAttributes: ( + clientId: string, + attributes: Record< string, any > + ) => void; + clearOverlay: ( clientId: string ) => void; + setCommentId: ( clientId: string, commentId: number | null ) => void; + setSyncedOpsKey: ( clientId: string, syncedOpsKey: string | null ) => void; + setStructuralOp: ( + clientId: string, + blockName: string, + op: SuggestionOperation + ) => void; + hasOverlay: ( clientId: string ) => boolean; + requestInterceptorBypass: ( clientId: string ) => void; + consumeInterceptorBypass: ( clientId: string ) => boolean; + registerFormatHandler: ( handler: SuggestionRequestHandler ) => () => void; + requestFormatSuggestion: ( request: any ) => boolean; + registerContentHandler: ( handler: SuggestionRequestHandler ) => () => void; + requestContentSuggestion: ( request: any ) => boolean; + enqueueSuggestionWrite: ( + clientId: string, + task: () => unknown + ) => Promise< unknown >; + markDeferredInsertion: ( clientId: string ) => void; + unmarkDeferredInsertion: ( clientId: string ) => void; + isDeferredInsertion: ( clientId: string ) => boolean; + clearDeferredInsertions: () => void; + getLastContentCaptureSeq: () => number; + armUndoRedoAdoption: () => void; + consumeUndoRedoAdoption: () => boolean; +} -const EMPTY_ENTRIES = Object.freeze( {} ); - -const OverlayContext = createContext( { +type OverlayAction = + | { + type: 'CAPTURE_BASELINE'; + clientId: string; + blockName: string; + attributes: Record< string, any >; + } + | { + type: 'SET_OVERLAY_ATTRIBUTES'; + clientId: string; + attributes: Record< string, any >; + seq?: number; + } + | { type: 'CLEAR_OVERLAY'; clientId: string } + | { + type: 'SET_COMMENT_ID'; + clientId: string; + commentId: number | null; + } + | { + type: 'SET_SYNCED_OPS_KEY'; + clientId: string; + syncedOpsKey: string | null; + } + | { + type: 'SET_STRUCTURAL_OP'; + clientId: string; + blockName: string; + op: SuggestionOperation; + seq?: number; + } + | { + type: 'PRUNE_ORPHANS'; + liveClientIds: string[] | Set< string >; + }; + +const EMPTY_ENTRIES: OverlayEntries = Object.freeze( {} ); + +const OverlayContext = createContext< OverlayContextValue >( { entries: EMPTY_ENTRIES, captureBaseline: () => {}, setOverlayAttributes: () => {}, @@ -105,7 +201,7 @@ const OverlayContext = createContext( { registerContentHandler: () => () => {}, requestContentSuggestion: () => false, // Standalone default (no provider mounted): run the task immediately. - enqueueSuggestionWrite: ( clientId, task ) => task(), + enqueueSuggestionWrite: ( _clientId, task ) => Promise.resolve( task() ), markDeferredInsertion: () => {}, unmarkDeferredInsertion: () => {}, isDeferredInsertion: () => false, @@ -118,11 +214,14 @@ const OverlayContext = createContext( { /** * Reducer managing the map of pending block overlays. * - * @param {Object} state Current state. - * @param {Object} action Action. - * @return {Object} Next state. + * @param state Current state. + * @param action Action. + * @return Next state. */ -export function overlayReducer( state, action ) { +export function overlayReducer( + state: OverlayEntries, + action: OverlayAction +): OverlayEntries { switch ( action.type ) { case 'CAPTURE_BASELINE': { if ( state[ action.clientId ] ) { @@ -225,7 +324,7 @@ export function overlayReducer( state, action ) { : action.liveClientIds; const keys = Object.keys( state ); let changed = false; - const next = {}; + const next: OverlayEntries = {}; for ( const key of keys ) { if ( liveIds.has( key ) ) { next[ key ] = state[ key ]; @@ -247,14 +346,21 @@ export function overlayReducer( state, action ) { * changes per `clientId` so a block can render the user's in-progress * suggestion without mutating the real block-editor state. * - * @param {{ children: React.ReactNode }} props */ -export function SuggestionOverlayProvider( { children } ) { +export function SuggestionOverlayProvider( { + children, +}: { + children: ReactNode; +} ) { const [ entries, dispatch ] = useReducer( overlayReducer, EMPTY_ENTRIES ); const registry = useRegistry(); const captureBaseline = useCallback( - ( clientId, blockName, attributes ) => + ( + clientId: string, + blockName: string, + attributes: Record< string, any > + ) => dispatch( { type: 'CAPTURE_BASELINE', clientId, @@ -265,7 +371,7 @@ export function SuggestionOverlayProvider( { children } ) { ); const setOverlayAttributes = useCallback( - ( clientId, attributes ) => + ( clientId: string, attributes: Record< string, any > ) => dispatch( { type: 'SET_OVERLAY_ATTRIBUTES', clientId, @@ -276,18 +382,18 @@ export function SuggestionOverlayProvider( { children } ) { ); const clearOverlay = useCallback( - ( clientId ) => dispatch( { type: 'CLEAR_OVERLAY', clientId } ), + ( clientId: string ) => dispatch( { type: 'CLEAR_OVERLAY', clientId } ), [] ); const setCommentId = useCallback( - ( clientId, commentId ) => + ( clientId: string, commentId: number | null ) => dispatch( { type: 'SET_COMMENT_ID', clientId, commentId } ), [] ); const setSyncedOpsKey = useCallback( - ( clientId, syncedOpsKey ) => + ( clientId: string, syncedOpsKey: string | null ) => dispatch( { type: 'SET_SYNCED_OPS_KEY', clientId, syncedOpsKey } ), [] ); @@ -307,20 +413,23 @@ export function SuggestionOverlayProvider( { children } ) { [] ); - const setStructuralOp = useCallback( ( clientId, blockName, op ) => { - dispatch( { - type: 'SET_STRUCTURAL_OP', - clientId, - blockName, - op, - seq: nextCaptureSeq(), - } ); - }, [] ); + const setStructuralOp = useCallback( + ( clientId: string, blockName: string, op: SuggestionOperation ) => { + dispatch( { + type: 'SET_STRUCTURAL_OP', + clientId, + blockName, + op, + seq: nextCaptureSeq(), + } ); + }, + [] + ); const hasEntries = Object.keys( entries ).length > 0; const hasOverlay = useCallback( - ( clientId ) => { + ( clientId: string ) => { const entry = entries[ clientId ]; return ( !! entry && Object.keys( entry.overlayAttributes ).length > 0 @@ -336,9 +445,9 @@ export function SuggestionOverlayProvider( { children } ) { // A ref-set rather than reducer state because the value is consumed // inside `registry.subscribe` (which doesn't react to React state) and // must clear synchronously when the dispatch is processed. - const bypassClientIdsRef = useRef( new Set() ); + const bypassClientIdsRef = useRef( new Set< string >() ); - const requestInterceptorBypass = useCallback( ( clientId ) => { + const requestInterceptorBypass = useCallback( ( clientId: string ) => { if ( clientId ) { bypassClientIdsRef.current.add( clientId ); /* @@ -353,7 +462,7 @@ export function SuggestionOverlayProvider( { children } ) { } }, [] ); - const consumeInterceptorBypass = useCallback( ( clientId ) => { + const consumeInterceptorBypass = useCallback( ( clientId: string ) => { const set = bypassClientIdsRef.current; if ( ! set.has( clientId ) ) { return false; @@ -369,18 +478,21 @@ export function SuggestionOverlayProvider( { children } ) { // heavy `useSuggestionsProvider` out of every block's render is why this is // a singleton rather than a per-block hook. A ref (not state) so // registering doesn't re-render every subscribed block. - const formatHandlerRef = useRef( null ); - - const registerFormatHandler = useCallback( ( handler ) => { - formatHandlerRef.current = handler; - return () => { - if ( formatHandlerRef.current === handler ) { - formatHandlerRef.current = null; - } - }; - }, [] ); + const formatHandlerRef = useRef< SuggestionRequestHandler | null >( null ); + + const registerFormatHandler = useCallback( + ( handler: SuggestionRequestHandler ) => { + formatHandlerRef.current = handler; + return () => { + if ( formatHandlerRef.current === handler ) { + formatHandlerRef.current = null; + } + }; + }, + [] + ); - const requestFormatSuggestion = useCallback( ( request ) => { + const requestFormatSuggestion = useCallback( ( request: any ) => { const handler = formatHandlerRef.current; if ( ! handler ) { return false; @@ -400,18 +512,21 @@ export function SuggestionOverlayProvider( { children } ) { // composition, autocorrect, a drag-drop, a multi-line paste). The per-block // HOC runs the cheap diff and hands a ready marker plan here; this single // mounted component owns note creation and the marker write. - const contentHandlerRef = useRef( null ); - - const registerContentHandler = useCallback( ( handler ) => { - contentHandlerRef.current = handler; - return () => { - if ( contentHandlerRef.current === handler ) { - contentHandlerRef.current = null; - } - }; - }, [] ); + const contentHandlerRef = useRef< SuggestionRequestHandler | null >( null ); + + const registerContentHandler = useCallback( + ( handler: SuggestionRequestHandler ) => { + contentHandlerRef.current = handler; + return () => { + if ( contentHandlerRef.current === handler ) { + contentHandlerRef.current = null; + } + }; + }, + [] + ); - const requestContentSuggestion = useCallback( ( request ) => { + const requestContentSuggestion = useCallback( ( request: any ) => { const handler = contentHandlerRef.current; if ( ! handler ) { return false; @@ -427,12 +542,13 @@ export function SuggestionOverlayProvider( { children } ) { * previously let one of each race on the same block). A ref because the * queue is imperative state consumed outside React's render cycle. */ - const writeQueueRef = useRef( null ); + const writeQueueRef = useRef< SuggestionWriteQueue | null >( null ); if ( writeQueueRef.current === null ) { writeQueueRef.current = createSuggestionWriteQueue(); } const enqueueSuggestionWrite = useCallback( - ( clientId, task ) => writeQueueRef.current.enqueue( clientId, task ), + ( clientId: string, task: () => unknown ) => + writeQueueRef.current!.enqueue( clientId, task ), [] ); @@ -447,20 +563,20 @@ export function SuggestionOverlayProvider( { children } ) { // A ref-set for the same reason as the bypass set above: it is written // from inside `registry.subscribe` and read synchronously during event // handling, neither of which can wait on React state. - const deferredInsertionsRef = useRef( new Set() ); + const deferredInsertionsRef = useRef( new Set< string >() ); - const markDeferredInsertion = useCallback( ( clientId ) => { + const markDeferredInsertion = useCallback( ( clientId: string ) => { if ( clientId ) { deferredInsertionsRef.current.add( clientId ); } }, [] ); - const unmarkDeferredInsertion = useCallback( ( clientId ) => { + const unmarkDeferredInsertion = useCallback( ( clientId: string ) => { deferredInsertionsRef.current.delete( clientId ); }, [] ); const isDeferredInsertion = useCallback( - ( clientId ) => deferredInsertionsRef.current.has( clientId ), + ( clientId: string ) => deferredInsertionsRef.current.has( clientId ), [] ); @@ -482,7 +598,7 @@ export function SuggestionOverlayProvider( { children } ) { * block-related. A counter-of-expiries rather than a boolean so two quick * undo presses arm two adoptions. */ - const undoAdoptionExpiriesRef = useRef( [] ); + const undoAdoptionExpiriesRef = useRef< number[] >( [] ); const armUndoRedoAdoption = useCallback( () => { undoAdoptionExpiriesRef.current.push( @@ -513,7 +629,7 @@ export function SuggestionOverlayProvider( { children } ) { if ( ! hasEntries ) { return 0; } - const blockEditor = select( BLOCK_EDITOR_STORE_NAME ); + const blockEditor: any = select( BLOCK_EDITOR_STORE_NAME ); return blockEditor?.getClientIdsWithDescendants?.().length ?? 0; }, [ hasEntries ] @@ -522,9 +638,8 @@ export function SuggestionOverlayProvider( { children } ) { if ( ! hasEntries ) { return; } - const getLive = registry.select( - BLOCK_EDITOR_STORE_NAME - )?.getClientIdsWithDescendants; + const getLive = registry.select( BLOCK_EDITOR_STORE_NAME ) + ?.getClientIdsWithDescendants; if ( ! getLive ) { return; } @@ -599,8 +714,8 @@ export function SuggestionOverlayProvider( { children } ) { /** * Hook returning the suggestion overlay API. * - * @return {OverlayContextValue} Overlay API. + * @return Overlay API. */ -export function useSuggestionOverlay() { +export function useSuggestionOverlay(): OverlayContextValue { return useContext( OverlayContext ); } diff --git a/packages/editor/src/components/suggestion-mode/provider.js b/packages/editor/src/components/suggestion-mode/provider.ts similarity index 76% rename from packages/editor/src/components/suggestion-mode/provider.js rename to packages/editor/src/components/suggestion-mode/provider.ts index a7b288e68a0ec8..0030876321414c 100644 --- a/packages/editor/src/components/suggestion-mode/provider.js +++ b/packages/editor/src/components/suggestion-mode/provider.ts @@ -1,12 +1,15 @@ import { useCallback, useMemo } from '@wordpress/element'; import { useDispatch, useRegistry, useSelect } from '@wordpress/data'; import { store as coreStore } from '@wordpress/core-data'; +// @ts-expect-error No exported types import { store as blockEditorStore } from '@wordpress/block-editor'; +// @ts-expect-error No exported types import { store as interfaceStore } from '@wordpress/interface'; import { store as noticesStore } from '@wordpress/notices'; import { __ } from '@wordpress/i18n'; import { EDITOR_STORE_NAME } from './constants'; import { useSuggestionOverlay } from './overlay-context'; +import type { SuggestionOperation } from './overlay-context'; import { addNoteIdToMetadata, getNoteIdsFromMetadata, @@ -14,33 +17,38 @@ import { import { ALL_NOTES_SIDEBAR, SIDEBARS } from '../collab-sidebar/constants'; /** - * @typedef {Object} SuggestionOperation - * @property {'attribute-set'|'inline-suggestion'|'block-insert-after'|'block-remove'|'block-move'} type - * Operation type. `attribute-set` and `inline-suggestion` ship in - * Phase 2; the structural variants ship in Phase 6 (issue #77434). - * @property {string} [attribute] The attribute being changed (`attribute-set`) or - * carrying the marker (`inline-suggestion`). - * @property {'del'|'add'|'format'} [suggestionType] Inline marker kind (`inline-suggestion` only): `del` - * wraps existing text proposed for removal, `add` wraps proposed - * new text, `format` wraps a run whose formatting changed (text - * unchanged). - * @property {string} [beforeHTML] Original run HTML captured for a `format` suggestion, so a - * reject can restore the pre-suggestion formatting. - * @property {string} [afterHTML] Proposed run HTML for a `format` suggestion, used to - * summarize which formats changed. - * @property {*} [before] The baseline value (`attribute-set`). - * @property {*} [after] The proposed value (`attribute-set`). + * A single suggestion operation. + * + * `type` is one of `attribute-set` / `inline-suggestion` (Phase 2) or the + * structural variants `block-insert-after` / `block-remove` / `block-move` + * (Phase 6, issue #77434). Other fields vary by type: + * - `attribute` The attribute being changed (`attribute-set`) or + * carrying the marker (`inline-suggestion`). + * - `suggestionType` Inline marker kind (`inline-suggestion` only): `del` + * wraps existing text proposed for removal, `add` wraps + * proposed new text, `format` wraps a run whose + * formatting changed (text unchanged). + * - `beforeHTML` Original run HTML captured for a `format` suggestion, + * so a reject can restore the pre-suggestion formatting. + * - `afterHTML` Proposed run HTML for a `format` suggestion, used to + * summarize which formats changed. + * - `before`/`after` The baseline and proposed values (`attribute-set`). */ +export type { SuggestionOperation }; -/** - * @typedef {Object} SuggestionPayload - * @property {number} schemaVersion Payload schema version. - * @property {string} blockName Block name at capture time. - * @property {string|null} baseRevision Post `modified_gmt` at - * capture, used by Phase 3 to - * detect stale suggestions. - * @property {SuggestionOperation[]} operations Ordered operations. - */ +export interface SuggestionPayload { + /** Payload schema version. */ + schemaVersion: number; + /** Block name at capture time. */ + blockName: string; + /** + * Post `modified_gmt` at capture, used by Phase 3 to detect stale + * suggestions. + */ + baseRevision: string | null; + /** Ordered operations. */ + operations: SuggestionOperation[]; +} /** * Suggestion payload schema version. v1 emitted only `attribute-set` @@ -70,10 +78,10 @@ const PAYLOAD_MAX_BYTES = 65536; * Byte length of a serialized payload, measured the way PHP `strlen()` * counts (UTF-8 bytes, not chars). * - * @param {SuggestionPayload} payload - * @return {number} UTF-8 byte length of the serialized JSON. + * @param payload Payload to measure. + * @return UTF-8 byte length of the serialized JSON. */ -function payloadByteLength( payload ) { +function payloadByteLength( payload: SuggestionPayload ): number { const serialized = JSON.stringify( payload ); if ( typeof TextEncoder !== 'undefined' ) { return new TextEncoder().encode( serialized ).length; @@ -89,12 +97,15 @@ function payloadByteLength( payload ) { * captured baseline. Attributes whose value differs are emitted; unchanged * or absent keys are skipped. * - * @param {Object} baselineAttributes Attributes captured on first edit. - * @param {Object} overlayAttributes Pending attribute changes. - * @return {SuggestionOperation[]} Operations describing the suggestion. + * @param baselineAttributes Attributes captured on first edit. + * @param overlayAttributes Pending attribute changes. + * @return Operations describing the suggestion. */ -export function operationsFromOverlay( baselineAttributes, overlayAttributes ) { - const operations = []; +export function operationsFromOverlay( + baselineAttributes: Record< string, any > | null | undefined, + overlayAttributes: Record< string, any > | null | undefined +): SuggestionOperation[] { + const operations: SuggestionOperation[] = []; for ( const [ attribute, after ] of Object.entries( overlayAttributes || {} ) ) { @@ -119,11 +130,11 @@ export function operationsFromOverlay( baselineAttributes, overlayAttributes ) { * based compare produces spurious "changed" detections when block code re- * emits a `style` object with reordered keys. The recursive walk avoids that. * - * @param {*} a First value. - * @param {*} b Second value. - * @return {boolean} True when the values are structurally equal. + * @param a First value. + * @param b Second value. + * @return True when the values are structurally equal. */ -function isAttributeEqual( a, b ) { +function isAttributeEqual( a: any, b: any ): boolean { if ( a === b ) { return true; } @@ -203,10 +214,12 @@ const STRUCTURAL_OP_TYPES = new Set( [ * structural mutation as its own note); attribute-set ops can ride along * inside the same payload but the structural op leads. * - * @param {SuggestionOperation[]} operations Payload operations. - * @return {SuggestionOperation|null} Structural op, or null when none. + * @param operations Payload operations. + * @return Structural op, or null when none. */ -export function findStructuralOp( operations ) { +export function findStructuralOp( + operations: SuggestionOperation[] | null | undefined +): SuggestionOperation | null { if ( ! Array.isArray( operations ) ) { return null; } @@ -223,10 +236,12 @@ export function findStructuralOp( operations ) { * while preserving every other metadata field. Used by Apply (after the * mutation lands) and by Reject (to drop the pending state). * - * @param {Object} currentAttributes Block's current attributes. - * @return {Object} Partial attributes payload safe for `updateBlockAttributes`. + * @param currentAttributes Block's current attributes. + * @return Partial attributes payload safe for `updateBlockAttributes`. */ -export function clearSuggestionMarkerAttributes( currentAttributes ) { +export function clearSuggestionMarkerAttributes( + currentAttributes: Record< string, any > | null | undefined +): { metadata: Record< string, any > } | null { const meta = currentAttributes?.metadata; if ( ! meta || meta.suggestion === undefined ) { return null; @@ -239,12 +254,15 @@ export function clearSuggestionMarkerAttributes( currentAttributes ) { * Apply a suggestion payload's operations to a block's current attributes * to produce the new attributes. Pure function — no side effects. * - * @param {Object} currentAttributes Block's current attributes. - * @param {SuggestionOperation[]} operations Operations from the payload. - * @return {Object} Merged attributes with suggestions applied. + * @param currentAttributes Block's current attributes. + * @param operations Operations from the payload. + * @return Merged attributes with suggestions applied. */ -export function applyOperations( currentAttributes, operations ) { - const result = { ...currentAttributes }; +export function applyOperations( + currentAttributes: Record< string, any > | null | undefined, + operations: SuggestionOperation[] +): Record< string, any > { + const result: Record< string, any > = { ...currentAttributes }; for ( const op of operations ) { if ( op.type === 'attribute-set' ) { result[ op.attribute ] = op.after; @@ -260,11 +278,14 @@ export function applyOperations( currentAttributes, operations ) { * captured at suggest-time differs from the attribute's current value — * simply reopening the post after any auto-save doesn't qualify. * - * @param {Object} currentAttributes Block's current attributes. - * @param {SuggestionOperation[]} operations Operations from the payload. - * @return {boolean} True if at least one targeted attribute has diverged. + * @param currentAttributes Block's current attributes. + * @param operations Operations from the payload. + * @return True if at least one targeted attribute has diverged. */ -export function hasAttributeConflict( currentAttributes, operations ) { +export function hasAttributeConflict( + currentAttributes: Record< string, any > | null | undefined, + operations: SuggestionOperation[] | null | undefined +): boolean { if ( ! Array.isArray( operations ) ) { return false; } @@ -303,10 +324,12 @@ export function hasAttributeConflict( currentAttributes, operations ) { * Add a new `case` per future bump; never remove old cases, since the * comment-meta store may contain payloads written by every prior version. * - * @param {Object} parsed Parsed JSON payload of a known older version. - * @return {Object} Payload upgraded to the current schema. + * @param parsed Parsed JSON payload of a known older version. + * @return Payload upgraded to the current schema. */ -function migrateSuggestionPayload( parsed ) { +function migrateSuggestionPayload( + parsed: SuggestionPayload +): SuggestionPayload { let next = parsed; if ( next.schemaVersion === 1 ) { next = { ...next, schemaVersion: 2 }; @@ -320,15 +343,17 @@ function migrateSuggestionPayload( parsed ) { * apply can't drop op types this consumer doesn't understand. Migrates * older payloads forward to the current shape. * - * @param {string|undefined} raw The raw JSON string from comment meta. - * @return {SuggestionPayload|null} Parsed payload, or null when the input is + * @param raw The raw JSON string from comment meta. + * @return Parsed payload, or null when the input is * malformed or the payload was written by a newer editor. */ -export function parseSuggestionPayload( raw ) { +export function parseSuggestionPayload( + raw: string | null | undefined +): SuggestionPayload | null { if ( ! raw ) { return null; } - let parsed; + let parsed: any; try { parsed = JSON.parse( raw ); } catch { @@ -366,15 +391,17 @@ export function parseSuggestionPayload( raw ) { * `useSuggestionsProvider` is instantiated once per consumer and the guard * must be shared across all of them. */ -const decisionsInFlight = new Set(); +const decisionsInFlight = new Set< string >(); /** * Whether an apply/reject decision for the given comment is in flight. * - * @param {number|string} commentId Comment id to check. - * @return {boolean} True while a decision is being processed. + * @param commentId Comment id to check. + * @return True while a decision is being processed. */ -export function isSuggestionDecisionInFlight( commentId ) { +export function isSuggestionDecisionInFlight( + commentId: number | string +): boolean { return decisionsInFlight.has( String( commentId ) ); } @@ -382,11 +409,13 @@ export function isSuggestionDecisionInFlight( commentId ) { * Wrap a decision callback (apply/reject) so its comment id is registered as * in flight for the duration of the call. * - * @param {Function} decide Decision callback taking `{ commentId, ... }`. - * @return {Function} Wrapped callback. + * @param decide Decision callback taking `{ commentId, ... }`. + * @return Wrapped callback. */ -function withDecisionInFlight( decide ) { - return async ( args ) => { +function withDecisionInFlight< Args extends { commentId?: number | string } >( + decide: ( args: Args ) => Promise< unknown > +) { + return async ( args: Args ) => { const key = String( args?.commentId ); decisionsInFlight.add( key ); try { @@ -405,15 +434,11 @@ function withDecisionInFlight( decide ) { * the `_wp_suggestion` comment meta. Linkage to a block reuses the existing * `metadata.noteId` block attribute. * - * @return {{ - * createSuggestion: Function, - * applySuggestion: Function, - * rejectSuggestion: Function, - * }} Suggestions API. + * @return Suggestions API. */ export function useSuggestionsProvider() { const { postId, postModified } = useSelect( ( select ) => { - const editor = select( EDITOR_STORE_NAME ); + const editor: any = select( EDITOR_STORE_NAME ); const id = editor?.getCurrentPostId?.() ?? null; const postType = editor?.getCurrentPostType?.() ?? null; const record = @@ -426,7 +451,7 @@ export function useSuggestionsProvider() { : null; return { postId: id, - postModified: record?.modified_gmt ?? null, + postModified: ( record as any )?.modified_gmt ?? null, }; }, [] ); @@ -449,7 +474,15 @@ export function useSuggestionsProvider() { const registry = useRegistry(); const createSuggestion = useCallback( - async ( { clientId, blockName, operations } ) => { + async ( { + clientId, + blockName, + operations, + }: { + clientId: string; + blockName: string; + operations: SuggestionOperation[]; + } ) => { if ( ! postId ) { throw new Error( 'No post id available for suggestion.' ); } @@ -457,12 +490,12 @@ export function useSuggestionsProvider() { return null; } - const payload = /** @type {SuggestionPayload} */ ( { + const payload: SuggestionPayload = { schemaVersion: SCHEMA_VERSION, blockName, baseRevision: postModified, operations, - } ); + }; if ( payloadByteLength( payload ) > PAYLOAD_MAX_BYTES ) { const error = new Error( @@ -476,7 +509,7 @@ export function useSuggestionsProvider() { } try { - const savedRecord = await saveEntityRecord( + const savedRecord: any = await saveEntityRecord( 'root', 'comment', { @@ -526,7 +559,7 @@ export function useSuggestionsProvider() { } return savedRecord; - } catch ( error ) { + } catch ( error: any ) { createNotice( 'error', error?.message || __( 'Unable to submit suggestion.' ), @@ -554,26 +587,32 @@ export function useSuggestionsProvider() { * status, or thread identity, so the user sees a single note * accumulating edits rather than a new note per save burst. * - * @param {Object} args Update arguments. - * @param {number|string} args.commentId Comment id of the - * existing suggestion. - * @param {string} args.blockName Block name (recorded - * on the payload). - * @param {SuggestionOperation[]} args.operations Latest operations. - * @return {Promise} The saved comment record. + * @param args Update arguments. + * @param args.commentId Comment id of the existing suggestion. + * @param args.blockName Block name (recorded on the payload). + * @param args.operations Latest operations. + * @return The saved comment record. */ const updateSuggestion = useCallback( - async ( { commentId, blockName, operations } ) => { + async ( { + commentId, + blockName, + operations, + }: { + commentId: number | string; + blockName: string; + operations: SuggestionOperation[]; + } ) => { if ( ! commentId ) { throw new Error( 'No comment id for suggestion update.' ); } - const payload = /** @type {SuggestionPayload} */ ( { + const payload: SuggestionPayload = { schemaVersion: SCHEMA_VERSION, blockName, baseRevision: postModified, operations, - } ); + }; if ( payloadByteLength( payload ) > PAYLOAD_MAX_BYTES ) { const error = new Error( @@ -598,7 +637,7 @@ export function useSuggestionsProvider() { }, { throwOnError: true } ); - } catch ( error ) { + } catch ( error: any ) { createNotice( 'error', error?.message || __( 'Unable to update suggestion.' ), @@ -615,12 +654,11 @@ export function useSuggestionsProvider() { * fully reverted to baseline — the user retracted their edit, so the * note no longer carries a meaningful suggestion. * - * @param {Object} args Delete arguments. - * @param {number|string} args.commentId Comment id to trash. - * @return {Promise} + * @param args Delete arguments. + * @param args.commentId Comment id to trash. */ const deleteSuggestion = useCallback( - async ( { commentId } ) => { + async ( { commentId }: { commentId: number | string | null } ) => { if ( ! commentId ) { return; } @@ -631,7 +669,7 @@ export function useSuggestionsProvider() { { id: commentId, status: 'trash' }, { throwOnError: true } ); - } catch ( error ) { + } catch ( error: any ) { createNotice( 'error', error?.message || __( 'Unable to remove suggestion.' ), @@ -648,23 +686,26 @@ export function useSuggestionsProvider() { * status to the comment meta. On a server failure the block is rolled * back so the UI is never left in a half-applied state. * - * @param {Object} args Apply arguments. - * @param {number|string} args.commentId Comment id holding the - * suggestion (`_wp_suggestion` - * meta). - * @param {string} args.clientId Block client id of the apply - * target. May be undefined if - * the acting user opened the - * post fresh and the metadata - * linkage was never persisted — - * the apply path then scans the - * live tree by `metadata.noteId`. - * @param {SuggestionPayload} args.payload Parsed payload (from - * `parseSuggestionPayload`). - * @return {Promise} + * @param args Apply arguments. + * @param args.commentId Comment id holding the suggestion + * (`_wp_suggestion` meta). + * @param args.clientId Block client id of the apply target. May be + * undefined if the acting user opened the post + * fresh and the metadata linkage was never + * persisted — the apply path then scans the live + * tree by `metadata.noteId`. + * @param args.payload Parsed payload (from `parseSuggestionPayload`). */ const applySuggestion = useCallback( - async ( { commentId, clientId, payload } ) => { + async ( { + commentId, + clientId, + payload, + }: { + commentId: number | string; + clientId?: string; + payload: SuggestionPayload | null; + } ) => { if ( ! payload || ! Array.isArray( payload.operations ) ) { createNotice( 'error', __( 'Invalid suggestion payload.' ), { type: 'snackbar', @@ -690,7 +731,12 @@ export function useSuggestionsProvider() { const ids = getNoteIdsFromMetadata( selectBlockAttributes( id )?.metadata ); - if ( ids.some( ( n ) => String( n ) === commentIdKey ) ) { + if ( + ids.some( + ( n: number | string ) => + String( n ) === commentIdKey + ) + ) { targetClientId = id; break; } @@ -794,11 +840,15 @@ export function useSuggestionsProvider() { clearOverlay( targetClientId ); } - createNotice( 'snackbar', __( 'Suggestion applied.' ), { - type: 'snackbar', - isDismissible: true, - } ); - } catch ( error ) { + createNotice( + 'snackbar' as any, + __( 'Suggestion applied.' ), + { + type: 'snackbar', + isDismissible: true, + } + ); + } catch ( error: any ) { createNotice( 'error', error?.message || @@ -823,7 +873,7 @@ export function useSuggestionsProvider() { // to override them. Listing each touched key with its original // value (or `undefined` when the key was added by this apply) // restores the block cleanly. - const rollbackPayload = {}; + const rollbackPayload: Record< string, any > = {}; for ( const op of payload.operations ) { if ( op.type !== 'attribute-set' ) { continue; @@ -833,7 +883,7 @@ export function useSuggestionsProvider() { currentAttributes ?? {}, op.attribute ) - ? currentAttributes[ op.attribute ] + ? currentAttributes?.[ op.attribute ] : undefined; } @@ -860,11 +910,11 @@ export function useSuggestionsProvider() { { throwOnError: true } ); - createNotice( 'snackbar', __( 'Suggestion applied.' ), { + createNotice( 'snackbar' as any, __( 'Suggestion applied.' ), { type: 'snackbar', isDismissible: true, } ); - } catch ( error ) { + } catch ( error: any ) { // Roll back the block change so the UI isn't left in a // half-applied state if the server rejected the update. requestInterceptorBypass( targetClientId ); @@ -896,20 +946,23 @@ export function useSuggestionsProvider() { * `metadata.suggestion` marker on the live block so the dimmed/struck * visual treatment goes away. * - * @param {Object} args Reject arguments. - * @param {number|string} args.commentId Comment id of the rejected - * suggestion. - * @param {string} [args.clientId] Target block clientId, if - * known. - * @param {SuggestionPayload} [args.payload] Parsed suggestion payload — - * inspected to detect a - * structural op so the marker - * can be cleared on the live - * block. - * @return {Promise} + * @param args Reject arguments. + * @param args.commentId Comment id of the rejected suggestion. + * @param args.clientId Target block clientId, if known. + * @param args.payload Parsed suggestion payload — inspected to detect + * a structural op so the marker can be cleared on + * the live block. */ const rejectSuggestion = useCallback( - async ( { commentId, clientId, payload } ) => { + async ( { + commentId, + clientId, + payload, + }: { + commentId: number | string; + clientId?: string; + payload?: SuggestionPayload | null; + } ) => { // Reject behavior depends on the structural op type: // - block-remove: drop the marker (block stays). // - block-insert-after: dispatch removeBlock to undo the @@ -995,11 +1048,11 @@ export function useSuggestionsProvider() { } } - createNotice( 'snackbar', __( 'Suggestion rejected.' ), { + createNotice( 'snackbar' as any, __( 'Suggestion rejected.' ), { type: 'snackbar', isDismissible: true, } ); - } catch ( error ) { + } catch ( error: any ) { createNotice( 'error', error?.message || __( 'Failed to reject suggestion.' ), diff --git a/packages/editor/src/components/suggestion-mode/suggestion-write-queue.js b/packages/editor/src/components/suggestion-mode/suggestion-write-queue.ts similarity index 72% rename from packages/editor/src/components/suggestion-mode/suggestion-write-queue.js rename to packages/editor/src/components/suggestion-mode/suggestion-write-queue.ts index b81c97ba687d55..8537cd2a390a96 100644 --- a/packages/editor/src/components/suggestion-mode/suggestion-write-queue.js +++ b/packages/editor/src/components/suggestion-mode/suggestion-write-queue.ts @@ -13,13 +13,18 @@ * while tasks for different blocks stay independent. */ +export interface SuggestionWriteQueue { + enqueue: ( clientId: string, task: () => unknown ) => Promise< unknown >; + hasPending: ( clientId: string ) => boolean; +} + /** * Create a write queue keyed by block client id. * - * @return {{enqueue: Function, hasPending: Function}} Queue API. + * @return Queue API. */ -export function createSuggestionWriteQueue() { - const chains = new Map(); +export function createSuggestionWriteQueue(): SuggestionWriteQueue { + const chains = new Map< string, Promise< void > >(); return { /** @@ -27,12 +32,12 @@ export function createSuggestionWriteQueue() { * block has settled. A rejected task never poisons later tasks — * the stored chain always resolves. * - * @param {string} clientId Block client id the write targets. - * @param {Function} task Async task to run. - * @return {Promise<*>} The task's own settlement (observable by the + * @param clientId Block client id the write targets. + * @param task Async task to run. + * @return The task's own settlement (observable by the * caller, including rejection). */ - enqueue( clientId, task ) { + enqueue( clientId: string, task: () => unknown ) { const previous = chains.get( clientId ) ?? Promise.resolve(); const run = previous.then( () => task() ); const settled = run.then( @@ -53,10 +58,10 @@ export function createSuggestionWriteQueue() { /** * Whether a task is queued or in flight for the block. * - * @param {string} clientId Block client id. - * @return {boolean} True when the block has pending writes. + * @param clientId Block client id. + * @return True when the block has pending writes. */ - hasPending( clientId ) { + hasPending( clientId: string ) { return chains.has( clientId ); }, }; diff --git a/packages/editor/src/components/suggestion-mode/test/auto-save.js b/packages/editor/src/components/suggestion-mode/test/auto-save.tsx similarity index 96% rename from packages/editor/src/components/suggestion-mode/test/auto-save.js rename to packages/editor/src/components/suggestion-mode/test/auto-save.tsx index fc0ec8d210775f..46ad26f94c17cb 100644 --- a/packages/editor/src/components/suggestion-mode/test/auto-save.js +++ b/packages/editor/src/components/suggestion-mode/test/auto-save.tsx @@ -42,14 +42,14 @@ afterEach( () => { jest.useRealTimers(); } ); -function renderInSuggestMode( ui ) { +function renderInSuggestMode( ui: React.ReactElement ) { const registry = createRegistry(); registry.register( noticesStore ); registry.register( coreStore ); registry.register( editorStore ); unlock( registry.dispatch( editorStore ) ).setEditorIntent( 'suggest' ); - const wrapper = ( { children } ) => ( + const wrapper = ( { children }: { children?: React.ReactNode } ) => ( { children } @@ -61,7 +61,7 @@ function renderInSuggestMode( ui ) { // Seed a comment record so `getEntityRecord( 'root', 'comment', id )` resolves // without an HTTP fetch — mirrors what `useNoteThreads`'s entity query would // have populated by the time a suggestion is in flight. -function seedComment( registry, comment ) { +function seedComment( registry: any, comment: any ) { registry .dispatch( coreStore ) .receiveEntityRecords( 'root', 'comment', [ comment ] ); @@ -69,7 +69,7 @@ function seedComment( registry, comment ) { // Test harness exposes the overlay API via a render-prop ref so tests can // drive the reducer directly. -let overlayHandle; +let overlayHandle: ReturnType< typeof useSuggestionOverlay >; function CaptureOverlay() { overlayHandle = useSuggestionOverlay(); return null; @@ -215,7 +215,7 @@ describe( 'SuggestionAutoSave', () => { it( 'does not duplicate work when the user keeps typing during an in-flight save', async () => { // `createSuggestion` resolves only when we explicitly let it. - let resolveCreate; + let resolveCreate!: ( value?: unknown ) => void; createSuggestion.mockImplementation( () => new Promise( ( resolve ) => { @@ -401,7 +401,7 @@ describe( 'SuggestionAutoSave', () => { registry.register( editorStore ); unlock( registry.dispatch( editorStore ) ).setEditorIntent( 'edit' ); - const wrapper = ( { children } ) => ( + const wrapper = ( { children }: { children?: React.ReactNode } ) => ( { children } @@ -490,6 +490,8 @@ describe( 'operationsForEntry', () => { blockName: 'core/paragraph', baselineAttributes: { content: 'a' }, overlayAttributes: { content: 'b' }, + commentId: null, + syncedOpsKey: null, } ) ).toEqual( [ { @@ -513,6 +515,8 @@ describe( 'operationsForEntry', () => { baselineAttributes: {}, overlayAttributes: {}, structuralOp: op, + commentId: null, + syncedOpsKey: null, } ) ).toEqual( [ op ] ); } ); @@ -525,6 +529,8 @@ describe( 'operationsForEntry', () => { baselineAttributes: { content: 'a' }, overlayAttributes: { content: 'b' }, structuralOp: op, + commentId: null, + syncedOpsKey: null, } ) ).toEqual( [ op, diff --git a/packages/editor/src/components/suggestion-mode/test/overlay-context.js b/packages/editor/src/components/suggestion-mode/test/overlay-context.ts similarity index 97% rename from packages/editor/src/components/suggestion-mode/test/overlay-context.js rename to packages/editor/src/components/suggestion-mode/test/overlay-context.ts index e267c8c243cc48..3365473fd6a543 100644 --- a/packages/editor/src/components/suggestion-mode/test/overlay-context.js +++ b/packages/editor/src/components/suggestion-mode/test/overlay-context.ts @@ -84,7 +84,7 @@ describe( 'overlayReducer', () => { } ); it( 'returns the same reference for unknown actions', () => { - const next = overlayReducer( INITIAL, { type: 'UNKNOWN' } ); + const next = overlayReducer( INITIAL, { type: 'UNKNOWN' } as any ); expect( next ).toBe( INITIAL ); } ); @@ -152,11 +152,15 @@ describe( 'overlayReducer', () => { blockName: 'core/paragraph', baselineAttributes: {}, overlayAttributes: { content: 'hi' }, + commentId: null, + syncedOpsKey: null, }, 'orphan-1': { blockName: 'core/paragraph', baselineAttributes: {}, overlayAttributes: { content: 'gone' }, + commentId: null, + syncedOpsKey: null, }, }; const next = overlayReducer( state, { @@ -218,6 +222,8 @@ describe( 'overlayReducer', () => { blockName: 'core/paragraph', baselineAttributes: {}, overlayAttributes: {}, + commentId: null, + syncedOpsKey: null, }, }; const next = overlayReducer( state, { diff --git a/packages/editor/src/components/suggestion-mode/test/provider.js b/packages/editor/src/components/suggestion-mode/test/provider.tsx similarity index 96% rename from packages/editor/src/components/suggestion-mode/test/provider.js rename to packages/editor/src/components/suggestion-mode/test/provider.tsx index ee52c6f0b123fa..73de7bd6e3c102 100644 --- a/packages/editor/src/components/suggestion-mode/test/provider.js +++ b/packages/editor/src/components/suggestion-mode/test/provider.tsx @@ -4,6 +4,7 @@ import { createReduxStore, RegistryProvider, } from '@wordpress/data'; +// @ts-expect-error No exported types import { store as blockEditorStore } from '@wordpress/block-editor'; import { store as noticesStore } from '@wordpress/notices'; import { @@ -196,12 +197,12 @@ describe( 'applyOperations', () => { describe( 'payloadByteLength', () => { it( 'measures ASCII payload byte length', () => { // {"a":"hello"} is 13 bytes. - expect( payloadByteLength( { a: 'hello' } ) ).toBe( 13 ); + expect( payloadByteLength( { a: 'hello' } as any ) ).toBe( 13 ); } ); it( 'counts multi-byte characters by UTF-8 byte length', () => { // {"a":"€"} = 8 ASCII bytes + 3 bytes for the euro sign = 11. - expect( payloadByteLength( { a: '€' } ) ).toBe( 11 ); + expect( payloadByteLength( { a: '€' } as any ) ).toBe( 11 ); } ); it( 'exposes a numeric size cap', () => { @@ -355,7 +356,7 @@ describe( 'parseSuggestionPayload', () => { }, ], } ); - const result = parseSuggestionPayload( raw ); + const result = parseSuggestionPayload( raw )!; expect( result ).not.toBeNull(); expect( result.schemaVersion ).toBe( 2 ); expect( result.operations ).toHaveLength( 1 ); @@ -376,7 +377,7 @@ describe( 'parseSuggestionPayload', () => { }, ], } ); - const result = parseSuggestionPayload( raw ); + const result = parseSuggestionPayload( raw )!; expect( result ).not.toBeNull(); expect( result.schemaVersion ).toBe( 2 ); expect( result.operations ).toHaveLength( 1 ); @@ -395,7 +396,7 @@ describe( 'parseSuggestionPayload', () => { }, ], } ); - const result = parseSuggestionPayload( raw ); + const result = parseSuggestionPayload( raw )!; expect( result ).not.toBeNull(); expect( result.schemaVersion ).toBe( 2 ); } ); @@ -490,16 +491,16 @@ describe( 'clearSuggestionMarkerAttributes', () => { */ function createStubInterfaceStore() { return createReduxStore( 'core/interface', { - reducer: ( state = { activeArea: null }, action ) => + reducer: ( state: any = { activeArea: null }, action: any ) => action.type === 'ENABLE_AREA' ? { activeArea: action.area } : state, actions: { - enableComplementaryArea: ( scope, area ) => ( { + enableComplementaryArea: ( scope: string, area: string ) => ( { type: 'ENABLE_AREA', area, } ), }, selectors: { - getActiveComplementaryArea: ( state ) => state.activeArea, + getActiveComplementaryArea: ( state: any ) => state.activeArea, }, } ); } @@ -556,7 +557,7 @@ describe( 'rejectSuggestion (block-move)', () => { } ); } - function setup( initialBlocks ) { + function setup( initialBlocks: any[] ) { const registry = createRegistry(); registry.register( noticesStore ); registry.register( blockEditorStore ); @@ -564,7 +565,7 @@ describe( 'rejectSuggestion (block-move)', () => { registry.register( createStubInterfaceStore() ); registry.dispatch( blockEditorStore ).resetBlocks( initialBlocks ); - let providerHandle; + let providerHandle: ReturnType< typeof useSuggestionsProvider >; function CaptureProvider() { providerHandle = useSuggestionsProvider(); return null; @@ -579,7 +580,7 @@ describe( 'rejectSuggestion (block-move)', () => { return { registry, getProvider: () => providerHandle }; } - function movePayload( structuralOp ) { + function movePayload( structuralOp: any ) { return { schemaVersion: 2, blockName: PARAGRAPH, @@ -709,7 +710,7 @@ describe( 'decision failures leave the block tree untouched', () => { } ); } - function setup( initialBlocks ) { + function setup( initialBlocks: any[] ) { const registry = createRegistry(); registry.register( noticesStore ); registry.register( blockEditorStore ); @@ -717,7 +718,7 @@ describe( 'decision failures leave the block tree untouched', () => { registry.register( createStubInterfaceStore() ); registry.dispatch( blockEditorStore ).resetBlocks( initialBlocks ); - let providerHandle; + let providerHandle: ReturnType< typeof useSuggestionsProvider >; function CaptureProvider() { providerHandle = useSuggestionsProvider(); return null; @@ -876,7 +877,7 @@ describe( 'createSuggestion (notes sidebar switch)', () => { } ); } - function setup( { activeArea } ) { + function setup( { activeArea }: { activeArea?: string | null } ) { const registry = createRegistry(); registry.register( noticesStore ); registry.register( blockEditorStore ); @@ -893,7 +894,7 @@ describe( 'createSuggestion (notes sidebar switch)', () => { const block = createBlock( PARAGRAPH, { content: 'Hello' } ); registry.dispatch( blockEditorStore ).resetBlocks( [ block ] ); - let providerHandle; + let providerHandle: ReturnType< typeof useSuggestionsProvider >; function CaptureProvider() { providerHandle = useSuggestionsProvider(); return null; @@ -908,7 +909,7 @@ describe( 'createSuggestion (notes sidebar switch)', () => { return { registry, block, getProvider: () => providerHandle }; } - async function createAttributeSuggestion( getProvider, block ) { + async function createAttributeSuggestion( getProvider: any, block: any ) { await act( async () => { await getProvider().createSuggestion( { clientId: block.clientId, diff --git a/packages/editor/src/components/suggestion-mode/test/suggestion-write-queue.js b/packages/editor/src/components/suggestion-mode/test/suggestion-write-queue.ts similarity index 93% rename from packages/editor/src/components/suggestion-mode/test/suggestion-write-queue.js rename to packages/editor/src/components/suggestion-mode/test/suggestion-write-queue.ts index e2ce30370db2a1..8118001fdb7606 100644 --- a/packages/editor/src/components/suggestion-mode/test/suggestion-write-queue.js +++ b/packages/editor/src/components/suggestion-mode/test/suggestion-write-queue.ts @@ -2,8 +2,8 @@ import { createSuggestionWriteQueue } from '../suggestion-write-queue'; /** Create a promise whose resolution the test controls. */ function deferred() { - let resolve; - let reject; + let resolve!: ( value?: unknown ) => void; + let reject!: ( reason?: unknown ) => void; const promise = new Promise( ( res, rej ) => { resolve = res; reject = rej; @@ -22,7 +22,7 @@ describe( 'createSuggestionWriteQueue', () => { it( 'runs tasks for the same block strictly one after another', async () => { const queue = createSuggestionWriteQueue(); const first = deferred(); - const order = []; + const order: string[] = []; queue.enqueue( 'a', async () => { order.push( 'first:start' ); @@ -49,7 +49,7 @@ describe( 'createSuggestionWriteQueue', () => { it( 'lets tasks for different blocks run independently', async () => { const queue = createSuggestionWriteQueue(); const blockedForever = deferred(); - const order = []; + const order: string[] = []; queue.enqueue( 'a', async () => { await blockedForever.promise; @@ -64,7 +64,7 @@ describe( 'createSuggestionWriteQueue', () => { it( 'does not let a rejected task poison later tasks on the block', async () => { const queue = createSuggestionWriteQueue(); - const order = []; + const order: string[] = []; const failing = queue.enqueue( 'a', async () => { throw new Error( 'boom' ); diff --git a/tools/eslint/suppressions.json b/tools/eslint/suppressions.json index 327acaca0cf1c6..cc1fab86b26fc9 100644 --- a/tools/eslint/suppressions.json +++ b/tools/eslint/suppressions.json @@ -1419,12 +1419,12 @@ "count": 1 } }, - "packages/editor/src/components/suggestion-mode/auto-save.js": { + "packages/editor/src/components/suggestion-mode/auto-save.ts": { "react-hooks/refs": { "count": 6 } }, - "packages/editor/src/components/suggestion-mode/test/auto-save.js": { + "packages/editor/src/components/suggestion-mode/test/auto-save.tsx": { "react-hooks/globals": { "count": 1 } From 3304c95eaa229a0fb3be265a37c09291ca384481 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Mon, 24 Aug 2026 16:22:53 -0700 Subject: [PATCH 06/23] Repoint the suggestion-mode imports at the store constants The local mirror of the store name and the suggest intent value is gone; `store/constants` imports nothing, so reading them from there closes no cycle. --- packages/editor/src/components/suggestion-mode/auto-save.ts | 6 +++--- packages/editor/src/components/suggestion-mode/provider.ts | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/editor/src/components/suggestion-mode/auto-save.ts b/packages/editor/src/components/suggestion-mode/auto-save.ts index 60d92b20eaf692..e8988a3f49b1a1 100644 --- a/packages/editor/src/components/suggestion-mode/auto-save.ts +++ b/packages/editor/src/components/suggestion-mode/auto-save.ts @@ -40,7 +40,7 @@ import { useCallback, useEffect, useRef } from '@wordpress/element'; import { useSuggestionOverlay } from './overlay-context'; import type { OverlayEntry, SuggestionOperation } from './overlay-context'; import { operationsFromOverlay, useSuggestionsProvider } from './provider'; -import { EDITOR_STORE_NAME, SUGGEST_INTENT } from './constants'; +import { STORE_NAME, EDITOR_INTENT_SUGGEST } from '../../store/constants'; import { unlock } from '../../lock-unlock'; const AUTOSAVE_DEBOUNCE_MS = 1500; @@ -112,8 +112,8 @@ export default function SuggestionAutoSave() { const isSuggestMode = useSelect( ( select ) => // `getEditorIntent` is private while Suggest mode is experimental. - unlock( select( EDITOR_STORE_NAME ) ).getEditorIntent() === - SUGGEST_INTENT, + unlock( select( STORE_NAME ) ).getEditorIntent() === + EDITOR_INTENT_SUGGEST, [] ); diff --git a/packages/editor/src/components/suggestion-mode/provider.ts b/packages/editor/src/components/suggestion-mode/provider.ts index 0030876321414c..38f5ae457ef429 100644 --- a/packages/editor/src/components/suggestion-mode/provider.ts +++ b/packages/editor/src/components/suggestion-mode/provider.ts @@ -7,7 +7,7 @@ import { store as blockEditorStore } from '@wordpress/block-editor'; import { store as interfaceStore } from '@wordpress/interface'; import { store as noticesStore } from '@wordpress/notices'; import { __ } from '@wordpress/i18n'; -import { EDITOR_STORE_NAME } from './constants'; +import { STORE_NAME } from '../../store/constants'; import { useSuggestionOverlay } from './overlay-context'; import type { SuggestionOperation } from './overlay-context'; import { @@ -438,7 +438,7 @@ function withDecisionInFlight< Args extends { commentId?: number | string } >( */ export function useSuggestionsProvider() { const { postId, postModified } = useSelect( ( select ) => { - const editor: any = select( EDITOR_STORE_NAME ); + const editor: any = select( STORE_NAME ); const id = editor?.getCurrentPostId?.() ?? null; const postType = editor?.getCurrentPostType?.() ?? null; const record = From c7782413868ba2315bdfb83d97768ca2b5a699aa Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Tue, 25 Aug 2026 15:53:53 -0700 Subject: [PATCH 07/23] Move the storage-layer review fixes into the storage layer Carry the final overlay-context module and its tests into the layer that introduces the suggestion overlay, so the storage PR reviews the context API it defines rather than an earlier draft of it. --- .../suggestion-mode/overlay-context.tsx | 53 ++++++++++++++ .../suggestion-mode/test/auto-save.tsx | 5 ++ .../suggestion-mode/test/overlay-context.ts | 71 +++++++++++++++++++ 3 files changed, 129 insertions(+) diff --git a/packages/editor/src/components/suggestion-mode/overlay-context.tsx b/packages/editor/src/components/suggestion-mode/overlay-context.tsx index 6a63b282e5cacb..f919fa27f50f0f 100644 --- a/packages/editor/src/components/suggestion-mode/overlay-context.tsx +++ b/packages/editor/src/components/suggestion-mode/overlay-context.tsx @@ -108,6 +108,18 @@ export type OverlayEntries = Record< string, OverlayEntry >; */ type SuggestionRequestHandler = ( request: any ) => unknown; +/** + * A block can hold more than one pending suggestion at a time — an + * attribute-set overlay (heading level) alongside inline markers in the same + * block's content, which is a legitimate combination because the two describe + * disjoint parts of the block. The overlay entry belongs to whichever + * suggestion opened it, so a decision on one of the block's other suggestions + * must leave it alone. `clearOverlayForComment` is the guarded clear used by + * those paths: it removes the entry only when the entry is the one linked to + * the comment being resolved. See finding F-14 in the Suggest mode testing + * document. + */ + export interface OverlayContextValue { entries: OverlayEntries; captureBaseline: ( @@ -120,6 +132,10 @@ export interface OverlayContextValue { attributes: Record< string, any > ) => void; clearOverlay: ( clientId: string ) => void; + clearOverlayForComment: ( + clientId: string, + commentId: number | string | null | undefined + ) => void; setCommentId: ( clientId: string, commentId: number | null ) => void; setSyncedOpsKey: ( clientId: string, syncedOpsKey: string | null ) => void; setStructuralOp: ( @@ -161,6 +177,11 @@ type OverlayAction = seq?: number; } | { type: 'CLEAR_OVERLAY'; clientId: string } + | { + type: 'CLEAR_OVERLAY_FOR_COMMENT'; + clientId: string; + commentId: number | string | null | undefined; + } | { type: 'SET_COMMENT_ID'; clientId: string; @@ -190,6 +211,7 @@ const OverlayContext = createContext< OverlayContextValue >( { captureBaseline: () => {}, setOverlayAttributes: () => {}, clearOverlay: () => {}, + clearOverlayForComment: () => {}, setCommentId: () => {}, setSyncedOpsKey: () => {}, setStructuralOp: () => {}, @@ -266,6 +288,25 @@ export function overlayReducer( const { [ action.clientId ]: _removed, ...rest } = state; return rest; } + case 'CLEAR_OVERLAY_FOR_COMMENT': { + /* + * Guarded clear for decision paths that act on a suggestion which + * does not live in the overlay (an inline marker). The block may + * still hold an unrelated pending attribute suggestion, and that + * entry is the only anchor keeping its note alive — dropping it + * would send the note to the garbage collector and take the + * proposed value off the canvas with it. + */ + const entry = state[ action.clientId ]; + if ( entry?.commentId === null || entry?.commentId === undefined ) { + return state; + } + if ( String( entry.commentId ) !== String( action.commentId ) ) { + return state; + } + const { [ action.clientId ]: _dropped, ...remaining } = state; + return remaining; + } case 'SET_COMMENT_ID': { const entry = state[ action.clientId ]; if ( ! entry ) { @@ -386,6 +427,16 @@ export function SuggestionOverlayProvider( { [] ); + const clearOverlayForComment = useCallback( + ( clientId: string, commentId: number | string | null | undefined ) => + dispatch( { + type: 'CLEAR_OVERLAY_FOR_COMMENT', + clientId, + commentId, + } ), + [] + ); + const setCommentId = useCallback( ( clientId: string, commentId: number | null ) => dispatch( { type: 'SET_COMMENT_ID', clientId, commentId } ), @@ -659,6 +710,7 @@ export function SuggestionOverlayProvider( { captureBaseline, setOverlayAttributes, clearOverlay, + clearOverlayForComment, setCommentId, setSyncedOpsKey, setStructuralOp, @@ -683,6 +735,7 @@ export function SuggestionOverlayProvider( { captureBaseline, setOverlayAttributes, clearOverlay, + clearOverlayForComment, setCommentId, setSyncedOpsKey, setStructuralOp, diff --git a/packages/editor/src/components/suggestion-mode/test/auto-save.tsx b/packages/editor/src/components/suggestion-mode/test/auto-save.tsx index 46ad26f94c17cb..d4c04338d6c383 100644 --- a/packages/editor/src/components/suggestion-mode/test/auto-save.tsx +++ b/packages/editor/src/components/suggestion-mode/test/auto-save.tsx @@ -2,6 +2,7 @@ import { render, act } from '@testing-library/react'; import { createRegistry, RegistryProvider } from '@wordpress/data'; import { store as coreStore } from '@wordpress/core-data'; import { store as noticesStore } from '@wordpress/notices'; +import { store as preferencesStore } from '@wordpress/preferences'; import SuggestionAutoSave, { operationsForEntry } from '../auto-save'; import { SuggestionOverlayProvider, @@ -46,6 +47,9 @@ function renderInSuggestMode( ui: React.ReactElement ) { const registry = createRegistry(); registry.register( noticesStore ); registry.register( coreStore ); + // `setEditorIntent` compares the editor mode across the change so it can + // announce a canvas swap, and `getEditorMode` reads the preferences store. + registry.register( preferencesStore ); registry.register( editorStore ); unlock( registry.dispatch( editorStore ) ).setEditorIntent( 'suggest' ); @@ -398,6 +402,7 @@ describe( 'SuggestionAutoSave', () => { it( 'does nothing when the editor is not in Suggest intent', async () => { const registry = createRegistry(); registry.register( noticesStore ); + registry.register( preferencesStore ); registry.register( editorStore ); unlock( registry.dispatch( editorStore ) ).setEditorIntent( 'edit' ); diff --git a/packages/editor/src/components/suggestion-mode/test/overlay-context.ts b/packages/editor/src/components/suggestion-mode/test/overlay-context.ts index 3365473fd6a543..26e4b699ba3087 100644 --- a/packages/editor/src/components/suggestion-mode/test/overlay-context.ts +++ b/packages/editor/src/components/suggestion-mode/test/overlay-context.ts @@ -83,6 +83,77 @@ describe( 'overlayReducer', () => { expect( cleared ).not.toBe( withEntry ); } ); + describe( 'CLEAR_OVERLAY_FOR_COMMENT', () => { + const withComment = ( commentId: number | string ) => { + const withEntry = overlayReducer( INITIAL, { + type: 'CAPTURE_BASELINE', + clientId: CLIENT_ID, + blockName: 'core/heading', + attributes: { level: 2 }, + } ); + return overlayReducer( withEntry, { + type: 'SET_COMMENT_ID', + clientId: CLIENT_ID, + commentId: commentId as any, + } ); + }; + + it( 'removes the entry when the comment owns it', () => { + const state = withComment( 42 ); + const cleared = overlayReducer( state, { + type: 'CLEAR_OVERLAY_FOR_COMMENT', + clientId: CLIENT_ID, + commentId: 42, + } ); + expect( cleared ).toEqual( {} ); + } ); + + it( 'matches across the string/number boundary', () => { + const state = withComment( 42 ); + const cleared = overlayReducer( state, { + type: 'CLEAR_OVERLAY_FOR_COMMENT', + clientId: CLIENT_ID, + commentId: '42', + } ); + expect( cleared ).toEqual( {} ); + } ); + + it( 'keeps an entry that belongs to another suggestion', () => { + const state = withComment( 42 ); + const next = overlayReducer( state, { + type: 'CLEAR_OVERLAY_FOR_COMMENT', + clientId: CLIENT_ID, + commentId: 43, + } ); + expect( next ).toBe( state ); + expect( next[ CLIENT_ID ].commentId ).toBe( 42 ); + } ); + + it( 'keeps an entry that has no comment yet', () => { + const state = overlayReducer( INITIAL, { + type: 'CAPTURE_BASELINE', + clientId: CLIENT_ID, + blockName: 'core/heading', + attributes: { level: 2 }, + } ); + const next = overlayReducer( state, { + type: 'CLEAR_OVERLAY_FOR_COMMENT', + clientId: CLIENT_ID, + commentId: 42, + } ); + expect( next ).toBe( state ); + } ); + + it( 'is a no-op when the block has no entry', () => { + const next = overlayReducer( INITIAL, { + type: 'CLEAR_OVERLAY_FOR_COMMENT', + clientId: CLIENT_ID, + commentId: 42, + } ); + expect( next ).toBe( INITIAL ); + } ); + } ); + it( 'returns the same reference for unknown actions', () => { const next = overlayReducer( INITIAL, { type: 'UNKNOWN' } as any ); expect( next ).toBe( INITIAL ); From ec0f118d4942d4f064b1b5df5bdaf76608691b88 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Mon, 31 Aug 2026 16:24:06 -0700 Subject: [PATCH 08/23] Name DOM unit tests for the jsdom Jest project Trunk splits the Jest run into node and jsdom projects, and only *.jsdom.test.* files are collected by the jsdom one, so these suites failed with 'window is not defined' after the trunk merge. Carry the auto-save ESLint suppression across the rename. --- .../test/{auto-save.tsx => auto-save.jsdom.test.tsx} | 0 .../test/{provider.tsx => provider.jsdom.test.tsx} | 0 tools/eslint/suppressions.json | 2 +- 3 files changed, 1 insertion(+), 1 deletion(-) rename packages/editor/src/components/suggestion-mode/test/{auto-save.tsx => auto-save.jsdom.test.tsx} (100%) rename packages/editor/src/components/suggestion-mode/test/{provider.tsx => provider.jsdom.test.tsx} (100%) diff --git a/packages/editor/src/components/suggestion-mode/test/auto-save.tsx b/packages/editor/src/components/suggestion-mode/test/auto-save.jsdom.test.tsx similarity index 100% rename from packages/editor/src/components/suggestion-mode/test/auto-save.tsx rename to packages/editor/src/components/suggestion-mode/test/auto-save.jsdom.test.tsx diff --git a/packages/editor/src/components/suggestion-mode/test/provider.tsx b/packages/editor/src/components/suggestion-mode/test/provider.jsdom.test.tsx similarity index 100% rename from packages/editor/src/components/suggestion-mode/test/provider.tsx rename to packages/editor/src/components/suggestion-mode/test/provider.jsdom.test.tsx diff --git a/tools/eslint/suppressions.json b/tools/eslint/suppressions.json index 89e0e2d0056f31..7b8887f58231f7 100644 --- a/tools/eslint/suppressions.json +++ b/tools/eslint/suppressions.json @@ -7116,7 +7116,7 @@ "count": 6 } }, - "packages/editor/src/components/suggestion-mode/test/auto-save.tsx": { + "packages/editor/src/components/suggestion-mode/test/auto-save.jsdom.test.tsx": { "react-hooks/globals": { "count": 1 } From 15093baa1ab6430b5ecf07935758e92d2b01975f Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Fri, 4 Sep 2026 09:13:46 -0700 Subject: [PATCH 09/23] Type the interface store import and the plugin global without directives The Vitest convention validator type checks a test's import graph with '@wordpress/interface' declared as a module and the gutenberg-env globals loaded, so the @ts-expect-error directives the editor package needed for both became unused errors there while staying required in the package build. Map the interface package to a typing stub, as commands already is, and give the editor tsconfigs the gutenberg-env types, so neither config needs a directive. --- .../editor/src/components/suggestion-mode/provider.ts | 1 - packages/editor/src/dataviews/store/private-actions.ts | 1 - packages/editor/tsconfig.build.json | 2 +- packages/editor/tsconfig.json | 2 +- tsconfig.base.json | 2 +- typings/untyped-packages/wordpress-interface.d.ts | 10 ++++++++++ 6 files changed, 13 insertions(+), 5 deletions(-) create mode 100644 typings/untyped-packages/wordpress-interface.d.ts diff --git a/packages/editor/src/components/suggestion-mode/provider.ts b/packages/editor/src/components/suggestion-mode/provider.ts index 38f5ae457ef429..0072ed73794dec 100644 --- a/packages/editor/src/components/suggestion-mode/provider.ts +++ b/packages/editor/src/components/suggestion-mode/provider.ts @@ -3,7 +3,6 @@ import { useDispatch, useRegistry, useSelect } from '@wordpress/data'; import { store as coreStore } from '@wordpress/core-data'; // @ts-expect-error No exported types import { store as blockEditorStore } from '@wordpress/block-editor'; -// @ts-expect-error No exported types import { store as interfaceStore } from '@wordpress/interface'; import { store as noticesStore } from '@wordpress/notices'; import { __ } from '@wordpress/i18n'; diff --git a/packages/editor/src/dataviews/store/private-actions.ts b/packages/editor/src/dataviews/store/private-actions.ts index 7cc271e6599890..4bc2e2241ad82d 100644 --- a/packages/editor/src/dataviews/store/private-actions.ts +++ b/packages/editor/src/dataviews/store/private-actions.ts @@ -204,7 +204,6 @@ export const registerPostTypeSchema = canCreate && duplicatePost; - // @ts-expect-error `globalThis` has no index signature for this build-time global. if ( ! globalThis.IS_GUTENBERG_PLUGIN ) { // Outside Gutenberg, disable duplication. canDuplicate = undefined; diff --git a/packages/editor/tsconfig.build.json b/packages/editor/tsconfig.build.json index cfccaef3819363..83b6ed1a686a32 100644 --- a/packages/editor/tsconfig.build.json +++ b/packages/editor/tsconfig.build.json @@ -2,7 +2,7 @@ "$schema": "https://json.schemastore.org/tsconfig.json", "extends": "../../tsconfig.base.json", "compilerOptions": { - "types": [ "style-imports", "react-css-custom-properties" ], + "types": [ "style-imports", "react-css-custom-properties", "gutenberg-env" ], "checkJs": false }, "references": [ diff --git a/packages/editor/tsconfig.json b/packages/editor/tsconfig.json index 4032ea781b5f2f..297af63cfea5a0 100644 --- a/packages/editor/tsconfig.json +++ b/packages/editor/tsconfig.json @@ -2,7 +2,7 @@ "$schema": "https://json.schemastore.org/tsconfig.json", "extends": "../../tsconfig.dev.base.json", "compilerOptions": { - "types": [ "jest", "style-imports", "react-css-custom-properties" ] + "types": [ "jest", "style-imports", "react-css-custom-properties", "gutenberg-env" ] }, "files": [ "global.d.ts" ], "references": [ diff --git a/tsconfig.base.json b/tsconfig.base.json index 3d1391acbb7df4..f6d62fb1bfddbd 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -16,7 +16,7 @@ "./typings/untyped-packages/any-module.d.ts" ], "@wordpress/interface": [ - "./typings/untyped-packages/any-module.d.ts" + "./typings/untyped-packages/wordpress-interface.d.ts" ] }, "checkJs": true, diff --git a/typings/untyped-packages/wordpress-interface.d.ts b/typings/untyped-packages/wordpress-interface.d.ts new file mode 100644 index 00000000000000..e810da89a8cbeb --- /dev/null +++ b/typings/untyped-packages/wordpress-interface.d.ts @@ -0,0 +1,10 @@ +/* + * `@wordpress/interface` ships untyped JavaScript resolved through gitignored + * build artifacts; the base tsconfig maps the package here so type checking + * never depends on those artifacts existing. + */ +declare module '@wordpress/interface' { + import type { StoreDescriptor } from '@wordpress/data'; + + export const store: StoreDescriptor; +} From fb2285f777fdcac5fb0c019ca0a475ebd971659e Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Fri, 4 Sep 2026 09:13:46 -0700 Subject: [PATCH 10/23] Run the data layer's unit tests under Vitest Import the test API explicitly, hoist the provider mock's functions with vi.hoisted, and opt in to the matchMedia mock the viewport package needs while loading. --- .../test/auto-save.jsdom.test.tsx | 75 +++++++++++-------- .../suggestion-mode/test/overlay-context.ts | 1 + .../test/provider.jsdom.test.tsx | 7 ++ .../test/suggestion-write-queue.ts | 1 + 4 files changed, 52 insertions(+), 32 deletions(-) diff --git a/packages/editor/src/components/suggestion-mode/test/auto-save.jsdom.test.tsx b/packages/editor/src/components/suggestion-mode/test/auto-save.jsdom.test.tsx index d4c04338d6c383..18bfbffcd770bc 100644 --- a/packages/editor/src/components/suggestion-mode/test/auto-save.jsdom.test.tsx +++ b/packages/editor/src/components/suggestion-mode/test/auto-save.jsdom.test.tsx @@ -1,3 +1,4 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { render, act } from '@testing-library/react'; import { createRegistry, RegistryProvider } from '@wordpress/data'; import { store as coreStore } from '@wordpress/core-data'; @@ -11,36 +12,46 @@ import { import { store as editorStore } from '../../../store'; import { unlock } from '../../../lock-unlock'; -// jest.mock factories may only reference variables prefixed with `mock`. -const mockCreateSuggestion = jest.fn(); -const mockUpdateSuggestion = jest.fn(); -const mockDeleteSuggestion = jest.fn(); +// The editor store pulls in `@wordpress/viewport`, which reads +// `window.matchMedia` while loading. +vi.hoisted( () => { + globalThis.wpVitest.mockMatchMedia(); +} ); -jest.mock( '../provider', () => { - const actual = jest.requireActual( '../provider' ); +// The mock factory is hoisted above the imports, so the functions it hands +// out have to be created there too. +const { createSuggestion, updateSuggestion, deleteSuggestion } = vi.hoisted( + () => ( { + createSuggestion: vi.fn(), + updateSuggestion: vi.fn(), + deleteSuggestion: vi.fn(), + } ) +); + +vi.mock( import( '../provider' ), async ( importOriginal ) => { + const actual = await importOriginal(); return { ...actual, - useSuggestionsProvider: () => ( { - createSuggestion: mockCreateSuggestion, - updateSuggestion: mockUpdateSuggestion, - deleteSuggestion: mockDeleteSuggestion, - } ), + useSuggestionsProvider: () => + ( { + createSuggestion, + updateSuggestion, + deleteSuggestion, + } ) as unknown as ReturnType< + typeof actual.useSuggestionsProvider + >, }; } ); -const createSuggestion = mockCreateSuggestion; -const updateSuggestion = mockUpdateSuggestion; -const deleteSuggestion = mockDeleteSuggestion; - beforeEach( () => { createSuggestion.mockReset(); updateSuggestion.mockReset(); deleteSuggestion.mockReset(); - jest.useFakeTimers(); + vi.useFakeTimers(); } ); afterEach( () => { - jest.useRealTimers(); + vi.useRealTimers(); } ); function renderInSuggestMode( ui: React.ReactElement ) { @@ -108,7 +119,7 @@ describe( 'SuggestionAutoSave', () => { expect( createSuggestion ).not.toHaveBeenCalled(); await act( async () => { - jest.advanceTimersByTime( 1500 ); + vi.advanceTimersByTime( 1500 ); } ); await flushPromises(); await flushPromises(); @@ -144,7 +155,7 @@ describe( 'SuggestionAutoSave', () => { } ); await act( async () => { - jest.advanceTimersByTime( 1500 ); + vi.advanceTimersByTime( 1500 ); } ); await flushPromises(); await flushPromises(); @@ -159,7 +170,7 @@ describe( 'SuggestionAutoSave', () => { } ); await act( async () => { - jest.advanceTimersByTime( 1500 ); + vi.advanceTimersByTime( 1500 ); } ); await flushPromises(); await flushPromises(); @@ -197,7 +208,7 @@ describe( 'SuggestionAutoSave', () => { } ); await act( async () => { - jest.advanceTimersByTime( 1500 ); + vi.advanceTimersByTime( 1500 ); } ); await flushPromises(); await flushPromises(); @@ -208,7 +219,7 @@ describe( 'SuggestionAutoSave', () => { } ); await act( async () => { - jest.advanceTimersByTime( 1500 ); + vi.advanceTimersByTime( 1500 ); } ); await flushPromises(); await flushPromises(); @@ -243,7 +254,7 @@ describe( 'SuggestionAutoSave', () => { } ); await act( async () => { - jest.advanceTimersByTime( 1500 ); + vi.advanceTimersByTime( 1500 ); } ); await flushPromises(); @@ -255,7 +266,7 @@ describe( 'SuggestionAutoSave', () => { } ); await act( async () => { - jest.advanceTimersByTime( 1500 ); + vi.advanceTimersByTime( 1500 ); } ); await flushPromises(); @@ -310,7 +321,7 @@ describe( 'SuggestionAutoSave', () => { } ); await act( async () => { - jest.advanceTimersByTime( 1500 ); + vi.advanceTimersByTime( 1500 ); } ); await flushPromises(); await flushPromises(); @@ -329,7 +340,7 @@ describe( 'SuggestionAutoSave', () => { } ); await act( async () => { - jest.advanceTimersByTime( 1500 ); + vi.advanceTimersByTime( 1500 ); } ); await flushPromises(); await flushPromises(); @@ -370,7 +381,7 @@ describe( 'SuggestionAutoSave', () => { } ); await act( async () => { - jest.advanceTimersByTime( 1500 ); + vi.advanceTimersByTime( 1500 ); } ); await flushPromises(); await flushPromises(); @@ -387,7 +398,7 @@ describe( 'SuggestionAutoSave', () => { } ); await act( async () => { - jest.advanceTimersByTime( 1500 ); + vi.advanceTimersByTime( 1500 ); } ); await flushPromises(); await flushPromises(); @@ -430,7 +441,7 @@ describe( 'SuggestionAutoSave', () => { } ); await act( async () => { - jest.advanceTimersByTime( 5000 ); + vi.advanceTimersByTime( 5000 ); } ); await flushPromises(); @@ -456,7 +467,7 @@ describe( 'SuggestionAutoSave', () => { // Leave Suggest mode mid-debounce. await act( async () => { - jest.advanceTimersByTime( 500 ); + vi.advanceTimersByTime( 500 ); } ); act( () => { unlock( registry.dispatch( editorStore ) ).setEditorIntent( @@ -465,7 +476,7 @@ describe( 'SuggestionAutoSave', () => { } ); await act( async () => { - jest.advanceTimersByTime( 5000 ); + vi.advanceTimersByTime( 5000 ); } ); await flushPromises(); @@ -479,7 +490,7 @@ describe( 'SuggestionAutoSave', () => { } ); await act( async () => { - jest.advanceTimersByTime( 1500 ); + vi.advanceTimersByTime( 1500 ); } ); await flushPromises(); await flushPromises(); diff --git a/packages/editor/src/components/suggestion-mode/test/overlay-context.ts b/packages/editor/src/components/suggestion-mode/test/overlay-context.ts index 26e4b699ba3087..9ee356f2688094 100644 --- a/packages/editor/src/components/suggestion-mode/test/overlay-context.ts +++ b/packages/editor/src/components/suggestion-mode/test/overlay-context.ts @@ -1,3 +1,4 @@ +import { describe, expect, it } from 'vitest'; import { overlayReducer } from '../overlay-context'; describe( 'overlayReducer', () => { diff --git a/packages/editor/src/components/suggestion-mode/test/provider.jsdom.test.tsx b/packages/editor/src/components/suggestion-mode/test/provider.jsdom.test.tsx index 73de7bd6e3c102..8012fb215fdb92 100644 --- a/packages/editor/src/components/suggestion-mode/test/provider.jsdom.test.tsx +++ b/packages/editor/src/components/suggestion-mode/test/provider.jsdom.test.tsx @@ -1,3 +1,4 @@ +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; import { render, act } from '@testing-library/react'; import { createRegistry, @@ -25,6 +26,12 @@ import { useSuggestionsProvider, } from '../provider'; +// The editor store pulls in `@wordpress/viewport`, which reads +// `window.matchMedia` while loading. +vi.hoisted( () => { + globalThis.wpVitest.mockMatchMedia(); +} ); + describe( 'operationsFromOverlay', () => { it( 'emits one attribute-set op per changed key', () => { const ops = operationsFromOverlay( diff --git a/packages/editor/src/components/suggestion-mode/test/suggestion-write-queue.ts b/packages/editor/src/components/suggestion-mode/test/suggestion-write-queue.ts index 8118001fdb7606..2fb69030c47705 100644 --- a/packages/editor/src/components/suggestion-mode/test/suggestion-write-queue.ts +++ b/packages/editor/src/components/suggestion-mode/test/suggestion-write-queue.ts @@ -1,3 +1,4 @@ +import { describe, expect, it } from 'vitest'; import { createSuggestionWriteQueue } from '../suggestion-write-queue'; /** Create a promise whose resolution the test controls. */ From 5487e49f09b3970aeb911b463eebb367b8cdfb47 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Fri, 11 Sep 2026 16:38:03 -0700 Subject: [PATCH 11/23] Suggest mode: drop the `@wordpress/interface` typings shim The shim declared the store as `any` because the package shipped untyped JavaScript resolved through gitignored build artifacts. #82753 typed the package, so the shim is unreferenced and would only hide the real types. Claude-Session: https://claude.ai/code/session_01YXYKyujAzMtLoT6QMxquHd --- typings/untyped-packages/wordpress-interface.d.ts | 10 ---------- 1 file changed, 10 deletions(-) delete mode 100644 typings/untyped-packages/wordpress-interface.d.ts diff --git a/typings/untyped-packages/wordpress-interface.d.ts b/typings/untyped-packages/wordpress-interface.d.ts deleted file mode 100644 index e810da89a8cbeb..00000000000000 --- a/typings/untyped-packages/wordpress-interface.d.ts +++ /dev/null @@ -1,10 +0,0 @@ -/* - * `@wordpress/interface` ships untyped JavaScript resolved through gitignored - * build artifacts; the base tsconfig maps the package here so type checking - * never depends on those artifacts existing. - */ -declare module '@wordpress/interface' { - import type { StoreDescriptor } from '@wordpress/data'; - - export const store: StoreDescriptor; -} From d8b1597826151937132f21aa39d0b1b236f59468 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Wed, 16 Sep 2026 15:53:48 -0700 Subject: [PATCH 12/23] Suggest mode: reformat for Prettier 3.9.6 Trunk bumped Prettier from 3.0.3 to 3.9.6, which indents these two continuation lines differently. Formatting only, no behavior change. Claude-Session: https://claude.ai/code/session_015VuPwr3z18cAox2kB4wp2H --- .../src/components/suggestion-mode/overlay-context.tsx | 5 +++-- packages/editor/src/components/suggestion-mode/provider.ts | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/editor/src/components/suggestion-mode/overlay-context.tsx b/packages/editor/src/components/suggestion-mode/overlay-context.tsx index f919fa27f50f0f..611953990f6fa2 100644 --- a/packages/editor/src/components/suggestion-mode/overlay-context.tsx +++ b/packages/editor/src/components/suggestion-mode/overlay-context.tsx @@ -689,8 +689,9 @@ export function SuggestionOverlayProvider( { if ( ! hasEntries ) { return; } - const getLive = registry.select( BLOCK_EDITOR_STORE_NAME ) - ?.getClientIdsWithDescendants; + const getLive = registry.select( + BLOCK_EDITOR_STORE_NAME + )?.getClientIdsWithDescendants; if ( ! getLive ) { return; } diff --git a/packages/editor/src/components/suggestion-mode/provider.ts b/packages/editor/src/components/suggestion-mode/provider.ts index 0072ed73794dec..bb2b417b729e36 100644 --- a/packages/editor/src/components/suggestion-mode/provider.ts +++ b/packages/editor/src/components/suggestion-mode/provider.ts @@ -446,7 +446,7 @@ export function useSuggestionsProvider() { 'postType', postType, id - ) + ) : null; return { postId: id, From e1998eeeea36957fc8b35466d9b34c62d8ea4d48 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Mon, 28 Sep 2026 22:42:45 -0700 Subject: [PATCH 13/23] Suggest mode: clear the overlay when rejecting an attribute suggestion An attribute-only suggestion has no structural op, so rejecting it skipped every clearOverlay() call. The overlay entry kept rendering the rejected value and carried it into the next proposal. Clear it with the guarded clearOverlayForComment() so an entry already reused by a newer suggestion survives. Claude-Session: https://claude.ai/code/session_01EmSXGAGtuLnk3UEe5kXPSU --- .../components/suggestion-mode/provider.ts | 17 ++- .../test/provider.jsdom.test.tsx | 135 ++++++++++++++++++ 2 files changed, 150 insertions(+), 2 deletions(-) diff --git a/packages/editor/src/components/suggestion-mode/provider.ts b/packages/editor/src/components/suggestion-mode/provider.ts index bb2b417b729e36..6914b2b1348de6 100644 --- a/packages/editor/src/components/suggestion-mode/provider.ts +++ b/packages/editor/src/components/suggestion-mode/provider.ts @@ -469,7 +469,8 @@ export function useSuggestionsProvider() { getBlockRootClientId: selectBlockRootClientId, getClientIdsWithDescendants: selectClientIdsWithDescendants, } = useSelect( blockEditorStore ); - const { requestInterceptorBypass, clearOverlay } = useSuggestionOverlay(); + const { requestInterceptorBypass, clearOverlay, clearOverlayForComment } = + useSuggestionOverlay(); const registry = useRegistry(); const createSuggestion = useCallback( @@ -970,7 +971,8 @@ export function useSuggestionsProvider() { // - block-move: clear the marker, then dispatch // moveBlockToPosition to put the block back at its // pre-move parent + index. - // - attribute-set (no structural op): no live-block change. + // - attribute-set (no structural op): no live-block change; the + // overlay entry holding the proposed value is cleared. const structuralOp = findStructuralOp( payload?.operations ); try { @@ -1045,6 +1047,16 @@ export function useSuggestionsProvider() { } clearOverlay( clientId ); } + } else if ( clientId ) { + /* + * An attribute-only suggestion lives entirely in the + * overlay: the live block never took the proposed value, + * so there is nothing to roll back, but the overlay entry + * must go or it keeps rendering the rejected value and + * feeds it into the next proposal. Guarded, because the + * entry may already belong to a newer suggestion. + */ + clearOverlayForComment( clientId, commentId ); } createNotice( 'snackbar' as any, __( 'Suggestion rejected.' ), { @@ -1069,6 +1081,7 @@ export function useSuggestionsProvider() { moveBlockToPosition, requestInterceptorBypass, clearOverlay, + clearOverlayForComment, registry, ] ); diff --git a/packages/editor/src/components/suggestion-mode/test/provider.jsdom.test.tsx b/packages/editor/src/components/suggestion-mode/test/provider.jsdom.test.tsx index 8012fb215fdb92..81f74594022d8c 100644 --- a/packages/editor/src/components/suggestion-mode/test/provider.jsdom.test.tsx +++ b/packages/editor/src/components/suggestion-mode/test/provider.jsdom.test.tsx @@ -25,6 +25,10 @@ import { clearSuggestionMarkerAttributes, useSuggestionsProvider, } from '../provider'; +import { + SuggestionOverlayProvider, + useSuggestionOverlay, +} from '../overlay-context'; // The editor store pulls in `@wordpress/viewport`, which reads // `window.matchMedia` while loading. @@ -677,6 +681,137 @@ describe( 'rejectSuggestion (block-move)', () => { } ); } ); +describe( 'rejectSuggestion (attribute-set)', () => { + const PARAGRAPH = 'core/test-reject-attribute-paragraph'; + + beforeAll( () => { + registerBlockType( PARAGRAPH, { + apiVersion: 3, + attributes: { + content: { type: 'string', default: '' }, + align: { type: 'string' }, + metadata: { type: 'object' }, + }, + save: () => null, + category: 'text', + title: 'Test Reject Attribute Paragraph', + } ); + } ); + + afterAll( () => { + getBlockTypes().forEach( ( block ) => + unregisterBlockType( block.name ) + ); + } ); + + function setup( initialBlocks: any[] ) { + const registry = createRegistry(); + registry.register( noticesStore ); + registry.register( blockEditorStore ); + registry.register( + createReduxStore( 'core', { + reducer: ( state = {} ) => state, + actions: { + saveEntityRecord: () => ( { type: 'SAVE_ENTITY_RECORD' } ), + }, + selectors: { + getEditedEntityRecord: () => null, + getEntityRecord: () => null, + getCurrentUser: () => null, + }, + } ) + ); + registry.register( createStubInterfaceStore() ); + registry.dispatch( blockEditorStore ).resetBlocks( initialBlocks ); + + let providerHandle: ReturnType< typeof useSuggestionsProvider >; + let overlayHandle: ReturnType< typeof useSuggestionOverlay >; + function Capture() { + providerHandle = useSuggestionsProvider(); + overlayHandle = useSuggestionOverlay(); + return null; + } + + render( + + + + + + ); + + return { + getProvider: () => providerHandle, + getOverlay: () => overlayHandle, + }; + } + + function attributePayload() { + return { + schemaVersion: 2, + blockName: PARAGRAPH, + baseRevision: null, + operations: [ + { + type: 'attribute-set', + attribute: 'align', + before: null, + after: 'center', + }, + ], + }; + } + + function proposeAlignment( + getOverlay: () => ReturnType< typeof useSuggestionOverlay >, + clientId: string, + commentId: number + ) { + act( () => { + getOverlay().captureBaseline( clientId, PARAGRAPH, { + content: 'Hello', + } ); + } ); + act( () => { + getOverlay().setOverlayAttributes( clientId, { align: 'center' } ); + getOverlay().setCommentId( clientId, commentId ); + } ); + } + + it( 'drops the overlay entry so the rejected value stops rendering', async () => { + const block = createBlock( PARAGRAPH, { content: 'Hello' } ); + const { getProvider, getOverlay } = setup( [ block ] ); + proposeAlignment( getOverlay, block.clientId, 7 ); + expect( getOverlay().hasOverlay( block.clientId ) ).toBe( true ); + + await act( async () => { + await getProvider().rejectSuggestion( { + commentId: 7, + clientId: block.clientId, + payload: attributePayload(), + } ); + } ); + + expect( getOverlay().hasOverlay( block.clientId ) ).toBe( false ); + } ); + + it( 'keeps an overlay entry that now belongs to another suggestion', async () => { + const block = createBlock( PARAGRAPH, { content: 'Hello' } ); + const { getProvider, getOverlay } = setup( [ block ] ); + proposeAlignment( getOverlay, block.clientId, 8 ); + + await act( async () => { + await getProvider().rejectSuggestion( { + commentId: 7, + clientId: block.clientId, + payload: attributePayload(), + } ); + } ); + + expect( getOverlay().hasOverlay( block.clientId ) ).toBe( true ); + } ); +} ); + describe( 'decision failures leave the block tree untouched', () => { const PARAGRAPH = 'core/test-failure-paragraph'; From ceb3997222505dcfee202e04bf3a7b11e875d194 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Mon, 28 Sep 2026 22:45:23 -0700 Subject: [PATCH 14/23] Suggest mode: resolve the live parent when rejecting a move The restore used the recorded fromParentClientId as its destination, but client ids are regenerated on every parse, so rejecting even a same-parent move inside a Group after a reload targeted a parent that no longer exists. A same-parent move now restores within the block's live parent; a cross-parent move uses the recorded parent only while it still exists. Claude-Session: https://claude.ai/code/session_01EmSXGAGtuLnk3UEe5kXPSU --- .../components/suggestion-mode/provider.ts | 27 ++++++++++++-- .../test/provider.jsdom.test.tsx | 35 +++++++++++++++++++ 2 files changed, 60 insertions(+), 2 deletions(-) diff --git a/packages/editor/src/components/suggestion-mode/provider.ts b/packages/editor/src/components/suggestion-mode/provider.ts index 6914b2b1348de6..86b086ca2bfd42 100644 --- a/packages/editor/src/components/suggestion-mode/provider.ts +++ b/packages/editor/src/components/suggestion-mode/provider.ts @@ -1004,6 +1004,29 @@ export function useSuggestionsProvider() { const clearAttrs = clearSuggestionMarkerAttributes( selectBlockAttributes( clientId ) ); + const liveParent = + selectBlockRootClientId( clientId ) ?? ''; + /* + * The recorded parents are session-local client ids, + * regenerated whenever the post is parsed again. A move + * within one parent needs no recorded id at all: the + * block's live parent is the parent it came from. A move + * across parents restores to the recorded parent only + * while that block still exists; after a reload it + * cannot be resolved, so the block is restored within + * its current parent (a documented limitation). + */ + const recordedFrom = + structuralOp.fromParentClientId ?? ''; + const recordedTo = structuralOp.toParentClientId ?? ''; + let restoreParent = liveParent; + if ( + recordedFrom !== recordedTo && + ( recordedFrom === '' || + selectBlockAttributes( recordedFrom ) ) + ) { + restoreParent = recordedFrom; + } requestInterceptorBypass( clientId ); clearOverlay( clientId ); /* @@ -1032,8 +1055,8 @@ export function useSuggestionsProvider() { * no-op. `moveBlockToPosition` expects '' (not null) * for the root. */ - selectBlockRootClientId( clientId ) ?? '', - structuralOp.fromParentClientId ?? '', + liveParent, + restoreParent, structuralOp.fromIndex ?? 0 ); } ); diff --git a/packages/editor/src/components/suggestion-mode/test/provider.jsdom.test.tsx b/packages/editor/src/components/suggestion-mode/test/provider.jsdom.test.tsx index 81f74594022d8c..96a637d78fc296 100644 --- a/packages/editor/src/components/suggestion-mode/test/provider.jsdom.test.tsx +++ b/packages/editor/src/components/suggestion-mode/test/provider.jsdom.test.tsx @@ -679,6 +679,41 @@ describe( 'rejectSuggestion (block-move)', () => { '' ); } ); + + it( 'restores a nested same-parent move after a reload regenerated the ids', async () => { + const first = createBlock( PARAGRAPH, { content: 'First' } ); + const moved = createBlock( PARAGRAPH, { + content: 'Moved', + metadata: { suggestion: { type: 'pending-move' } }, + } ); + // Current order inside the group: [First, Moved]; the block was + // suggested-moved from index 0 in an earlier session. + const group = createBlock( GROUP, {}, [ first, moved ] ); + + const { registry, getProvider } = setup( [ group ] ); + + await act( async () => { + await getProvider().rejectSuggestion( { + commentId: 3, + clientId: moved.clientId, + payload: movePayload( { + type: 'block-move', + clientId: moved.clientId, + blockName: PARAGRAPH, + // The group's id from the session that recorded the move. + fromParentClientId: 'previous-session-group', + fromIndex: 0, + toParentClientId: 'previous-session-group', + } ), + } ); + } ); + + const blockEditor = registry.select( blockEditorStore ); + expect( blockEditor.getBlockRootClientId( moved.clientId ) ).toBe( + group.clientId + ); + expect( blockEditor.getBlockIndex( moved.clientId ) ).toBe( 0 ); + } ); } ); describe( 'rejectSuggestion (attribute-set)', () => { From e4fdd2fb76e86578cf1f0d260f0545e08a5676d7 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Mon, 28 Sep 2026 22:46:38 -0700 Subject: [PATCH 15/23] Suggest mode: flush pending suggestions when leaving Suggesting Leaving the suggest intent cleared every debounce timer, so a proposal made moments before switching to Editing lived only in React state and was lost on save and reload. Switching intents is a normal part of the review workflow, so enqueue the pending saves immediately instead. Claude-Session: https://claude.ai/code/session_01EmSXGAGtuLnk3UEe5kXPSU --- .../components/suggestion-mode/auto-save.ts | 18 ++++++++++-------- .../test/auto-save.jsdom.test.tsx | 18 +++++++----------- 2 files changed, 17 insertions(+), 19 deletions(-) diff --git a/packages/editor/src/components/suggestion-mode/auto-save.ts b/packages/editor/src/components/suggestion-mode/auto-save.ts index e8988a3f49b1a1..ae7040b49fd36f 100644 --- a/packages/editor/src/components/suggestion-mode/auto-save.ts +++ b/packages/editor/src/components/suggestion-mode/auto-save.ts @@ -255,17 +255,19 @@ export default function SuggestionAutoSave() { const timers = timersRef.current; /* - * Leaving Suggest mode cancels every pending debounce. The component - * stays mounted across intent changes (it is gated on the experiment - * flag, not the intent), so without this a timer scheduled moments - * before the switch still fires and POSTs a note for an edit the - * user walked away from. Cancelling is not lossy: the overlay entry - * keeps its unsynced fingerprint, so re-entering Suggest re-runs this - * effect and reschedules the save. + * Leaving Suggest mode flushes every pending debounce instead of + * waiting it out. The component stays mounted across intent changes + * (it is gated on the experiment flag, not the intent), and switching + * to Editing is a normal step in reviewing or publishing: the edit + * was made as a suggestion, so it must reach the server now. Dropping + * the timer instead would leave the proposal only in React state, + * lost on the next reload, and could leave a saved pending marker + * with no note behind it. */ if ( ! isSuggestMode ) { - for ( const timer of timers.values() ) { + for ( const [ clientId, timer ] of timers ) { clearTimeout( timer ); + enqueueSync( clientId ); } timers.clear(); return undefined; diff --git a/packages/editor/src/components/suggestion-mode/test/auto-save.jsdom.test.tsx b/packages/editor/src/components/suggestion-mode/test/auto-save.jsdom.test.tsx index 18bfbffcd770bc..8da1e5c2f60212 100644 --- a/packages/editor/src/components/suggestion-mode/test/auto-save.jsdom.test.tsx +++ b/packages/editor/src/components/suggestion-mode/test/auto-save.jsdom.test.tsx @@ -448,7 +448,7 @@ describe( 'SuggestionAutoSave', () => { expect( createSuggestion ).not.toHaveBeenCalled(); } ); - it( 'drops a pending save when the user leaves Suggest intent, and resumes it on return', async () => { + it( 'saves a pending suggestion right away when the user leaves Suggest intent', async () => { createSuggestion.mockResolvedValue( { id: 42 } ); const { registry } = renderInSuggestMode( @@ -465,7 +465,8 @@ describe( 'SuggestionAutoSave', () => { overlayHandle.setOverlayAttributes( 'a', { content: 'Hello' } ); } ); - // Leave Suggest mode mid-debounce. + // Leave Suggest mode mid-debounce. The suggestion was already made, + // so it must not wait for a return to Suggest intent to persist. await act( async () => { vi.advanceTimersByTime( 500 ); } ); @@ -474,26 +475,21 @@ describe( 'SuggestionAutoSave', () => { 'edit' ); } ); - - await act( async () => { - vi.advanceTimersByTime( 5000 ); - } ); + await flushPromises(); await flushPromises(); - expect( createSuggestion ).not.toHaveBeenCalled(); + expect( createSuggestion ).toHaveBeenCalledTimes( 1 ); - // Returning to Suggest reschedules the still-unsynced entry. + // Returning to Suggest does not save the same proposal twice. act( () => { unlock( registry.dispatch( editorStore ) ).setEditorIntent( 'suggest' ); } ); - await act( async () => { - vi.advanceTimersByTime( 1500 ); + vi.advanceTimersByTime( 5000 ); } ); await flushPromises(); - await flushPromises(); expect( createSuggestion ).toHaveBeenCalledTimes( 1 ); } ); From ce423659b97ca34082e663b48d9cc312fd7e79d4 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Tue, 29 Sep 2026 22:28:24 -0700 Subject: [PATCH 16/23] Suggestions: KSES every string leaf of applied payload values Structured `after` values such as table rows and snapshot attributes were stored unfiltered for users without unfiltered_html. Reuse core's filter_block_kses_value() so they get the same walk parsed block attributes get. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01YHz7zkCC2S8crriSWsYPDk --- .../wordpress-7.1/block-suggestions.php | 21 +++-- ...est-comments-controller-gutenberg-test.php | 83 +++++++++++++++++++ 2 files changed, 97 insertions(+), 7 deletions(-) diff --git a/lib/compat/wordpress-7.1/block-suggestions.php b/lib/compat/wordpress-7.1/block-suggestions.php index ab51db331f8c89..32ab0e2859c4ba 100644 --- a/lib/compat/wordpress-7.1/block-suggestions.php +++ b/lib/compat/wordpress-7.1/block-suggestions.php @@ -25,8 +25,10 @@ * block snapshot carried inside a suggestion operation (`op.block` on * `block-remove` / `block-insert-after` ops), recursing into `innerBlocks`. * - * Only `innerHTML` and `originalContent` are filtered: they are the fields a - * consumer turns back into markup when the block is re-inserted on accept. + * `innerHTML` and `originalContent` are the fields a consumer turns back + * into markup when the block is re-inserted. `attributes` get the same + * string-leaf walk core applies to parsed blocks, so no nested value + * escapes the filter. * * @param array $block Serialized block snapshot (decoded from JSON). * @return array Snapshot with HTML-bearing fields filtered. @@ -37,6 +39,9 @@ function gutenberg_kses_suggestion_block_snapshot( $block ) { $block[ $key ] = wp_kses_post( $block[ $key ] ); } } + if ( isset( $block['attributes'] ) && is_array( $block['attributes'] ) ) { + $block['attributes'] = filter_block_kses_value( $block['attributes'], 'post' ); + } if ( isset( $block['innerBlocks'] ) && is_array( $block['innerBlocks'] ) ) { foreach ( $block['innerBlocks'] as $index => $inner_block ) { if ( is_array( $inner_block ) ) { @@ -59,9 +64,9 @@ function gutenberg_kses_suggestion_block_snapshot( $block ) { * * - Users with `unfiltered_html` store the payload as-is — the same * freedom they already have in post content. - * - Everyone else has `wp_kses_post()` applied to the string values that - * get APPLIED to content on accept/reject: `after`, `afterHTML`, - * `beforeHTML`, and the serialized block snapshot in `block`. + * - Everyone else has `wp_kses_post()` applied to every string leaf of + * the values that get APPLIED to content on accept/reject: `after`, + * `afterHTML`, `beforeHTML`, and the serialized block snapshot in `block`. * * `before` is intentionally NOT filtered: it is only compared against live * content for conflict detection, never applied. Filtering it would produce @@ -92,9 +97,11 @@ function gutenberg_sanitize_suggestion_payload( $value ) { if ( ! is_array( $operation ) ) { continue; } + // `after` can be structured (a table's `body` rows, a gallery's + // `images`), so every string leaf is filtered, not only strings. foreach ( array( 'after', 'afterHTML', 'beforeHTML' ) as $key ) { - if ( isset( $operation[ $key ] ) && is_string( $operation[ $key ] ) ) { - $operation[ $key ] = wp_kses_post( $operation[ $key ] ); + if ( isset( $operation[ $key ] ) ) { + $operation[ $key ] = filter_block_kses_value( $operation[ $key ], 'post' ); } } if ( isset( $operation['block'] ) && is_array( $operation['block'] ) ) { diff --git a/phpunit/experimental/class-wp-rest-comments-controller-gutenberg-test.php b/phpunit/experimental/class-wp-rest-comments-controller-gutenberg-test.php index 8af677b9ffca58..05995f59241e52 100644 --- a/phpunit/experimental/class-wp-rest-comments-controller-gutenberg-test.php +++ b/phpunit/experimental/class-wp-rest-comments-controller-gutenberg-test.php @@ -607,6 +607,89 @@ public function test_suggestion_payload_is_ksesed_for_user_without_unfiltered_ht $this->assertStringContainsString( 'world', $after, 'Allowed markup must survive KSES.' ); } + /** + * Test that structured `after` values (a table's `body` rows, for example) + * are filtered down to every string leaf, as are the attributes of a block + * snapshot, for a user without `unfiltered_html`. + */ + public function test_suggestion_payload_kses_reaches_nested_values() { + wp_set_current_user( self::$author_id ); + $post_id = self::factory()->post->create( array( 'post_author' => self::$author_id ) ); + + $payload = wp_json_encode( + array( + 'schemaVersion' => 2, + 'blockName' => 'core/table', + 'baseRevision' => null, + 'operations' => array( + array( + 'type' => 'attribute-set', + 'attribute' => 'body', + 'before' => array(), + 'after' => array( + array( + 'cells' => array( + array( + 'content' => 'cell', + 'tag' => 'td', + ), + ), + ), + ), + ), + array( + 'type' => 'block-insert-after', + 'clientId' => 'abc', + 'blockName' => 'core/paragraph', + 'block' => array( + 'name' => 'core/paragraph', + 'attributes' => array( + 'content' => 'text', + ), + 'innerBlocks' => array( + array( + 'name' => 'core/paragraph', + 'attributes' => array( 'content' => 'inner' ), + ), + ), + ), + ), + ), + ) + ); + + $request = new WP_REST_Request( 'POST', '/wp/v2/comments' ); + $request->add_header( 'Content-Type', 'application/json' ); + $request->set_body( + wp_json_encode( + array( + 'post' => $post_id, + 'content' => '', + 'type' => 'note', + 'author' => self::$author_id, + 'meta' => array( + '_wp_suggestion' => $payload, + ), + ) + ) + ); + + $response = rest_get_server()->dispatch( $request ); + $this->assertSame( 201, $response->get_status() ); + + $data = $response->get_data(); + $decoded = json_decode( get_comment_meta( $data['id'], '_wp_suggestion', true ), true ); + $cell = $decoded['operations'][0]['after'][0]['cells'][0]; + $snapshot = $decoded['operations'][1]['block']; + $inner_text = $snapshot['innerBlocks'][0]['attributes']['content']; + + $this->assertStringNotContainsString( '