Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SEO Assistant: Add initial error handling to title generation #41649

Merged
merged 5 commits into from
Feb 11, 2025
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: other

SEO Assistant: Add initial error handling to title generation
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Button, Icon, Tooltip } from '@wordpress/components';
import { Button, Icon, Tooltip, Notice } from '@wordpress/components';
import { useState, useEffect, useRef, useMemo, useCallback } from '@wordpress/element';
import { __ } from '@wordpress/i18n';
import { next, closeSmall, chevronLeft } from '@wordpress/icons';
Expand Down Expand Up @@ -64,7 +64,10 @@ export default function AssistantWizard( { close } ) {

const handleNext = useCallback( () => {
debug( 'handleNext, stepsCount', stepsCount );
let nextStep;
let nextStep: number;

steps[ currentStep ].resetState?.();

setCurrentStep( prev => {
if ( prev + 1 < stepsCount ) {
nextStep = prev + 1;
Expand All @@ -74,7 +77,7 @@ export default function AssistantWizard( { close } ) {
}
return prev;
} );
}, [ stepsCount, steps ] );
}, [ stepsCount, steps, currentStep ] );

useEffect( () => {
const currentId = currentStepData?.id;
Expand Down Expand Up @@ -157,6 +160,7 @@ export default function AssistantWizard( { close } ) {
setIsBusy( true );
setAssistantFlowAction( 'backwards' );
debug( 'moving back to ' + ( currentStep - 1 ) );
steps[ currentStep ].resetState?.();
setCurrentStep( currentStep - 1 );
setCurrentStepData( steps[ currentStep - 1 ] );
}
Expand Down Expand Up @@ -189,7 +193,7 @@ export default function AssistantWizard( { close } ) {
const handleRetry = useCallback( async () => {
debug( 'handleRetry' );
setIsBusy( true );
await steps[ currentStep ].onRetry?.();
await steps[ currentStep ].onRetry?.( {} );
setIsBusy( false );
}, [ currentStep, steps ] );

Expand Down Expand Up @@ -229,6 +233,12 @@ export default function AssistantWizard( { close } ) {
isBusy={ isBusy }
/>
) ) }

{ steps[ currentStep ].hasFailed && (
<Notice status="error" isDismissible={ false }>
{ __( 'Something went wrong. Please try again or skip this step.', 'jetpack' ) }
</Notice>
) }
<div ref={ stepsEndRef } />
</div>

Expand All @@ -245,6 +255,7 @@ export default function AssistantWizard( { close } ) {
{ currentStep === 2 && steps[ currentStep ].type === 'options' && (
<OptionsInput
disabled={ ! steps[ currentStep ].hasSelection }
loading={ isBusy }
submitCtaLabel={ steps[ currentStep ].submitCtaLabel }
retryCtaLabel={ steps[ currentStep ].retryCtaLabel }
handleRetry={ handleRetry }
Expand All @@ -254,6 +265,7 @@ export default function AssistantWizard( { close } ) {
{ currentStep === 3 && steps[ currentStep ].type === 'options' && (
<OptionsInput
disabled={ ! steps[ currentStep ].hasSelection }
loading={ isBusy }
submitCtaLabel={ steps[ currentStep ].submitCtaLabel }
retryCtaLabel={ steps[ currentStep ].retryCtaLabel }
handleRetry={ handleRetry }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,17 @@ export interface Results {
};
}

type OnStart = ( options?: { fromSkip?: boolean; results?: Results } ) => Promise< void | string >;

export interface Step {
id: string;
title: string;
label?: string;
messages: Message[];
type: StepType;
onStart?: ( options?: { fromSkip: boolean; results: Results } ) => void;
onStart?: OnStart;
onSubmit?: () => Promise< string >;
onSkip?: () => void;
onSkip?: () => Promise< void >;
value?: string;
setValue?:
| React.Dispatch< React.SetStateAction< string > >
Expand All @@ -43,7 +45,9 @@ export interface Step {
options?: OptionMessage[];
onSelect?: ( option: OptionMessage ) => void;
submitCtaLabel?: string;
onRetry?: () => void;
onRetry?: OnStart;
retryCtaLabel?: string;
hasSelection?: boolean;
hasFailed?: boolean;
resetState?: () => void;
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,13 @@
import { askQuestionSync, usePostContent } from '@automattic/jetpack-ai-client';
import { useDispatch, useSelect } from '@wordpress/data';
import { store as editorStore } from '@wordpress/editor';
import { useCallback, useState, createInterpolateElement, useMemo } from '@wordpress/element';
import {
useCallback,
useState,
createInterpolateElement,
useMemo,
useEffect,
} from '@wordpress/element';
import { __ } from '@wordpress/i18n';
/*
* Internal dependencies
Expand Down Expand Up @@ -38,6 +44,8 @@ export const useTitleStep = ( {
const postContent = usePostContent();
const postId = useSelect( select => select( editorStore ).getCurrentPostId(), [] );
const [ generatedCount, setGeneratedCount ] = useState( 0 );
const [ hasFailed, setHasFailed ] = useState( false );
const [ failurePoint, setFailurePoint ] = useState< 'generate' | 'regenerate' | null >( null );

const prevStepHasChanged = useMemo( () => keywords !== lastValue, [ keywords, lastValue ] );

Expand Down Expand Up @@ -87,29 +95,49 @@ export const useTitleStep = ( {
return newTitles;
}, [ generatedCount, request ] );

useEffect( () => {
if ( ! hasFailed ) {
// Reset the failure point when the request is successful
setFailurePoint( null );
}
}, [ hasFailed ] );

const handleTitleGenerate = useCallback(
async ( { fromSkip } ) => {
let newTitles = [ ...titleOptions ];
const previousLastValue = lastValue;

setLastValue( keywords );
const initialMessage = fromSkip
? {
content: createInterpolateElement(
__( "Skipped!<br />Let's optimise your title first.", 'jetpack' ),
{ br: <br /> }
),
showIcon: true,
}
: {
content: __( "Let's optimise your title first.", 'jetpack' ),
showIcon: true,
};
setMessages( [ initialMessage ] );

if ( ! hasFailed ) {
const initialMessage = fromSkip
? {
content: createInterpolateElement(
__( "Skipped!<br />Let's optimise your title first.", 'jetpack' ),
{ br: <br /> }
),
showIcon: true,
}
: {
content: __( "Let's optimise your title first.", 'jetpack' ),
showIcon: true,
};
setMessages( [ initialMessage ] );
}

// we only generate if options are empty
if ( newTitles.length === 0 || prevStepHasChanged ) {
setSelectedTitle( '' );
newTitles = await getTitles();
try {
setSelectedTitle( '' );
setHasFailed( false );
newTitles = await getTitles();
} catch {
setFailurePoint( 'generate' );
setHasFailed( true );
// reset the last value to the previous value on failure to avoid a wrong value for prevStepHasChanged
setLastValue( previousLastValue );
return;
}
}

const readyMessageSuffix = createInterpolateElement(
Expand All @@ -122,29 +150,37 @@ export const useTitleStep = ( {
if ( newTitles.length ) {
// this sets the title options for internal state
setTitleOptions( newTitles );
// this addes title options as message-buttons
// this adds title options as message-buttons
newTitles.forEach( title => addMessage( { ...title, type: 'option', isUser: true } ) );
}
return value;
},
[
titleOptions,
prevStepHasChanged,
lastValue,
keywords,
setMessages,
hasFailed,
prevStepHasChanged,
editLastMessage,
value,
setMessages,
getTitles,
addMessage,
value,
]
);

const handleTitleRegenerate = useCallback( async () => {
const newTitles = await getTitles();

setTitleOptions( [ ...titleOptions, ...newTitles ] );
newTitles.forEach( title => addMessage( { ...title, type: 'option', isUser: true } ) );
}, [ addMessage, getTitles, titleOptions ] );
try {
setHasFailed( false );
const newTitles = await getTitles();

setTitleOptions( [ ...titleOptions, ...newTitles ] );
newTitles.forEach( title => addMessage( { ...title, type: 'option', isUser: true } ) );
} catch {
setFailurePoint( 'regenerate' );
setHasFailed( true );
}
}, [ getTitles, titleOptions, addMessage ] );

const handleTitleSubmit = useCallback( async () => {
setValue( selectedTitle );
Expand All @@ -153,6 +189,15 @@ export const useTitleStep = ( {
return selectedTitle;
}, [ selectedTitle, addMessage, editPost ] );

const resetState = useCallback( () => {
setHasFailed( false );
setFailurePoint( null );
}, [] );

// The build fails if we use i18n strings directly in a ternary operator.
const tryAgainLabel = __( 'Try again', 'jetpack' );
const regenerateLabel = __( 'Regenerate', 'jetpack' );

return {
id: 'title',
title: __( 'Optimise Title', 'jetpack' ),
Expand All @@ -163,12 +208,14 @@ export const useTitleStep = ( {
onSelect: handleTitleSelect,
onSubmit: handleTitleSubmit,
submitCtaLabel: __( 'Insert', 'jetpack' ),
onRetry: handleTitleRegenerate,
retryCtaLabel: __( 'Regenerate', 'jetpack' ),
onRetry: failurePoint === 'generate' ? handleTitleGenerate : handleTitleRegenerate,
retryCtaLabel: failurePoint === 'generate' ? tryAgainLabel : regenerateLabel,
onStart: handleTitleGenerate,
value,
setValue,
includeInResults: true,
hasSelection: !! selectedTitle,
hasFailed,
resetState,
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,20 @@ import { arrowRight } from '@wordpress/icons';

export const OptionsInput = ( {
disabled,
loading,
handleRetry,
retryCtaLabel,
handleSubmit,
submitCtaLabel,
} ) => {
return (
<div className="assistant-wizard__actions">
<Button variant="secondary" className="assistant-wizard__submit" onClick={ handleRetry }>
<Button
variant="secondary"
className="assistant-wizard__submit"
onClick={ handleRetry }
disabled={ loading }
>
{ retryCtaLabel }
</Button>

Expand Down
Loading