From 921d76275de25893bd921cb6151d3cdf6f0213fc Mon Sep 17 00:00:00 2001 From: Siddarthan Sarumathi Pandian Date: Fri, 7 Feb 2025 10:41:33 +0530 Subject: [PATCH] Newsletter imports: Handle errors correctly (#99162) * Newsletter imports: Handle errors correctly rebase. * Update props. * Pass tests * Make TS happy. * remove optional param * Show the notice till the user dimisses it. * Error handling and add upgrade link based on site. * Make tests happy. * dispatch all errors correctly. * Make test happy * Update atomic site checks. --- .../add-subscribers-modal.tsx | 11 ++- .../subscribers-page-context.tsx | 96 +++++++++++++------ .../data-stores/src/subscriber/actions.ts | 10 +- .../subscriber/src/hooks/use-import-error.ts | 13 +++ 4 files changed, 92 insertions(+), 38 deletions(-) create mode 100644 packages/subscriber/src/hooks/use-import-error.ts diff --git a/client/my-sites/subscribers/components/add-subscribers-modal/add-subscribers-modal.tsx b/client/my-sites/subscribers/components/add-subscribers-modal/add-subscribers-modal.tsx index 7419418a28fdbd..dd1870a8eaaff8 100644 --- a/client/my-sites/subscribers/components/add-subscribers-modal/add-subscribers-modal.tsx +++ b/client/my-sites/subscribers/components/add-subscribers-modal/add-subscribers-modal.tsx @@ -7,6 +7,7 @@ import { SiteDetails } from '@automattic/data-stores'; import { localizeUrl } from '@automattic/i18n-utils'; import { AddSubscriberForm, UploadSubscribersForm } from '@automattic/subscriber'; import { useHasStaleImportJobs } from '@automattic/subscriber/src/hooks/use-has-stale-import-jobs'; +import { useImportError } from '@automattic/subscriber/src/hooks/use-import-error'; import { useInProgressState } from '@automattic/subscriber/src/hooks/use-in-progress-state'; import { ExternalLink, Modal, __experimentalVStack as VStack } from '@wordpress/components'; import { copy, upload, reusableBlock } from '@wordpress/icons'; @@ -67,15 +68,17 @@ const AddSubscribersModal = ( { site }: AddSubscribersModalProps ) => { const [ isUploading, setIsUploading ] = useState( false ); const onImportStarted = ( hasFile: boolean ) => setIsUploading( hasFile ); + + const importError = useImportError(); + const isImportInProgress = useInProgressState(); + const hasStaleImportJobs = useHasStaleImportJobs(); + const onImportFinished = () => { setIsUploading( false ); setAddingMethod( '' ); - addSubscribersCallback(); + addSubscribersCallback( importError ); }; - const isImportInProgress = useInProgressState(); - const hasStaleImportJobs = useHasStaleImportJobs(); - if ( ! showAddSubscribersModal ) { return null; } diff --git a/client/my-sites/subscribers/components/subscribers-page/subscribers-page-context.tsx b/client/my-sites/subscribers/components/subscribers-page/subscribers-page-context.tsx index 152b02b3d3d0f7..96a5c600850aec 100644 --- a/client/my-sites/subscribers/components/subscribers-page/subscribers-page-context.tsx +++ b/client/my-sites/subscribers/components/subscribers-page/subscribers-page-context.tsx @@ -1,5 +1,6 @@ import { isEnabled } from '@automattic/calypso-config'; import { updateLaunchpadSettings } from '@automattic/data-stores'; +import { ImportSubscribersError } from '@automattic/data-stores/src/subscriber/types'; import { useActiveJobRecognition } from '@automattic/subscriber'; import { useQueryClient } from '@tanstack/react-query'; import { translate } from 'i18n-calypso'; @@ -10,6 +11,7 @@ import { usePagination } from 'calypso/my-sites/subscribers/hooks'; import { Subscriber } from 'calypso/my-sites/subscribers/types'; import { useSelector } from 'calypso/state'; import { successNotice, errorNotice } from 'calypso/state/notices/actions'; +import isJetpackSite from 'calypso/state/sites/selectors/is-jetpack-site'; import { getSelectedSiteSlug } from 'calypso/state/ui/selectors'; import { SubscribersFilterBy, SubscribersSortBy } from '../../constants'; import useManySubsSite from '../../hooks/use-many-subs-site'; @@ -49,7 +51,7 @@ type SubscribersPageContextProps = { showAddSubscribersModal: boolean; showMigrateSubscribersModal: boolean; setShowAddSubscribersModal: ( show: boolean ) => void; - addSubscribersCallback: () => void; + addSubscribersCallback: ( importError?: ImportSubscribersError ) => void; migrateSubscribersCallback: ( sourceSiteId: number, targetSiteId: number ) => void; closeAllModals: typeof closeAllModals; siteId: number | null; @@ -94,7 +96,9 @@ export const SubscribersPageProvider = ( { const [ showAddSubscribersModal, setShowAddSubscribersModal ] = useState( false ); const [ showMigrateSubscribersModal, setShowMigrateSubscribersModal ] = useState( false ); const [ debouncedSearchTerm ] = useDebounce( searchTerm, 300 ); - + const isJetpackNonAtomic = useSelector( + ( state ) => siteId && isJetpackSite( state, siteId, { treatAtomicAsJetpackSite: false } ) + ); useEffect( () => { if ( hasManySubscribers ) { setDataViewFilterOption( SubscribersFilterBy.WPCOM ); @@ -161,42 +165,72 @@ export const SubscribersPageProvider = ( { queryClient.invalidateQueries( { queryKey: [ 'launchpad' ] } ); }; - const addSubscribersCallback = () => { + const addSubscribersCallback = ( importError?: ImportSubscribersError ) => { setShowAddSubscribersModal( false ); completeImportSubscribersTask(); - if ( completedJob ) { - const { email_count, subscribed_count, already_subscribed_count, failed_subscribed_count } = - completedJob; - dispatch( - successNotice( - translate( - 'Import completed. %(added)d subscribed, %(skipped)d already subscribed, and %(failed)d failed out of %(total)d %(totalLabel)s.', + if ( ! importError ) { + if ( completedJob ) { + const { email_count, subscribed_count, already_subscribed_count, failed_subscribed_count } = + completedJob; + dispatch( + successNotice( + translate( + 'Import completed. %(added)d subscribed, %(skipped)d already subscribed, and %(failed)d failed out of %(total)d %(totalLabel)s.', + { + args: { + added: subscribed_count, + skipped: already_subscribed_count, + failed: failed_subscribed_count, + total: email_count, + totalLabel: translate( 'subscriber', 'subscribers', { + count: email_count, + } ), + }, + } + ) + ) + ); + } else { + dispatch( + successNotice( + translate( + "Your subscriber list is being processed. We'll send you an email when it's finished importing." + ), { - args: { - added: subscribed_count, - skipped: already_subscribed_count, - failed: failed_subscribed_count, - total: email_count, - totalLabel: translate( 'subscriber', 'subscribers', { - count: email_count, - } ), - }, + duration: 5000, } ) - ) - ); + ); + } } else { - dispatch( - successNotice( - translate( - "Your subscriber list is being processed. We'll send you an email when it's finished importing." - ), - { - duration: 5000, - } - ) - ); + let notice = translate( 'An unknown error has occurred. Please try again in a second.' ); + interface NoticeArgs { + isPersistent: boolean; + button?: string; + href?: string; + } + const noticeArgs: NoticeArgs = { isPersistent: true }; + + if ( + 'error' in importError && + typeof importError.error === 'object' && + importError.error && + 'code' in importError.error && + 'message' in importError.error + ) { + const { code, message } = importError.error; + notice = message as string; + if ( code === 'subscriber_import_limit_reached' && typeof message === 'string' ) { + noticeArgs.button = translate( 'Upgrade' ); + const siteSlug = selectedSiteSlug || ''; // Use a default if siteSlug is not available + noticeArgs.href = isJetpackNonAtomic + ? `https://cloud.jetpack.com/pricing/${ siteSlug }` + : `https://wordpress.com/plans/${ siteSlug }`; + } + } + + dispatch( errorNotice( notice, noticeArgs ) ); } }; diff --git a/packages/data-stores/src/subscriber/actions.ts b/packages/data-stores/src/subscriber/actions.ts index 1c0bf28292e2e0..607c8feb0f9782 100644 --- a/packages/data-stores/src/subscriber/actions.ts +++ b/packages/data-stores/src/subscriber/actions.ts @@ -71,15 +71,19 @@ export function createActions() { [ 'parse_only', String( parseOnly ) ] as [ string, string ], ]; - const data: ImportSubscribersResponse = await wpcomProxyRequest( { + const data = ( await wpcomProxyRequest( { path: `/sites/${ encodeURIComponent( siteId ) }/subscribers/import`, method: 'POST', apiNamespace: 'wpcom/v2', token: typeof token === 'string' ? token : undefined, formData: formDataEntries as ( string | File )[][], - } ); + } ) ) as ImportSubscribersResponse & { error?: { code: string; message: string } }; - dispatch.importCsvSubscribersStartSuccess( siteId, data.upload_id ); + if ( data.upload_id ) { + dispatch.importCsvSubscribersStartSuccess( siteId, data.upload_id ); + } else { + dispatch.importCsvSubscribersStartFailed( siteId, data as ImportSubscribersError ); + } } catch ( error ) { dispatch.importCsvSubscribersStartFailed( siteId, error as ImportSubscribersError ); } diff --git a/packages/subscriber/src/hooks/use-import-error.ts b/packages/subscriber/src/hooks/use-import-error.ts new file mode 100644 index 00000000000000..ad271c77c4a409 --- /dev/null +++ b/packages/subscriber/src/hooks/use-import-error.ts @@ -0,0 +1,13 @@ +import { Subscriber } from '@automattic/data-stores'; +import { useSelect } from '@wordpress/data'; + +export function useImportError() { + const { importSelector } = useSelect( ( select ) => { + const subscriber = select( Subscriber.store ); + return { + importSelector: subscriber.getImportSubscribersSelector(), + }; + }, [] ); + + return importSelector?.error; +}