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..816009c50687 --- /dev/null +++ b/projects/packages/videopress/changelog/update-videopress-delete-media-progress-indicator @@ -0,0 +1,4 @@ +Significance: patch +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 eb26ad495c91..bb9fe4e6164f 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(); @@ -113,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 @@ -209,33 +213,80 @@ const StageInner = () => { promoteLocal, retryUpload, openVideoDetails, - deleteItems: ( ids: string[] ) => { - deleteVideo( ids, { - onSuccess: () => { - createSuccessNotice( - sprintf( - /* translators: %d: number of deleted videos. */ - _n( - '%d video deleted.', - '%d videos deleted.', - ids.length, - 'jetpack-videopress-pkg' - ), - ids.length - ) - ); - }, - onError: () => { - createErrorNotice( + 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 + // 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. + let failedIds = new Set< string >(); + try { + await deleteVideo( ids ); + createSuccessNotice( + sprintf( + /* translators: %d: number of deleted videos. */ _n( - 'Failed to delete video.', - 'Failed to delete videos.', + '%d video deleted.', + '%d videos deleted.', ids.length, 'jetpack-videopress-pkg' - ) - ); - }, + ), + ids.length + ), + { id: noticeId } + ); + } 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 + ), + { id: noticeId } + ); + } + 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( @@ -265,6 +316,7 @@ const StageInner = () => { openVideoDetails, createSuccessNotice, createErrorNotice, + createInfoNotice, ] ); @@ -296,19 +348,21 @@ 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 = 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 ] ); + }, [ 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..1f5c729cab0f 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,17 +160,37 @@ const Editor = ( { type StageReadyProps = { video: LibraryItem }; +// 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(); const { mutate: updateMeta, isPending: isSaving } = useUpdateVideoMeta(); - const { mutate: deleteVideo } = useDeleteVideo(); - const { createSuccessNotice, createErrorNotice } = useGlobalNotices(); + const { mutateAsync: deleteVideo, isPending: isDeleting } = useDeleteVideo(); + 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 + // 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 ( { updateMeta( { id: video.id, patch: values }, @@ -186,15 +206,34 @@ const StageReady = ( { video }: StageReadyProps ) => { ); } } onDelete={ () => { - deleteVideo( Number( video.id ), { - onSuccess: () => { - createSuccessNotice( __( 'Video deleted.', 'jetpack-videopress-pkg' ) ); - navigate( { href: '/library' } ); - }, - onError: () => { - createErrorNotice( __( 'Failed to delete video.', 'jetpack-videopress-pkg' ) ); - }, + 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: deletingNoticeId( video.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( () => { + createSuccessNotice( __( 'Video deleted.', 'jetpack-videopress-pkg' ), { + id: deletingNoticeId( video.id ), + } ); + if ( isMountedRef.current ) { + navigate( { href: '/library' } ); + } + } ) + .catch( () => { + createErrorNotice( __( 'Failed to delete video.', 'jetpack-videopress-pkg' ), { + id: deletingNoticeId( video.id ), + } ); + } ); } } 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..71c896bbad56 100644 --- a/projects/packages/videopress/src/dashboard/components/library/actions.ts +++ b/projects/packages/videopress/src/dashboard/components/library/actions.ts @@ -10,14 +10,19 @@ 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.type === 'videopress' && item.upload.status === 'idle'; /** * 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 +46,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 +58,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 +70,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 +82,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..c25be3e05b80 --- /dev/null +++ b/projects/packages/videopress/src/dashboard/components/library/test/actions.test.ts @@ -0,0 +1,32 @@ +import { makeLibraryItem as item } from '../../../test-utils/library-item'; +import { buildLibraryActions } from '../actions'; + +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..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(), @@ -95,6 +74,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 +102,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..0979fc119450 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" @@ -113,7 +113,7 @@ export default function ThumbnailField( { item }: Props ) { ) : null } - { upload.status === 'promoting' ? ( + { upload.status === 'promoting' || upload.status === 'deleting' ? ( - { __( 'Uploading…', 'jetpack-videopress-pkg' ) } + + { upload.status === 'deleting' + ? __( 'Deleting…', 'jetpack-videopress-pkg' ) + : __( 'Uploading…', 'jetpack-videopress-pkg' ) } + ) : null } 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 aad282eae6be..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 @@ -1,7 +1,7 @@ import { renderHook, act } from '@testing-library/react'; import { mockApiFetch } from '../../test-utils/mock-api-fetch'; import { createTestQueryClient, createTestWrapper } from '../../test-utils/query-client-wrapper'; -import { useDeleteVideo } from '../use-delete-video'; +import { DeleteVideosError, useDeleteVideo } from '../use-delete-video'; describe( 'useDeleteVideo', () => { it( 'sends DELETE for each id', async () => { @@ -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,5 +27,85 @@ 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 () => { + 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 client = createTestQueryClient(); + const removeSpy = jest.spyOn( client, 'removeQueries' ); + const wrapper = createTestWrapper( client ); + 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 ); + // 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 () => { + 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..d0a77ea2dce0 100644 --- a/projects/packages/videopress/src/dashboard/hooks/use-delete-video.ts +++ b/projects/packages/videopress/src/dashboard/hooks/use-delete-video.ts @@ -1,12 +1,37 @@ 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; /** - * 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,44 @@ 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 ] } ); + 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-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-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 ) ); } 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, + }; +} 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 {