diff --git a/src/js/components/ManageMenu/SnippetsTable/ActionFeedback.tsx b/src/js/components/ManageMenu/SnippetsTable/ActionFeedback.tsx new file mode 100644 index 000000000..282f3454a --- /dev/null +++ b/src/js/components/ManageMenu/SnippetsTable/ActionFeedback.tsx @@ -0,0 +1,35 @@ +import React from 'react' +import { __ } from '@wordpress/i18n' +import { failureMessage, useActionFeedback } from '../../../hooks/useActionFeedback' +import { DismissibleNotice } from '../../common/Notice' + +/** + * Show any snippet action that did not complete. + * + * Rendered above the table so a failure appears where the person is already + * looking, rather than only in the browser console. + */ +export const ActionFeedback: React.FC = () => { + const { failures, dismissFailure } = useActionFeedback() + + if (0 === failures.length) { + return null + } + + return ( + <> + {failures.map(failure => + dismissFailure(failure.id)} + > +

+ {failureMessage(failure)}{' '} + {__('Nothing has been changed.', 'code-snippets')} +

+
)} + + ) +} diff --git a/src/js/components/ManageMenu/SnippetsTable/ManageSnippetCard.tsx b/src/js/components/ManageMenu/SnippetsTable/ManageSnippetCard.tsx index 7e766f5e9..430d8b1a6 100644 --- a/src/js/components/ManageMenu/SnippetsTable/ManageSnippetCard.tsx +++ b/src/js/components/ManageMenu/SnippetsTable/ManageSnippetCard.tsx @@ -4,9 +4,9 @@ import { RawHTML } from '@wordpress/element' import { __, sprintf } from '@wordpress/i18n' import React, { useState } from 'react' import { ConfirmDeleteDialog, useDeleteSnippet } from '../../common/snippets/ConfirmDeleteDialog' +import { useActionFeedback } from '../../../hooks/useActionFeedback' import { useSnippetsAPI } from '../../../hooks/useSnippetsAPI' import { useSnippetsList } from '../../../hooks/useSnippetsList' -import { handleUnknownError } from '../../../utils/errors' import { downloadSnippetExportFile } from '../../../utils/files' import { canModifySnippet, cloneSnippetObject, getSnippetDisplayName, getSnippetEditUrl, getSnippetType, isNetworkOnlySnippet, isSnippetActive } from '../../../utils/snippets/snippets' import { Button } from '../../common/Button' @@ -39,6 +39,7 @@ const CardPreviewButton: React.FC = ({ snippet }) => { const CloneExportMenuItems: React.FC = ({ snippet }) => { const api = useSnippetsAPI() const { refreshSnippetsList } = useSnippetsList() + const { reportFailure } = useActionFeedback() return ( <> @@ -46,7 +47,7 @@ const CloneExportMenuItems: React.FC = ({ snippet }) => onSelect={() => { api.create(cloneSnippetObject(snippet)) .then(refreshSnippetsList) - .catch(handleUnknownError) + .catch((error: unknown) => reportFailure(__('clone this snippet', 'code-snippets'), error)) }} > {__('Clone', 'code-snippets')} @@ -56,7 +57,7 @@ const CloneExportMenuItems: React.FC = ({ snippet }) => onSelect={() => { api.export(snippet) .then(response => downloadSnippetExportFile(response, snippet)) - .catch(handleUnknownError) + .catch((error: unknown) => reportFailure(__('export this snippet', 'code-snippets'), error)) }} > {__('Export', 'code-snippets')} @@ -77,6 +78,7 @@ const RestoreDeleteMenuItems: React.FC = ({ }) => { const api = useSnippetsAPI() const { refreshSnippetsList } = useSnippetsList() + const { reportFailure } = useActionFeedback() return ( <> @@ -87,7 +89,7 @@ const RestoreDeleteMenuItems: React.FC = ({ onSelect={() => { api.restore(snippet) .then(refreshSnippetsList) - .catch(handleUnknownError) + .catch((error: unknown) => reportFailure(__('restore this snippet', 'code-snippets'), error)) }} > {__('Restore', 'code-snippets')} @@ -103,11 +105,17 @@ const RestoreDeleteMenuItems: React.FC = ({ const CardActionsMenu: React.FC = ({ snippet }) => { const { refreshSnippetsList } = useSnippetsList() + const { reportFailure } = useActionFeedback() const canModify = canModifySnippet(snippet) const { requestDelete, deleteDialogProps } = useDeleteSnippet({ snippet, onSuccess: refreshSnippetsList, - onError: handleUnknownError + onError: (error: unknown) => reportFailure( + snippet.trashed + ? __('delete this snippet', 'code-snippets') + : __('move this snippet to the trash', 'code-snippets'), + error + ) }) return ( diff --git a/src/js/components/ManageMenu/SnippetsTable/SnippetsTable.tsx b/src/js/components/ManageMenu/SnippetsTable/SnippetsTable.tsx index 2be7c018e..41f9cc4f4 100644 --- a/src/js/components/ManageMenu/SnippetsTable/SnippetsTable.tsx +++ b/src/js/components/ManageMenu/SnippetsTable/SnippetsTable.tsx @@ -12,10 +12,12 @@ import { isLicensed } from '../../../utils/screen' import { SNIPPET_TYPE_LABELS, getSnippetAddNewUrl, getSnippetType, isProType } from '../../../utils/snippets/snippets' import { buildUrl } from '../../../utils/urls' import { Badge } from '../../common/Badge' +import { WithActionFeedbackContext } from '../../../hooks/useActionFeedback' import { Notice } from '../../common/Notice' import { ScreenMetaSlot } from '../../common/ScreenMetaSlot' import { UpsellPage } from '../../common/UpsellDialog' import { WithSnippetsTableFilters, useSnippetsFilters } from './WithSnippetsTableFilters' +import { ActionFeedback } from './ActionFeedback' import { WithFilteredSnippetsContext } from './WithFilteredSnippetsContext' import { SnippetsListTable } from './SnippetsListTable' import type { SnippetType } from '../../../types/Snippet' @@ -121,6 +123,8 @@ const SnippetsTableInner = () => { return ( <> + +
- - - + + + + + diff --git a/src/js/components/ManageMenu/SnippetsTable/TableColumns.tsx b/src/js/components/ManageMenu/SnippetsTable/TableColumns.tsx index d44968392..30879b547 100644 --- a/src/js/components/ManageMenu/SnippetsTable/TableColumns.tsx +++ b/src/js/components/ManageMenu/SnippetsTable/TableColumns.tsx @@ -3,8 +3,8 @@ import { RawHTML } from '@wordpress/element' import { __ } from '@wordpress/i18n' import React, { Fragment } from 'react' import { useSnippetsAPI } from '../../../hooks/useSnippetsAPI' +import { useActionFeedback } from '../../../hooks/useActionFeedback' import { useSnippetsList } from '../../../hooks/useSnippetsList' -import { handleUnknownError } from '../../../utils/errors' import { isNetworkAdmin } from '../../../utils/screen' import { getSnippetDisplayName, getSnippetEditUrl, getSnippetType } from '../../../utils/snippets/snippets' import { buildUrl } from '../../../utils/urls' @@ -35,6 +35,7 @@ const RunOnceButton: React.FC = ({ snippet }) => const ActivationSwitch: React.FC = ({ snippet }) => { const { activate, deactivate } = useSnippetsAPI() const { refreshSnippetsList } = useSnippetsList() + const { reportFailure } = useActionFeedback() const actionText = snippet.network && !snippet.shared_network ? snippet.active ? __('Network Deactivate', 'code-snippets') : __('Network Activate', 'code-snippets') @@ -53,7 +54,12 @@ const ActivationSwitch: React.FC = ({ snippet }) => { onChange={() => { (snippet.active ? deactivate(snippet) : activate(snippet)) .then(refreshSnippetsList) - .catch(handleUnknownError) + .catch((error: unknown) => reportFailure( + snippet.active + ? __('deactivate this snippet', 'code-snippets') + : __('activate this snippet', 'code-snippets'), + error + )) }} /> ) diff --git a/src/js/components/ManageMenu/SnippetsTable/useApplyBulkAction.ts b/src/js/components/ManageMenu/SnippetsTable/useApplyBulkAction.ts index 6fa763444..7d8d10bf8 100644 --- a/src/js/components/ManageMenu/SnippetsTable/useApplyBulkAction.ts +++ b/src/js/components/ManageMenu/SnippetsTable/useApplyBulkAction.ts @@ -1,7 +1,7 @@ -import { __ } from '@wordpress/i18n' +import { __, sprintf } from '@wordpress/i18n' import { useSnippetsAPI } from '../../../hooks/useSnippetsAPI' +import { useActionFeedback } from '../../../hooks/useActionFeedback' import { useSnippetsList } from '../../../hooks/useSnippetsList' -import { handleUnknownError } from '../../../utils/errors' import { downloadBulkSnippetExportFile } from '../../../utils/files' import { cloneSnippetObject } from '../../../utils/snippets/snippets' import type { ListTableAction } from '../../common/ListTable' @@ -81,22 +81,71 @@ const submitBulkSnippetDownloadsIndividually = (snippets: readonly Snippet[]): P const applyAndRefresh = async ( targets: Snippet[], action: (snippet: Snippet) => Promise | Promise, - refresh: () => Promise + refresh: () => Promise, + onFailure: (failed: number, total: number, error: unknown) => void ): Promise => { if (0 < targets.length) { + let failed = 0 + let firstError: unknown + + // Every snippet is attempted even when one fails, so a single bad + // snippet does not silently halt the rest of the batch. Failures used + // to be discarded here, which is why a bulk action that did nothing + // looked exactly like one that had worked. for (const snippet of targets) { - await action(snippet).catch(handleUnknownError) + try { + await action(snippet) + } catch (error: unknown) { + failed += 1 + firstError ??= error + } } await refresh() + + if (0 < failed) { + onFailure(failed, targets.length, firstError) + } } } +/** + * Build the failure reporter for one bulk action. + * + * The count is included because a batch can partly succeed, and "three of ten + * failed" is a very different situation to "nothing happened". + */ +const bulkFailureReporter = ( + reportFailure: (action: string, error: unknown) => void, + label: string +) => (failed: number, total: number, error: unknown) => + reportFailure( + sprintf( + /* translators: 1: what was being done, 2: number that failed, 3: number attempted. */ + __('%1$s (%2$d of %3$d failed)', 'code-snippets'), + label, + failed, + total + ), + error + ) + +/** + * Send the selected snippets to the browser as downloads. + * + * Falls back to one download per snippet where the server cannot build a zip. + */ +const submitBulkDownload = (selectedSnippets: Snippet[]): Promise => + 1 < selectedSnippets.length && !window.CODE_SNIPPETS_MANAGE?.supportsZipDownloads + ? submitBulkSnippetDownloadsIndividually(selectedSnippets) + : submitBulkSnippetDownload(selectedSnippets) + export const useApplyBulkAction = ( allSnippets: Snippet[] ): (action: SnippetsTableAction | undefined, selected: Set) => Promise => { const api = useSnippetsAPI() const { refreshSnippetsList } = useSnippetsList() + const { reportFailure } = useActionFeedback() return async (action, selected) => { switch (action) { @@ -104,41 +153,40 @@ export const useApplyBulkAction = ( await applyAndRefresh( allSnippets.filter(snippet => selected.has(snippet.id) && !snippet.active), snippet => api.activate({ id: snippet.id, network: snippet.network }), - refreshSnippetsList) + refreshSnippetsList, + bulkFailureReporter(reportFailure, __('activate the selected snippets', 'code-snippets'))) break case 'deactivate': await applyAndRefresh( allSnippets.filter(snippet => selected.has(snippet.id) && snippet.active), snippet => api.deactivate({ id: snippet.id, network: snippet.network }), - refreshSnippetsList) + refreshSnippetsList, + bulkFailureReporter(reportFailure, __('deactivate the selected snippets', 'code-snippets'))) break case 'clone': await applyAndRefresh( allSnippets.filter(snippet => selected.has(snippet.id) && !snippet.trashed), snippet => api.create(cloneSnippetObject(snippet)), - refreshSnippetsList) + refreshSnippetsList, + bulkFailureReporter(reportFailure, __('clone the selected snippets', 'code-snippets'))) break case 'export': downloadBulkSnippetExportFile(allSnippets.filter(snippet => selected.has(snippet.id))) break - case 'download': { - const selectedSnippets = allSnippets.filter(snippet => selected.has(snippet.id)) - - return 1 < selectedSnippets.length && !window.CODE_SNIPPETS_MANAGE?.supportsZipDownloads - ? submitBulkSnippetDownloadsIndividually(selectedSnippets) - : submitBulkSnippetDownload(selectedSnippets) - } + case 'download': + return submitBulkDownload(allSnippets.filter(snippet => selected.has(snippet.id))) case 'trash': case 'delete': await applyAndRefresh( allSnippets.filter(snippet => selected.has(snippet.id)), snippet => api.delete({ id: snippet.id, network: snippet.network }), - refreshSnippetsList) + refreshSnippetsList, + bulkFailureReporter(reportFailure, __('remove the selected snippets', 'code-snippets'))) break case undefined: diff --git a/src/js/hooks/useActionFeedback.tsx b/src/js/hooks/useActionFeedback.tsx new file mode 100644 index 000000000..ec33a80a3 --- /dev/null +++ b/src/js/hooks/useActionFeedback.tsx @@ -0,0 +1,68 @@ +import React, { useCallback, useMemo, useState } from 'react' +import { __, sprintf } from '@wordpress/i18n' +import { createContextHook } from '../utils/bootstrap' +import { describeError, handleUnknownError } from '../utils/errors' +import type { PropsWithChildren } from 'react' + +export interface ActionFailure { + id: number + /** What the person was trying to do, already translated. */ + action: string + /** What went wrong, in terms they can act on. */ + detail: string +} + +export interface ActionFeedbackContext { + failures: readonly ActionFailure[] + /** + * Report that an action did not complete. + * + * Every snippet action used to send its error to the console and nothing + * else, so a failed request looked identical to a click that had never + * registered: the row did not change and nothing explained why. That left + * people unable to tell a permissions problem from a plugin conflict, and + * left us unable to ask them anything useful. + */ + reportFailure: (action: string, error: unknown) => void + dismissFailure: (id: number) => void +} + +const [Context, useActionFeedback] = createContextHook('useActionFeedback') + +export const WithActionFeedbackContext: React.FC = ({ children }) => { + const [failures, setFailures] = useState([]) + + const reportFailure = useCallback((action: string, error: unknown) => { + // Still logged, so the full object remains available in the console. + handleUnknownError(error) + + setFailures(current => [ + ...current.filter(failure => failure.action !== action), + { id: Date.now() + current.length, action, detail: describeError(error) } + ]) + }, []) + + const dismissFailure = useCallback((id: number) => { + setFailures(current => current.filter(failure => failure.id !== id)) + }, []) + + const value = useMemo( + () => ({ failures, reportFailure, dismissFailure }), + [failures, reportFailure, dismissFailure] + ) + + return {children} +} + +/** + * Build the sentence shown to the person, given what they were doing. + */ +export const failureMessage = (failure: ActionFailure): string => + sprintf( + /* translators: 1: what the user was trying to do, 2: reason it did not work. */ + __('Could not %1$s. %2$s', 'code-snippets'), + failure.action, + failure.detail + ) + +export { useActionFeedback } diff --git a/src/js/utils/errors.ts b/src/js/utils/errors.ts index 7df075577..dc55562fc 100644 --- a/src/js/utils/errors.ts +++ b/src/js/utils/errors.ts @@ -1,6 +1,10 @@ -import { __ } from '@wordpress/i18n' +import { __, sprintf } from '@wordpress/i18n' import { isAxiosError } from 'axios' +const HTTP_FORBIDDEN = 403 +const HTTP_NOT_FOUND = 404 +const HTTP_SERVER_ERROR = 500 + export const handleUnknownError = (error: unknown) => { console.error(error) } @@ -20,3 +24,58 @@ export const unpackErrorResponse = (error: unknown): string => { return __('An unknown error occurred.', 'code-snippets') } + +/** + * Describe a failed request in terms the person reading it can act on. + * + * The HTTP status is deliberately included. Requests to the snippets API are + * blocked by security rules and firewalls often enough that "it did nothing" + * is impossible to diagnose without it, and asking people to open developer + * tools is a poor substitute for the plugin simply saying what happened. + */ +export const describeError = (error: unknown): string => { + if (isAxiosError(error)) { + if (!error.response) { + return __( + 'The request did not reach your site. It may have been blocked by a firewall or security plugin.', + 'code-snippets' + ) + } + + const status = error.response.status + const message = unpackErrorResponse(error) + + if (HTTP_FORBIDDEN === status) { + return sprintf( + /* translators: %s: error message returned by the site. */ + __('Your site refused the request (403). Try reloading the page, and check whether a security plugin is blocking it. %s', 'code-snippets'), + message + ) + } + + if (HTTP_NOT_FOUND === status) { + return __( + 'The snippets API could not be found (404). It may be disabled or blocked on your site.', + 'code-snippets' + ) + } + + if (HTTP_SERVER_ERROR <= status) { + return sprintf( + /* translators: 1: HTTP status code, 2: error message returned by the site. */ + __('Your site returned an error (%1$d). %2$s', 'code-snippets'), + status, + message + ) + } + + return sprintf( + /* translators: 1: HTTP status code, 2: error message returned by the site. */ + __('Your site returned %1$d. %2$s', 'code-snippets'), + status, + message + ) + } + + return unpackErrorResponse(error) +}