Skip to content

Commit 0aadbd1

Browse files
whyisjakeclaude
andcommitted
Core Data: report autosave resolution on every exit path
The completion dispatch sat on the success path only, so an early return (unknown author, post type without autosave support) or a rejected request skipped it. hasFetchedAutosaves then stayed false and isEditedPostAutosaveable never enabled autosaving for the session. This was a regression against the delegation it replaced: that completed getAutosaves resolution unconditionally, and hasFinishedResolution treats an errored resolution as complete, so even a failed fetch counted as fetched. Move the dispatch into a finally, and correct the docblock, which claimed getAutosaves was unaffected. Reporting against its key also suppresses that resolver via hasStartedResolution, so a later direct call returns the stored single-author array rather than refetching. Two existing tests asserted the dispatch was NOT made on the early-return paths, encoding the bug as intended behavior. They now assert the opposite, and cover the failed-request path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent c940518 commit 0aadbd1

3 files changed

Lines changed: 131 additions & 34 deletions

File tree

packages/core-data/src/resolvers.js

Lines changed: 44 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -628,11 +628,23 @@ export const getAutosaves =
628628
* long-lived post with many editors, the difference is the entire collection
629629
* versus one row.
630630
*
631-
* `getAutosaves` is intentionally left alone rather than being narrowed, so
632-
* calling it directly still returns the full collection. Its resolution is
633-
* marked finished here because `hasFetchedAutosaves` reports on it, and
634-
* `isEditedPostAutosaveable` in @wordpress/editor treats an unfinished
635-
* resolution as "not yet known" — without this, autosaving would never enable.
631+
* The `getAutosaves` resolver itself is left unchanged. This one reports
632+
* completion against the `getAutosaves` resolution key because that is what
633+
* `hasFetchedAutosaves` reads, and the editor package's
634+
* `isEditedPostAutosaveable` treats an unfinished resolution as "not yet
635+
* known" — without it, autosaving would never enable.
636+
*
637+
* That report happens in a `finally` so it covers every exit path. The previous
638+
* delegation to `getAutosaves` completed its resolution unconditionally, and a
639+
* failed request still counted as finished (`hasFinishedResolution` treats
640+
* `error` as complete). Reporting only on the success path would leave
641+
* autosaving disabled for the rest of the session after a single early return
642+
* or network failure.
643+
*
644+
* Note that reporting against `getAutosaves` also suppresses that resolver for
645+
* the post: `hasStartedResolution` becomes true, so a later direct call to the
646+
* `getAutosaves` selector returns whatever this resolver stored rather than
647+
* refetching the full collection.
636648
*
637649
* @param {string} postType The type of the parent post.
638650
* @param {number} postId The id of the parent post.
@@ -641,36 +653,38 @@ export const getAutosaves =
641653
export const getAutosave =
642654
( postType, postId, authorId ) =>
643655
async ( { dispatch, resolveSelect } ) => {
644-
if ( authorId === undefined ) {
645-
return;
646-
}
656+
try {
657+
if ( authorId === undefined ) {
658+
return;
659+
}
647660

648-
const {
649-
rest_base: restBase,
650-
rest_namespace: restNamespace = 'wp/v2',
651-
supports,
652-
} = await resolveSelect.getPostType( postType );
661+
const {
662+
rest_base: restBase,
663+
rest_namespace: restNamespace = 'wp/v2',
664+
supports,
665+
} = await resolveSelect.getPostType( postType );
653666

654-
if ( ! supports?.autosave ) {
655-
return;
656-
}
667+
if ( ! supports?.autosave ) {
668+
return;
669+
}
657670

658-
const autosaves = await apiFetch( {
659-
path: addQueryArgs(
660-
`/${ restNamespace }/${ restBase }/${ postId }/autosaves`,
661-
{
662-
context: 'edit',
663-
per_page: 1,
664-
author: authorId,
665-
}
666-
),
667-
} );
671+
const autosaves = await apiFetch( {
672+
path: addQueryArgs(
673+
`/${ restNamespace }/${ restBase }/${ postId }/autosaves`,
674+
{
675+
context: 'edit',
676+
per_page: 1,
677+
author: authorId,
678+
}
679+
),
680+
} );
668681

669-
if ( autosaves && autosaves.length ) {
670-
dispatch.receiveAutosaves( postId, autosaves );
682+
if ( autosaves && autosaves.length ) {
683+
dispatch.receiveAutosaves( postId, autosaves );
684+
}
685+
} finally {
686+
dispatch.finishResolution( 'getAutosaves', [ postType, postId ] );
671687
}
672-
673-
dispatch.finishResolution( 'getAutosaves', [ postType, postId ] );
674688
};
675689

676690
export const __experimentalGetCurrentGlobalStylesId =

packages/core-data/src/test/autosaves.js

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,58 @@ describe( 'autosaves', () => {
124124
} );
125125
} );
126126

127+
it( 'reports hasFetchedAutosaves after a failed request', async () => {
128+
const registry = createTestRegistry();
129+
triggerFetch.mockImplementation( ( { path, parse } ) => {
130+
if ( path.startsWith( '/wp/v2/types' ) ) {
131+
const postType = {
132+
slug: 'post',
133+
rest_base: 'posts',
134+
rest_namespace: 'wp/v2',
135+
supports: { autosave: true },
136+
};
137+
138+
if ( parse === false ) {
139+
return {
140+
json: async () => postType,
141+
headers: { get: () => '' },
142+
};
143+
}
144+
145+
return postType;
146+
}
147+
148+
throw new Error( 'Network error' );
149+
} );
150+
151+
await registry
152+
.resolveSelect( coreDataStore )
153+
.getAutosave( 'post', POST_ID, AUTHOR_ID )
154+
.catch( () => {} );
155+
156+
// Autosaving must not stay disabled for the session after one failure.
157+
expect(
158+
registry
159+
.select( coreDataStore )
160+
.hasFetchedAutosaves( 'post', POST_ID )
161+
).toBe( true );
162+
} );
163+
164+
it( 'reports hasFetchedAutosaves when the author is not yet known', async () => {
165+
const registry = createTestRegistry();
166+
mockFetch( [ AUTOSAVE ] );
167+
168+
await registry
169+
.resolveSelect( coreDataStore )
170+
.getAutosave( 'post', POST_ID, undefined );
171+
172+
expect(
173+
registry
174+
.select( coreDataStore )
175+
.hasFetchedAutosaves( 'post', POST_ID )
176+
).toBe( true );
177+
} );
178+
127179
it( 'does not return another author’s autosave', async () => {
128180
const registry = createTestRegistry();
129181
mockFetch( [ { id: 100, author: 999, parent: POST_ID } ] );

packages/core-data/src/test/resolvers.js

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -958,7 +958,7 @@ describe( 'getAutosave', () => {
958958
);
959959
} );
960960

961-
it( 'does not fetch when the author is unknown', async () => {
961+
it( 'does not fetch when the author is unknown, but still finishes resolution', async () => {
962962
const { dispatch, resolveSelect } = setUp( SUCCESSFUL_RESPONSE );
963963

964964
await getAutosave(
@@ -968,10 +968,15 @@ describe( 'getAutosave', () => {
968968
)( { dispatch, resolveSelect } );
969969

970970
expect( triggerFetch ).not.toHaveBeenCalled();
971-
expect( dispatch.finishResolution ).not.toHaveBeenCalled();
971+
// Bailing without reporting would leave autosaving disabled for the
972+
// session if the current user id never arrives.
973+
expect( dispatch.finishResolution ).toHaveBeenCalledWith(
974+
'getAutosaves',
975+
[ postType, postId ]
976+
);
972977
} );
973978

974-
it( 'does not fetch when the post type does not support autosaves', async () => {
979+
it( 'does not fetch when the post type does not support autosaves, but still finishes resolution', async () => {
975980
const { dispatch } = setUp( SUCCESSFUL_RESPONSE );
976981
const resolveSelect = Object.assign( jest.fn(), {
977982
getPostType: jest.fn( () => ( {
@@ -987,7 +992,33 @@ describe( 'getAutosave', () => {
987992
)( { dispatch, resolveSelect } );
988993

989994
expect( triggerFetch ).not.toHaveBeenCalled();
990-
expect( dispatch.finishResolution ).not.toHaveBeenCalled();
995+
expect( dispatch.finishResolution ).toHaveBeenCalledWith(
996+
'getAutosaves',
997+
[ postType, postId ]
998+
);
999+
} );
1000+
1001+
it( 'finishes resolution even when the request fails', async () => {
1002+
const { dispatch, resolveSelect } = setUp( SUCCESSFUL_RESPONSE );
1003+
triggerFetch.mockImplementation( () => {
1004+
throw new Error( 'Network error' );
1005+
} );
1006+
1007+
await expect(
1008+
getAutosave(
1009+
postType,
1010+
postId,
1011+
AUTHOR_ID
1012+
)( { dispatch, resolveSelect } )
1013+
).rejects.toThrow( 'Network error' );
1014+
1015+
expect( dispatch.receiveAutosaves ).not.toHaveBeenCalled();
1016+
// A failed fetch previously still counted as fetched, because
1017+
// hasFinishedResolution treats an errored resolution as complete.
1018+
expect( dispatch.finishResolution ).toHaveBeenCalledWith(
1019+
'getAutosaves',
1020+
[ postType, postId ]
1021+
);
9911022
} );
9921023

9931024
it( 'leaves the getAutosaves resolver behavior unchanged', async () => {

0 commit comments

Comments
 (0)