From 399ddfe6230411b465f26d50433a3bee22f2e48b Mon Sep 17 00:00:00 2001 From: William Viana Date: Thu, 11 Jun 2026 16:05:33 +0200 Subject: [PATCH 1/3] VideoPress: show a deleting progress state in the dashboard library MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deleting media from the modernized VideoPress dashboard gave no feedback until the listing refetched, so the action felt frozen (CFT item VP09). - Add a 'deleting' upload status that rides the same in-flight channel as 'promoting': rows get a "Deleting…" overlay with an indeterminate progress bar in the grid layout and a "Deleting…" pill in the table layout, and stay non-interactive (no title link, no thumbnail button, no eligible row actions) until the refetched listing removes them. - Run bulk deletions in parallel (Promise.allSettled) instead of sequentially, and reject with a DeleteVideosError carrying the failed ids so partial failures report a precise count and keep the surviving rows selected. - Invalidate the library cache on settle and await the refetch before clearing the overlay so rows never flash back to their normal state ahead of removal; scope the invalidation away from item queries so the details page doesn't refetch the id it just deleted. - React via the mutateAsync promise instead of mutate-level callbacks, which TanStack drops when another mutation starts on the same observer or the component unmounts mid-flight. - Details page: show a persistent "Deleting video…" snackbar while the delete is in flight, guard against double-fire, and skip the post-delete navigation if the user already left the page. Fixes VIDP-251. Co-Authored-By: Claude Fable 5 --- ...videopress-delete-media-progress-indicator | 4 + .../videopress/routes/library/stage.tsx | 94 ++++++++++++++----- .../videopress/routes/video/stage.tsx | 52 ++++++++-- .../dashboard/components/library/actions.ts | 16 ++-- .../dashboard/components/library/fields.tsx | 5 + .../components/library/test/actions.test.ts | 54 +++++++++++ .../library/test/thumbnail-field.test.tsx | 20 ++++ .../components/library/thumbnail-field.tsx | 15 ++- .../hooks/test/use-delete-video.test.ts | 65 ++++++++++++- .../src/dashboard/hooks/use-delete-video.ts | 59 ++++++++++-- .../videopress/src/dashboard/types/library.ts | 6 +- 11 files changed, 336 insertions(+), 54 deletions(-) create mode 100644 projects/packages/videopress/changelog/update-videopress-delete-media-progress-indicator create mode 100644 projects/packages/videopress/src/dashboard/components/library/test/actions.test.ts diff --git a/projects/packages/videopress/changelog/update-videopress-delete-media-progress-indicator b/projects/packages/videopress/changelog/update-videopress-delete-media-progress-indicator new file mode 100644 index 000000000000..0a79e5d12971 --- /dev/null +++ b/projects/packages/videopress/changelog/update-videopress-delete-media-progress-indicator @@ -0,0 +1,4 @@ +Significance: patch +Type: fixed + +VideoPress dashboard: show a Deleting progress state on library rows and the video details page while media is being deleted, and run bulk deletions in parallel. diff --git a/projects/packages/videopress/routes/library/stage.tsx b/projects/packages/videopress/routes/library/stage.tsx index eb26ad495c91..177f3be68fe5 100644 --- a/projects/packages/videopress/routes/library/stage.tsx +++ b/projects/packages/videopress/routes/library/stage.tsx @@ -10,7 +10,7 @@ import { buildLibraryActions } from '../../src/dashboard/components/library/acti import { libraryFields } from '../../src/dashboard/components/library/fields'; import { UploadActionsProvider } from '../../src/dashboard/components/library/upload-actions-context'; import QueryClientWrapper from '../../src/dashboard/components/query-client-wrapper'; -import { useDeleteVideo } from '../../src/dashboard/hooks/use-delete-video'; +import { DeleteVideosError, useDeleteVideo } from '../../src/dashboard/hooks/use-delete-video'; import { useFreeTier } from '../../src/dashboard/hooks/use-free-tier'; import { useLibrary } from '../../src/dashboard/hooks/use-library'; import { useUpdateVideoMeta } from '../../src/dashboard/hooks/use-update-video-meta'; @@ -65,10 +65,14 @@ const StageInner = () => { // The upload-from-library endpoint doesn't report progress, so we just // need to know which rows to overlay with an "Uploading…" state. const [ promotingIds, setPromotingIds ] = useState< Set< string > >( () => new Set() ); + // IDs currently being deleted. Same overlay technique as promotingIds: + // rows get a "Deleting…" state (thumbnail overlay in grid, title pill in + // table) until the post-delete refetch removes them from the listing. + const [ deletingIds, setDeletingIds ] = useState< Set< string > >( () => new Set() ); const { items, isLoading, paginationInfo } = useLibrary( view ); const { uploadQueue, startUpload, retryUpload } = useUpload(); - const { mutate: deleteVideo } = useDeleteVideo(); + const { mutateAsync: deleteVideo } = useDeleteVideo(); const { mutate: updateMeta } = useUpdateVideoMeta(); const { mutate: uploadFromLibrary } = useUploadFromLibrary(); const { isAtLimit, isFree, isUnlimited, videoCount, limit } = useFreeTier(); @@ -210,8 +214,20 @@ const StageInner = () => { retryUpload, openVideoDetails, deleteItems: ( ids: string[] ) => { - deleteVideo( ids, { - onSuccess: () => { + setDeletingIds( prev => { + const next = new Set( prev ); + ids.forEach( id => next.add( id ) ); + return next; + } ); + // React via the mutateAsync promise, not mutate-level callbacks: + // those are dropped if another delete starts while this one is in + // flight (TanStack detaches the observer), which would strand + // rows in the "Deleting…" state. The promise settles only after + // the hook's awaited library refetch, so the cleanup below can't + // flash rows back to their normal state ahead of their removal + // from the listing. + deleteVideo( ids ) + .then( () => { createSuccessNotice( sprintf( /* translators: %d: number of deleted videos. */ @@ -224,18 +240,41 @@ const StageInner = () => { ids.length ) ); - }, - onError: () => { + return new Set< string >(); + } ) + .catch( ( error: Error ) => { + // Unknown error shape → assume nothing was deleted. + const failedIds = + error instanceof DeleteVideosError + ? new Set( error.failedIds.map( String ) ) + : new Set( ids ); createErrorNotice( - _n( - 'Failed to delete video.', - 'Failed to delete videos.', - ids.length, - 'jetpack-videopress-pkg' + sprintf( + /* translators: %d: number of videos that could not be deleted. */ + _n( + 'Failed to delete %d video.', + 'Failed to delete %d videos.', + failedIds.size, + 'jetpack-videopress-pkg' + ), + failedIds.size ) ); - }, - } ); + return failedIds; + } ) + .then( failedIds => { + setDeletingIds( prev => { + const next = new Set( prev ); + ids.forEach( id => next.delete( id ) ); + return next; + } ); + // Prune rows that are now gone from the DataViews selection + // so the bulk-actions toolbar doesn't keep counting them. On + // partial failure the failed rows survive and stay selected. + setSelection( prev => + prev.filter( id => ! ids.includes( id ) || failedIds.has( id ) ) + ); + } ); }, setPrivacy: ( id: string, privacy: LibraryItemPrivacy ) => { updateMeta( @@ -296,19 +335,24 @@ const StageInner = () => { shortcode: '', isProcessing: false, } ) ); - // Overlay an "uploading"-style state on items currently being - // promoted from local-storage to VideoPress, so the title-cell - // pill and the thumbnail overlay reflect the in-flight state - // without needing a parallel signal at every render site. - const overlaid = promotingIds.size - ? items.map( item => - promotingIds.has( item.id ) - ? { ...item, upload: { status: 'promoting' as const, progress: 0 } } - : item - ) - : items; + // Overlay an in-flight state on items currently being promoted from + // local-storage to VideoPress or being deleted, so the title-cell + // pill and the thumbnail overlay reflect the operation without + // needing a parallel signal at every render site. + const overlaid = + promotingIds.size || deletingIds.size + ? items.map( item => { + if ( promotingIds.has( item.id ) ) { + return { ...item, upload: { status: 'promoting' as const, progress: 0 } }; + } + if ( deletingIds.has( item.id ) ) { + return { ...item, upload: { status: 'deleting' as const, progress: 0 } }; + } + return item; + } ) + : items; return [ ...inFlight, ...overlaid ]; - }, [ uploadQueue, items, promotingIds ] ); + }, [ uploadQueue, items, promotingIds, deletingIds ] ); const getItemId = useCallback( ( item: LibraryItem ) => item.id, [] ); diff --git a/projects/packages/videopress/routes/video/stage.tsx b/projects/packages/videopress/routes/video/stage.tsx index 347f8c665a93..5f8d7e0161d5 100644 --- a/projects/packages/videopress/routes/video/stage.tsx +++ b/projects/packages/videopress/routes/video/stage.tsx @@ -1,7 +1,7 @@ import AdminPage from '@automattic/jetpack-components/admin-page'; import { useGlobalNotices } from '@automattic/jetpack-components/global-notices'; import { Breadcrumbs } from '@wordpress/admin-ui'; -import { useCallback, useState } from '@wordpress/element'; +import { useCallback, useEffect, useRef, useState } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; import { Link, useNavigate, useParams } from '@wordpress/route'; import { Stack, Text } from '@wordpress/ui'; @@ -160,12 +160,27 @@ const Editor = ( { type StageReadyProps = { video: LibraryItem }; +// Stable id so the in-progress snackbar can be replaced/removed once the +// delete settles instead of stacking a second notice next to it. +const DELETING_NOTICE_ID = 'vp-video-deleting'; + const StageReady = ( { video }: StageReadyProps ) => { const navigate = useNavigate(); const { mutate: updateMeta, isPending: isSaving } = useUpdateVideoMeta(); - const { mutate: deleteVideo } = useDeleteVideo(); - const { createSuccessNotice, createErrorNotice } = useGlobalNotices(); + const { mutateAsync: deleteVideo, isPending: isDeleting } = useDeleteVideo(); + const { createSuccessNotice, createErrorNotice, createInfoNotice, removeNotice } = + useGlobalNotices(); const [ chaptersOpen, setChaptersOpen ] = useState( false ); + // Deletes keep running after an unmount (the user can navigate away via + // the breadcrumb mid-flight). The notice cleanup below must still happen + // then, but we shouldn't yank them to the Library if they've moved on. + const isMountedRef = useRef( true ); + useEffect( () => { + isMountedRef.current = true; + return () => { + isMountedRef.current = false; + }; + }, [] ); return ( { ); } } onDelete={ () => { - deleteVideo( Number( video.id ), { - onSuccess: () => { + if ( isDeleting ) { + return; + } + // Deleting can take several seconds (the backend also removes the + // remote VideoPress copy); surface progress immediately so the + // action doesn't feel frozen. `explicitDismiss` keeps the snackbar + // from auto-expiring before the request settles. + createInfoNotice( __( 'Deleting video…', 'jetpack-videopress-pkg' ), { + id: DELETING_NOTICE_ID, + explicitDismiss: true, + } ); + // Promise chain rather than mutate-level callbacks: those are + // dropped when the component unmounts mid-flight, which would + // orphan the explicitDismiss notice above forever. + deleteVideo( Number( video.id ) ) + .then( () => { + removeNotice( DELETING_NOTICE_ID ); createSuccessNotice( __( 'Video deleted.', 'jetpack-videopress-pkg' ) ); - navigate( { href: '/library' } ); - }, - onError: () => { + if ( isMountedRef.current ) { + navigate( { href: '/library' } ); + } + } ) + .catch( () => { + removeNotice( DELETING_NOTICE_ID ); createErrorNotice( __( 'Failed to delete video.', 'jetpack-videopress-pkg' ) ); - }, - } ); + } ); } } onDownload={ () => { if ( video.sourceUrl ) { diff --git a/projects/packages/videopress/src/dashboard/components/library/actions.ts b/projects/packages/videopress/src/dashboard/components/library/actions.ts index 195ea151e9cf..8a76c07f4d6e 100644 --- a/projects/packages/videopress/src/dashboard/components/library/actions.ts +++ b/projects/packages/videopress/src/dashboard/components/library/actions.ts @@ -11,13 +11,17 @@ type Api = { }; const isVideoPressIdle = ( item: LibraryItem ) => - item.type === 'videopress' && item.upload.status !== 'failed'; + item.type === 'videopress' && + item.upload.status !== 'failed' && + item.upload.status !== 'deleting'; /** * Build the DataViews actions array for the Library tab. Eligibility predicates * gate per-row availability based on `item.type` and `item.upload.status`. The * Delete action sets `supportsBulk: true` and `isEligible` to filter local items - * out of mixed selections (DataViews silently skips ineligible items). + * out of mixed selections (DataViews silently skips ineligible items). Rows with + * a delete already in flight are ineligible for every action so a slow delete + * can't be double-fired or raced by an edit. * * @param api - Hook mutators forwarded into the action callbacks. * @return The actions array for ``. @@ -41,7 +45,7 @@ export function buildLibraryActions( api: Api ): Action< LibraryItem >[] { id: 'set-privacy-public', label: __( 'Make public', 'jetpack-videopress-pkg' ), supportsBulk: false, - isEligible: item => item.type === 'videopress' && item.privacy !== 'public', + isEligible: item => isVideoPressIdle( item ) && item.privacy !== 'public', callback: items => { const [ item ] = items; if ( item ) { @@ -53,7 +57,7 @@ export function buildLibraryActions( api: Api ): Action< LibraryItem >[] { id: 'set-privacy-private', label: __( 'Make private', 'jetpack-videopress-pkg' ), supportsBulk: false, - isEligible: item => item.type === 'videopress' && item.privacy !== 'private', + isEligible: item => isVideoPressIdle( item ) && item.privacy !== 'private', callback: items => { const [ item ] = items; if ( item ) { @@ -65,7 +69,7 @@ export function buildLibraryActions( api: Api ): Action< LibraryItem >[] { id: 'set-privacy-site', label: __( 'Reset to site default', 'jetpack-videopress-pkg' ), supportsBulk: false, - isEligible: item => item.type === 'videopress' && item.privacy !== 'site-default', + isEligible: item => isVideoPressIdle( item ) && item.privacy !== 'site-default', callback: items => { const [ item ] = items; if ( item ) { @@ -77,7 +81,7 @@ export function buildLibraryActions( api: Api ): Action< LibraryItem >[] { id: 'delete', label: __( 'Delete', 'jetpack-videopress-pkg' ), supportsBulk: true, - isEligible: item => item.type === 'videopress', + isEligible: isVideoPressIdle, callback: items => { api.deleteItems( items.map( i => i.id ) ); }, diff --git a/projects/packages/videopress/src/dashboard/components/library/fields.tsx b/projects/packages/videopress/src/dashboard/components/library/fields.tsx index 90c30fdd340a..2b54a7a34c20 100644 --- a/projects/packages/videopress/src/dashboard/components/library/fields.tsx +++ b/projects/packages/videopress/src/dashboard/components/library/fields.tsx @@ -67,6 +67,11 @@ const TitleCell = ( { item }: { item: LibraryItem } ) => { intent: 'informational', label: __( 'Uploading…', 'jetpack-videopress-pkg' ), }; + } else if ( upload.status === 'deleting' ) { + pill = { + intent: 'informational', + label: __( 'Deleting…', 'jetpack-videopress-pkg' ), + }; } else if ( upload.status === 'failed' ) { pill = { intent: 'high', diff --git a/projects/packages/videopress/src/dashboard/components/library/test/actions.test.ts b/projects/packages/videopress/src/dashboard/components/library/test/actions.test.ts new file mode 100644 index 000000000000..741bb00dad2e --- /dev/null +++ b/projects/packages/videopress/src/dashboard/components/library/test/actions.test.ts @@ -0,0 +1,54 @@ +import { buildLibraryActions } from '../actions'; +import type { LibraryItem } from '../../../types/library'; + +const item = ( overrides: Partial< LibraryItem > = {} ): LibraryItem => ( { + id: '42', + guid: 'abc123', + type: 'videopress', + title: 'My Clip', + filename: 'clip.mp4', + thumbnailUrl: null, + durationSeconds: 0, + uploadDate: '2026-01-01T00:00:00', + privacy: 'public', + isPrivate: false, + fileSizeBytes: 0, + upload: { status: 'idle', progress: 0 }, + description: '', + rating: 'G', + displayEmbed: true, + allowDownloads: false, + shortcode: '', + isProcessing: false, + ...overrides, +} ); + +const makeApi = () => ( { + promoteLocal: jest.fn(), + retryUpload: jest.fn(), + deleteItems: jest.fn(), + setPrivacy: jest.fn(), + openVideoDetails: jest.fn(), +} ); + +describe( 'buildLibraryActions — eligibility while deleting', () => { + it( 'makes a row with a delete in flight ineligible for every action', () => { + const actions = buildLibraryActions( makeApi() ); + const deleting = item( { upload: { status: 'deleting', progress: 0 } } ); + + for ( const action of actions ) { + expect( { id: action.id, eligible: action.isEligible?.( deleting ) } ).toEqual( { + id: action.id, + eligible: false, + } ); + } + } ); + + it( 'keeps an idle VideoPress row eligible for delete and edit', () => { + const actions = buildLibraryActions( makeApi() ); + const idle = item(); + + expect( actions.find( a => a.id === 'delete' )?.isEligible?.( idle ) ).toBe( true ); + expect( actions.find( a => a.id === 'edit-details' )?.isEligible?.( idle ) ).toBe( true ); + } ); +} ); diff --git a/projects/packages/videopress/src/dashboard/components/library/test/thumbnail-field.test.tsx b/projects/packages/videopress/src/dashboard/components/library/test/thumbnail-field.test.tsx index d1275e083ee4..6508b11e2f1d 100644 --- a/projects/packages/videopress/src/dashboard/components/library/test/thumbnail-field.test.tsx +++ b/projects/packages/videopress/src/dashboard/components/library/test/thumbnail-field.test.tsx @@ -95,6 +95,15 @@ describe( 'ThumbnailField — grid Details access', () => { expect( screen.queryByRole( 'button', { name: /Edit details/ } ) ).not.toBeInTheDocument(); expect( screen.getByText( '40%' ) ).toBeInTheDocument(); } ); + + it( 'shows a Deleting… overlay and removes the open-details button while deleting', () => { + renderField( + , + makeActions() + ); + expect( screen.getByText( 'Deleting…' ) ).toBeInTheDocument(); + expect( screen.queryByRole( 'button', { name: /Edit details/ } ) ).not.toBeInTheDocument(); + } ); } ); describe( 'TitleCell — grid Details access', () => { @@ -114,4 +123,15 @@ describe( 'TitleCell — grid Details access', () => { expect( screen.getByText( 'Raw Footage' ) ).toBeInTheDocument(); expect( screen.queryByRole( 'button', { name: 'Raw Footage' } ) ).not.toBeInTheDocument(); } ); + + it( 'renders a Deleting… pill and a non-interactive title while deleting', () => { + renderField( + , + makeActions() + ); + expect( screen.getByText( 'Deleting…' ) ).toBeInTheDocument(); + expect( screen.queryByRole( 'button', { name: 'Doomed' } ) ).not.toBeInTheDocument(); + } ); } ); diff --git a/projects/packages/videopress/src/dashboard/components/library/thumbnail-field.tsx b/projects/packages/videopress/src/dashboard/components/library/thumbnail-field.tsx index c8b5da75cb5e..547aab7892cc 100644 --- a/projects/packages/videopress/src/dashboard/components/library/thumbnail-field.tsx +++ b/projects/packages/videopress/src/dashboard/components/library/thumbnail-field.tsx @@ -10,7 +10,7 @@ type Props = { item: LibraryItem }; /** * Render the media-area for one DataViews grid card. Layers (priority order): - * 1. uploading → ProgressBar overlay + * 1. uploading / promoting / deleting → ProgressBar overlay * 2. failed → red overlay with Retry * 3. local → "Local video" placeholder + hover-revealed Upload button * 4. videopress → thumbnail image + duration badge + click/hover "Edit details" @@ -126,6 +126,19 @@ export default function ThumbnailField( { item }: Props ) { ) : null } + { upload.status === 'deleting' ? ( + + { __( 'Deleting…', 'jetpack-videopress-pkg' ) } + + + ) : null } + { upload.status === 'failed' ? ( { it( 'sends DELETE for each id', async () => { @@ -26,4 +26,67 @@ describe( 'useDeleteVideo', () => { '/wp/v2/media/3?force=true', ] ); } ); + + it( 'rejects with a DeleteVideosError listing only the failed ids, still attempting every id', async () => { + const paths: string[] = []; + mockApiFetch( async ( { path, method } ) => { + if ( method === 'DELETE' ) { + paths.push( path ?? '' ); + if ( path === '/wp/v2/media/2?force=true' ) { + throw new Error( 'boom' ); + } + return { deleted: true }; + } + throw new Error( 'unexpected' ); + } ); + + const wrapper = createTestWrapper( createTestQueryClient() ); + const { result } = renderHook( () => useDeleteVideo(), { wrapper } ); + + let caught: unknown; + await act( async () => { + try { + await result.current.mutateAsync( [ 1, 2, 3 ] ); + } catch ( error ) { + caught = error; + } + } ); + + expect( caught ).toBeInstanceOf( DeleteVideosError ); + expect( ( caught as DeleteVideosError ).failedIds ).toEqual( [ 2 ] ); + // A failure must not short-circuit the rest of the batch. + expect( paths ).toHaveLength( 3 ); + } ); + + it( 'invalidates library list queries (not item queries) even when deletions fail', async () => { + mockApiFetch( async ( { method } ) => { + if ( method === 'DELETE' ) { + throw new Error( 'boom' ); + } + throw new Error( 'unexpected' ); + } ); + + const client = createTestQueryClient(); + const invalidateSpy = jest.spyOn( client, 'invalidateQueries' ); + const wrapper = createTestWrapper( client ); + const { result } = renderHook( () => useDeleteVideo(), { wrapper } ); + + await act( async () => { + await result.current.mutateAsync( 1 ).catch( () => { + // Rejection is expected; the assertion below is about invalidation. + } ); + } ); + + expect( invalidateSpy ).toHaveBeenCalledTimes( 1 ); + const { predicate } = invalidateSpy.mock.calls[ 0 ][ 0 ] as unknown as { + predicate: ( query: { queryKey: unknown[] } ) => boolean; + }; + // List queries refetch; the (possibly just-deleted) item query must + // not — refetching it would 404 and stall the mutation settling. + expect( predicate( { queryKey: [ 'jetpack-videopress-library', { page: 1 } ] } ) ).toBe( true ); + expect( predicate( { queryKey: [ 'jetpack-videopress-library', 'item', '9' ] } ) ).toBe( + false + ); + expect( predicate( { queryKey: [ 'unrelated' ] } ) ).toBe( false ); + } ); } ); diff --git a/projects/packages/videopress/src/dashboard/hooks/use-delete-video.ts b/projects/packages/videopress/src/dashboard/hooks/use-delete-video.ts index 9d44838c463a..162fd03fb253 100644 --- a/projects/packages/videopress/src/dashboard/hooks/use-delete-video.ts +++ b/projects/packages/videopress/src/dashboard/hooks/use-delete-video.ts @@ -5,8 +5,33 @@ import { LIBRARY_QUERY_KEY } from './use-library'; type Id = number | string; /** - * Return a mutation that permanently deletes one or more videos via DELETE /wp/v2/media/{id}?force=true - * and invalidates the library cache on success. + * Thrown when one or more deletions in a batch fail. Carries the ids that + * failed so callers can report precise counts and keep the surviving rows + * selected; the other ids in the batch were deleted successfully. + */ +export class DeleteVideosError extends Error { + failedIds: Id[]; + + constructor( failedIds: Id[] ) { + super( `Failed to delete ${ failedIds.length } video(s)` ); + this.name = 'DeleteVideosError'; + this.failedIds = failedIds; + } +} + +/** + * Return a mutation that permanently deletes one or more videos via DELETE + * /wp/v2/media/{id}?force=true. Requests run in parallel; if any fail the + * mutation rejects with a DeleteVideosError listing the failed ids (the rest + * stay deleted). The library cache is invalidated on settle — success and + * partial failure both change the listing — and the invalidation promise is + * returned so the mutation settles only after active queries refetch, letting + * row-level "Deleting…" states persist until rows actually disappear. + * + * Callers should react via the mutateAsync promise rather than mutate-level + * callbacks: TanStack detaches the observer from an in-flight mutation when + * the same hook instance starts another one, silently dropping that call's + * mutate-level callbacks — the promise always settles. * * @return A TanStack Query mutation object whose mutateAsync accepts a single id or an array of ids. */ @@ -15,15 +40,29 @@ export function useDeleteVideo() { return useMutation< void, Error, Id | Id[] >( { mutationFn: async input => { const ids = Array.isArray( input ) ? input : [ input ]; - for ( const id of ids ) { - await apiFetch( { - path: `/wp/v2/media/${ id }?force=true`, - method: 'DELETE', - } ); + const results = await Promise.allSettled( + ids.map( id => + apiFetch( { + path: `/wp/v2/media/${ id }?force=true`, + method: 'DELETE', + } ) + ) + ); + const failedIds = ids.filter( ( _, index ) => results[ index ].status === 'rejected' ); + if ( failedIds.length > 0 ) { + throw new DeleteVideosError( failedIds ); } }, - onSuccess: () => { - client.invalidateQueries( { queryKey: [ LIBRARY_QUERY_KEY ] } ); - }, + // Predicate rather than a key prefix: plain [ LIBRARY_QUERY_KEY ] would + // also match useVideo's [ LIBRARY_QUERY_KEY, 'item', id ] — on the + // details page that query is active for the id being deleted, and the + // refetch would 404 (+ one retry) and stall settling by seconds. + // Deleting can't change other items' detail data, so item queries are + // excluded entirely. + onSettled: () => + client.invalidateQueries( { + predicate: query => + query.queryKey[ 0 ] === LIBRARY_QUERY_KEY && query.queryKey[ 1 ] !== 'item', + } ), } ); } diff --git a/projects/packages/videopress/src/dashboard/types/library.ts b/projects/packages/videopress/src/dashboard/types/library.ts index 85dcd5a25a9a..4259935d5487 100644 --- a/projects/packages/videopress/src/dashboard/types/library.ts +++ b/projects/packages/videopress/src/dashboard/types/library.ts @@ -1,6 +1,10 @@ export type LibraryItemType = 'videopress' | 'local'; export type LibraryItemPrivacy = 'public' | 'private' | 'site-default'; -export type UploadStatus = 'idle' | 'uploading' | 'promoting' | 'failed'; +// `upload.status` doubles as the row's single in-flight-operation slot: +// 'deleting' isn't an upload state, but riding this channel means every +// render site that keys interactivity off `status === 'idle'` (title link, +// thumbnail button, action eligibility) handles it without extra plumbing. +export type UploadStatus = 'idle' | 'uploading' | 'promoting' | 'deleting' | 'failed'; export type VideoRating = 'G' | 'PG-13' | 'R'; export interface UploadState { From bdff83a1722ead1f364850e4a2ee47b6aa832552 Mon Sep 17 00:00:00 2001 From: William Viana Date: Thu, 11 Jun 2026 17:27:24 +0200 Subject: [PATCH 2/3] VideoPress: simplify delete-progress implementation Cleanup pass over the previous commit, no behavior changes: - Hoist the duplicated LibraryItem test fixture into test-utils/library-item.ts and reuse it in both library test suites. - Merge the promoting/deleting thumbnail overlays into one block (they differed only in the label). - Replace the settle notices' remove-then-create dance with same-id replacement (the notices store drops an existing notice with the same id on create). - Flip isVideoPressIdle from a status blocklist to an allowlist on 'idle', matching the other render sites, so future statuses are excluded from row actions by default. - Flatten the value-threaded .then/.catch/.then chain in deleteItems into async/await with a linear cleanup tail; hoist the requested-ids Set out of the selection filter. - Drop the no-op size guard around the renderedItems overlay map. - Share the item-query key segment between use-video and the delete hook's invalidation predicate instead of re-encoding 'item' at each site. Co-Authored-By: Claude Fable 5 --- .../videopress/routes/library/stage.tsx | 122 ++++++++---------- .../videopress/routes/video/stage.tsx | 18 +-- .../dashboard/components/library/actions.ts | 7 +- .../components/library/test/actions.test.ts | 24 +--- .../library/test/thumbnail-field.test.tsx | 23 +--- .../components/library/thumbnail-field.tsx | 21 +-- .../src/dashboard/hooks/use-delete-video.ts | 14 +- .../src/dashboard/hooks/use-library.ts | 6 + .../src/dashboard/hooks/use-video.ts | 4 +- .../src/dashboard/test-utils/library-item.ts | 32 +++++ 10 files changed, 124 insertions(+), 147 deletions(-) create mode 100644 projects/packages/videopress/src/dashboard/test-utils/library-item.ts diff --git a/projects/packages/videopress/routes/library/stage.tsx b/projects/packages/videopress/routes/library/stage.tsx index 177f3be68fe5..b51edadbbb6f 100644 --- a/projects/packages/videopress/routes/library/stage.tsx +++ b/projects/packages/videopress/routes/library/stage.tsx @@ -213,12 +213,8 @@ const StageInner = () => { promoteLocal, retryUpload, openVideoDetails, - deleteItems: ( ids: string[] ) => { - setDeletingIds( prev => { - const next = new Set( prev ); - ids.forEach( id => next.add( id ) ); - return next; - } ); + deleteItems: async ( ids: string[] ) => { + setDeletingIds( prev => new Set( [ ...prev, ...ids ] ) ); // React via the mutateAsync promise, not mutate-level callbacks: // those are dropped if another delete starts while this one is in // flight (TanStack detaches the observer), which would strand @@ -226,55 +222,50 @@ const StageInner = () => { // the hook's awaited library refetch, so the cleanup below can't // flash rows back to their normal state ahead of their removal // from the listing. - deleteVideo( ids ) - .then( () => { - createSuccessNotice( - sprintf( - /* translators: %d: number of deleted videos. */ - _n( - '%d video deleted.', - '%d videos deleted.', - ids.length, - 'jetpack-videopress-pkg' - ), - ids.length - ) - ); - return new Set< string >(); - } ) - .catch( ( error: Error ) => { - // Unknown error shape → assume nothing was deleted. - const failedIds = - error instanceof DeleteVideosError - ? new Set( error.failedIds.map( String ) ) - : new Set( ids ); - createErrorNotice( - sprintf( - /* translators: %d: number of videos that could not be deleted. */ - _n( - 'Failed to delete %d video.', - 'Failed to delete %d videos.', - failedIds.size, - 'jetpack-videopress-pkg' - ), - failedIds.size - ) - ); - return failedIds; - } ) - .then( failedIds => { - setDeletingIds( prev => { - const next = new Set( prev ); - ids.forEach( id => next.delete( id ) ); - return next; - } ); - // Prune rows that are now gone from the DataViews selection - // so the bulk-actions toolbar doesn't keep counting them. On - // partial failure the failed rows survive and stay selected. - setSelection( prev => - prev.filter( id => ! ids.includes( id ) || failedIds.has( id ) ) - ); - } ); + let failedIds = new Set< string >(); + try { + await deleteVideo( ids ); + createSuccessNotice( + sprintf( + /* translators: %d: number of deleted videos. */ + _n( + '%d video deleted.', + '%d videos deleted.', + ids.length, + 'jetpack-videopress-pkg' + ), + ids.length + ) + ); + } catch ( error ) { + // Unknown error shape → assume nothing was deleted. + failedIds = + error instanceof DeleteVideosError + ? new Set( error.failedIds.map( String ) ) + : new Set( ids ); + createErrorNotice( + sprintf( + /* translators: %d: number of videos that could not be deleted. */ + _n( + 'Failed to delete %d video.', + 'Failed to delete %d videos.', + failedIds.size, + 'jetpack-videopress-pkg' + ), + failedIds.size + ) + ); + } + setDeletingIds( prev => { + const next = new Set( prev ); + ids.forEach( id => next.delete( id ) ); + return next; + } ); + // Prune rows that are now gone from the DataViews selection so + // the bulk-actions toolbar doesn't keep counting them. On partial + // failure the failed rows survive and stay selected. + const requested = new Set( ids ); + setSelection( prev => prev.filter( id => ! requested.has( id ) || failedIds.has( id ) ) ); }, setPrivacy: ( id: string, privacy: LibraryItemPrivacy ) => { updateMeta( @@ -339,18 +330,15 @@ const StageInner = () => { // local-storage to VideoPress or being deleted, so the title-cell // pill and the thumbnail overlay reflect the operation without // needing a parallel signal at every render site. - const overlaid = - promotingIds.size || deletingIds.size - ? items.map( item => { - if ( promotingIds.has( item.id ) ) { - return { ...item, upload: { status: 'promoting' as const, progress: 0 } }; - } - if ( deletingIds.has( item.id ) ) { - return { ...item, upload: { status: 'deleting' as const, progress: 0 } }; - } - return item; - } ) - : items; + const overlaid = items.map( item => { + if ( promotingIds.has( item.id ) ) { + return { ...item, upload: { status: 'promoting' as const, progress: 0 } }; + } + if ( deletingIds.has( item.id ) ) { + return { ...item, upload: { status: 'deleting' as const, progress: 0 } }; + } + return item; + } ); return [ ...inFlight, ...overlaid ]; }, [ uploadQueue, items, promotingIds, deletingIds ] ); diff --git a/projects/packages/videopress/routes/video/stage.tsx b/projects/packages/videopress/routes/video/stage.tsx index 5f8d7e0161d5..42ef0ed813fd 100644 --- a/projects/packages/videopress/routes/video/stage.tsx +++ b/projects/packages/videopress/routes/video/stage.tsx @@ -160,16 +160,16 @@ const Editor = ( { type StageReadyProps = { video: LibraryItem }; -// Stable id so the in-progress snackbar can be replaced/removed once the -// delete settles instead of stacking a second notice next to it. +// Stable id so the settle notices replace the in-progress snackbar in place +// (the notices store drops an existing notice with the same id on create) +// instead of stacking a second notice next to it. const DELETING_NOTICE_ID = 'vp-video-deleting'; const StageReady = ( { video }: StageReadyProps ) => { const navigate = useNavigate(); const { mutate: updateMeta, isPending: isSaving } = useUpdateVideoMeta(); const { mutateAsync: deleteVideo, isPending: isDeleting } = useDeleteVideo(); - const { createSuccessNotice, createErrorNotice, createInfoNotice, removeNotice } = - useGlobalNotices(); + const { createSuccessNotice, createErrorNotice, createInfoNotice } = useGlobalNotices(); const [ chaptersOpen, setChaptersOpen ] = useState( false ); // Deletes keep running after an unmount (the user can navigate away via // the breadcrumb mid-flight). The notice cleanup below must still happen @@ -217,15 +217,17 @@ const StageReady = ( { video }: StageReadyProps ) => { // orphan the explicitDismiss notice above forever. deleteVideo( Number( video.id ) ) .then( () => { - removeNotice( DELETING_NOTICE_ID ); - createSuccessNotice( __( 'Video deleted.', 'jetpack-videopress-pkg' ) ); + createSuccessNotice( __( 'Video deleted.', 'jetpack-videopress-pkg' ), { + id: DELETING_NOTICE_ID, + } ); if ( isMountedRef.current ) { navigate( { href: '/library' } ); } } ) .catch( () => { - removeNotice( DELETING_NOTICE_ID ); - createErrorNotice( __( 'Failed to delete video.', 'jetpack-videopress-pkg' ) ); + createErrorNotice( __( 'Failed to delete video.', 'jetpack-videopress-pkg' ), { + id: DELETING_NOTICE_ID, + } ); } ); } } onDownload={ () => { diff --git a/projects/packages/videopress/src/dashboard/components/library/actions.ts b/projects/packages/videopress/src/dashboard/components/library/actions.ts index 8a76c07f4d6e..71c896bbad56 100644 --- a/projects/packages/videopress/src/dashboard/components/library/actions.ts +++ b/projects/packages/videopress/src/dashboard/components/library/actions.ts @@ -10,10 +10,11 @@ type Api = { openVideoDetails: ( id: string ) => void; }; +// Allowlist on 'idle' (matching TitleText and ThumbnailField) rather than a +// blocklist of known in-flight statuses, so any future status is excluded +// from row actions by default instead of silently slipping through. const isVideoPressIdle = ( item: LibraryItem ) => - item.type === 'videopress' && - item.upload.status !== 'failed' && - item.upload.status !== 'deleting'; + item.type === 'videopress' && item.upload.status === 'idle'; /** * Build the DataViews actions array for the Library tab. Eligibility predicates diff --git a/projects/packages/videopress/src/dashboard/components/library/test/actions.test.ts b/projects/packages/videopress/src/dashboard/components/library/test/actions.test.ts index 741bb00dad2e..c25be3e05b80 100644 --- a/projects/packages/videopress/src/dashboard/components/library/test/actions.test.ts +++ b/projects/packages/videopress/src/dashboard/components/library/test/actions.test.ts @@ -1,27 +1,5 @@ +import { makeLibraryItem as item } from '../../../test-utils/library-item'; import { buildLibraryActions } from '../actions'; -import type { LibraryItem } from '../../../types/library'; - -const item = ( overrides: Partial< LibraryItem > = {} ): LibraryItem => ( { - id: '42', - guid: 'abc123', - type: 'videopress', - title: 'My Clip', - filename: 'clip.mp4', - thumbnailUrl: null, - durationSeconds: 0, - uploadDate: '2026-01-01T00:00:00', - privacy: 'public', - isPrivate: false, - fileSizeBytes: 0, - upload: { status: 'idle', progress: 0 }, - description: '', - rating: 'G', - displayEmbed: true, - allowDownloads: false, - shortcode: '', - isProcessing: false, - ...overrides, -} ); const makeApi = () => ( { promoteLocal: jest.fn(), diff --git a/projects/packages/videopress/src/dashboard/components/library/test/thumbnail-field.test.tsx b/projects/packages/videopress/src/dashboard/components/library/test/thumbnail-field.test.tsx index 6508b11e2f1d..a7728f29af73 100644 --- a/projects/packages/videopress/src/dashboard/components/library/test/thumbnail-field.test.tsx +++ b/projects/packages/videopress/src/dashboard/components/library/test/thumbnail-field.test.tsx @@ -1,5 +1,6 @@ import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; +import { makeLibraryItem as item } from '../../../test-utils/library-item'; import { libraryFields } from '../fields'; import ThumbnailField from '../thumbnail-field'; import { UploadActionsProvider, type UploadActions } from '../upload-actions-context'; @@ -11,28 +12,6 @@ jest.mock( '../../../hooks/use-poster-url', () => ( { usePosterUrl: jest.fn( () => null ), } ) ); -const item = ( overrides: Partial< LibraryItem > = {} ): LibraryItem => ( { - id: '42', - guid: 'abc123', - type: 'videopress', - title: 'My Clip', - filename: 'clip.mp4', - thumbnailUrl: null, - durationSeconds: 0, - uploadDate: '2026-01-01T00:00:00', - privacy: 'public', - isPrivate: false, - fileSizeBytes: 0, - upload: { status: 'idle', progress: 0 }, - description: '', - rating: 'G', - displayEmbed: true, - allowDownloads: false, - shortcode: '', - isProcessing: false, - ...overrides, -} ); - const makeActions = ( overrides: Partial< UploadActions > = {} ): UploadActions => ( { promoteLocal: jest.fn(), retryUpload: jest.fn(), diff --git a/projects/packages/videopress/src/dashboard/components/library/thumbnail-field.tsx b/projects/packages/videopress/src/dashboard/components/library/thumbnail-field.tsx index 547aab7892cc..0979fc119450 100644 --- a/projects/packages/videopress/src/dashboard/components/library/thumbnail-field.tsx +++ b/projects/packages/videopress/src/dashboard/components/library/thumbnail-field.tsx @@ -113,7 +113,7 @@ export default function ThumbnailField( { item }: Props ) { ) : null } - { upload.status === 'promoting' ? ( + { upload.status === 'promoting' || upload.status === 'deleting' ? ( - { __( 'Uploading…', 'jetpack-videopress-pkg' ) } - - - ) : null } - - { upload.status === 'deleting' ? ( - - { __( 'Deleting…', 'jetpack-videopress-pkg' ) } + + { upload.status === 'deleting' + ? __( 'Deleting…', 'jetpack-videopress-pkg' ) + : __( 'Uploading…', 'jetpack-videopress-pkg' ) } + ) : null } diff --git a/projects/packages/videopress/src/dashboard/hooks/use-delete-video.ts b/projects/packages/videopress/src/dashboard/hooks/use-delete-video.ts index 162fd03fb253..577896c0a14e 100644 --- a/projects/packages/videopress/src/dashboard/hooks/use-delete-video.ts +++ b/projects/packages/videopress/src/dashboard/hooks/use-delete-video.ts @@ -1,6 +1,6 @@ import { useMutation, useQueryClient } from '@tanstack/react-query'; import apiFetch from '@wordpress/api-fetch'; -import { LIBRARY_QUERY_KEY } from './use-library'; +import { LIBRARY_ITEM_QUERY_SEGMENT, LIBRARY_QUERY_KEY } from './use-library'; type Id = number | string; @@ -54,15 +54,15 @@ export function useDeleteVideo() { } }, // Predicate rather than a key prefix: plain [ LIBRARY_QUERY_KEY ] would - // also match useVideo's [ LIBRARY_QUERY_KEY, 'item', id ] — on the - // details page that query is active for the id being deleted, and the - // refetch would 404 (+ one retry) and stall settling by seconds. - // Deleting can't change other items' detail data, so item queries are - // excluded entirely. + // also match useVideo's item queries — on the details page that query + // is active for the id being deleted, and the refetch would 404 + // (+ one retry) and stall settling by seconds. Deleting can't change + // other items' detail data, so item queries are excluded entirely. onSettled: () => client.invalidateQueries( { predicate: query => - query.queryKey[ 0 ] === LIBRARY_QUERY_KEY && query.queryKey[ 1 ] !== 'item', + query.queryKey[ 0 ] === LIBRARY_QUERY_KEY && + query.queryKey[ 1 ] !== LIBRARY_ITEM_QUERY_SEGMENT, } ), } ); } diff --git a/projects/packages/videopress/src/dashboard/hooks/use-library.ts b/projects/packages/videopress/src/dashboard/hooks/use-library.ts index 654f3d62e752..f7eec6addb16 100644 --- a/projects/packages/videopress/src/dashboard/hooks/use-library.ts +++ b/projects/packages/videopress/src/dashboard/hooks/use-library.ts @@ -244,3 +244,9 @@ export function useLibrary( view: View ) { } export const LIBRARY_QUERY_KEY = 'jetpack-videopress-library' as const; + +// Second tuple segment of item-detail query keys +// ([ LIBRARY_QUERY_KEY, LIBRARY_ITEM_QUERY_SEGMENT, id ] — see use-video.ts). +// Shared so cache invalidation can address list vs item queries without +// re-encoding the key shape at each site. +export const LIBRARY_ITEM_QUERY_SEGMENT = 'item' as const; diff --git a/projects/packages/videopress/src/dashboard/hooks/use-video.ts b/projects/packages/videopress/src/dashboard/hooks/use-video.ts index 1172a011e569..ab145c7f9e94 100644 --- a/projects/packages/videopress/src/dashboard/hooks/use-video.ts +++ b/projects/packages/videopress/src/dashboard/hooks/use-video.ts @@ -1,7 +1,7 @@ import { useQuery } from '@tanstack/react-query'; import apiFetch from '@wordpress/api-fetch'; import { buildShortcode } from '../utils/format'; -import { privacyIntToString } from './use-library'; +import { LIBRARY_ITEM_QUERY_SEGMENT, LIBRARY_QUERY_KEY, privacyIntToString } from './use-library'; import type { LibraryItem } from '../types/library'; type ApiMediaItem = { @@ -73,7 +73,7 @@ function toLibraryItem( raw: ApiMediaItem ): LibraryItem { */ export function useVideo( id: number | string ) { const query = useQuery< LibraryItem >( { - queryKey: [ 'jetpack-videopress-library', 'item', String( id ) ], + queryKey: [ LIBRARY_QUERY_KEY, LIBRARY_ITEM_QUERY_SEGMENT, String( id ) ], queryFn: async () => { const raw = await apiFetch< ApiMediaItem >( { path: `/wp/v2/media/${ id }` } ); return toLibraryItem( raw ); diff --git a/projects/packages/videopress/src/dashboard/test-utils/library-item.ts b/projects/packages/videopress/src/dashboard/test-utils/library-item.ts new file mode 100644 index 000000000000..ba0844cdd3d8 --- /dev/null +++ b/projects/packages/videopress/src/dashboard/test-utils/library-item.ts @@ -0,0 +1,32 @@ +import type { LibraryItem } from '../types/library'; + +/** + * Build a LibraryItem test fixture. Defaults model an idle, public + * VideoPress video; pass overrides for the fields under test. + * + * @param overrides - Fields to override on the base fixture. + * @return A complete LibraryItem. + */ +export function makeLibraryItem( overrides: Partial< LibraryItem > = {} ): LibraryItem { + return { + id: '42', + guid: 'abc123', + type: 'videopress', + title: 'My Clip', + filename: 'clip.mp4', + thumbnailUrl: null, + durationSeconds: 0, + uploadDate: '2026-01-01T00:00:00', + privacy: 'public', + isPrivate: false, + fileSizeBytes: 0, + upload: { status: 'idle', progress: 0 }, + description: '', + rating: 'G', + displayEmbed: true, + allowDownloads: false, + shortcode: '', + isProcessing: false, + ...overrides, + }; +} From 2b544af04ccae29216cf724bae6618ac9601ba50 Mon Sep 17 00:00:00 2001 From: William Viana Date: Thu, 11 Jun 2026 17:50:01 +0200 Subject: [PATCH 3/3] VideoPress: address delete-progress review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From a recall-mode review pass over the branch: - Drop deleted ids' item-detail queries from the cache on settle. Invalidating doesn't help (the refetch 404s and stale data is retained), so a cached entry let a back-navigation render a ghost editor for a deleted video. - Key the details page's in-progress notice by video id so two overlapping deletes (start one, navigate away mid-flight, delete another) can't replace each other's notices. - Disable Save on the details page while a delete is in flight so a slow delete can't be raced by a meta update. - Announce library deletes to screen readers: the row overlay/pill is purely visual, so surface an in-progress notice (replaced in place by the settle notice) matching the details-page pattern. - Finish the LIBRARY_ITEM_QUERY_SEGMENT migration in use-update-video-meta and use-update-video-poster, which still hardcoded 'item'. - Changelog type fixed → added (new UI state + behavior change, not a bug fix). Co-Authored-By: Claude Fable 5 --- ...videopress-delete-media-progress-indicator | 2 +- .../videopress/routes/library/stage.tsx | 28 +++++++++++++++-- .../videopress/routes/video/stage.tsx | 21 ++++++++----- .../hooks/test/use-delete-video.test.ts | 23 ++++++++++++-- .../src/dashboard/hooks/use-delete-video.ts | 31 ++++++++++++++----- .../dashboard/hooks/use-update-video-meta.ts | 4 +-- .../hooks/use-update-video-poster.ts | 4 +-- 7 files changed, 87 insertions(+), 26 deletions(-) diff --git a/projects/packages/videopress/changelog/update-videopress-delete-media-progress-indicator b/projects/packages/videopress/changelog/update-videopress-delete-media-progress-indicator index 0a79e5d12971..816009c50687 100644 --- a/projects/packages/videopress/changelog/update-videopress-delete-media-progress-indicator +++ b/projects/packages/videopress/changelog/update-videopress-delete-media-progress-indicator @@ -1,4 +1,4 @@ Significance: patch -Type: fixed +Type: added VideoPress dashboard: show a Deleting progress state on library rows and the video details page while media is being deleted, and run bulk deletions in parallel. diff --git a/projects/packages/videopress/routes/library/stage.tsx b/projects/packages/videopress/routes/library/stage.tsx index b51edadbbb6f..bb9fe4e6164f 100644 --- a/projects/packages/videopress/routes/library/stage.tsx +++ b/projects/packages/videopress/routes/library/stage.tsx @@ -117,7 +117,7 @@ const StageInner = () => { [ navigate ] ); - const { createSuccessNotice, createErrorNotice } = useGlobalNotices(); + const { createSuccessNotice, createErrorNotice, createInfoNotice } = useGlobalNotices(); // Drag-and-drop entry point. Mirrors the file-picker's `startUpload` // path but accepts multiple files and enforces the free-tier cap up @@ -215,6 +215,25 @@ const StageInner = () => { openVideoDetails, deleteItems: async ( ids: string[] ) => { setDeletingIds( prev => new Set( [ ...prev, ...ids ] ) ); + // The row overlay/pill is purely visual; this notice is what + // announces the in-flight state to screen readers. Per-batch id + // (rows mid-delete are ineligible for another delete, so the + // first id can't repeat across concurrent batches) lets the + // settle notices below replace it in place rather than stack. + const noticeId = `vp-library-deleting-${ ids[ 0 ] }-${ ids.length }`; + createInfoNotice( + sprintf( + /* translators: %d: number of videos being deleted. */ + _n( + 'Deleting %d video…', + 'Deleting %d videos…', + ids.length, + 'jetpack-videopress-pkg' + ), + ids.length + ), + { id: noticeId, explicitDismiss: true } + ); // React via the mutateAsync promise, not mutate-level callbacks: // those are dropped if another delete starts while this one is in // flight (TanStack detaches the observer), which would strand @@ -235,7 +254,8 @@ const StageInner = () => { 'jetpack-videopress-pkg' ), ids.length - ) + ), + { id: noticeId } ); } catch ( error ) { // Unknown error shape → assume nothing was deleted. @@ -253,7 +273,8 @@ const StageInner = () => { 'jetpack-videopress-pkg' ), failedIds.size - ) + ), + { id: noticeId } ); } setDeletingIds( prev => { @@ -295,6 +316,7 @@ const StageInner = () => { openVideoDetails, createSuccessNotice, createErrorNotice, + createInfoNotice, ] ); diff --git a/projects/packages/videopress/routes/video/stage.tsx b/projects/packages/videopress/routes/video/stage.tsx index 42ef0ed813fd..1f5c729cab0f 100644 --- a/projects/packages/videopress/routes/video/stage.tsx +++ b/projects/packages/videopress/routes/video/stage.tsx @@ -160,10 +160,12 @@ const Editor = ( { type StageReadyProps = { video: LibraryItem }; -// Stable id so the settle notices replace the in-progress snackbar in place -// (the notices store drops an existing notice with the same id on create) -// instead of stacking a second notice next to it. -const DELETING_NOTICE_ID = 'vp-video-deleting'; +// Per-video id so the settle notices replace the in-progress snackbar in +// place (the notices store drops an existing notice with the same id on +// create) instead of stacking a second notice next to it. Keyed by video id +// so two overlapping deletes — start one, navigate away mid-flight, delete +// another — can't clobber each other's notices. +const deletingNoticeId = ( videoId: string ) => `vp-video-deleting-${ videoId }`; const StageReady = ( { video }: StageReadyProps ) => { const navigate = useNavigate(); @@ -185,7 +187,10 @@ const StageReady = ( { video }: StageReadyProps ) => { return ( { updateMeta( { id: video.id, patch: values }, @@ -209,7 +214,7 @@ const StageReady = ( { video }: StageReadyProps ) => { // action doesn't feel frozen. `explicitDismiss` keeps the snackbar // from auto-expiring before the request settles. createInfoNotice( __( 'Deleting video…', 'jetpack-videopress-pkg' ), { - id: DELETING_NOTICE_ID, + id: deletingNoticeId( video.id ), explicitDismiss: true, } ); // Promise chain rather than mutate-level callbacks: those are @@ -218,7 +223,7 @@ const StageReady = ( { video }: StageReadyProps ) => { deleteVideo( Number( video.id ) ) .then( () => { createSuccessNotice( __( 'Video deleted.', 'jetpack-videopress-pkg' ), { - id: DELETING_NOTICE_ID, + id: deletingNoticeId( video.id ), } ); if ( isMountedRef.current ) { navigate( { href: '/library' } ); @@ -226,7 +231,7 @@ const StageReady = ( { video }: StageReadyProps ) => { } ) .catch( () => { createErrorNotice( __( 'Failed to delete video.', 'jetpack-videopress-pkg' ), { - id: DELETING_NOTICE_ID, + id: deletingNoticeId( video.id ), } ); } ); } } diff --git a/projects/packages/videopress/src/dashboard/hooks/test/use-delete-video.test.ts b/projects/packages/videopress/src/dashboard/hooks/test/use-delete-video.test.ts index d140fbc4b4b6..661501f564be 100644 --- a/projects/packages/videopress/src/dashboard/hooks/test/use-delete-video.test.ts +++ b/projects/packages/videopress/src/dashboard/hooks/test/use-delete-video.test.ts @@ -14,7 +14,9 @@ describe( 'useDeleteVideo', () => { throw new Error( 'unexpected' ); } ); - const wrapper = createTestWrapper( createTestQueryClient() ); + const client = createTestQueryClient(); + const removeSpy = jest.spyOn( client, 'removeQueries' ); + const wrapper = createTestWrapper( client ); const { result } = renderHook( () => useDeleteVideo(), { wrapper } ); await act( async () => { await result.current.mutateAsync( [ 1, 2, 3 ] ); @@ -25,6 +27,12 @@ describe( 'useDeleteVideo', () => { '/wp/v2/media/2?force=true', '/wp/v2/media/3?force=true', ] ); + // Deleted ids' item-detail queries are dropped so a back-navigation + // can't render a ghost editor from cache. + expect( removeSpy ).toHaveBeenCalledTimes( 3 ); + expect( removeSpy ).toHaveBeenCalledWith( { + queryKey: [ 'jetpack-videopress-library', 'item', '2' ], + } ); } ); it( 'rejects with a DeleteVideosError listing only the failed ids, still attempting every id', async () => { @@ -40,7 +48,9 @@ describe( 'useDeleteVideo', () => { throw new Error( 'unexpected' ); } ); - const wrapper = createTestWrapper( createTestQueryClient() ); + const client = createTestQueryClient(); + const removeSpy = jest.spyOn( client, 'removeQueries' ); + const wrapper = createTestWrapper( client ); const { result } = renderHook( () => useDeleteVideo(), { wrapper } ); let caught: unknown; @@ -56,6 +66,15 @@ describe( 'useDeleteVideo', () => { expect( ( caught as DeleteVideosError ).failedIds ).toEqual( [ 2 ] ); // A failure must not short-circuit the rest of the batch. expect( paths ).toHaveLength( 3 ); + // Only the SUCCEEDED ids' item queries are dropped — the failed row + // still exists and its cached details remain valid. + expect( removeSpy ).toHaveBeenCalledTimes( 2 ); + expect( removeSpy ).toHaveBeenCalledWith( { + queryKey: [ 'jetpack-videopress-library', 'item', '1' ], + } ); + expect( removeSpy ).toHaveBeenCalledWith( { + queryKey: [ 'jetpack-videopress-library', 'item', '3' ], + } ); } ); it( 'invalidates library list queries (not item queries) even when deletions fail', async () => { diff --git a/projects/packages/videopress/src/dashboard/hooks/use-delete-video.ts b/projects/packages/videopress/src/dashboard/hooks/use-delete-video.ts index 577896c0a14e..d0a77ea2dce0 100644 --- a/projects/packages/videopress/src/dashboard/hooks/use-delete-video.ts +++ b/projects/packages/videopress/src/dashboard/hooks/use-delete-video.ts @@ -53,16 +53,31 @@ export function useDeleteVideo() { throw new DeleteVideosError( failedIds ); } }, - // Predicate rather than a key prefix: plain [ LIBRARY_QUERY_KEY ] would - // also match useVideo's item queries — on the details page that query - // is active for the id being deleted, and the refetch would 404 - // (+ one retry) and stall settling by seconds. Deleting can't change - // other items' detail data, so item queries are excluded entirely. - onSettled: () => - client.invalidateQueries( { + onSettled: ( _data, error, input ) => { + // Drop the deleted ids' item-detail queries outright. Invalidating + // them is no help — a refetch 404s and TanStack retains the stale + // data — so a cached entry would let a back-navigation render a + // ghost editor for a video that no longer exists. + const ids = Array.isArray( input ) ? input : [ input ]; + const failedIds = error instanceof DeleteVideosError ? new Set( error.failedIds ) : null; + ids + .filter( id => ! failedIds?.has( id ) ) + .forEach( id => + client.removeQueries( { + queryKey: [ LIBRARY_QUERY_KEY, LIBRARY_ITEM_QUERY_SEGMENT, String( id ) ], + } ) + ); + // Predicate rather than a key prefix: plain [ LIBRARY_QUERY_KEY ] + // would also match useVideo's item queries — on the details page + // that query is active for the id being deleted, and the refetch + // would 404 (+ one retry) and stall settling by seconds. Deleting + // can't change other items' detail data, so item queries are + // excluded entirely. + return client.invalidateQueries( { predicate: query => query.queryKey[ 0 ] === LIBRARY_QUERY_KEY && query.queryKey[ 1 ] !== LIBRARY_ITEM_QUERY_SEGMENT, - } ), + } ); + }, } ); } diff --git a/projects/packages/videopress/src/dashboard/hooks/use-update-video-meta.ts b/projects/packages/videopress/src/dashboard/hooks/use-update-video-meta.ts index fbd2d3f43ee1..e25345cba211 100644 --- a/projects/packages/videopress/src/dashboard/hooks/use-update-video-meta.ts +++ b/projects/packages/videopress/src/dashboard/hooks/use-update-video-meta.ts @@ -1,6 +1,6 @@ import { useMutation, useQueryClient } from '@tanstack/react-query'; import apiFetch from '@wordpress/api-fetch'; -import { privacyStringToInt, LIBRARY_QUERY_KEY } from './use-library'; +import { privacyStringToInt, LIBRARY_ITEM_QUERY_SEGMENT, LIBRARY_QUERY_KEY } from './use-library'; import type { VideoDetailsPatch } from '../types/library'; type ApiPatch = { @@ -61,7 +61,7 @@ export function useUpdateVideoMeta() { onSuccess: ( _data, { id } ) => { client.invalidateQueries( { queryKey: [ LIBRARY_QUERY_KEY ] } ); client.invalidateQueries( { - queryKey: [ LIBRARY_QUERY_KEY, 'item', String( id ) ], + queryKey: [ LIBRARY_QUERY_KEY, LIBRARY_ITEM_QUERY_SEGMENT, String( id ) ], } ); }, } ); diff --git a/projects/packages/videopress/src/dashboard/hooks/use-update-video-poster.ts b/projects/packages/videopress/src/dashboard/hooks/use-update-video-poster.ts index f298ef3b3601..49bfd47f8690 100644 --- a/projects/packages/videopress/src/dashboard/hooks/use-update-video-poster.ts +++ b/projects/packages/videopress/src/dashboard/hooks/use-update-video-poster.ts @@ -1,6 +1,6 @@ import { useMutation, useQueryClient } from '@tanstack/react-query'; import apiFetch from '@wordpress/api-fetch'; -import { LIBRARY_QUERY_KEY } from './use-library'; +import { LIBRARY_ITEM_QUERY_SEGMENT, LIBRARY_QUERY_KEY } from './use-library'; import type { LibraryItem } from '../types/library'; export type UpdatePosterVars = @@ -93,7 +93,7 @@ export function useUpdateVideoPoster() { onSuccess: ( { poster }, vars ) => { if ( poster ) { client.setQueryData( - [ LIBRARY_QUERY_KEY, 'item', String( vars.id ) ], + [ LIBRARY_QUERY_KEY, LIBRARY_ITEM_QUERY_SEGMENT, String( vars.id ) ], ( old: LibraryItem | undefined ) => ( old ? { ...old, thumbnailUrl: poster } : old ) ); }