Skip to content

Commit a8200eb

Browse files
fredrikekelundfredrikekelund
authored
Refactor and clean up site deletion logic in app (#2915)
* Refactor site deletion in app * Nit fix
1 parent 9d64145 commit a8200eb

File tree

2 files changed

+43
-192
lines changed

2 files changed

+43
-192
lines changed

‎apps/studio/src/hooks/use-site-details.tsx‎

Lines changed: 37 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,10 @@ import {
1111
useRef,
1212
useState,
1313
} from 'react';
14-
import { useAuth } from 'src/hooks/use-auth';
1514
import { useContentTabs } from 'src/hooks/use-content-tabs';
1615
import { useIpcListener } from 'src/hooks/use-ipc-listener';
17-
import { useOffline } from 'src/hooks/use-offline';
1816
import { simplifyErrorForDisplay } from 'src/lib/error-formatting';
1917
import { getIpcApi } from 'src/lib/get-ipc-api';
20-
import { useAppDispatch } from 'src/stores';
21-
import { snapshotThunks } from 'src/stores/snapshot-slice';
2218
import type { Blueprint } from 'src/stores/wpcom-api';
2319

2420
interface SiteDetailsContext {
@@ -114,61 +110,6 @@ function useSelectedSite( firstSiteId: string | null ) {
114110
};
115111
}
116112

117-
function useDeleteSite() {
118-
const [ isLoading, setIsLoading ] = useState< Record< string, boolean > >( {} );
119-
const dispatch = useAppDispatch();
120-
const isOffline = useOffline();
121-
const { isAuthenticated } = useAuth();
122-
123-
const deleteSite = useCallback(
124-
async ( siteId: string, removeLocal: boolean ): Promise< void > => {
125-
if ( ! siteId ) {
126-
return;
127-
}
128-
129-
const shouldDeletePreviewSites = ! isOffline && isAuthenticated;
130-
131-
const allSiteRemovePromises = shouldDeletePreviewSites
132-
? dispatch( snapshotThunks.deleteAllSnapshotsForSite( { siteId } ) )
133-
: Promise.resolve();
134-
135-
try {
136-
setIsLoading( ( loading ) => ( { ...loading, [ siteId ]: true } ) );
137-
138-
await getIpcApi().deleteSite( siteId, removeLocal );
139-
140-
if ( shouldDeletePreviewSites ) {
141-
await allSiteRemovePromises;
142-
}
143-
144-
// After site is deleted successfully, clean up wpcom connections
145-
try {
146-
const connectedSites = await getIpcApi().getConnectedWpcomSites( siteId );
147-
const connectedSiteIds = connectedSites.map( ( site ) => site.id );
148-
if ( connectedSiteIds.length > 0 ) {
149-
await getIpcApi().disconnectWpcomSites( [
150-
{
151-
siteIds: connectedSiteIds,
152-
localSiteId: siteId,
153-
},
154-
] );
155-
}
156-
} catch ( error ) {
157-
// If disconnection fails, log but don't fail the deletion
158-
console.error( 'Failed to disconnect wpcom sites:', error );
159-
}
160-
} catch ( error ) {
161-
console.error( 'Error during site deletion:', error );
162-
throw error;
163-
} finally {
164-
setIsLoading( ( loading ) => ( { ...loading, [ siteId ]: false } ) );
165-
}
166-
},
167-
[ dispatch, isOffline, isAuthenticated ]
168-
);
169-
return { deleteSite, isLoading };
170-
}
171-
172113
export function SiteDetailsProvider( { children }: SiteDetailsProviderProps ) {
173114
const { Provider } = siteDetailsContext;
174115

@@ -187,7 +128,7 @@ export function SiteDetailsProvider( { children }: SiteDetailsProviderProps ) {
187128
);
188129
const { selectedSiteId, setSelectedSiteId } = useSelectedSite( firstSite?.id );
189130
const [ uploadingSites, setUploadingSites ] = useState< { [ siteId: string ]: boolean } >( {} );
190-
const { deleteSite, isLoading: isDeleting } = useDeleteSite();
131+
const [ isDeleting, setIsDeleting ] = useState< Record< string, boolean > >( {} );
191132
const { setSelectedTab, selectedTab } = useContentTabs();
192133

193134
useIpcListener( 'on-site-create-progress', ( _, { siteId, message } ) => {
@@ -244,12 +185,42 @@ export function SiteDetailsProvider( { children }: SiteDetailsProviderProps ) {
244185
}, [] );
245186

246187
const onDeleteSite = useCallback(
247-
async ( id: string, removeLocal: boolean ) => {
248-
await deleteSite( id, removeLocal );
188+
async ( siteId: string, shouldDeleteFiles: boolean ) => {
189+
if ( ! siteId ) {
190+
return;
191+
}
192+
193+
try {
194+
setIsDeleting( ( prev ) => ( { ...prev, [ siteId ]: true } ) );
195+
196+
await getIpcApi().deleteSite( siteId, shouldDeleteFiles );
197+
198+
// After site is deleted successfully, clean up wpcom connections
199+
try {
200+
const connectedSites = await getIpcApi().getConnectedWpcomSites( siteId );
201+
const connectedSiteIds = connectedSites.map( ( site ) => site.id );
202+
if ( connectedSiteIds.length > 0 ) {
203+
await getIpcApi().disconnectWpcomSites( [
204+
{
205+
siteIds: connectedSiteIds,
206+
localSiteId: siteId,
207+
},
208+
] );
209+
}
210+
} catch ( error ) {
211+
// If disconnection fails, log but don't fail the deletion
212+
console.error( 'Failed to disconnect wpcom sites:', error );
213+
}
214+
} catch ( error ) {
215+
console.error( 'Error during site deletion:', error );
216+
throw error;
217+
} finally {
218+
setIsDeleting( ( prev ) => ( { ...prev, [ siteId ]: false } ) );
219+
}
220+
221+
// No need to update the `sites` state. That's handled in the `site-event` listener.
222+
249223
const newSites = await getIpcApi().getSiteDetails();
250-
setSites( sortSites( newSites ) );
251-
// Use functional update to access current selectedSiteId value
252-
// Tab reset is handled in SiteContentTabs when it detects the previous site was deleted
253224
setSelectedSiteId( ( currentSelectedId ) => {
254225
const selectedSiteStillExists = newSites.some( ( site ) => site.id === currentSelectedId );
255226
if ( ! selectedSiteStillExists ) {
@@ -258,7 +229,7 @@ export function SiteDetailsProvider( { children }: SiteDetailsProviderProps ) {
258229
return currentSelectedId;
259230
} );
260231
},
261-
[ deleteSite, setSelectedSiteId ]
232+
[ setSelectedSiteId ]
262233
);
263234

264235
const createSite = useCallback(

‎apps/studio/src/stores/snapshot-slice.ts‎

Lines changed: 6 additions & 126 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import {
33
createAsyncThunk,
44
createSelector,
55
createSlice,
6-
isAnyOf,
76
PayloadAction,
87
} from '@reduxjs/toolkit';
98
import { SNAPSHOT_EVENTS } from '@studio/common/lib/cli-events';
@@ -41,14 +40,7 @@ type DeleteOperation = BaseOperation & {
4140
optimistic?: boolean;
4241
};
4342

44-
type BulkOperation = Omit< BaseOperation, 'progress' > & {
45-
operationIds: UUID[];
46-
siteId?: string;
47-
type: 'bulk';
48-
userId?: number;
49-
};
50-
51-
export type SnapshotOperation = CreateOperation | UpdateOperation | DeleteOperation | BulkOperation;
43+
export type SnapshotOperation = CreateOperation | UpdateOperation | DeleteOperation;
5244

5345
type SnapshotState = {
5446
operations: Record< UUID, SnapshotOperation >;
@@ -97,41 +89,6 @@ const deleteSnapshot = createTypedAsyncThunk(
9789
}
9890
);
9991

100-
async function deleteMultipleSnapshots(
101-
snapshots: Snapshot[]
102-
): Promise< [ url: string, operationId: UUID ][] > {
103-
return await Promise.all(
104-
snapshots.map( async ( snapshot ) => {
105-
const { operationId } = await getIpcApi().deleteSnapshot( snapshot.url );
106-
return [ snapshot.url, operationId ];
107-
} )
108-
);
109-
}
110-
111-
const deleteAllSnapshotsForSite = createTypedAsyncThunk<
112-
{ operations: [ url: string, operationId: UUID ][]; bulkOperationId: UUID },
113-
{ siteId: string }
114-
>( 'snapshot/deleteAllSnapshotsForSite', async ( { siteId }, thunkAPI ) => {
115-
const state = thunkAPI.getState();
116-
const snapshots = snapshotSelectors.selectSnapshotsBySite( state, siteId );
117-
const operations = await deleteMultipleSnapshots( snapshots );
118-
const bulkOperationId = window.crypto.randomUUID();
119-
120-
return { operations, bulkOperationId };
121-
} );
122-
123-
const deleteAllSnapshotsForUser = createTypedAsyncThunk<
124-
{ operations: [ url: string, operationId: UUID ][]; bulkOperationId: UUID },
125-
{ userId: number }
126-
>( 'snapshot/deleteAllSnapshotsForUser', async ( { userId }, thunkAPI ) => {
127-
const state = thunkAPI.getState();
128-
const snapshots = snapshotSelectors.selectSnapshotsByUser( state, userId );
129-
const operations = await deleteMultipleSnapshots( snapshots );
130-
const bulkOperationId = window.crypto.randomUUID();
131-
132-
return { operations, bulkOperationId };
133-
} );
134-
13592
const updateSnapshotLocally = createAction< {
13693
atomicSiteId: number;
13794
snapshot: Partial< Omit< Snapshot, 'atomicSiteId' > >;
@@ -200,47 +157,9 @@ const snapshotSlice = createSlice( {
200157
type: 'delete',
201158
optimistic: action.meta.arg.optimistic ?? false,
202159
};
203-
} )
204-
.addMatcher(
205-
isAnyOf( deleteAllSnapshotsForSite.fulfilled, deleteAllSnapshotsForUser.fulfilled ),
206-
( state, action ) => {
207-
if ( ! action.payload.operations.length ) {
208-
return;
209-
}
210-
211-
const bulkOperation: BulkOperation = {
212-
error: null,
213-
operationIds: action.payload.operations.map( ( [ _, operationId ] ) => operationId ),
214-
status: 'pending',
215-
type: 'bulk',
216-
};
217-
218-
if ( 'siteId' in action.meta.arg ) {
219-
bulkOperation.siteId = action.meta.arg.siteId;
220-
} else if ( 'userId' in action.meta.arg ) {
221-
bulkOperation.userId = action.meta.arg.userId;
222-
}
223-
224-
state.operations[ action.payload.bulkOperationId ] = bulkOperation;
225-
226-
action.payload.operations.forEach( ( [ url, operationId ] ) => {
227-
state.operations[ operationId ] = {
228-
error: null,
229-
progress: 0,
230-
snapshotUrl: url,
231-
status: 'pending',
232-
type: 'delete',
233-
};
234-
} );
235-
}
236-
);
160+
} );
237161
},
238162
selectors: {
239-
selectActiveBulkOperationForUser: ( state, userId: number ): BulkOperation | undefined =>
240-
Object.values( state.operations ).find(
241-
( operation ): operation is BulkOperation =>
242-
operation.status === 'pending' && operation.type === 'bulk' && operation.userId === userId
243-
),
244163
selectActiveCreateOperationForSite: ( state, siteId: string ): CreateOperation | undefined =>
245164
Object.values( state.operations ).find(
246165
( operation ): operation is CreateOperation =>
@@ -268,6 +187,9 @@ const snapshotSlice = createSlice( {
268187
},
269188
} );
270189

190+
// We use the `createSelector` API here to memoize the result. Whenever a selector returns a
191+
// reference type (array, object, etc), it's good to use `createSelector` to avoid unnecessary
192+
// React re-renders.
271193
const selectActiveCreateUpdateOperationsForAnySite = createSelector(
272194
[ ( state: RootState ) => state.snapshot.operations ],
273195
( operations ) =>
@@ -373,27 +295,6 @@ function getOperation( operationId: UUID ) {
373295
return state.snapshot.operations[ operationId ];
374296
}
375297

376-
function getAssociatedBulkOperation( targetOperationId: UUID ): [ UUID, BulkOperation ] | [] {
377-
const state = store.getState();
378-
// `Object.entries()` always returns a string type for the key, but we know the type is actually constrained to UUID
379-
const entries = Object.entries( state.snapshot.operations ) as [ UUID, SnapshotOperation ][];
380-
381-
for ( const [ operationId, operation ] of entries ) {
382-
if ( operation.type === 'bulk' && operation.operationIds.includes( targetOperationId ) ) {
383-
return [ operationId, operation ];
384-
}
385-
}
386-
387-
return [];
388-
}
389-
390-
function isBulkOperationSettled( bulkOperation: BulkOperation ) {
391-
return bulkOperation.operationIds.every( ( operationId ) => {
392-
const operation = getOperation( operationId );
393-
return operation && operation.status !== 'pending';
394-
} );
395-
}
396-
397298
window.ipcListener.subscribe( 'snapshot-event', ( _, snapshotEvent ) => {
398299
const { event: eventType } = snapshotEvent;
399300

@@ -528,18 +429,6 @@ window.ipcListener.subscribe( 'snapshot-success', ( event, payload ) => {
528429
} )
529430
);
530431

531-
const [ bulkOperationId, bulkOperation ] = getAssociatedBulkOperation( payload.operationId );
532-
const bulkOperationIsSettled = bulkOperation && isBulkOperationSettled( bulkOperation );
533-
534-
if ( bulkOperationId && bulkOperationIsSettled ) {
535-
store.dispatch(
536-
snapshotActions.updateOperation( {
537-
operationId: bulkOperationId,
538-
operation: { status: 'fulfilled' },
539-
} )
540-
);
541-
}
542-
543432
const { snapshotUrl = '' } = operation;
544433

545434
if ( operation.type === 'create' ) {
@@ -553,18 +442,11 @@ window.ipcListener.subscribe( 'snapshot-success', ( event, payload ) => {
553442
body: sprintf( __( "Preview site '%s' has been updated." ), snapshotUrl ),
554443
} );
555444
} else if ( operation.type === 'delete' ) {
556-
if ( operation.optimistic ) {
557-
// Do nothing
558-
} else if ( ! bulkOperation ) {
445+
if ( ! operation.optimistic ) {
559446
getIpcApi().showNotification( {
560447
title: operation.snapshotName,
561448
body: sprintf( __( "Preview site '%s' has been deleted." ), snapshotUrl ),
562449
} );
563-
} else if ( bulkOperationIsSettled ) {
564-
getIpcApi().showNotification( {
565-
title: __( 'Delete Successful' ),
566-
body: __( 'All preview sites have been deleted.' ),
567-
} );
568450
}
569451
}
570452
} );
@@ -584,7 +466,5 @@ export const snapshotThunks = {
584466
createSnapshot,
585467
updateSnapshot,
586468
deleteSnapshot,
587-
deleteAllSnapshotsForSite,
588-
deleteAllSnapshotsForUser,
589469
};
590470
export const reducer = snapshotSlice.reducer;

0 commit comments

Comments
 (0)