diff --git a/packages/editor/CHANGELOG.md b/packages/editor/CHANGELOG.md index 2d7679be422238..45aa4d89fbd4ca 100644 --- a/packages/editor/CHANGELOG.md +++ b/packages/editor/CHANGELOG.md @@ -10,6 +10,7 @@ - `setEditorIntent` now surfaces mode transitions with a snackbar ('You're suggesting' / 'You're editing' / 'You're viewing') alongside the existing a11y announcement. - Suggestions: introduce a per-attribute conflict check (`hasAttributeConflict`) and a `SuggestionSummary` renderer that replaces the post-`modified_gmt` staleness compare. Adds the `wp/suggestions` architecture doc and updates the `core/editor` data reference to cover the new selectors. - Suggestions: surface Apply / Reject actions in the collaboration sidebar via a shared `useSuggestionDecision` hook. Note headers expose icon-only Apply / Reject buttons, the note body renders the suggestion summary plus the staleness confirmation dialog, and the e2e coverage for block notes and the intent switcher is extended to the new UI. +- Suggestions: replace the manual commit-bar with a background auto-save subsystem. Pending overlay edits flush as a `_wp_suggestion` note after a short idle window, and subsequent edits on the same block update the existing note rather than creating a new one — keeping the live block tree free of pending suggestion state. ## 14.45.0 (2026-04-29) diff --git a/packages/editor/src/components/provider/index.js b/packages/editor/src/components/provider/index.js index 62740b5b64aa72..74eb706f840408 100644 --- a/packages/editor/src/components/provider/index.js +++ b/packages/editor/src/components/provider/index.js @@ -48,7 +48,7 @@ import TemplatePartMenuItems from '../template-part-menu-items'; import MediaEditorModalMount from '../media/media-editor-modal'; import { SuggestionOverlayProvider, - SuggestionCommitBar, + SuggestionAutoSave, SuggestionStoreInterceptor, registerSuggestionOverlayFilter, } from '../suggestion-mode'; @@ -479,7 +479,7 @@ export const ExperimentalEditorProvider = withRegistryProvider( - + { window?.__experimentalMediaEditorModal && ( ) } 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..c023452ef18782 --- /dev/null +++ b/packages/editor/src/components/suggestion-mode/auto-save.js @@ -0,0 +1,245 @@ +/** + * 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'; + +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 ''; + } +} + +/** + * 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 ) => + 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() ); + + const syncOnce = useCallback( + async ( clientId ) => { + const entry = entriesRef.current[ clientId ]; + if ( ! entry ) { + return; + } + const operations = operationsFromOverlay( + entry.baselineAttributes, + entry.overlayAttributes + ); + 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`. + let commentId = entry.commentId; + if ( commentId ) { + const linkedComment = registry + .select( coreStore ) + .getEntityRecord( 'root', 'comment', commentId ); + if ( linkedComment && linkedComment.status !== 'hold' ) { + commentId = null; + setCommentIdRef.current( clientId, null ); + } + } + + try { + if ( operations.length === 0 ) { + if ( commentId ) { + await deleteRef.current( { commentId } ); + setCommentIdRef.current( 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 ) { + setCommentIdRef.current( 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 ] + ); + + 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 = operationsFromOverlay( + entry.baselineAttributes, + entry.overlayAttributes + ); + 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/commit-bar.js b/packages/editor/src/components/suggestion-mode/commit-bar.js deleted file mode 100644 index feeaa4905ccafa..00000000000000 --- a/packages/editor/src/components/suggestion-mode/commit-bar.js +++ /dev/null @@ -1,109 +0,0 @@ -/** - * WordPress dependencies - */ -import { __ } from '@wordpress/i18n'; -import { - BlockControls, - store as blockEditorStore, -} from '@wordpress/block-editor'; -import { ToolbarGroup, ToolbarButton } from '@wordpress/components'; -import { useSelect } from '@wordpress/data'; -import { useCallback, useState } from '@wordpress/element'; - -/** - * Internal dependencies - */ -import { useSuggestionOverlay } from './overlay-context'; -import { operationsFromOverlay, useSuggestionsProvider } from './provider'; -import { EDITOR_STORE_NAME } from './constants'; - -/** - * Block toolbar group that surfaces Submit / Discard controls whenever the - * currently selected block has a pending suggestion overlay. - * - * The bar is a shared singleton — mounted once per editor provider rather - * than once per block — because the block-editor's `BlockControls` fill - * automatically targets the selected block's toolbar slot. - * - * @return {React.ReactNode|null} Toolbar markup, or null if nothing pending. - */ -export default function SuggestionCommitBar() { - const { entries, clearOverlay } = useSuggestionOverlay(); - const { createSuggestion } = useSuggestionsProvider(); - - const { selectedClientId, isSuggestMode } = useSelect( ( select ) => { - return { - selectedClientId: - select( blockEditorStore ).getSelectedBlockClientId(), - isSuggestMode: - select( EDITOR_STORE_NAME ).getEditorIntent?.() === 'suggest', - }; - }, [] ); - - const [ isSubmitting, setIsSubmitting ] = useState( false ); - const entry = selectedClientId ? entries[ selectedClientId ] : null; - const hasOverlay = - !! entry && Object.keys( entry.overlayAttributes ).length > 0; - - const onSubmit = useCallback( async () => { - if ( ! entry || isSubmitting ) { - return; - } - const operations = operationsFromOverlay( - entry.baselineAttributes, - entry.overlayAttributes - ); - if ( operations.length === 0 ) { - clearOverlay( selectedClientId ); - return; - } - setIsSubmitting( true ); - try { - await createSuggestion( { - clientId: selectedClientId, - blockName: entry.blockName, - operations, - } ); - clearOverlay( selectedClientId ); - } catch { - // Notice surfaced by the provider. - } finally { - setIsSubmitting( false ); - } - }, [ - entry, - isSubmitting, - selectedClientId, - clearOverlay, - createSuggestion, - ] ); - - const onDiscard = useCallback( () => { - if ( selectedClientId ) { - clearOverlay( selectedClientId ); - } - }, [ selectedClientId, clearOverlay ] ); - - if ( ! isSuggestMode || ! hasOverlay ) { - return null; - } - - return ( - - - - { isSubmitting - ? __( 'Submitting…' ) - : __( 'Submit suggestion' ) } - - - { __( 'Discard' ) } - - - - ); -} diff --git a/packages/editor/src/components/suggestion-mode/index.js b/packages/editor/src/components/suggestion-mode/index.js index 52e389d570d6e4..19eaec10652496 100644 --- a/packages/editor/src/components/suggestion-mode/index.js +++ b/packages/editor/src/components/suggestion-mode/index.js @@ -7,7 +7,7 @@ export { default as withSuggestionOverlay, registerSuggestionOverlayFilter, } from './with-suggestion-overlay'; -export { default as SuggestionCommitBar } from './commit-bar'; +export { default as SuggestionAutoSave } from './auto-save'; export { default as SuggestionStoreInterceptor } from './store-interceptor'; export { useSuggestionsProvider, diff --git a/packages/editor/src/components/suggestion-mode/overlay-context.js b/packages/editor/src/components/suggestion-mode/overlay-context.js index ffc393a53d7282..43d560fa671236 100644 --- a/packages/editor/src/components/suggestion-mode/overlay-context.js +++ b/packages/editor/src/components/suggestion-mode/overlay-context.js @@ -81,6 +81,8 @@ const OverlayContext = createContext( { captureBaseline: () => {}, setOverlayAttributes: () => {}, clearOverlay: () => {}, + setCommentId: () => {}, + setSyncedOpsKey: () => {}, hasOverlay: () => false, requestInterceptorBypass: () => {}, consumeInterceptorBypass: () => false, @@ -105,6 +107,8 @@ export function overlayReducer( state, action ) { blockName: action.blockName, baselineAttributes: action.attributes, overlayAttributes: {}, + commentId: null, + syncedOpsKey: null, }, }; } @@ -131,6 +135,32 @@ export function overlayReducer( state, action ) { 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 'PRUNE_ORPHANS': { // Action carries a serializable array; the reducer materializes a // Set internally for the lookup. Keeps actions Redux-DevTools- @@ -194,6 +224,18 @@ export function SuggestionOverlayProvider( { children } ) { [] ); + 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 } ), + [] + ); + const hasEntries = Object.keys( entries ).length > 0; const hasOverlay = useCallback( @@ -271,6 +313,8 @@ export function SuggestionOverlayProvider( { children } ) { captureBaseline, setOverlayAttributes, clearOverlay, + setCommentId, + setSyncedOpsKey, hasOverlay, requestInterceptorBypass, consumeInterceptorBypass, @@ -280,6 +324,8 @@ export function SuggestionOverlayProvider( { children } ) { captureBaseline, setOverlayAttributes, clearOverlay, + setCommentId, + setSyncedOpsKey, hasOverlay, requestInterceptorBypass, consumeInterceptorBypass, diff --git a/packages/editor/src/components/suggestion-mode/provider.js b/packages/editor/src/components/suggestion-mode/provider.js index 4ea2cc50c8666b..6be6f6a8946cf0 100644 --- a/packages/editor/src/components/suggestion-mode/provider.js +++ b/packages/editor/src/components/suggestion-mode/provider.js @@ -362,6 +362,101 @@ export function useSuggestionsProvider() { ] ); + /** + * 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 @@ -546,6 +641,8 @@ export function useSuggestionsProvider() { return { createSuggestion, + updateSuggestion, + deleteSuggestion, applySuggestion, rejectSuggestion, }; 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..9d09f0e910f837 --- /dev/null +++ b/packages/editor/src/components/suggestion-mode/test/auto-save.js @@ -0,0 +1,444 @@ +/** + * 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 from '../auto-save'; +import { + SuggestionOverlayProvider, + useSuggestionOverlay, +} from '../overlay-context'; +import { store as editorStore } from '../../../store'; + +// 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 ); + 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 ); + 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(); + } ); +} ); 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 e5b90094f7d67b..d0a49e8a697dd4 100644 --- a/packages/editor/src/components/suggestion-mode/test/overlay-context.js +++ b/packages/editor/src/components/suggestion-mode/test/overlay-context.js @@ -18,6 +18,8 @@ describe( 'overlayReducer', () => { blockName: 'core/paragraph', baselineAttributes: { content: 'Hello' }, overlayAttributes: {}, + commentId: null, + syncedOpsKey: null, } ); const afterSecond = overlayReducer( afterFirst, { @@ -109,6 +111,52 @@ describe( 'overlayReducer', () => { 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': { diff --git a/tools/eslint/suppressions.json b/tools/eslint/suppressions.json index 0e9b84b1d96a42..3e89d014c4743a 100644 --- a/tools/eslint/suppressions.json +++ b/tools/eslint/suppressions.json @@ -1089,6 +1089,11 @@ "count": 2 } }, + "packages/editor/src/components/suggestion-mode/auto-save.js": { + "react-hooks/refs": { + "count": 6 + } + }, "packages/editor/src/components/suggestion-mode/store-interceptor.js": { "react-hooks/refs": { "count": 4 @@ -1104,6 +1109,11 @@ "count": 1 } }, + "packages/editor/src/components/suggestion-mode/test/auto-save.js": { + "react-hooks/globals": { + "count": 1 + } + }, "packages/editor/src/components/suggestion-mode/test/with-suggestion-overlay.js": { "react-hooks/globals": { "count": 1