Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions packages/editor/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@
"@wordpress/deprecated": "file:../deprecated",
"@wordpress/dom": "file:../dom",
"@wordpress/element": "file:../element",
"@wordpress/escape-html": "file:../escape-html",
"@wordpress/fields": "file:../fields",
"@wordpress/global-styles-engine": "file:../global-styles-engine",
"@wordpress/global-styles-ui": "file:../global-styles-ui",
Expand Down
13 changes: 13 additions & 0 deletions packages/editor/src/components/save-error-details/style.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
.editor-save-error-details {
display: inline;
}

.editor-save-error-details summary {
display: inline;
cursor: var(--wpds-cursor-control);
margin-left: 0.35em;
}

.editor-save-error-details__message {
white-space: break-spaces;
}
31 changes: 31 additions & 0 deletions packages/editor/src/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*/
import { speak } from '@wordpress/a11y';
import apiFetch from '@wordpress/api-fetch';
import { escapeHTML } from '@wordpress/escape-html';
import deprecated from '@wordpress/deprecated';
import {
parse,
Expand Down Expand Up @@ -38,6 +39,11 @@ import { getCanvasWidthByDeviceType } from '../utils/device-type';

const { getEntitySnapshot } = unlock( coreDataPrivateApis );

// Error codes whose message is generated by the client rather than returned by
// the server. Their messages restate what the notice already says, so there are
// no details worth disclosing.
const CLIENT_GENERATED_ERROR_CODES = [ 'offline_error', 'fetch_error' ];

/**
* Returns an action generator used in signalling that editor has initialized with
* the specified post object and editor settings.
Expand Down Expand Up @@ -294,8 +300,33 @@ export const savePost =
post: previousRecord,
edits,
error,
options,
} );
if ( args.length ) {
const [ noticeMessage ] = args;
if (
error.message &&
! CLIENT_GENERATED_ERROR_CODES.includes( error.code ) &&
! /<\/?[^>]*>/.test( error.message )
) {
args[ 0 ] = `${ escapeHTML(
noticeMessage
) } <details class="editor-save-error-details"><summary>${ escapeHTML(
__( 'Show details' )
) }</summary><span class="editor-save-error-details__message">${ escapeHTML(
error.message
) }</span></details>`;
args[ 1 ] = {
...args[ 1 ],
__unstableHTML: true,
speak: false,
};
// The notices store doesn't support a separate spoken message
// when rendering raw HTML content, so announce the plain text
// manually to avoid reading the markup aloud.
speak( noticeMessage, 'assertive' );
}

registry.dispatch( noticesStore ).createErrorNotice( ...args );
}
} else {
Expand Down
134 changes: 134 additions & 0 deletions packages/editor/src/store/test/actions.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
/**
* WordPress dependencies
*/
import { speak } from '@wordpress/a11y';
import apiFetch from '@wordpress/api-fetch';
import { store as blockEditorStore } from '@wordpress/block-editor';
import { store as coreStore } from '@wordpress/core-data';
import { createRegistry } from '@wordpress/data';
import { store as noticesStore } from '@wordpress/notices';
import { store as preferencesStore } from '@wordpress/preferences';

jest.mock( '@wordpress/a11y', () => ( {
speak: jest.fn(),
} ) );

/**
* Internal dependencies
*/
Expand Down Expand Up @@ -268,6 +273,135 @@ describe( 'Post actions', () => {
},
] );
} );

it( 'adds a details action for save failures with a plain-text error message', async () => {
const post = {
id: postId,
type: 'post',
title: 'bar',
content: 'bar',
excerpt: 'crackers',
status: 'publish',
};

apiFetch.setFetchHandler( async ( options ) => {
const method = getMethod( options );
const { path } = options;

if (
method === 'PUT' &&
path.startsWith( `/wp/v2/posts/${ postId }` )
) {
throw {
code: 'test_save_failure',
message: 'Details from server.',
};
}

throw {
code: 'unknown_path',
message: `Unknown path: ${ method } ${ path }`,
};
} );

const registry = createRegistryWithStores();

registry
.dispatch( coreStore )
.receiveEntityRecords( 'postType', 'post', post );

registry.dispatch( editorStore ).setupEditor( post, {
content: 'new bar',
} );

await registry.dispatch( editorStore ).savePost();

const [ notice ] = registry.select( noticesStore ).getNotices();
expect( notice ).toMatchObject( {
status: 'error',
__unstableHTML: true,
} );
expect( notice.content ).toContain(
'Updating failed. We’ll try to save a backup in this browser. Please try updating again.'
);
expect( notice.content ).toContain(
'<details class="editor-save-error-details">'
);
expect( notice.content ).toContain(
'<summary>Show details</summary>'
);
expect( notice.content ).toContain( 'Details from server.' );
expect( notice.actions ).toEqual( [] );
expect( notice.spokenMessage ).toBeNull();
expect( notice.content ).toContain(
'<span class="editor-save-error-details__message">Details from server.</span>'
);
expect( speak ).toHaveBeenCalledWith(
'Updating failed. We’ll try to save a backup in this browser. Please try updating again.',
'assertive'
);
} );

// These errors are generated by the client rather than returned by the
// server, so their message only restates the notice itself.
[
[
'offline_error',
'Unable to connect. Please check your Internet connection.',
'Updating failed because you were offline. Please verify your connection and try again.',
],
[
'fetch_error',
'Could not get a valid response from the server.',
'Updating failed. We’ll try to save a backup in this browser. Please try updating again.',
],
].forEach( ( [ code, message, expectedNotice ] ) => {
it( `omits the details disclosure for ${ code } failures`, async () => {
const post = {
id: postId,
type: 'post',
title: 'bar',
content: 'bar',
excerpt: 'crackers',
status: 'publish',
};

apiFetch.setFetchHandler( async ( options ) => {
const method = getMethod( options );
const { path } = options;

if (
method === 'PUT' &&
path.startsWith( `/wp/v2/posts/${ postId }` )
) {
throw { code, message };
}

throw {
code: 'unknown_path',
message: `Unknown path: ${ method } ${ path }`,
};
} );

const registry = createRegistryWithStores();

registry
.dispatch( coreStore )
.receiveEntityRecords( 'postType', 'post', post );

registry.dispatch( editorStore ).setupEditor( post, {
content: 'new bar',
} );

await registry.dispatch( editorStore ).savePost();

const [ notice ] = registry.select( noticesStore ).getNotices();
expect( notice.content ).toBe( expectedNotice );
expect( notice.content ).not.toContain( '<details' );
expect( notice.content ).not.toContain( 'Show details' );
expect( notice.__unstableHTML ).toBeUndefined();
} );
} );
} );

describe( 'autosave()', () => {
Expand Down
39 changes: 29 additions & 10 deletions packages/editor/src/store/utils/notice-builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,14 @@ import { __ } from '@wordpress/i18n';
*/
import { ATTACHMENT_POST_TYPE } from '../constants';

const AUTO_SAVE_FAILURE_NOTICE = __(
'Auto-save failed. We’ll try to save a backup in this browser. You can also save manually.'
);

const AUTO_SAVE_OFFLINE_FAILURE_NOTICE = __(
'Auto-save failed because you were offline. We’ll try to save a backup in this browser. Please verify your connection and save manually.'
);

/**
* Builds the arguments for a success notification dispatch.
*
Expand Down Expand Up @@ -89,7 +97,7 @@ export function getNotificationArgumentsForSaveSuccess( data ) {
* notification should be sent.
*/
export function getNotificationArgumentsForSaveFail( data ) {
const { post, edits, error } = data;
const { post, edits, error, options } = data;
if ( error && 'rest_autosave_no_changes' === error.code ) {
// Autosave requested a new autosave, but there were no changes. This shouldn't
// result in an error notice for the user.
Expand All @@ -115,31 +123,42 @@ export function getNotificationArgumentsForSaveFail( data ) {
),
};

const noticeMessage =
let noticeMessage =
! isPublished && edits.status in messages
? messages[ edits.status ]
: messages.default;

if ( options?.isAutosave ) {
noticeMessage = AUTO_SAVE_OFFLINE_FAILURE_NOTICE;
}

return [ noticeMessage, { id: 'editor-save' } ];
}

const messages = {
publish: __( 'Publishing failed.' ),
private: __( 'Publishing failed.' ),
future: __( 'Scheduling failed.' ),
default: __( 'Updating failed.' ),
publish: __(
'Publishing failed. We’ll try to save a backup in this browser. Please try publishing again.'
),
private: __(
'Publishing failed. We’ll try to save a backup in this browser. Please try publishing again.'
),
future: __(
'Scheduling failed. We’ll try to save a backup in this browser. Please try scheduling again.'
),
default: __(
'Updating failed. We’ll try to save a backup in this browser. Please try updating again.'
),
};

let noticeMessage =
! isPublished && edits.status in messages
? messages[ edits.status ]
: messages.default;

// Check if message string contains HTML. Notice text is currently only
// supported as plaintext, and stripping the tags may muddle the meaning.
if ( error.message && ! /<\/?[^>]*>/.test( error.message ) ) {
noticeMessage = [ noticeMessage, error.message ].join( ' ' );
if ( options?.isAutosave ) {
noticeMessage = AUTO_SAVE_FAILURE_NOTICE;
}

return [
noticeMessage,
{
Expand Down
23 changes: 19 additions & 4 deletions packages/editor/src/store/utils/test/notice-builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ describe( 'getNotificationArgumentsForSaveFail()', () => {
'',
[ 'draft', 'publish' ],
[
'Publishing failed. Something went wrong.',
'Publishing failed. We’ll try to save a backup in this browser. Please try publishing again.',
defaultExpectedAction,
],
],
Expand All @@ -129,7 +129,7 @@ describe( 'getNotificationArgumentsForSaveFail()', () => {
'',
[ 'draft', 'private' ],
[
'Publishing failed. Something went wrong.',
'Publishing failed. We’ll try to save a backup in this browser. Please try publishing again.',
defaultExpectedAction,
],
],
Expand All @@ -138,22 +138,36 @@ describe( 'getNotificationArgumentsForSaveFail()', () => {
'',
[ 'draft', 'future' ],
[
'Scheduling failed. Something went wrong.',
'Scheduling failed. We’ll try to save a backup in this browser. Please try scheduling again.',
defaultExpectedAction,
],
],
[
'when post is published and edits is published',
'',
[ 'publish', 'publish' ],
[ 'Updating failed. Something went wrong.', defaultExpectedAction ],
[
'Updating failed. We’ll try to save a backup in this browser. Please try updating again.',
defaultExpectedAction,
],
],
[
'when the save is an autosave',
'',
[ 'publish', 'publish' ],
[
'Auto-save failed. We’ll try to save a backup in this browser. You can also save manually.',
defaultExpectedAction,
],
{ isAutosave: true },
],
].forEach(
( [
description,
errorCode,
[ postStatus, editsStatus ],
expectedValue,
options = {},
] ) => {
// eslint-disable-next-line jest/valid-title
it( description, () => {
Expand All @@ -165,6 +179,7 @@ describe( 'getNotificationArgumentsForSaveFail()', () => {
post,
edits,
error,
options,
} )
).toEqual( expectedValue );
} );
Expand Down
1 change: 1 addition & 0 deletions packages/editor/src/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
@use "./components/post-trash/style.scss" as *;
@use "./components/preview-dropdown/style.scss" as *;
@use "./components/resizable-editor/style.scss" as *;
@use "./components/save-error-details/style.scss" as *;
@use "./components/save-publish-panels/style.scss" as *;
@use "./components/start-page-options/style.scss" as *;
@use "./components/start-template-options/style.scss" as *;
Expand Down
1 change: 1 addition & 0 deletions packages/editor/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
{ "path": "../deprecated" },
{ "path": "../dom" },
{ "path": "../element" },
{ "path": "../escape-html" },
{ "path": "../fields" },
{ "path": "../global-styles-engine" },
{ "path": "../global-styles-ui" },
Expand Down
Loading