Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: 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.
130 changes: 92 additions & 38 deletions projects/packages/videopress/routes/library/stage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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
Expand Down Expand 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(
Expand Down Expand Up @@ -265,6 +316,7 @@ const StageInner = () => {
openVideoDetails,
createSuccessNotice,
createErrorNotice,
createInfoNotice,
]
);

Expand Down Expand Up @@ -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, [] );

Expand Down
63 changes: 51 additions & 12 deletions projects/packages/videopress/routes/video/stage.tsx
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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 (
<Editor
video={ video }
isSaving={ isSaving }
// Treat an in-flight delete like an in-flight save: Save stays
// disabled so a slow delete can't be raced by a meta update
// against the attachment being removed.
isSaving={ isSaving || isDeleting }
onSave={ ( values, reset ) => {
updateMeta(
{ id: video.id, patch: values },
Expand All @@ -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 ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<DataViews>`.
Expand All @@ -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 ) {
Expand All @@ -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 ) {
Expand All @@ -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 ) {
Expand All @@ -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 ) );
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
Original file line number Diff line number Diff line change
@@ -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 );
} );
} );
Loading
Loading