Skip to content

Studio: Add failed state when the preview site fails to update #977

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.

Sign up for GitHub

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

Merged
katinthehatsite merged 8 commits into from
Feb 27, 2025
Merged

Studio: Add failed state when the preview site fails to update #977

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b1b4dd1
Add failed state
Feb 25, 2025
cb361fc
Remove testing instructions
Feb 25, 2025
ca3b289
Fix unintended changes
Feb 25, 2025
6d3e5d2
Adjust approach
Feb 25, 2025
69c4b08
Add cleanup
Feb 27, 2025
3f8cd26
Simplify state management
Feb 27, 2025
e57be3a
Cleanup when user retries
Feb 27, 2025
37ff84c
Merge branch 'trunk' of github.com:Automattic/studio into fix/add-fai…
Feb 27, 2025
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 21 additions & 5 deletions src/hooks/use-update-demo-site.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@ import { getIpcApi } from 'src/lib/get-ipc-api';
interface DemoSiteUpdateContextType {
updateDemoSite: ( snapshot: Snapshot, localSite: SiteDetails ) => Promise< void >;
isDemoSiteUpdating: ( atomicSiteId: number ) => boolean;
hasDemoSiteError: ( atomicSiteId: number ) => boolean;
}

const DemoSiteUpdateContext = createContext< DemoSiteUpdateContextType >( {
updateDemoSite: async () => undefined,
isDemoSiteUpdating: () => false,
hasDemoSiteError: () => false,
} );

interface DemoSiteUpdateProviderProps {
Expand All @@ -25,6 +27,7 @@ export const DemoSiteUpdateProvider: React.FC< DemoSiteUpdateProviderProps > = (
const { client } = useAuth();
const { __ } = useI18n();
const [ updatingSites, setUpdatingSites ] = useState< Set< number > >( new Set() );
const [ errorSites, setErrorSites ] = useState< Set< number > >( new Set() );
const { updateSnapshot } = useSnapshots();

Comment thread
katinthehatsite marked this conversation as resolved.
const updateDemoSite = useCallback(
Expand All @@ -34,6 +37,14 @@ export const DemoSiteUpdateProvider: React.FC< DemoSiteUpdateProviderProps > = (
return;
}
setUpdatingSites( ( prev ) => new Set( prev ).add( snapshot.atomicSiteId ) );
// Clear error when user retries
if ( errorSites.has( snapshot.atomicSiteId ) ) {
setErrorSites( ( prev ) => {
const next = new Set( prev );
next.delete( snapshot.atomicSiteId );
return next;
} );
}

let archivePath = '';
try {
Expand Down Expand Up @@ -96,6 +107,7 @@ export const DemoSiteUpdateProvider: React.FC< DemoSiteUpdateProviderProps > = (
} );
return response;
} catch ( error ) {
setErrorSites( ( prev ) => new Set( prev ).add( snapshot.atomicSiteId ) );

@sejas sejas Feb 25, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we only add sites to errorSites and never remove them. Is that correct?

@katinthehatsite katinthehatsite Feb 27, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the state should be cleared by itself when the component unmounts. And the UI should be reset after 1 minute timeout:

		const timeoutId = setTimeout( () => {
			setShowUpdatedMessage( false );
			setShowFailedMessage( false );
		}, UPDATED_MESSAGE_DURATION_MS );

In any case, I added some clean up when we attemp another update: https://github.com/Automattic/studio/pull/977/commits/69c4b08f0e6ecfc1bd493fdf35a377d288d91d76k
Let me know what you think.
getIpcApi().showErrorMessageBox( {
title: __( 'Update failed' ),
message: sprintf(
Expand All @@ -116,22 +128,26 @@ export const DemoSiteUpdateProvider: React.FC< DemoSiteUpdateProviderProps > = (
}
}
},
[ __, client, updateSnapshot ]
[ __, client, errorSites, updateSnapshot ]
);

const isDemoSiteUpdating = useCallback(
( atomicSiteId: number ) => {
return updatingSites.has( atomicSiteId );
},
( atomicSiteId: number ) => updatingSites.has( atomicSiteId ),
[ updatingSites ]
);

const hasDemoSiteError = useCallback(
( atomicSiteId: number ) => errorSites.has( atomicSiteId ),
[ errorSites ]
);

const contextValue = useMemo(
() => ( {
updateDemoSite,
isDemoSiteUpdating,
hasDemoSiteError,
} ),
[ updateDemoSite, isDemoSiteUpdating ]
[ updateDemoSite, isDemoSiteUpdating, hasDemoSiteError ]
);

return (
Expand Down
21 changes: 17 additions & 4 deletions src/modules/preview-site/components/preview-site-row.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Spinner } from '@wordpress/components';
import { sprintf } from '@wordpress/i18n';
import { Icon, published } from '@wordpress/icons';
import { Icon, published, warning } from '@wordpress/icons';
import { useI18n } from '@wordpress/react-i18n';
import { useEffect, useState, useRef } from 'react';
import { ArrowIcon } from 'src/components/arrow-icon';
Expand Down Expand Up @@ -35,8 +35,9 @@ export function PreviewSiteRow( {
const { url, date, isDeleting } = snapshot;
const { countDown, expireDateString, isExpired } = useExpirationDate( date );
const { fetchSnapshotUsage, removeSnapshot } = useSnapshots();
const { isDemoSiteUpdating } = useUpdateDemoSite();
const { isDemoSiteUpdating, hasDemoSiteError } = useUpdateDemoSite();
const isPreviewSiteUpdating = isDemoSiteUpdating( snapshot.atomicSiteId );
const hasError = hasDemoSiteError( snapshot.atomicSiteId );
const { formatRelativeTime } = useFormatLocalizedTimestamps();
const [ showUpdatedMessage, setShowUpdatedMessage ] = useState( false );
const wasUpdating = useRef( false );
Expand All @@ -52,14 +53,17 @@ export function PreviewSiteRow( {
return;
}
wasUpdating.current = false;
setShowUpdatedMessage( true );

if ( ! hasError ) {
setShowUpdatedMessage( true );
}

const timeoutId = setTimeout( () => {
setShowUpdatedMessage( false );
}, UPDATED_MESSAGE_DURATION_MS );

return () => clearTimeout( timeoutId );
}, [ isPreviewSiteUpdating ] );
}, [ hasError, isPreviewSiteUpdating ] );

const getLastUpdateTimeText = () => {
if ( ! date ) {
Expand All @@ -75,6 +79,15 @@ export function PreviewSiteRow( {
);
}

if ( hasError ) {
return (
<div className="flex items-center">
<Icon icon={ warning } className="!mt-0 mr-1 fill-a8c-red-50" />
<span className="text-a8c-red-50">{ __( 'Failed' ) }</span>
</div>
);
}

const timeDistance = formatRelativeTime( new Date( date ).toISOString() );
return sprintf( __( '%s ago' ), timeDistance );
};
Expand Down