diff --git a/projects/packages/videopress/changelog/add-videopress-trim-cut b/projects/packages/videopress/changelog/add-videopress-trim-cut index 965fbeac9cce..1482c2006e66 100644 --- a/projects/packages/videopress/changelog/add-videopress-trim-cut +++ b/projects/packages/videopress/changelog/add-videopress-trim-cut @@ -1,4 +1,4 @@ Significance: minor Type: added -Add an optional trim and cut editor with preview, undo, original video restoration, and a choice to update or save a new video. Keep the editor available during processing and resume pending copies when returning to the page. Reduce background status checks, refresh delayed timeline thumbnails, and allow retrying failed edits without creating another video. +Add an optional trim and cut editor with preview, undo, and original video restoration. Keep the editor available during processing, reduce background status checks, refresh delayed timeline thumbnails, and allow retrying failed edits. diff --git a/projects/packages/videopress/src/class-wpcom-rest-api-v2-endpoint-videopress-edits.php b/projects/packages/videopress/src/class-wpcom-rest-api-v2-endpoint-videopress-edits.php index 14b6b58bf85a..d6a678d9cf4f 100644 --- a/projects/packages/videopress/src/class-wpcom-rest-api-v2-endpoint-videopress-edits.php +++ b/projects/packages/videopress/src/class-wpcom-rest-api-v2-endpoint-videopress-edits.php @@ -135,46 +135,6 @@ public function register_routes() { 'permission_callback' => array( $this, 'permissions_check' ), ) ); - - $copy_args = array_merge( - $guid_arg, - array( - 'request_id' => array( - 'type' => 'string', - 'pattern' => '^[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}$', - 'required' => true, - ), - ) - ); - register_rest_route( - 'wpcom/v2', - 'videopress/(?P[A-Za-z0-9]{8})/edits/copy', - array( - 'args' => array_merge( - $copy_args, - $edit_args, - array( - 'title' => array( - 'type' => 'string', - 'maxLength' => 1000, - ), - ) - ), - 'methods' => WP_REST_Server::CREATABLE, - 'callback' => array( $this, 'copy_edits' ), - 'permission_callback' => array( $this, 'permissions_check' ), - ) - ); - register_rest_route( - 'wpcom/v2', - 'videopress/(?P[A-Za-z0-9]{8})/edits/copy/(?P[a-f0-9-]{36})', - array( - 'args' => $copy_args, - 'methods' => WP_REST_Server::READABLE, - 'callback' => array( $this, 'get_copy' ), - 'permission_callback' => array( $this, 'permissions_check' ), - ) - ); } /** @@ -230,34 +190,6 @@ public function retry_edits( $request ) { ); } - /** - * Create an independent video from original-timeline edits. - * - * @param WP_REST_Request $request The request object. - * @return WP_REST_Response|WP_Error - */ - public function copy_edits( $request ) { - $body = array( - 'base_revision' => $request['base_revision'], - 'operations' => $request['operations'], - 'request_id' => $request['request_id'], - ); - if ( isset( $request['title'] ) ) { - $body['title'] = $request['title']; - } - return $this->proxy_request( sprintf( 'videos/%s/edits/copy', $request['guid'] ), 'POST', $body, true ); - } - - /** - * Fetch progress for the same idempotent copy request. - * - * @param WP_REST_Request $request The request object. - * @return WP_REST_Response|WP_Error - */ - public function get_copy( $request ) { - return $this->proxy_request( sprintf( 'videos/%s/edits/copy/%s', $request['guid'], $request['request_id'] ) ); - } - /** * Restore the original using the upstream API's POST deletion convention. * @@ -284,10 +216,9 @@ public function get_storyboard( $request ) { * @param string $path WordPress.com REST v1.1 path. * @param string $method HTTP method. * @param array|null $body JSON request data. - * @param bool $as_user Preserve the connected actor when creating a copy. * @return WP_REST_Response|WP_Error */ - private function proxy_request( $path, $method = 'GET', $body = null, $as_user = false ) { + private function proxy_request( $path, $method = 'GET', $body = null ) { $args = array( 'method' => $method ); if ( null !== $body ) { $args['headers'] = array( 'content-type' => 'application/json' ); @@ -307,8 +238,6 @@ private function proxy_request( $path, $method = 'GET', $body = null, $as_user = $url = Constants::get_constant( 'JETPACK__WPCOM_JSON_API_BASE' ) . '/rest/v1.1/' . $path; // @phan-suppress-next-line PhanAccessMethodInternal -- Use the poster transport; the direct client only dispatches v2 routes. $response = Client::_wp_remote_request( $url, $args ); - } elseif ( $as_user ) { - $response = Client::wpcom_json_api_request_as_user( $path, '1.1', $args, $body, 'rest' ); } else { $response = Client::wpcom_json_api_request_as_blog( $path, '1.1', $args, $body, 'rest' ); } diff --git a/projects/packages/videopress/src/class-xmlrpc.php b/projects/packages/videopress/src/class-xmlrpc.php index 454783d03791..a477b9d18362 100644 --- a/projects/packages/videopress/src/class-xmlrpc.php +++ b/projects/packages/videopress/src/class-xmlrpc.php @@ -7,7 +7,6 @@ namespace Automattic\Jetpack\VideoPress; -use WP_Error; use WP_User; /** @@ -68,51 +67,12 @@ public function xmlrpc_methods( $methods, $core_methods, $user ) { } $methods['jetpack.createMediaItem'] = array( $this, 'create_media_item' ); - $methods['jetpack.createVideoPressCopy'] = array( $this, 'create_videopress_copy' ); - $methods['jetpack.authorizeVideoPressCopy'] = array( $this, 'authorize_videopress_copy' ); $methods['jetpack.updateVideoPressMediaItem'] = array( $this, 'update_videopress_media_item' ); $methods['jetpack.updateVideoPressPosterImage'] = array( $this, 'update_poster_image' ); return $methods; } - /** - * Check permissions on the real attachment before WordPress.com copies its retained master. - * - * @since $$next-version$$ - * @param string $guid The source VideoPress GUID. - * @return array Permission acknowledgement or an error response. - */ - public function authorize_videopress_copy( $guid ) { - $this->authenticate_user(); - if ( ! is_string( $guid ) || ! preg_match( '/^[A-Za-z0-9]{8}$/D', $guid ) - || ! $this->current_user || ! $this->current_user->exists() || ! current_user_can( 'upload_files' ) ) { - return array( 'errors' => array( 'videopress_copy_forbidden' => __( 'You cannot copy this video.', 'jetpack-videopress-pkg' ) ) ); - } - $post_id = WPCOM_REST_API_V2_Endpoint_VideoPress::get_video_attachment_id( $guid ); - if ( ! $post_id || ! current_user_can( 'edit_post', $post_id ) ) { - return array( 'errors' => array( 'videopress_copy_forbidden' => __( 'You cannot copy this video.', 'jetpack-videopress-pkg' ) ) ); - } - return array( - 'authorized' => true, - 'guid' => $guid, - ); - } - - /** - * Create one idempotent copy attachment; older clients reject this method before inserting a row. - * - * @since $$next-version$$ - * @param array $media A single media item carrying its copy request identifier. - * @return array The attachment or an error response. - */ - public function create_videopress_copy( $media ) { - if ( ! is_array( $media ) || count( $media ) !== 1 || ! isset( $media[0] ) || ! is_array( $media[0] ) || ! array_key_exists( 'videopress_copy_request_id', $media[0] ) ) { - return array( 'errors' => array( 'videopress_copy_invalid_request' => __( 'Invalid video copy request.', 'jetpack-videopress-pkg' ) ) ); - } - return $this->create_media_item( $media ); - } - /** * This is used by the WPCOM VideoPress uploader in order to create a media item with * specific meta data about an uploaded file. After this, the transcoding session will @@ -134,16 +94,6 @@ public function create_media_item( $media ) { : sanitize_title( basename( $url ) ); $guid = $media['guid'] ?? null; - if ( array_key_exists( 'videopress_copy_request_id', $media_item ) ) { - $media_id = $this->create_copy_attachment( $title, $media_item['videopress_copy_request_id'] ); - if ( is_wp_error( $media_id ) ) { - return array( 'errors' => array( $media_id->get_error_code() => $media_id->get_error_message() ) ); - } - $media_item['post'] = get_post( $media_id ); - $media_item['videopress_copy_request_id_ack'] = $media_item['videopress_copy_request_id']; - continue; - } - $media_id = videopress_create_new_media_item( $title, $guid ); $post_update = array(); @@ -173,45 +123,6 @@ public function create_media_item( $media ) { return array( 'media' => $media ); } - /** - * Reserve a copy request before inserting its attachment so retries cannot create duplicates. - * - * @param string $title Attachment title. - * @param mixed $request_id Source GUID and copy request UUID. - * @return int|WP_Error The existing or newly created attachment ID. - */ - private function create_copy_attachment( $title, $request_id ) { - if ( ! is_string( $request_id ) || ! preg_match( '/^[A-Za-z0-9]{8}:[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}$/D', $request_id ) ) { - return new WP_Error( 'videopress_copy_invalid_request', __( 'Invalid video copy request identifier.', 'jetpack-videopress-pkg' ) ); - } - if ( ! $this->current_user || ! $this->current_user->exists() || ! current_user_can( 'upload_files' ) ) { - return new WP_Error( 'videopress_copy_forbidden', __( 'You cannot create a video copy.', 'jetpack-videopress-pkg' ) ); - } - - $option = 'videopress_copy_attachment_' . hash( 'sha256', $request_id ); - if ( ! add_option( $option, 0, '', false ) ) { - $attachment_id = (int) get_option( $option, 0 ); - if ( ! $attachment_id ) { - return new WP_Error( 'videopress_copy_attachment_pending', __( 'The video copy attachment is still being created.', 'jetpack-videopress-pkg' ) ); - } - if ( 'attachment' !== get_post_type( $attachment_id ) || get_post_meta( $attachment_id, '_videopress_copy_request_id', true ) !== $request_id ) { - return new WP_Error( 'videopress_copy_attachment_unavailable', __( 'The video copy attachment is unavailable.', 'jetpack-videopress-pkg' ) ); - } - return $attachment_id; - } - - // A pending reservation never expires: a timed-out insert may already have created the attachment. - $attachment_id = videopress_create_new_media_item( $title ); - if ( is_wp_error( $attachment_id ) || ! $attachment_id ) { - return new WP_Error( 'videopress_copy_attachment_failed', __( 'The video copy attachment could not be created.', 'jetpack-videopress-pkg' ) ); - } - wp_update_attachment_metadata( $attachment_id, array( 'original' => array( 'url' => '' ) ) ); - if ( ! add_post_meta( $attachment_id, '_videopress_copy_request_id', $request_id, true ) || ! update_option( $option, $attachment_id, false ) ) { - return new WP_Error( 'videopress_copy_attachment_failed', __( 'The video copy attachment could not be recorded.', 'jetpack-videopress-pkg' ) ); - } - return $attachment_id; - } - /** * Update VideoPress metadata for a media item. * diff --git a/projects/packages/videopress/src/dashboard/components/editor/copy-status-banner.tsx b/projects/packages/videopress/src/dashboard/components/editor/copy-status-banner.tsx deleted file mode 100644 index 75cfec7b120f..000000000000 --- a/projects/packages/videopress/src/dashboard/components/editor/copy-status-banner.tsx +++ /dev/null @@ -1,118 +0,0 @@ -import { ProgressBar } from '@wordpress/components'; -import { __ } from '@wordpress/i18n'; -import { Notice } from '@wordpress/ui'; -import { videoTabPath } from '../video-nav'; -import type { useCopySession } from './use-copy-session'; - -type Props = { - session: ReturnType< typeof useCopySession >; - onReload: () => void; - onOpenVideo: ( href: string ) => void; -}; - -/** - * Report copy progress and preserve safe retries when the acceptance response is lost. - * - * @param props - Component props. - * @param props.session - Copy request lifecycle. - * @param props.onReload - Reload a conflicting source revision. - * @param props.onOpenVideo - Open a created video to retry its failed processing. - * @return Copy status, or nothing before a request. - */ -export default function CopyStatusBanner( { session, onReload, onOpenVideo }: Props ) { - if ( ! session.request ) { - return null; - } - let message: string = __( - 'Creating your new video… Your current video stays unchanged.', - 'jetpack-videopress-pkg' - ); - let action; - if ( session.conflict ) { - message = __( - 'The source video changed. Reload the latest edits before creating a copy.', - 'jetpack-videopress-pkg' - ); - action = ( - - { __( 'Reload latest', 'jetpack-videopress-pkg' ) } - - ); - } else if ( session.failed || session.rejected ) { - message = - ( session.rejected && session.error?.message ) || - ( session.status.data?.attachment_id - ? __( - 'The new video was created, but its edits could not be processed. Your current video and edits are unchanged.', - 'jetpack-videopress-pkg' - ) - : __( - 'The new video could not be created. Your current video and edits are unchanged.', - 'jetpack-videopress-pkg' - ) ); - const attachmentId = session.status.data?.attachment_id; - if ( attachmentId ) { - action = ( - onOpenVideo( videoTabPath( String( attachmentId ), 'editor' ) ) } - > - { __( 'Open new video', 'jetpack-videopress-pkg' ) } - - ); - } - } else if ( ! session.submitting ) { - if ( session.needsAssistance ) { - message = __( - 'We could not confirm whether the new video was created. Your current video is unchanged. We’ll keep checking automatically. Please contact support if it remains unconfirmed.', - 'jetpack-videopress-pkg' - ); - } else if ( - session.recoverable || - ( ( session.error || session.status.isError ) && ! session.status.data ) - ) { - message = __( - 'We could not confirm the new video’s status. We’ll keep checking automatically.', - 'jetpack-videopress-pkg' - ); - } - if ( - ! session.needsAssistance && - ( session.recoverable || ( session.error && ! session.status.data ) ) - ) { - action = ( - void session.retry() }> - { __( 'Retry', 'jetpack-videopress-pkg' ) } - - ); - } - } - const terminalError = session.failed || session.rejected; - const uncertain = - session.needsAssistance || session.recoverable || session.error || session.status.isError; - let intent: 'error' | 'warning' | 'info' = 'info'; - if ( terminalError ) { - intent = 'error'; - } else if ( session.conflict || uncertain ) { - intent = 'warning'; - } - return ( - - - { message } - { ! terminalError && ! session.conflict && ! uncertain && ( - - ) } - - { action && { action } } - { terminalError && } - - ); -} diff --git a/projects/packages/videopress/src/dashboard/components/editor/create-copy-request-id.ts b/projects/packages/videopress/src/dashboard/components/editor/create-copy-request-id.ts deleted file mode 100644 index 09185153ca90..000000000000 --- a/projects/packages/videopress/src/dashboard/components/editor/create-copy-request-id.ts +++ /dev/null @@ -1,14 +0,0 @@ -/** - * Create a UUIDv4 without requiring the secure-context-only randomUUID API. - * - * @return A unique identifier retained across retries of one copy request. - */ -export function createCopyRequestId(): string { - const bytes = crypto.getRandomValues( new Uint8Array( 16 ) ); - /* eslint-disable no-bitwise -- UUIDv4 fixes the version and variant bits. */ - bytes[ 6 ] = ( bytes[ 6 ] & 0x0f ) | 0x40; - bytes[ 8 ] = ( bytes[ 8 ] & 0x3f ) | 0x80; - /* eslint-enable no-bitwise */ - const hex = Array.from( bytes, byte => byte.toString( 16 ).padStart( 2, '0' ) ).join( '' ); - return `${ hex.slice( 0, 8 ) }-${ hex.slice( 8, 12 ) }-${ hex.slice( 12, 16 ) }-${ hex.slice( 16, 20 ) }-${ hex.slice( 20 ) }`; -} diff --git a/projects/packages/videopress/src/dashboard/components/editor/editor-screen.tsx b/projects/packages/videopress/src/dashboard/components/editor/editor-screen.tsx index 6a40d0d37b19..d245609e79a9 100644 --- a/projects/packages/videopress/src/dashboard/components/editor/editor-screen.tsx +++ b/projects/packages/videopress/src/dashboard/components/editor/editor-screen.tsx @@ -1,4 +1,3 @@ -import { useGlobalNotices } from '@automattic/jetpack-components/global-notices'; import { __ } from '@wordpress/i18n'; import { useNavigate } from '@wordpress/route'; import { Button, Notice, Text } from '@wordpress/ui'; @@ -11,16 +10,11 @@ import { useVideo } from '../../hooks/use-video'; import VideoLayout from '../video-layout'; import { videoTabPath } from '../video-nav'; import ConfirmDialog from './confirm-dialog'; -import CopyStatusBanner from './copy-status-banner'; -import { createCopyRequestId } from './create-copy-request-id'; import HeaderActions from './header-actions'; import PreviewPlayer from './preview/preview-player'; -import SaveVideoDialog from './save-dialog'; import { sessionEditsEqual } from './state/edit-session'; -import { sessionToOperations } from './state/serialize'; import StatusBanner from './status-banner'; import Timeline from './timeline/timeline'; -import { useCopySession } from './use-copy-session'; import { useEditSession } from './use-edit-session'; import './style.scss'; import type { EditorTool } from '../../../../routes/video-editor/operations-panel'; @@ -39,16 +33,13 @@ type ConfirmAction = 'save' | 'discard' | 'restore' | 'reload'; * @return The trim and cut editing screen. */ export default function TrimCutEditor( { video, onSelectTool }: Props ) { - const copySession = useCopySession( video.guid ); - const editor = useEditSession( video, copySession.request ); - const { createSuccessNotice } = useGlobalNotices(); - const completedCopyRef = useRef< string | null >( null ); + const editor = useEditSession( video ); const transport = usePreviewTransport(); const navigate = useNavigate(); const [ sourceReady, setSourceReady ] = useState( false ); const [ sourceDuration, setSourceDuration ] = useState( 0 ); const [ confirm, setConfirm ] = useState< ConfirmAction | null >( null ); - const hasUnsavedChanges = editor.hasUnsavedChanges && ! copySession.saved; + const { hasUnsavedChanges } = editor; const dirtyRef = useRef( hasUnsavedChanges ); dirtyRef.current = hasUnsavedChanges; const jobStatus = editor.edits?.job?.status; @@ -65,13 +56,7 @@ export default function TrimCutEditor( { video, onSelectTool }: Props ) { Math.abs( sourceDuration - editor.edits.original_duration_ms ) > 1000 ); const locked = - waitingForVideo || - editor.locked || - copySession.locked || - editor.conflict || - copySession.conflict || - ! sourceReady || - durationMismatch; + waitingForVideo || editor.locked || editor.conflict || ! sourceReady || durationMismatch; const confirmNavigation = useCallback( () => ! dirtyRef.current || @@ -103,24 +88,6 @@ export default function TrimCutEditor( { video, onSelectTool }: Props ) { }; }, [ hasUnsavedChanges, confirmNavigation, navigate, video.id ] ); - useEffect( () => { - const result = copySession.status.data; - if ( - result?.job?.status !== 'complete' || - ! result.attachment_id || - completedCopyRef.current === result.request_id - ) { - return; - } - completedCopyRef.current = result.request_id; - dirtyRef.current = false; - editor.discard(); - createSuccessNotice( - __( 'New video created. The original video is unchanged.', 'jetpack-videopress-pkg' ) - ); - navigate( { href: videoTabPath( String( result.attachment_id ), 'details' ) } ); - }, [ copySession.status.data, editor.discard, createSuccessNotice, navigate ] ); - const guardLink = ( event: MouseEvent< HTMLDivElement > ) => { if ( event.defaultPrevented || @@ -142,10 +109,15 @@ export default function TrimCutEditor( { video, onSelectTool }: Props ) { } }; - const copy: Record< - Exclude< ConfirmAction, 'save' >, - { title: string; message: string; label: string } - > = { + const copy: Record< ConfirmAction, { title: string; message: string; label: string } > = { + save: { + title: __( 'Update video?', 'jetpack-videopress-pkg' ), + message: __( + 'Viewers will see the edited video. Your original is kept and can be restored. Existing chapters may need to be adjusted after the video finishes processing.', + 'jetpack-videopress-pkg' + ), + label: __( 'Update video', 'jetpack-videopress-pkg' ), + }, discard: { title: __( 'Discard changes?', 'jetpack-videopress-pkg' ), message: __( @@ -172,12 +144,17 @@ export default function TrimCutEditor( { video, onSelectTool }: Props ) { }, }; const onConfirm = () => { - if ( confirm === 'restore' ) { + if ( confirm === 'save' ) { + // The session only guards its own lock; source readiness is checked here. + if ( locked ) { + return; + } + void editor.submit(); + } else if ( confirm === 'restore' ) { void editor.submit( true ); } else if ( confirm === 'discard' ) { editor.discard(); } else if ( confirm === 'reload' ) { - copySession.clear(); void editor.reload(); } setConfirm( null ); @@ -196,22 +173,15 @@ export default function TrimCutEditor( { video, onSelectTool }: Props ) { canRedo={ ! locked && canRedo( editor.history ) } onUndo={ () => editor.dispatch( { type: 'UNDO' } ) } onRedo={ () => editor.dispatch( { type: 'REDO' } ) } - canDiscard={ editor.dirty && ! editor.locked && ! copySession.locked } + canDiscard={ editor.dirty && ! editor.locked } onDiscard={ () => setConfirm( 'discard' ) } canSave={ editor.dirty && ! locked } - onSave={ () => { - if ( copySession.failed || copySession.rejected ) { - copySession.clear(); - } - setConfirm( 'save' ); - } } + onSave={ () => setConfirm( 'save' ) } canRestoreOriginal={ Boolean( editor.edits?.can_restore_original ) && ! waitingForVideo && ! editor.locked && - ! editor.conflict && - ! copySession.conflict && - ! copySession.locked + ! editor.conflict } onRestoreOriginal={ () => setConfirm( 'restore' ) } /> @@ -228,29 +198,16 @@ export default function TrimCutEditor( { video, onSelectTool }: Props ) { ) } - setConfirm( 'reload' ) } - onOpenVideo={ href => { - if ( confirmNavigation() ) { - navigate( { href } ); - } - } } - /> void editor.retryProcessing() - : undefined - } + onRetry={ editor.canRetry ? () => void editor.retryProcessing() : undefined } onReloadLatest={ () => setConfirm( 'reload' ) } />
{ if ( tool !== 'trim' && confirmNavigation() ) { onSelectTool( tool ); @@ -287,7 +244,7 @@ export default function TrimCutEditor( { video, onSelectTool }: Props ) {
- { confirm === 'save' && ( - setConfirm( null ) } - onSave={ ( mode, title ) => { - if ( locked || ! editor.baseline ) { - return; - } - setConfirm( null ); - if ( mode === 'update' ) { - void editor.submit(); - } else { - void copySession.submit( { - guid: video.guid, - baseRevision: editor.baseline.revision, - operations: sessionToOperations( editor.session, editor.session.durationMs ), - requestId: createCopyRequestId(), - title, - } ); - } - } } - /> - ) } - { confirm && confirm !== 'save' && ( + { confirm && ( setConfirm( null ) } /> diff --git a/projects/packages/videopress/src/dashboard/components/editor/save-dialog.tsx b/projects/packages/videopress/src/dashboard/components/editor/save-dialog.tsx deleted file mode 100644 index f0cac22095d9..000000000000 --- a/projects/packages/videopress/src/dashboard/components/editor/save-dialog.tsx +++ /dev/null @@ -1,100 +0,0 @@ -import { RadioControl, TextControl } from '@wordpress/components'; -import { __, _x, sprintf } from '@wordpress/i18n'; -import { Button, Dialog, Text } from '@wordpress/ui'; -import { useState } from 'react'; - -export type SaveMode = 'update' | 'copy'; - -type Props = { - title: string; - isBusy: boolean; - onSave: ( mode: SaveMode, title: string ) => void; - onCancel: () => void; -}; - -/** - * Choose whether edited video replaces the current video or becomes a separate library item. - * - * @param props - Component props. - * @param props.title - Source video title. - * @param props.isBusy - Whether a save request is in progress. - * @param props.onSave - Save the selected destination and title. - * @param props.onCancel - Close without saving. - * @return The save destination dialog. - */ -export default function SaveVideoDialog( { title, isBusy, onSave, onCancel }: Props ) { - const [ mode, setMode ] = useState< SaveMode >( 'update' ); - const [ copyTitle, setCopyTitle ] = useState< string >( () => - // translators: %s: original video title. - sprintf( __( '%s (edited)', 'jetpack-videopress-pkg' ), title ) - ); - return ( - { - if ( ! open && ! isBusy ) { - onCancel(); - } - } } - > - - - { __( 'Save video edits', 'jetpack-videopress-pkg' ) } - { ! isBusy && } - - -
- setMode( value as SaveMode ) } - options={ [ - { value: 'update', label: __( 'Update existing video', 'jetpack-videopress-pkg' ) }, - { value: 'copy', label: __( 'Save as new video', 'jetpack-videopress-pkg' ) }, - ] } - /> - { mode === 'copy' ? ( - <> - - - { __( - 'Add the edited version to your library. The current video stays unchanged.', - 'jetpack-videopress-pkg' - ) } - - - ) : ( - - { __( - 'Viewers will see the edited video. Your original is kept and can be restored. Existing chapters may need to be adjusted after the video finishes processing.', - 'jetpack-videopress-pkg' - ) } - - ) } -
- - - - - - - - ); -} diff --git a/projects/packages/videopress/src/dashboard/components/editor/style.scss b/projects/packages/videopress/src/dashboard/components/editor/style.scss index 1ce61e632da1..a757a0d4b0d8 100644 --- a/projects/packages/videopress/src/dashboard/components/editor/style.scss +++ b/projects/packages/videopress/src/dashboard/components/editor/style.scss @@ -10,13 +10,6 @@ padding: 32px; } -.vp-video-editor__save-options { - display: flex; - flex-direction: column; - gap: 16px; - padding-block-end: 8px; -} - .vp-video-editor__progress { display: block; margin-block-start: 8px; diff --git a/projects/packages/videopress/src/dashboard/components/editor/test/create-copy-request-id.test.ts b/projects/packages/videopress/src/dashboard/components/editor/test/create-copy-request-id.test.ts deleted file mode 100644 index dd1e61852007..000000000000 --- a/projects/packages/videopress/src/dashboard/components/editor/test/create-copy-request-id.test.ts +++ /dev/null @@ -1,25 +0,0 @@ -import { createCopyRequestId } from '../create-copy-request-id'; - -afterEach( () => jest.restoreAllMocks() ); - -it.each( [ - [ 0x00, '00000000-0000-4000-8000-000000000000' ], - [ 0xff, 'ffffffff-ffff-4fff-bfff-ffffffffffff' ], -] )( 'sets UUIDv4 version and variant bits for random bytes %i', ( byte, expected ) => { - const random = jest.spyOn( crypto, 'getRandomValues' ).mockImplementation( array => { - ( array as Uint8Array ).fill( byte as number ); - return array; - } ); - expect( createCopyRequestId() ).toBe( expected ); - expect( random ).toHaveBeenCalledTimes( 1 ); - expect( random ).toHaveBeenCalledWith( expect.any( Uint8Array ) ); - expect( random.mock.calls[ 0 ][ 0 ]?.byteLength ).toBe( 16 ); -} ); - -it( 'preserves the remaining random bytes in their original order', () => { - jest.spyOn( crypto, 'getRandomValues' ).mockImplementation( array => { - ( array as Uint8Array ).set( Array.from( { length: 16 }, ( _, index ) => index ) ); - return array; - } ); - expect( createCopyRequestId() ).toBe( '00010203-0405-4607-8809-0a0b0c0d0e0f' ); -} ); diff --git a/projects/packages/videopress/src/dashboard/components/editor/test/editor-screen.test.tsx b/projects/packages/videopress/src/dashboard/components/editor/test/editor-screen.test.tsx index 4248f2383956..620ed6cb5418 100644 --- a/projects/packages/videopress/src/dashboard/components/editor/test/editor-screen.test.tsx +++ b/projects/packages/videopress/src/dashboard/components/editor/test/editor-screen.test.tsx @@ -5,17 +5,11 @@ import userEvent from '@testing-library/user-event'; import { useNavigate } from '@wordpress/route'; import { useRestoreOriginal } from '../../../hooks/use-restore-original'; import { useRetryVideoProcessing } from '../../../hooks/use-retry-video-processing'; -import { - useSaveVideoCopy, - useVideoCopyStatus, - VideoCopyRejectedError, -} from '../../../hooks/use-save-video-copy'; -import { EditsConflictError, useSaveVideoEdits } from '../../../hooks/use-save-video-edits'; +import { useSaveVideoEdits } from '../../../hooks/use-save-video-edits'; import { useVideoEdits } from '../../../hooks/use-video-edits'; import { makeLibraryItem } from '../../../test-utils/library-item'; import { createTestWrapper } from '../../../test-utils/query-client-wrapper'; import TrimCutEditor from '../editor-screen'; -import type { SaveVideoCopyResponse } from '../../../hooks/use-save-video-copy'; import type { EditsJob, VideoEdits } from '../../../types/edits'; import type { ReactNode } from 'react'; @@ -57,14 +51,6 @@ jest.mock( '../../../hooks/use-save-video-edits', () => ( { ...jest.requireActual( '../../../hooks/use-save-video-edits' ), useSaveVideoEdits: jest.fn(), } ) ); -jest.mock( '../../../hooks/use-save-video-copy', () => ( { - ...jest.requireActual( '../../../hooks/use-save-video-copy' ), - useSaveVideoCopy: jest.fn(), - useVideoCopyStatus: jest.fn(), -} ) ); -jest.mock( '../create-copy-request-id', () => ( { - createCopyRequestId: () => '8b3d1700-1234-4567-89ab-123456789abc', -} ) ); jest.mock( '../preview/preview-player', () => { const { forwardRef, useImperativeHandle } = jest.requireActual( 'react' ); return { @@ -100,9 +86,7 @@ const video = makeLibraryItem( { guid: 'clip123', durationSeconds: 10 } ); const save = jest.fn(); const restore = jest.fn(); const retryProcessing = jest.fn(); -const copy = jest.fn(); const refetch = jest.fn(); -const refetchCopy = jest.fn(); const navigate = jest.fn(); const successNotice = jest.fn(); const errorNotice = jest.fn(); @@ -140,20 +124,6 @@ function setEdits( changes: Partial< VideoEdits > = {} ) { refetch.mockResolvedValue( { data: edits } ); } -/** - * Supply the separately polled copy job. - * - * @param data - Copy response, if the request can be found. - * @param isError - Whether polling failed. - */ -function setCopyStatus( data?: SaveVideoCopyResponse, isError = false ) { - jest.mocked( useVideoCopyStatus ).mockReturnValue( { - data, - isError, - refetch: refetchCopy, - } as never ); -} - /** * Report the original video's metadata through the media boundary. * @@ -185,36 +155,7 @@ function renderEditor( ready = true ) { }; } -/** - * Submit a copy using the actual timeline and save dialog. - * - * @param user - User interaction controller. - */ -async function saveCopy( user: ReturnType< typeof userEvent.setup > ) { - await user.click( screen.getByRole( 'button', { name: 'New cut' } ) ); - await user.click( screen.getByRole( 'button', { name: 'Save' } ) ); - await user.click( screen.getByRole( 'radio', { name: 'Save as new video' } ) ); - await user.click( screen.getByRole( 'button', { name: 'Save as new video' } ) ); -} - -/** - * Build a polled response for the submitted copy request. - * - * @param job - Current copy job. - * @return The source and destination copy identifiers. - */ -function copyResponse( job: EditsJob = processingJob ): SaveVideoCopyResponse { - return { - source_guid: video.guid, - request_id: copy.mock.calls[ 0 ][ 0 ].requestId, - guid: null, - attachment_id: null, - job, - }; -} - beforeEach( () => { - sessionStorage.clear(); jest.clearAllMocks(); confirmNavigation = jest.spyOn( window, 'confirm' ).mockReturnValue( false ); jest.mocked( useNavigate ).mockReturnValue( navigate ); @@ -233,17 +174,14 @@ beforeEach( () => { job: idleJob, updated: '2026-09-20T00:00:00Z', } ); - setCopyStatus(); jest.mocked( useSaveVideoEdits ).mockReturnValue( { mutateAsync: save } as never ); jest .mocked( useRetryVideoProcessing ) .mockReturnValue( { mutateAsync: retryProcessing } as never ); retryProcessing.mockResolvedValue( { guid: video.guid, revision: 2, job: processingJob } ); jest.mocked( useRestoreOriginal ).mockReturnValue( { mutateAsync: restore } as never ); - jest.mocked( useSaveVideoCopy ).mockReturnValue( { mutateAsync: copy } as never ); save.mockResolvedValue( { guid: video.guid, revision: 2, job: processingJob } ); restore.mockResolvedValue( { guid: video.guid, revision: 2, job: processingJob } ); - copy.mockResolvedValue( undefined ); } ); afterEach( () => { @@ -296,13 +234,15 @@ it( 'submits the current edits against their loaded revision and waits for the c const { user, refresh } = renderEditor(); await user.click( screen.getByRole( 'button', { name: 'New cut' } ) ); await user.click( screen.getByRole( 'button', { name: 'Save' } ) ); + expect( screen.getByRole( 'dialog', { name: 'Update video?' } ) ).toHaveTextContent( + 'Existing chapters may need to be adjusted' + ); await user.click( screen.getByRole( 'button', { name: 'Update video' } ) ); expect( save ).toHaveBeenCalledWith( { guid: video.guid, baseRevision: 2, operations: [ { type: 'cut', start_ms: 0, end_ms: 2000 } ], } ); - expect( copy ).not.toHaveBeenCalled(); expect( screen.queryByRole( 'dialog' ) ).not.toBeInTheDocument(); expect( screen.getByRole( 'button', { name: 'New cut' } ) ).toHaveAttribute( 'aria-disabled', @@ -380,7 +320,6 @@ it( 'retries an accepted save that failed while its unchanged draft remains in t await user.click( screen.getByRole( 'button', { name: 'Retry' } ) ); expect( retryProcessing ).toHaveBeenCalledWith( { guid: video.guid, jobId: processingJob.id } ); expect( save ).toHaveBeenCalledTimes( 1 ); - expect( copy ).not.toHaveBeenCalled(); } ); it( 'keeps a modified draft saveable without retrying older stored instructions', async () => { @@ -495,293 +434,6 @@ it( 'disables edits after a source error but still permits discarding the draft' expect( screen.getByRole( 'dialog', { name: 'Discard changes?' } ) ).toBeInTheDocument(); } ); -it( 'creates a separate video and navigates only once its attachment is ready', async () => { - const { user, refresh } = renderEditor(); - await saveCopy( user ); - expect( copy ).toHaveBeenCalledWith( { - guid: video.guid, - baseRevision: 2, - operations: [ { type: 'cut', start_ms: 0, end_ms: 2000 } ], - requestId: '8b3d1700-1234-4567-89ab-123456789abc', - title: `${ video.title } (edited)`, - } ); - setCopyStatus( copyResponse() ); - refresh(); - expect( - screen.getByText( 'Your current video stays unchanged.', { - exact: false, - ignore: '.a11y-speak-region, .a11y-speak-region *', - } ) - ).toBeInTheDocument(); - expect( screen.getByRole( 'button', { name: 'New cut' } ) ).toHaveAttribute( - 'aria-disabled', - 'true' - ); - expect( screen.getByRole( 'button', { name: 'Chapters' } ) ).toBeDisabled(); - await user.click( screen.getByRole( 'button', { name: 'Chapters' } ) ); - expect( selectTool ).not.toHaveBeenCalled(); - expect( screen.queryByRole( 'button', { name: 'Check status' } ) ).not.toBeInTheDocument(); - const complete = copyResponse( { ...processingJob, status: 'complete' } ); - setCopyStatus( complete ); - refresh(); - expect( navigate ).not.toHaveBeenCalled(); - setCopyStatus( { ...complete, guid: 'copy123', attachment_id: 99 } ); - refresh(); - expect( successNotice ).toHaveBeenCalledWith( - 'New video created. The original video is unchanged.' - ); - expect( navigate ).toHaveBeenCalledWith( { href: '/video/99' } ); - expect( screen.getByRole( 'button', { name: 'Discard changes' } ) ).toHaveAttribute( - 'aria-disabled', - 'true' - ); - setCopyStatus( { ...complete, guid: 'copy123', attachment_id: 99 } ); - refresh(); - expect( navigate ).toHaveBeenCalledTimes( 1 ); - expect( successNotice ).toHaveBeenCalledTimes( 1 ); - expect( save ).not.toHaveBeenCalled(); - expect( restore ).not.toHaveBeenCalled(); -} ); - -it( 'keeps an unconfirmed copy locked and retries the same captured request', async () => { - copy.mockRejectedValue( new Error( 'Connection lost' ) ); - const { user } = renderEditor(); - await saveCopy( user ); - expect( - screen.getByText( 'We could not confirm the new video’s status.', { - exact: false, - ignore: '.a11y-speak-region, .a11y-speak-region *', - } ) - ).toBeInTheDocument(); - expect( screen.getByRole( 'button', { name: 'Discard changes' } ) ).toHaveAttribute( - 'aria-disabled', - 'true' - ); - await user.click( screen.getByRole( 'button', { name: 'Retry' } ) ); - expect( copy ).toHaveBeenCalledTimes( 2 ); - expect( copy.mock.calls[ 1 ][ 0 ] ).toBe( copy.mock.calls[ 0 ][ 0 ] ); - expect( screen.queryByRole( 'button', { name: 'Check status' } ) ).not.toBeInTheDocument(); - expect( screen.queryByRole( 'button', { name: 'Dismiss' } ) ).not.toBeInTheDocument(); - expect( save ).not.toHaveBeenCalled(); -} ); - -it( 'allows polling after an accepted copy status request fails without offering a second submission', async () => { - const { user, refresh } = renderEditor(); - await saveCopy( user ); - setCopyStatus( undefined, true ); - refresh(); - expect( - screen.getByText( 'We could not confirm the new video’s status.', { - exact: false, - ignore: '.a11y-speak-region, .a11y-speak-region *', - } ) - ).toBeInTheDocument(); - expect( screen.queryByRole( 'button', { name: 'Retry' } ) ).not.toBeInTheDocument(); - expect( screen.queryByRole( 'button', { name: 'Check status' } ) ).not.toBeInTheDocument(); - expect( copy ).toHaveBeenCalledTimes( 1 ); -} ); - -it( 'keeps a recoverable attachment error locked and offers a safe retry', async () => { - const { user, refresh } = renderEditor(); - await saveCopy( user ); - setCopyStatus( - copyResponse( { - ...processingJob, - status: 'failed', - error: { code: 'copy_attachment_unconfirmed', message: 'Attachment response lost.' }, - } ) - ); - refresh(); - expect( - screen.getByText( 'We could not confirm the new video’s status.', { - exact: false, - ignore: '.a11y-speak-region, .a11y-speak-region *', - } ) - ).toBeInTheDocument(); - expect( screen.getByRole( 'button', { name: 'New cut' } ) ).toHaveAttribute( - 'aria-disabled', - 'true' - ); - await user.click( screen.getByRole( 'button', { name: 'Retry' } ) ); - expect( copy.mock.calls[ 1 ][ 0 ] ).toBe( copy.mock.calls[ 0 ][ 0 ] ); - expect( screen.queryByRole( 'button', { name: 'Dismiss' } ) ).not.toBeInTheDocument(); -} ); - -it( 'distinguishes a created copy from a failed transcode', async () => { - const { user, refresh } = renderEditor(); - await saveCopy( user ); - setCopyStatus( { - ...copyResponse( { - ...processingJob, - status: 'failed', - error: { code: 'transcode_failed', message: 'Transcoding failed.' }, - } ), - guid: 'copy1234', - attachment_id: 99, - } ); - refresh(); - expect( - screen.getByText( 'The new video was created, but its edits could not be processed.', { - exact: false, - ignore: '.a11y-speak-region, .a11y-speak-region *', - } ) - ).toBeInTheDocument(); - expect( navigate ).not.toHaveBeenCalled(); - expect( screen.getByRole( 'button', { name: 'Dismiss' } ) ).toBeInTheDocument(); -} ); - -it.each( [ 'Dismiss', 'Save' ] )( - 'preserves the draft after a failed copy when choosing %s', - async action => { - const { user, refresh } = renderEditor(); - await saveCopy( user ); - setCopyStatus( - copyResponse( { - ...processingJob, - status: 'failed', - error: { code: 'transcode_failed', message: 'Transcoding failed.' }, - } ) - ); - refresh(); - expect( - screen.getByText( 'Your current video and edits are unchanged.', { - exact: false, - ignore: '.a11y-speak-region, .a11y-speak-region *', - } ) - ).toBeInTheDocument(); - await user.click( screen.getByRole( 'button', { name: action } ) ); - expect( - screen.queryByText( 'Your current video and edits are unchanged.', { - exact: false, - ignore: '.a11y-speak-region, .a11y-speak-region *', - } ) - ).not.toBeInTheDocument(); - if ( action === 'Save' ) { - await user.click( screen.getByRole( 'button', { name: 'Cancel' } ) ); - } - expect( screen.getByRole( 'button', { name: 'Save' } ) ).not.toHaveAttribute( - 'aria-disabled', - 'true' - ); - await user.click( screen.getByRole( 'button', { name: 'Undo' } ) ); - expect( screen.getByRole( 'button', { name: 'Save' } ) ).toHaveAttribute( - 'aria-disabled', - 'true' - ); - expect( save ).not.toHaveBeenCalled(); - } -); - -it( 'requires reloading the source after a copy revision conflict', async () => { - setEdits( { - can_restore_original: true, - operations: [ { type: 'trim', start_ms: 0, end_ms: 9000 } ], - } ); - copy.mockRejectedValue( new EditsConflictError( 'Source changed.', 3 ) ); - const { user } = renderEditor(); - expect( screen.getByRole( 'button', { name: 'More actions' } ) ).toBeInTheDocument(); - await saveCopy( user ); - expect( - screen.getByText( 'The source video changed.', { - exact: false, - ignore: '.a11y-speak-region, .a11y-speak-region *', - } ) - ).toBeInTheDocument(); - expect( screen.getByRole( 'button', { name: 'Save' } ) ).toHaveAttribute( - 'aria-disabled', - 'true' - ); - expect( screen.getByRole( 'button', { name: 'New cut' } ) ).toHaveAttribute( - 'aria-disabled', - 'true' - ); - expect( screen.getByRole( 'button', { name: 'Undo' } ) ).toHaveAttribute( - 'aria-disabled', - 'true' - ); - expect( screen.queryByRole( 'button', { name: 'More actions' } ) ).not.toBeInTheDocument(); - await user.click( screen.getByRole( 'button', { name: 'Save' } ) ); - expect( screen.queryByRole( 'dialog' ) ).not.toBeInTheDocument(); - expect( copy ).toHaveBeenCalledTimes( 1 ); - await user.click( screen.getByRole( 'button', { name: 'Reload latest' } ) ); - expect( refetch ).not.toHaveBeenCalled(); - await user.click( - within( screen.getByRole( 'dialog' ) ).getByRole( 'button', { name: 'Reload latest' } ) - ); - expect( refetch ).toHaveBeenCalledTimes( 1 ); - expect( - screen.queryByText( 'The source video changed.', { - exact: false, - ignore: '.a11y-speak-region, .a11y-speak-region *', - } ) - ).not.toBeInTheDocument(); - expect( screen.getByRole( 'button', { name: 'New cut' } ) ).not.toHaveAttribute( - 'aria-disabled', - 'true' - ); - expect( screen.getByRole( 'button', { name: 'More actions' } ) ).toBeInTheDocument(); - expect( screen.getByRole( 'button', { name: 'Save' } ) ).toHaveAttribute( - 'aria-disabled', - 'true' - ); -} ); - -it( 'prevents duplicate submission and edits while copy acceptance is pending', async () => { - copy.mockReturnValue( new Promise( () => {} ) ); - const { user } = renderEditor(); - await saveCopy( user ); - expect( - screen.getByText( 'Creating your new video', { - exact: false, - ignore: '.a11y-speak-region, .a11y-speak-region *', - } ) - ).toBeInTheDocument(); - expect( screen.queryByRole( 'button', { name: 'Check status' } ) ).not.toBeInTheDocument(); - await user.click( screen.getByRole( 'button', { name: 'Save' } ) ); - await user.click( screen.getByRole( 'button', { name: 'Discard changes' } ) ); - await user.click( screen.getByRole( 'button', { name: 'New cut' } ) ); - expect( screen.queryByRole( 'dialog' ) ).not.toBeInTheDocument(); - expect( copy ).toHaveBeenCalledTimes( 1 ); -} ); - -it.each( [ 'Dismiss', 'Save' ] )( - 'preserves the draft after a rejected copy when choosing %s', - async action => { - copy.mockRejectedValue( - new VideoCopyRejectedError( 'quota_exceeded', 'Not enough video storage.' ) - ); - const { user } = renderEditor(); - await saveCopy( user ); - expect( - screen.getByText( 'Not enough video storage.', { - exact: false, - ignore: '.a11y-speak-region, .a11y-speak-region *', - } ) - ).toBeInTheDocument(); - expect( screen.getByRole( 'button', { name: 'New cut' } ) ).not.toHaveAttribute( - 'aria-disabled', - 'true' - ); - expect( screen.queryByRole( 'button', { name: 'Retry' } ) ).not.toBeInTheDocument(); - await user.click( screen.getByRole( 'button', { name: action } ) ); - expect( - screen.queryByText( 'Not enough video storage.', { - exact: false, - ignore: '.a11y-speak-region, .a11y-speak-region *', - } ) - ).not.toBeInTheDocument(); - if ( action === 'Save' ) { - await user.click( screen.getByRole( 'button', { name: 'Cancel' } ) ); - } - await user.click( screen.getByRole( 'button', { name: 'Undo' } ) ); - expect( screen.getByRole( 'button', { name: 'Save' } ) ).toHaveAttribute( - 'aria-disabled', - 'true' - ); - expect( save ).not.toHaveBeenCalled(); - } -); - it( 'shows indeterminate server progress and does not offer edits during processing', () => { setEdits( { can_restore_original: true, job: processingJob } ); renderEditor(); @@ -863,36 +515,25 @@ it( 'suspends editor shortcuts until the save dialog is dismissed', async () => ); } ); -it.each( [ 'update', 'copy' ] )( - 'stops warning on navigation after an accepted %s save', - async mode => { - const { user, refresh } = renderEditor(); - if ( mode === 'copy' ) { - await saveCopy( user ); - } else { - await user.click( screen.getByRole( 'button', { name: 'New cut' } ) ); - await user.click( screen.getByRole( 'button', { name: 'Save' } ) ); - await user.click( screen.getByRole( 'button', { name: 'Update video' } ) ); - } - const event = new Event( 'beforeunload', { cancelable: true } ); - window.dispatchEvent( event ); - expect( event.defaultPrevented ).toBe( false ); - await user.click( screen.getByRole( 'tab', { name: 'Details' } ) ); - expect( confirmNavigation ).not.toHaveBeenCalled(); +it( 'stops warning on navigation after an accepted save', async () => { + const { user, refresh } = renderEditor(); + await user.click( screen.getByRole( 'button', { name: 'New cut' } ) ); + await user.click( screen.getByRole( 'button', { name: 'Save' } ) ); + await user.click( screen.getByRole( 'button', { name: 'Update video' } ) ); + const event = new Event( 'beforeunload', { cancelable: true } ); + window.dispatchEvent( event ); + expect( event.defaultPrevented ).toBe( false ); + await user.click( screen.getByRole( 'tab', { name: 'Details' } ) ); + expect( confirmNavigation ).not.toHaveBeenCalled(); - if ( mode === 'copy' ) { - setCopyStatus( copyResponse( { ...processingJob, status: 'failed' } ) ); - } else { - setEdits( { job: { ...processingJob, status: 'failed' } } ); - } - refresh(); - const failedEvent = new Event( 'beforeunload', { cancelable: true } ); - window.dispatchEvent( failedEvent ); - expect( failedEvent.defaultPrevented ).toBe( true ); - } -); + setEdits( { job: { ...processingJob, status: 'failed' } } ); + refresh(); + const failedEvent = new Event( 'beforeunload', { cancelable: true } ); + window.dispatchEvent( failedEvent ); + expect( failedEvent.defaultPrevented ).toBe( true ); +} ); -it( 'keeps a processing copy in the editor and unlocks it when the job completes', () => { +it( 'keeps a processing video in the editor and unlocks it when the job completes', () => { const processingVideo = { ...video, durationSeconds: 0, isProcessing: true }; setEdits( { job: processingJob } ); const { rerender } = render( @@ -944,51 +585,7 @@ it( 'shows the failed job and retained original when attachment metadata never f expect( screen.queryByText( 'Video processing placeholder' ) ).not.toBeInTheDocument(); } ); -it( 'restores the pending copy draft after returning to the source editor', async () => { - const { user, unmount } = renderEditor(); - await saveCopy( user ); - unmount(); - renderEditor(); - expect( screen.getByRole( 'button', { name: 'New cut' } ) ).toHaveAttribute( - 'aria-disabled', - 'true' - ); - expect( screen.getAllByRole( 'slider', { name: /Cut/ } ) ).toHaveLength( 2 ); - const event = new Event( 'beforeunload', { cancelable: true } ); - window.dispatchEvent( event ); - expect( event.defaultPrevented ).toBe( false ); - expect( copy ).toHaveBeenCalledTimes( 1 ); -} ); - -it( 'continues to warn if a copy submission has not been confirmed', async () => { - copy.mockRejectedValueOnce( new Error( 'Connection interrupted' ) ); - const { user } = renderEditor(); - await saveCopy( user ); - const event = new Event( 'beforeunload', { cancelable: true } ); - window.dispatchEvent( event ); - expect( event.defaultPrevented ).toBe( true ); -} ); - -it( 'detects source edits made while a pending copy was away from the editor', async () => { - const { user, unmount } = renderEditor(); - await saveCopy( user ); - unmount(); - setEdits( { revision: 3, operations: [ { type: 'trim', start_ms: 1000, end_ms: 9000 } ] } ); - setCopyStatus( copyResponse( { ...processingJob, status: 'failed' } ) ); - renderEditor(); - expect( - screen.getByText( 'This video was edited somewhere else since you opened the editor.', { - ignore: '.a11y-speak-region, .a11y-speak-region *', - } ) - ).toBeInTheDocument(); - expect( screen.getByRole( 'button', { name: 'Save' } ) ).toHaveAttribute( - 'aria-disabled', - 'true' - ); - expect( screen.getAllByRole( 'slider', { name: /Cut/ } ) ).toHaveLength( 2 ); -} ); - -it( 'retries a failed copy after returning with missing playback metadata', async () => { +it( 'retries a failed edit after returning with missing playback metadata', async () => { setEdits( { can_retry: true, job: { ...processingJob, status: 'failed' } } ); const user = userEvent.setup(); render( diff --git a/projects/packages/videopress/src/dashboard/components/editor/test/save-dialog.test.tsx b/projects/packages/videopress/src/dashboard/components/editor/test/save-dialog.test.tsx deleted file mode 100644 index 347cbc963613..000000000000 --- a/projects/packages/videopress/src/dashboard/components/editor/test/save-dialog.test.tsx +++ /dev/null @@ -1,79 +0,0 @@ -import { render, screen } from '@testing-library/react'; -import userEvent from '@testing-library/user-event'; -import SaveVideoDialog from '../save-dialog'; - -const title = 'A very edited video'; - -it( 'defaults to updating the current video and explains the chapter impact', async () => { - const onSave = jest.fn(); - render( - - ); - expect( screen.getByRole( 'radio', { name: 'Update existing video' } ) ).toBeChecked(); - expect( screen.getByText( /Existing chapters may need to be adjusted/ ) ).toBeInTheDocument(); - expect( screen.queryByRole( 'textbox', { name: 'New video title' } ) ).not.toBeInTheDocument(); - await userEvent.setup().click( screen.getByRole( 'button', { name: 'Update video' } ) ); - expect( onSave ).toHaveBeenCalledWith( 'update', `${ title } (edited)` ); -} ); - -it( 'offers a separate video with a default title and trims the submitted title', async () => { - const onSave = jest.fn(); - const user = userEvent.setup(); - render( - - ); - await user.click( screen.getByRole( 'radio', { name: 'Save as new video' } ) ); - const input = screen.getByRole( 'textbox', { name: 'New video title' } ); - expect( input ).toHaveValue( `${ title } (edited)` ); - expect( screen.getByText( /The current video stays unchanged/ ) ).toBeInTheDocument(); - expect( - screen.queryByText( /Existing chapters may need to be adjusted/ ) - ).not.toBeInTheDocument(); - await user.clear( input ); - await user.type( input, ' A separate version ' ); - await user.click( screen.getByRole( 'button', { name: 'Save as new video' } ) ); - expect( onSave ).toHaveBeenCalledWith( 'copy', 'A separate version' ); -} ); - -it( 'rejects whitespace titles for copies while allowing the existing video to be updated', async () => { - const onSave = jest.fn(); - const user = userEvent.setup(); - render( - - ); - await user.click( screen.getByRole( 'radio', { name: 'Save as new video' } ) ); - const input = screen.getByRole( 'textbox', { name: 'New video title' } ); - await user.clear( input ); - await user.type( input, ' ' ); - const saveCopy = screen.getByRole( 'button', { name: 'Save as new video' } ); - expect( saveCopy ).toHaveAttribute( 'aria-disabled', 'true' ); - await user.click( saveCopy ); - expect( onSave ).not.toHaveBeenCalled(); - await user.click( screen.getByRole( 'radio', { name: 'Update existing video' } ) ); - expect( screen.getByRole( 'button', { name: 'Update video' } ) ).not.toHaveAttribute( - 'aria-disabled', - 'true' - ); -} ); - -it( 'prevents repeated save and cancellation while a request is busy', async () => { - const onSave = jest.fn(); - const onCancel = jest.fn(); - const user = userEvent.setup(); - const props = { title, onSave, onCancel }; - const { rerender } = render( ); - await user.click( screen.getByRole( 'radio', { name: 'Save as new video' } ) ); - rerender( ); - expect( screen.getByRole( 'textbox', { name: 'New video title' } ) ).toBeDisabled(); - expect( screen.getByRole( 'radio', { name: 'Update existing video' } ) ).toBeDisabled(); - expect( screen.queryByRole( 'button', { name: 'Close' } ) ).not.toBeInTheDocument(); - const save = screen.getByRole( 'button', { name: 'Save as new video' } ); - const cancel = screen.getByRole( 'button', { name: 'Cancel' } ); - expect( save ).toHaveAttribute( 'aria-disabled', 'true' ); - expect( cancel ).toHaveAttribute( 'aria-disabled', 'true' ); - await user.click( save ); - await user.click( cancel ); - await user.keyboard( '{Escape}' ); - expect( onSave ).not.toHaveBeenCalled(); - expect( onCancel ).not.toHaveBeenCalled(); -} ); diff --git a/projects/packages/videopress/src/dashboard/components/editor/test/use-copy-session.test.tsx b/projects/packages/videopress/src/dashboard/components/editor/test/use-copy-session.test.tsx deleted file mode 100644 index 8e07fc98ef0b..000000000000 --- a/projects/packages/videopress/src/dashboard/components/editor/test/use-copy-session.test.tsx +++ /dev/null @@ -1,313 +0,0 @@ -import { act, render, renderHook, screen } from '@testing-library/react'; -import { - useSaveVideoCopy, - useVideoCopyStatus, - VideoCopyRejectedError, -} from '../../../hooks/use-save-video-copy'; -import { EditsConflictError } from '../../../hooks/use-save-video-edits'; -import CopyStatusBanner from '../copy-status-banner'; -import { useCopySession } from '../use-copy-session'; -import type { SaveVideoCopyResponse, SaveVideoCopyVars } from '../../../hooks/use-save-video-copy'; - -jest.mock( '../../../hooks/use-save-video-copy', () => ( { - ...jest.requireActual( '../../../hooks/use-save-video-copy' ), - useSaveVideoCopy: jest.fn(), - useVideoCopyStatus: jest.fn(), -} ) ); - -const mutate = jest.fn(); -const refetch = jest.fn(); -const request: SaveVideoCopyVars = { - guid: 'source12', - baseRevision: 2, - operations: [ { type: 'cut', start_ms: 3000, end_ms: 5000 } ], - requestId: '32457391-3ebf-4c67-ac58-a34dd71399bf', - title: 'Video copy', -}; -const accepted: SaveVideoCopyResponse = { - source_guid: request.guid, - request_id: request.requestId, - guid: null, - attachment_id: null, - job: { id: 'copy-job', status: 'processing', target_revision: null, progress: null, error: null }, -}; -let status: SaveVideoCopyResponse | undefined; - -beforeEach( () => { - sessionStorage.clear(); - jest.clearAllMocks(); - status = undefined; - mutate.mockResolvedValue( accepted ); - jest.mocked( useSaveVideoCopy ).mockReturnValue( { mutateAsync: mutate } as never ); - jest.mocked( useVideoCopyStatus ).mockImplementation( - ( guid, requestId ) => - ( { - data: requestId ? status : undefined, - refetch, - } ) as never - ); -} ); - -describe( 'useCopySession', () => { - it.each( [ 'copy_storage_limit', 'copy_authorization_unavailable' ] )( - 'unlocks a rejected %s request without polling and allows saving the preserved draft again', - async code => { - const failure = new VideoCopyRejectedError( code, 'Cannot create a copy.' ); - mutate.mockRejectedValueOnce( failure ); - const { result } = renderHook( () => useCopySession( request.guid ) ); - await act( async () => result.current.submit( request ) ); - expect( result.current.rejected ).toBe( true ); - expect( result.current.locked ).toBe( false ); - expect( useVideoCopyStatus ).toHaveBeenLastCalledWith( request.guid, null ); - expect( result.current.request?.operations ).toEqual( request.operations ); - act( () => result.current.clear() ); - expect( result.current.request ).toBeNull(); - const next = { ...request, requestId: 'another-copy' }; - await act( async () => result.current.submit( next ) ); - expect( mutate ).toHaveBeenLastCalledWith( next ); - expect( result.current.locked ).toBe( true ); - } - ); - - it.each( [ - new VideoCopyRejectedError( 'rest_cookie_invalid_nonce' ), - new VideoCopyRejectedError( 'copy_authorization_unavailable' ), - new EditsConflictError(), - ] )( 'keeps an uncertain request locked when a retry returns %s', async retryError => { - mutate - .mockRejectedValueOnce( new Error( 'Response interrupted' ) ) - .mockRejectedValueOnce( retryError ); - const { result } = renderHook( () => useCopySession( request.guid ) ); - await act( async () => result.current.submit( request ) ); - await act( async () => result.current.retry() ); - expect( result.current.rejected ).toBe( false ); - expect( result.current.conflict ).toBe( false ); - expect( result.current.locked ).toBe( true ); - expect( useVideoCopyStatus ).toHaveBeenLastCalledWith( request.guid, request.requestId ); - act( () => result.current.clear() ); - expect( result.current.request ).toBe( request ); - } ); - - it( 'prevents duplicate submissions while the POST is unsettled', async () => { - let resolve: ( value: SaveVideoCopyResponse ) => void; - mutate.mockImplementation( - () => - new Promise( resolveRequest => { - resolve = resolveRequest; - } ) - ); - const { result } = renderHook( () => useCopySession( request.guid ) ); - act( () => { - void result.current.submit( request ); - } ); - expect( result.current.locked ).toBe( true ); - expect( result.current.submitting ).toBe( true ); - await act( async () => result.current.submit( { ...request, requestId: 'another-copy' } ) ); - expect( mutate ).toHaveBeenCalledTimes( 1 ); - await act( async () => resolve( accepted ) ); - expect( result.current.submitting ).toBe( false ); - } ); - - it( 'does not replace an accepted processing request with a second copy', async () => { - const { result, rerender } = renderHook( () => useCopySession( request.guid ) ); - await act( async () => result.current.submit( request ) ); - status = accepted; - rerender(); - await act( async () => result.current.submit( { ...request, requestId: 'another-copy' } ) ); - expect( mutate ).toHaveBeenCalledTimes( 1 ); - expect( result.current.request?.requestId ).toBe( request.requestId ); - } ); - - it( 'retains the exact request and source draft after an uncertain failure and retry', async () => { - const sourceDraft = JSON.parse( JSON.stringify( request.operations ) ); - const failure = new Error( 'Response interrupted' ); - mutate.mockRejectedValueOnce( failure ).mockResolvedValueOnce( accepted ); - const { result } = renderHook( () => useCopySession( request.guid ) ); - await act( async () => result.current.submit( request ) ); - expect( result.current.error ).toBe( failure ); - expect( result.current.locked ).toBe( true ); - await act( async () => result.current.retry() ); - expect( mutate ).toHaveBeenNthCalledWith( 1, request ); - expect( mutate ).toHaveBeenNthCalledWith( 2, request ); - expect( request.operations ).toEqual( sourceDraft ); - expect( result.current.error ).toBeNull(); - } ); - - it.each( [ 'failed', 'complete' ] as const )( - 'handles a terminal %s copy while preserving the request', - async nextStatus => { - const { result, rerender } = renderHook( () => useCopySession( request.guid ) ); - await act( async () => result.current.submit( request ) ); - status = accepted; - rerender(); - expect( result.current.locked ).toBe( true ); - status = { - ...accepted, - guid: nextStatus === 'complete' ? 'newcopy1' : null, - attachment_id: nextStatus === 'complete' ? 17 : null, - job: { ...accepted.job, status: nextStatus }, - }; - rerender(); - expect( result.current.locked ).toBe( nextStatus === 'complete' ); - expect( result.current.request ).toEqual( request ); - } - ); - - it( 'retries an unconfirmed remote attachment with the captured request while keeping edits locked', async () => { - const { result, rerender } = renderHook( () => useCopySession( request.guid ) ); - await act( async () => result.current.submit( request ) ); - status = { - ...accepted, - job: { - ...accepted.job, - status: 'failed', - error: { code: 'copy_attachment_unconfirmed', message: 'Attachment unconfirmed' }, - }, - }; - rerender(); - expect( result.current.recoverable ).toBe( true ); - expect( result.current.failed ).toBe( false ); - expect( result.current.locked ).toBe( true ); - act( () => result.current.clear() ); - expect( result.current.request ).toBe( request ); - await act( async () => result.current.retry() ); - expect( mutate ).toHaveBeenNthCalledWith( 2, request ); - expect( result.current.request?.operations ).toEqual( request.operations ); - } ); - - it( 'preserves a pending attachment request without retrying and accepts a late completion', async () => { - const { result, rerender } = renderHook( () => useCopySession( request.guid ) ); - await act( async () => result.current.submit( request ) ); - status = { - ...accepted, - job: { - ...accepted.job, - status: 'failed', - error: { code: 'copy_attachment_pending', message: 'Attachment pending' }, - }, - }; - rerender(); - expect( result.current.needsAssistance ).toBe( true ); - expect( result.current.recoverable ).toBe( false ); - expect( result.current.failed ).toBe( false ); - expect( result.current.locked ).toBe( true ); - act( () => result.current.clear() ); - await act( async () => result.current.retry() ); - await act( async () => result.current.submit( { ...request, requestId: 'another-copy' } ) ); - expect( mutate ).toHaveBeenCalledTimes( 1 ); - expect( result.current.request ).toBe( request ); - expect( useVideoCopyStatus ).toHaveBeenLastCalledWith( request.guid, request.requestId ); - - render( - - ); - expect( - screen.getByText( 'Your current video is unchanged.', { - exact: false, - ignore: '.a11y-speak-region, .a11y-speak-region *', - } ) - ).toBeInTheDocument(); - expect( - screen.getByText( 'contact support if it remains unconfirmed', { - exact: false, - ignore: '.a11y-speak-region, .a11y-speak-region *', - } ) - ).toBeInTheDocument(); - expect( screen.queryByRole( 'button', { name: 'Retry' } ) ).not.toBeInTheDocument(); - expect( screen.queryByRole( 'button', { name: 'Dismiss' } ) ).not.toBeInTheDocument(); - expect( screen.queryByRole( 'button', { name: 'Check status' } ) ).not.toBeInTheDocument(); - - status = { - ...accepted, - guid: 'newcopy1', - attachment_id: 17, - job: { ...accepted.job, status: 'complete' }, - }; - rerender(); - expect( result.current.needsAssistance ).toBe( false ); - expect( result.current.status.data?.guid ).toBe( 'newcopy1' ); - } ); - - it( 'releases the lock and exposes a source revision conflict', async () => { - mutate.mockRejectedValueOnce( new EditsConflictError() ); - const { result } = renderHook( () => useCopySession( request.guid ) ); - await act( async () => result.current.submit( request ) ); - expect( result.current.conflict ).toBe( true ); - expect( result.current.locked ).toBe( false ); - expect( result.current.request?.operations ).toEqual( request.operations ); - } ); - - it( 'keeps tracking an accepted job when clear is called before it finishes', async () => { - const { result, rerender } = renderHook( () => useCopySession( request.guid ) ); - await act( async () => result.current.submit( request ) ); - status = accepted; - rerender(); - act( () => result.current.clear() ); - expect( result.current.request?.requestId ).toBe( request.requestId ); - expect( result.current.locked ).toBe( true ); - } ); -} ); - -it( 'resumes an accepted copy after remounting without submitting another job', async () => { - const { result: firstResult, unmount: unmountFirst } = renderHook( () => - useCopySession( request.guid ) - ); - await act( async () => firstResult.current.submit( request ) ); - unmountFirst(); - const { - result: secondResult, - rerender: rerenderSecond, - unmount: unmountSecond, - } = renderHook( () => useCopySession( request.guid ) ); - expect( secondResult.current.request ).toEqual( request ); - expect( secondResult.current.saved ).toBe( true ); - expect( secondResult.current.locked ).toBe( true ); - expect( useVideoCopyStatus ).toHaveBeenLastCalledWith( request.guid, request.requestId ); - expect( mutate ).toHaveBeenCalledTimes( 1 ); - status = { ...accepted, attachment_id: 17, job: { ...accepted.job, status: 'complete' } }; - rerenderSecond(); - unmountSecond(); - const { result: thirdResult } = renderHook( () => useCopySession( request.guid ) ); - expect( thirdResult.current.request ).toBeNull(); -} ); - -it( 'keeps an interrupted copy request recoverable across navigation until acceptance is confirmed', async () => { - mutate.mockRejectedValueOnce( new Error( 'Connection lost' ) ); - const { result: firstResult, unmount: unmountFirst } = renderHook( () => - useCopySession( request.guid ) - ); - await act( async () => firstResult.current.submit( request ) ); - expect( firstResult.current.saved ).toBe( false ); - unmountFirst(); - const { result: secondResult, rerender: rerenderSecond } = renderHook( () => - useCopySession( request.guid ) - ); - expect( secondResult.current.request ).toEqual( request ); - expect( secondResult.current.saved ).toBe( false ); - status = accepted; - rerenderSecond(); - expect( secondResult.current.saved ).toBe( true ); - expect( mutate ).toHaveBeenCalledTimes( 1 ); -} ); - -it( 'can still save when browser storage is unavailable', async () => { - const get = jest.spyOn( Storage.prototype, 'getItem' ).mockImplementation( () => { - throw new Error( 'Storage blocked' ); - } ); - const set = jest.spyOn( Storage.prototype, 'setItem' ).mockImplementation( () => { - throw new Error( 'Storage blocked' ); - } ); - try { - const { result } = renderHook( () => useCopySession( request.guid ) ); - await act( async () => result.current.submit( request ) ); - expect( result.current.saved ).toBe( true ); - expect( mutate ).toHaveBeenCalledWith( request ); - } finally { - get.mockRestore(); - set.mockRestore(); - } -} ); diff --git a/projects/packages/videopress/src/dashboard/components/editor/use-copy-session.ts b/projects/packages/videopress/src/dashboard/components/editor/use-copy-session.ts deleted file mode 100644 index 0e0989a6f9b1..000000000000 --- a/projects/packages/videopress/src/dashboard/components/editor/use-copy-session.ts +++ /dev/null @@ -1,155 +0,0 @@ -import { useEffect, useRef, useState } from 'react'; -import { - useSaveVideoCopy, - useVideoCopyStatus, - VideoCopyRejectedError, -} from '../../hooks/use-save-video-copy'; -import { EditsConflictError } from '../../hooks/use-save-video-edits'; -import type { SaveVideoCopyVars } from '../../hooks/use-save-video-copy'; - -type StoredCopy = { request: SaveVideoCopyVars; accepted: boolean }; - -/** - * Read a pending copy from this browser tab; storage may be unavailable. - * @param guid - Source video GUID. - * @return The saved request, if it belongs to this source. - */ -function readPendingCopy( guid: string ): StoredCopy | null { - try { - const stored = JSON.parse( sessionStorage.getItem( `videopress-copy:${ guid }` ) || 'null' ); - return stored?.request?.guid === guid && - typeof stored.request.requestId === 'string' && - Array.isArray( stored.request.operations ) - ? stored - : null; - } catch { - return null; - } -} - -/** - * Preserve the idempotency key before sending a request that could outlive this page. - * @param guid - Source video GUID. - * @param value - Pending request, or null to forget a terminal result. - */ -function storePendingCopy( guid: string, value: StoredCopy | null ) { - try { - if ( value ) { - sessionStorage.setItem( `videopress-copy:${ guid }`, JSON.stringify( value ) ); - } else { - sessionStorage.removeItem( `videopress-copy:${ guid }` ); - } - } catch { - // Storage restrictions must not prevent saving or following the current request. - } -} - -/** - * Retain a copy request across uncertain responses so retrying cannot create duplicates. - * - * @param guid - Source video GUID. - * @return The copy lifecycle and actions. - */ -export function useCopySession( guid: string ) { - const mutation = useSaveVideoCopy(); - const [ stored ] = useState( () => readPendingCopy( guid ) ); - const [ request, setRequest ] = useState< SaveVideoCopyVars | null >( stored?.request ?? null ); - const [ accepted, setAccepted ] = useState( Boolean( stored?.accepted ) ); - const [ submitting, setSubmitting ] = useState( false ); - const [ error, setError ] = useState< Error | null >( - stored && ! stored.accepted ? new Error( 'Copy acceptance unconfirmed' ) : null - ); - const submittingRef = useRef( false ); - const requestRef = useRef< SaveVideoCopyVars | null >( request ); - const acceptanceUncertainRef = useRef( Boolean( stored ) ); - const rejected = error instanceof VideoCopyRejectedError && ! acceptanceUncertainRef.current; - const conflict = error instanceof EditsConflictError && ! acceptanceUncertainRef.current; - const status = useVideoCopyStatus( - guid, - submitting || rejected || conflict ? null : ( request?.requestId ?? null ) - ); - const recoverable = - status.data?.job?.status === 'failed' && - status.data.job.error?.code === 'copy_attachment_unconfirmed'; - const needsAssistance = - status.data?.job?.status === 'failed' && - status.data.job.error?.code === 'copy_attachment_pending'; - const failed = status.data?.job?.status === 'failed' && ! recoverable && ! needsAssistance; - - const confirmed = accepted || Boolean( status.data ); - const saved = Boolean( request ) && confirmed && ! failed && ! rejected && ! conflict; - useEffect( () => { - if ( ! request ) { - return; - } - if ( failed || rejected || conflict || status.data?.job?.status === 'complete' ) { - storePendingCopy( guid, null ); - } else { - storePendingCopy( guid, { request, accepted: confirmed } ); - } - }, [ guid, request, confirmed, failed, rejected, conflict, status.data?.job?.status ] ); - - const submit = async ( nextRequest: SaveVideoCopyVars ) => { - if ( - needsAssistance || - submittingRef.current || - ( requestRef.current && nextRequest !== requestRef.current ) - ) { - return; - } - submittingRef.current = true; - setSubmitting( true ); - setError( null ); - setRequest( nextRequest ); - requestRef.current = nextRequest; - storePendingCopy( guid, { request: nextRequest, accepted } ); - try { - await mutation.mutateAsync( nextRequest ); - setAccepted( true ); - acceptanceUncertainRef.current = true; - } catch ( caught ) { - // A later rejection cannot disprove acceptance of an earlier interrupted request. - if ( ! ( - caught instanceof VideoCopyRejectedError || caught instanceof EditsConflictError - ) ) { - acceptanceUncertainRef.current = true; - } else if ( ! acceptanceUncertainRef.current ) { - storePendingCopy( guid, null ); - } - setError( caught as Error ); - } finally { - submittingRef.current = false; - setSubmitting( false ); - } - }; - - return { - request, - saved, - submitting, - error, - recoverable, - needsAssistance, - conflict, - rejected, - failed, - locked: Boolean( request ) && ! failed && ! conflict && ! rejected, - status, - submit, - retry: () => request && submit( request ), - clear: () => { - if ( - submittingRef.current || - ( requestRef.current && ! failed && ! conflict && ! rejected ) - ) { - return; - } - requestRef.current = null; - acceptanceUncertainRef.current = false; - storePendingCopy( guid, null ); - setAccepted( false ); - setRequest( null ); - setError( null ); - }, - }; -} diff --git a/projects/packages/videopress/src/dashboard/components/editor/use-edit-session.ts b/projects/packages/videopress/src/dashboard/components/editor/use-edit-session.ts index 71cdb402ef78..c16df3b4ab97 100644 --- a/projects/packages/videopress/src/dashboard/components/editor/use-edit-session.ts +++ b/projects/packages/videopress/src/dashboard/components/editor/use-edit-session.ts @@ -13,7 +13,6 @@ import { createEditSession, editSessionReducer, sessionEditsEqual } from './stat import { isDirty, sessionToOperations } from './state/serialize'; import type { EditSession, EditSessionAction } from './state/edit-session'; import type { HistoryAction } from '../../../client/components/chapters-editor/state/history'; -import type { SaveVideoCopyVars } from '../../hooks/use-save-video-copy'; import type { SaveEditsResponse, VideoEdits } from '../../types/edits'; import type { LibraryItem } from '../../types/library'; @@ -25,11 +24,10 @@ const reducer = withHistory< EditSession, EditSessionAction >( editSessionReduce /** * Keep local edits against the revision they were made on until a processing job commits. * - * @param video - The attachment being edited. - * @param copyRequest - Recover the draft and its revision from a pending copy after navigation. + * @param video - The attachment being edited. * @return The edit session, processing state, and save/restore actions. */ -export function useEditSession( video: LibraryItem, copyRequest?: SaveVideoCopyVars | null ) { +export function useEditSession( video: LibraryItem ) { const query = useVideoEdits( video.guid ); const saveMutation = useSaveVideoEdits(); const restoreMutation = useRestoreOriginal(); @@ -107,14 +105,6 @@ export function useEditSession( video: LibraryItem, copyRequest?: SaveVideoCopyV } } else if ( ! currentBaseline ) { adopt( edits ); - if ( copyRequest ) { - dispatch( { - type: 'LOAD', - operations: copyRequest.operations, - durationMs: edits.original_duration_ms, - } ); - setConflict( copyRequest.baseRevision !== edits.revision ); - } } else if ( edits.revision !== currentBaseline.revision ) { if ( pendingJob || current.dirty ) { pendingRef.current = null; @@ -124,7 +114,7 @@ export function useEditSession( video: LibraryItem, copyRequest?: SaveVideoCopyV adopt( edits ); } } - }, [ query.edits, pending, adopt, copyRequest ] ); + }, [ query.edits, pending, adopt ] ); const guardedDispatch = useCallback( ( action: HistoryAction< EditSessionAction > ) => { if ( ! stateRef.current.locked && ! stateRef.current.conflict ) { diff --git a/projects/packages/videopress/src/dashboard/hooks/test/use-save-video-copy.test.ts b/projects/packages/videopress/src/dashboard/hooks/test/use-save-video-copy.test.ts deleted file mode 100644 index 8287578c30d9..000000000000 --- a/projects/packages/videopress/src/dashboard/hooks/test/use-save-video-copy.test.ts +++ /dev/null @@ -1,247 +0,0 @@ -import { act, renderHook, waitFor } from '@testing-library/react'; -import apiFetch from '@wordpress/api-fetch'; -import { createTestQueryClient, createTestWrapper } from '../../test-utils/query-client-wrapper'; -import { - LIBRARY_QUERY_KEY, - LIBRARY_POLL_INTERVAL_MS, - PROCESSING_POLL_MAX_MS, -} from '../use-library'; -import { - useSaveVideoCopy, - useVideoCopyStatus, - VIDEO_COPY_QUERY_KEY, - VideoCopyRejectedError, -} from '../use-save-video-copy'; -import { EditsConflictError } from '../use-save-video-edits'; -import { EDITS_QUERY_KEY } from '../use-video-edits'; -import type { SaveVideoCopyResponse, SaveVideoCopyVars } from '../use-save-video-copy'; - -jest.mock( '@wordpress/api-fetch', () => ( { __esModule: true, default: jest.fn() } ) ); - -const request: SaveVideoCopyVars = { - guid: 'source12', - baseRevision: 2, - operations: [ { type: 'cut', start_ms: 3000, end_ms: 5000 } ], - requestId: '32457391-3ebf-4c67-ac58-a34dd71399bf', - title: 'Video copy', -}; -const accepted: SaveVideoCopyResponse = { - source_guid: request.guid, - request_id: request.requestId, - guid: null, - attachment_id: null, - job: { id: 'copy-job', status: 'processing', target_revision: null, progress: null, error: null }, -}; - -beforeEach( () => { - jest.mocked( apiFetch ).mockReset(); -} ); - -afterEach( () => { - jest.useRealTimers(); -} ); - -describe( 'useSaveVideoCopy', () => { - it( 'does not cache a null job or treat it as a definitive rejection', async () => { - jest.mocked( apiFetch ).mockResolvedValue( { ...accepted, job: null } ); - const client = createTestQueryClient(); - const { result } = renderHook( useSaveVideoCopy, { wrapper: createTestWrapper( client ) } ); - await act( async () => { - await expect( result.current.mutateAsync( request ) ).rejects.not.toBeInstanceOf( - VideoCopyRejectedError - ); - } ); - expect( - client.getQueryData( [ VIDEO_COPY_QUERY_KEY, request.guid, request.requestId ] ) - ).toBeUndefined(); - } ); - - it.each( [ - [ 'copy_storage_limit', 403 ], - [ 'copy_source_unavailable', 409 ], - [ 'copy_authorization_unavailable', 424 ], - [ 'invalid_title', 400 ], - [ 'unknown_media', 404 ], - ] )( - 'distinguishes a rejected %s request from an uncertain acceptance', - async ( code, status ) => { - jest - .mocked( apiFetch ) - .mockRejectedValue( { code, message: 'Cannot copy this video.', data: { status } } ); - const { result } = renderHook( useSaveVideoCopy, { wrapper: createTestWrapper() } ); - await act( async () => { - await expect( result.current.mutateAsync( request ) ).rejects.toEqual( - new VideoCopyRejectedError( code as string, 'Cannot copy this video.' ) - ); - } ); - } - ); - - it.each( [ - [ 'copy_request_pending', 409 ], - [ 'copy_request_conflict', 409 ], - [ 'videopress_edits_request_failed', 502 ], - [ 'unknown_dependency_failure', 424 ], - ] )( 'retains uncertain acceptance for %s', async ( code, status ) => { - const failure = { code, data: { status } }; - jest.mocked( apiFetch ).mockRejectedValue( failure ); - const { result } = renderHook( useSaveVideoCopy, { wrapper: createTestWrapper() } ); - await act( async () => { - await expect( result.current.mutateAsync( request ) ).rejects.toBe( failure ); - } ); - } ); - - it( 'submits an idempotent copy request and leaves the source edit cache unchanged', async () => { - jest.mocked( apiFetch ).mockResolvedValue( accepted ); - const client = createTestQueryClient(); - client.setQueryDefaults( [ EDITS_QUERY_KEY ], { gcTime: Infinity } ); - client.setQueryDefaults( [ VIDEO_COPY_QUERY_KEY ], { gcTime: Infinity } ); - const original = { revision: 2, operations: [], job: { status: 'idle' } }; - client.setQueryData( [ EDITS_QUERY_KEY, request.guid ], original ); - const { result } = renderHook( useSaveVideoCopy, { wrapper: createTestWrapper( client ) } ); - await act( async () => { - await result.current.mutateAsync( request ); - } ); - expect( apiFetch ).toHaveBeenCalledWith( { - path: '/wpcom/v2/videopress/source12/edits/copy', - method: 'POST', - data: { - base_revision: 2, - operations: request.operations, - request_id: request.requestId, - title: 'Video copy', - }, - } ); - expect( client.getQueryData( [ EDITS_QUERY_KEY, request.guid ] ) ).toEqual( original ); - expect( - client.getQueryData( [ VIDEO_COPY_QUERY_KEY, request.guid, request.requestId ] ) - ).toEqual( accepted ); - } ); - - it( 'retries an uncertain request with the same identifier and captured operations', async () => { - const networkError = new Error( 'Connection interrupted' ); - jest.mocked( apiFetch ).mockRejectedValueOnce( networkError ).mockResolvedValueOnce( accepted ); - const { result } = renderHook( useSaveVideoCopy, { wrapper: createTestWrapper() } ); - await act( async () => { - await expect( result.current.mutateAsync( request ) ).rejects.toBe( networkError ); - } ); - await act( async () => { - await result.current.mutateAsync( request ); - } ); - expect( jest.mocked( apiFetch ).mock.calls[ 1 ] ).toEqual( - jest.mocked( apiFetch ).mock.calls[ 0 ] - ); - } ); - - it( 'reports source revision conflicts without accepting a new copy', async () => { - jest - .mocked( apiFetch ) - .mockRejectedValue( { code: 'edits_conflict', data: { current_revision: 3 } } ); - const client = createTestQueryClient(); - const { result } = renderHook( useSaveVideoCopy, { wrapper: createTestWrapper( client ) } ); - await act( async () => { - await expect( result.current.mutateAsync( request ) ).rejects.toBeInstanceOf( - EditsConflictError - ); - } ); - expect( - client.getQueryData( [ VIDEO_COPY_QUERY_KEY, request.guid, request.requestId ] ) - ).toBeUndefined(); - } ); -} ); - -describe( 'useVideoCopyStatus', () => { - it( 'automatically recovers after a response with a null job', async () => { - jest.useFakeTimers(); - jest - .mocked( apiFetch ) - .mockResolvedValueOnce( { ...accepted, job: null } ) - .mockResolvedValueOnce( accepted ); - const { result } = renderHook( () => useVideoCopyStatus( request.guid, request.requestId ), { - wrapper: createTestWrapper(), - } ); - await waitFor( () => expect( result.current.isError ).toBe( true ) ); - expect( result.current.data ).toBeUndefined(); - await act( async () => jest.advanceTimersByTime( LIBRARY_POLL_INTERVAL_MS ) ); - await waitFor( () => expect( result.current.data ).toEqual( accepted ) ); - } ); - - it.each( [ 'copy_attachment_pending', 'copy_attachment_unconfirmed' ] )( - 'continues polling %s until the copy completes', - async code => { - jest.useFakeTimers(); - jest - .mocked( apiFetch ) - .mockResolvedValueOnce( { - ...accepted, - job: { ...accepted.job, status: 'failed', error: { code, message: 'Unconfirmed' } }, - } ) - .mockResolvedValue( { - ...accepted, - guid: 'copy1234', - attachment_id: 17, - job: { ...accepted.job, status: 'complete' }, - } ); - const { result } = renderHook( () => useVideoCopyStatus( request.guid, request.requestId ), { - wrapper: createTestWrapper(), - } ); - await waitFor( () => expect( result.current.data?.job.error?.code ).toBe( code ) ); - await act( async () => jest.advanceTimersByTime( LIBRARY_POLL_INTERVAL_MS ) ); - await waitFor( () => expect( result.current.data?.job.status ).toBe( 'complete' ) ); - await act( async () => jest.advanceTimersByTime( LIBRARY_POLL_INTERVAL_MS * 3 ) ); - expect( apiFetch ).toHaveBeenCalledTimes( 2 ); - } - ); - - it( 'waits for a request before querying', () => { - renderHook( () => useVideoCopyStatus( request.guid, null ), { wrapper: createTestWrapper() } ); - expect( apiFetch ).not.toHaveBeenCalled(); - } ); - - it( 'refreshes the library when the destination appears and finishes, then stops polling', async () => { - jest.useFakeTimers(); - jest - .mocked( apiFetch ) - .mockResolvedValueOnce( accepted ) - .mockResolvedValueOnce( { ...accepted, guid: 'copy1234', attachment_id: 17 } ) - .mockResolvedValue( { - ...accepted, - guid: 'copy1234', - attachment_id: 17, - job: { ...accepted.job, status: 'complete' }, - } ); - const client = createTestQueryClient(); - const invalidate = jest.spyOn( client, 'invalidateQueries' ); - const { result } = renderHook( () => useVideoCopyStatus( request.guid, request.requestId ), { - wrapper: createTestWrapper( client ), - } ); - await waitFor( () => expect( result.current.data?.job.status ).toBe( 'processing' ) ); - expect( invalidate ).not.toHaveBeenCalled(); - await act( async () => jest.advanceTimersByTime( LIBRARY_POLL_INTERVAL_MS ) ); - await waitFor( () => expect( result.current.data?.attachment_id ).toBe( 17 ) ); - expect( invalidate ).toHaveBeenCalledWith( { queryKey: [ LIBRARY_QUERY_KEY ] } ); - await act( async () => jest.advanceTimersByTime( LIBRARY_POLL_INTERVAL_MS ) ); - await waitFor( () => expect( result.current.data?.job.status ).toBe( 'complete' ) ); - expect( invalidate ).toHaveBeenCalledTimes( 2 ); - await act( async () => jest.advanceTimersByTime( LIBRARY_POLL_INTERVAL_MS * 3 ) ); - expect( apiFetch ).toHaveBeenCalledTimes( 3 ); - } ); - - it( 'caps automatic processing polls while retaining manual status checks', async () => { - jest.useFakeTimers(); - jest.mocked( apiFetch ).mockResolvedValue( accepted ); - const { result } = renderHook( () => useVideoCopyStatus( request.guid, request.requestId ), { - wrapper: createTestWrapper(), - } ); - await waitFor( () => expect( result.current.data?.job.status ).toBe( 'processing' ) ); - jest.setSystemTime( Date.now() + PROCESSING_POLL_MAX_MS + 1 ); - await act( async () => jest.advanceTimersByTime( LIBRARY_POLL_INTERVAL_MS ) ); - const calls = jest.mocked( apiFetch ).mock.calls.length; - await act( async () => jest.advanceTimersByTime( LIBRARY_POLL_INTERVAL_MS * 3 ) ); - expect( apiFetch ).toHaveBeenCalledTimes( calls ); - await act( async () => { - await result.current.refetch(); - } ); - expect( apiFetch ).toHaveBeenCalledTimes( calls + 1 ); - } ); -} ); diff --git a/projects/packages/videopress/src/dashboard/hooks/use-save-video-copy.ts b/projects/packages/videopress/src/dashboard/hooks/use-save-video-copy.ts deleted file mode 100644 index beb1ae025038..000000000000 --- a/projects/packages/videopress/src/dashboard/hooks/use-save-video-copy.ts +++ /dev/null @@ -1,163 +0,0 @@ -import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query'; -import apiFetch from '@wordpress/api-fetch'; -import { __ } from '@wordpress/i18n'; -import { useEffect, useRef } from 'react'; -import { LIBRARY_QUERY_KEY, nextProcessingPoll } from './use-library'; -import { EditsConflictError } from './use-save-video-edits'; -import type { ProcessingPollAnchor } from './use-library'; -import type { SaveVideoEditsVars } from './use-save-video-edits'; -import type { EditsJob } from '../types/edits'; - -export const VIDEO_COPY_QUERY_KEY = 'videopress-video-copy'; - -/** A request rejected before the service could create a copy. */ -export class VideoCopyRejectedError extends Error { - constructor( - public code: string, - message?: string - ) { - super( message || __( 'The new video could not be created.', 'jetpack-videopress-pkg' ) ); - this.name = 'VideoCopyRejectedError'; - } -} - -export type SaveVideoCopyVars = SaveVideoEditsVars & { - /** Reuse this identifier when retrying an uncertain request. */ - requestId: string; - title?: string; -}; - -export type SaveVideoCopyResponse = { - source_guid: string; - request_id: string; - /** Destination identifiers appear once its attachment has been created. */ - guid: string | null; - attachment_id: number | null; - job: EditsJob; -}; - -/** - * Keep checking until the service can confirm a terminal result. - * - * @param response - The latest copy status, if available. - * @return Whether the copy still needs status updates. - */ -function isCopyPending( response?: SaveVideoCopyResponse ): boolean { - return ( - ! response || - response.job.status === 'processing' || - ( response.job.status === 'failed' && - [ 'copy_attachment_pending', 'copy_attachment_unconfirmed' ].includes( - response.job.error?.code ?? '' - ) ) - ); -} - -/** - * Keep incomplete service responses out of the copy status cache. - * - * @param response - The copy API response. - * @return The response with a confirmed job shape. - */ -function validateCopyResponse( response: SaveVideoCopyResponse ): SaveVideoCopyResponse { - if ( - ! response?.job || - ! [ 'processing', 'complete', 'failed' ].includes( response.job.status ) - ) { - throw new Error( - __( 'The video service did not return a valid copy status.', 'jetpack-videopress-pkg' ) - ); - } - return response; -} - -/** - * Create a separate edited video without changing the source's edit session. - * - * @return The copy mutation; callers retain the request ID for retries and polling. - */ -export function useSaveVideoCopy() { - const client = useQueryClient(); - return useMutation< SaveVideoCopyResponse, Error, SaveVideoCopyVars >( { - mutationFn: async ( { guid, baseRevision, operations, requestId, title } ) => { - try { - return validateCopyResponse( - await apiFetch< SaveVideoCopyResponse >( { - path: `/wpcom/v2/videopress/${ guid }/edits/copy`, - method: 'POST', - data: { - base_revision: baseRevision, - operations, - request_id: requestId, - ...( title === undefined ? {} : { title } ), - }, - } ) - ); - } catch ( error ) { - const restError = error as { - code?: string; - message?: string; - data?: { current_revision?: number; status?: number }; - }; - if ( restError?.code === 'edits_conflict' ) { - throw new EditsConflictError( - restError.message, - restError.data?.current_revision ?? null - ); - } - if ( - [ 400, 401, 403, 404, 405, 413, 422 ].includes( restError?.data?.status ?? 0 ) || - [ 'copy_source_unavailable', 'copy_authorization_unavailable' ].includes( - restError?.code ?? '' - ) - ) { - throw new VideoCopyRejectedError( restError.code ?? 'copy_rejected', restError.message ); - } - throw error; - } - }, - onSuccess: ( response, { guid, requestId } ) => { - client.setQueryData( [ VIDEO_COPY_QUERY_KEY, guid, requestId ], response ); - }, - } ); -} - -/** - * Follow a copy job until processing finishes, keeping the library current. - * - * @param guid - The source video GUID. - * @param requestId - The accepted or uncertain copy request, or null before saving. - * @return The copy job query, including retry and error state. - */ -export function useVideoCopyStatus( guid: string, requestId: string | null ) { - const client = useQueryClient(); - const processingStartRef = useRef< ProcessingPollAnchor | null >( null ); - const query = useQuery< SaveVideoCopyResponse >( { - queryKey: [ VIDEO_COPY_QUERY_KEY, guid, requestId ], - queryFn: async () => - validateCopyResponse( - await apiFetch< SaveVideoCopyResponse >( { - path: `/wpcom/v2/videopress/${ guid }/edits/copy/${ requestId }`, - } ) - ), - enabled: Boolean( guid && requestId ), - refetchInterval: state => { - const { anchor, interval } = nextProcessingPoll( - processingStartRef.current, - isCopyPending( state.state.data ) ? [ `${ guid }:${ requestId }` ] : [], - Date.now() - ); - processingStartRef.current = anchor; - return interval; - }, - refetchOnWindowFocus: state => ( isCopyPending( state.state.data ) ? 'always' : false ), - } ); - const attachmentId = query.data?.attachment_id; - const status = query.data?.job?.status; - useEffect( () => { - if ( attachmentId ) { - void client.invalidateQueries( { queryKey: [ LIBRARY_QUERY_KEY ] } ); - } - }, [ client, attachmentId, status ] ); - return query; -} diff --git a/projects/packages/videopress/tests/php/WPCOM_REST_API_V2_Endpoint_VideoPress_Edits_Test.php b/projects/packages/videopress/tests/php/WPCOM_REST_API_V2_Endpoint_VideoPress_Edits_Test.php index 13b88a8b6b44..1533e598cc02 100644 --- a/projects/packages/videopress/tests/php/WPCOM_REST_API_V2_Endpoint_VideoPress_Edits_Test.php +++ b/projects/packages/videopress/tests/php/WPCOM_REST_API_V2_Endpoint_VideoPress_Edits_Test.php @@ -29,22 +29,19 @@ class WPCOM_REST_API_V2_Endpoint_VideoPress_Edits_Test extends BaseTestCase { /** @var array Captured outbound requests. */ private $requests = array(); - /** @var int Attachment owner ID. */ - private $owner_id; - /** * Set up the connected owner, video, and REST server. */ public function setUp(): void { parent::setUp(); Constants::set_constant( 'JETPACK__WPCOM_JSON_API_BASE', 'https://public-api.wordpress.com' ); - $this->owner_id = $this->login_as( 'author' ); - $post_id = wp_insert_post( + $owner_id = $this->login_as( 'author' ); + $post_id = wp_insert_post( array( 'post_type' => 'attachment', 'post_status' => 'inherit', 'post_mime_type' => 'video/videopress', - 'post_author' => $this->owner_id, + 'post_author' => $owner_id, ) ); // WorDBless does not emulate the resolver's meta query. @@ -190,6 +187,16 @@ public static function denied_users() { return array( array( '', 401 ), array( 'subscriber', 403 ), array( 'contributor', 403 ), array( 'author', 403 ) ); } + /** + * Without a connected owner the proxy refuses before contacting WordPress.com. + */ + public function test_missing_owner_connection_is_rejected_before_proxying() { + \Jetpack_Options::delete_option( 'user_tokens' ); + ( new Connection_Manager() )->reset_connection_status(); + $this->assertSame( 403, $this->dispatch()->get_status() ); + $this->assertEmpty( $this->requests ); + } + /** * Test owners can read their video and obtain storyboard data. */ @@ -230,72 +237,6 @@ public function test_restore_uses_post_delete_route() { $this->assertStringContainsString( '/rest/v1.1/videos/AbCd1234/edits/delete', $this->requests[0]['url'] ); } - /** - * Copy requests forward the idempotency key without unrelated parameters. - */ - public function test_copy_forwards_request_and_status_routes() { - $request_id = 'a1b2c3d4-1234-4567-890a-b1c2d3e4f567'; - $body = array( - 'base_revision' => 2, - 'operations' => array(), - 'request_id' => $request_id, - 'title' => 'A new video', - ); - $this->upstream_response['response']['code'] = 202; - $this->assertSame( 202, $this->dispatch( 'POST', $body + array( 'unrelated' => 'ignored' ), 'edits/copy' )->get_status() ); - $this->assertSame( $body, json_decode( $this->requests[0]['args']['body'], true ) ); - $this->assertStringContainsString( '/rest/v1.1/videos/AbCd1234/edits/copy', $this->requests[0]['url'] ); - $this->assertStringContainsString( 'token="key:1:' . $this->owner_id . '"', $this->requests[0]['args']['headers']['Authorization'] ); - $this->dispatch( 'GET', null, 'edits/copy/' . $request_id ); - $this->assertStringContainsString( '/rest/v1.1/videos/AbCd1234/edits/copy/' . $request_id, $this->requests[1]['url'] ); - $this->assertStringContainsString( 'token="asdasd:1:0"', $this->requests[1]['args']['headers']['Authorization'] ); - } - - /** A missing creator connection cannot fall back to the blog token for a copy. */ - public function test_copy_requires_the_initiating_users_token() { - \Jetpack_Options::delete_option( 'user_tokens' ); - ( new Connection_Manager() )->reset_connection_status(); - $response = $this->dispatch( - 'POST', - array( - 'base_revision' => 0, - 'operations' => array(), - 'request_id' => 'a1b2c3d4-1234-4567-890a-b1c2d3e4f567', - ), - 'edits/copy' - ); - $this->assertSame( 403, $response->get_status() ); - $this->assertEmpty( $this->requests ); - } - - /** - * Invalid idempotency keys fail before contacting WordPress.com. - */ - public function test_copy_requires_a_valid_request_uuid() { - $body = array( - 'base_revision' => 0, - 'operations' => array(), - 'request_id' => 'invalid', - ); - $this->assertSame( 400, $this->dispatch( 'POST', $body, 'edits/copy' )->get_status() ); - $this->assertEmpty( $this->requests ); - } - - /** - * Copy endpoints use the same per-video authorization as updates. - */ - public function test_copy_rejects_other_authors() { - $this->login_as( 'author' ); - $body = array( - 'base_revision' => 0, - 'operations' => array(), - 'request_id' => 'a1b2c3d4-1234-4567-890a-b1c2d3e4f567', - ); - $this->assertSame( 403, $this->dispatch( 'POST', $body, 'edits/copy' )->get_status() ); - $this->assertSame( 403, $this->dispatch( 'GET', null, 'edits/copy/' . $body['request_id'] )->get_status() ); - $this->assertEmpty( $this->requests ); - } - /** * @dataProvider invalid_payloads * @param array $body Invalid JSON payload. @@ -425,8 +366,8 @@ public function test_wpcom_uses_authenticated_http_for_every_edit_route() { ) ); $token = \Mockery::mock( 'alias:' . VideoPressToken::class ); - $token->shouldReceive( 'videopress_onetime_upload_token' )->times( 6 )->andReturn( 'wpcom-test-token' ); - $token->shouldReceive( 'blog_id' )->times( 6 )->andReturn( get_current_blog_id() ); + $token->shouldReceive( 'videopress_onetime_upload_token' )->times( 4 )->andReturn( 'wpcom-test-token' ); + $token->shouldReceive( 'blog_id' )->times( 4 )->andReturn( get_current_blog_id() ); remove_filter( 'jetpack_videopress_trim_cut', '__return_true' ); $this->register_routes(); $body = array( @@ -438,15 +379,12 @@ public function test_wpcom_uses_authenticated_http_for_every_edit_route() { 'end_ms' => 5000, ), ), - 'request_id' => 'a1b2c3d4-1234-4567-890a-b1c2d3e4f567', ); $routes = array( array( 'GET', 'edits', 'edits' ), array( 'GET', 'storyboard', 'storyboard' ), array( 'POST', 'edits', 'edits' ), array( 'DELETE', 'edits', 'edits/delete' ), - array( 'POST', 'edits/copy', 'edits/copy' ), - array( 'GET', 'edits/copy/' . $body['request_id'], 'edits/copy/' . $body['request_id'] ), ); foreach ( $routes as $index => $route ) { $this->assertSame( 200, $this->dispatch( $route[0], 'POST' === $route[0] ? $body : null, $route[1] )->get_status() ); @@ -456,7 +394,7 @@ public function test_wpcom_uses_authenticated_http_for_every_edit_route() { $this->assertSame( 'X_UPLOAD_TOKEN token="wpcom-test-token" blog_id="' . get_current_blog_id() . '"', $request['args']['headers']['Authorization'] ); $this->assertSame( 0, $request['args']['redirection'] ); } - $this->assertSame( $body, json_decode( $this->requests[4]['args']['body'], true ) ); + $this->assertSame( $body, json_decode( $this->requests[2]['args']['body'], true ) ); } finally { \Brain\Monkey\tearDown(); } diff --git a/projects/packages/videopress/tests/php/XMLRPC_Test.php b/projects/packages/videopress/tests/php/XMLRPC_Test.php index 2d82e33e48e7..cb83086c6763 100644 --- a/projects/packages/videopress/tests/php/XMLRPC_Test.php +++ b/projects/packages/videopress/tests/php/XMLRPC_Test.php @@ -8,7 +8,6 @@ namespace Automattic\Jetpack\VideoPress; use PHPUnit\Framework\Attributes\BeforeClass; -use PHPUnit\Framework\Attributes\DataProvider; use WorDBless\BaseTestCase; use WorDBless\Posts; @@ -16,91 +15,6 @@ * Class to test the VideoPress XMLRPC class. */ class XMLRPC_Test extends BaseTestCase { - /** @var int The connected copy creator. */ - private $author_id; - - /** Authenticate callbacks as a connected author. */ - public function setUp(): void { - parent::setUp(); - $this->author_id = wp_insert_user( - array( - 'user_login' => 'video-copy-author', - 'user_pass' => 'password', - 'role' => 'author', - ) - ); - XMLRPC::init()->xmlrpc_methods( array(), array(), new \WP_User( $this->author_id ) ); - } - - /** Clear the callback signer between tests. */ - public function tearDown(): void { - XMLRPC::init()->xmlrpc_methods( array(), array(), new \WP_User( 0 ) ); - wp_set_current_user( 0 ); - delete_transient( 'videopress_get_post_id_by_guid_source12' ); - wp_cache_delete( 'get_post_by_guid_source12', 'videopress' ); - parent::tearDown(); - } - - /** - * @dataProvider copy_authorization_roles - * @param string $role The connected creator's role, or empty for no user. - * @param bool $owns_source Whether the creator owns the source attachment. - * @param bool $expected Whether the copy is permitted. - */ - #[DataProvider( 'copy_authorization_roles' )] - public function test_copy_preflight_requires_upload_and_source_edit_permissions( $role, $owns_source, $expected ) { - $actor = $role ? wp_insert_user( - array( - 'user_login' => 'copy-permission-actor', - 'user_pass' => 'password', - 'role' => $role, - ) - ) : 0; - $post_id = wp_insert_post( - array( - 'post_type' => 'attachment', - 'post_status' => 'inherit', - 'post_mime_type' => 'video/videopress', - 'post_author' => $owns_source ? $actor : $this->author_id, - ) - ); - // WorDBless does not emulate the resolver's meta query. - set_transient( 'videopress_get_post_id_by_guid_source12', $post_id, HOUR_IN_SECONDS ); - XMLRPC::init()->xmlrpc_methods( array(), array(), new \WP_User( $actor ) ); - wp_set_current_user( $this->author_id ); - $result = XMLRPC::init()->authorize_videopress_copy( 'source12' ); - if ( $expected ) { - $this->assertSame( - array( - 'authorized' => true, - 'guid' => 'source12', - ), - $result - ); - } else { - $this->assertArrayHasKey( 'videopress_copy_forbidden', $result['errors'] ); - } - $this->assertCount( 1, Posts::init()->posts ); - } - - /** @return array Creator permissions on the source attachment. */ - public static function copy_authorization_roles() { - return array( - 'Owner author' => array( 'author', true, true ), - 'Other author' => array( 'author', false, false ), - 'Editor' => array( 'editor', false, true ), - 'Subscriber owner' => array( 'subscriber', true, false ), - 'Contributor owner' => array( 'contributor', true, false ), - 'No signer' => array( '', false, false ), - ); - } - - /** Malformed or unmapped sources cannot be authorized. */ - public function test_copy_preflight_rejects_unknown_sources() { - $this->assertArrayHasKey( 'videopress_copy_forbidden', XMLRPC::init()->authorize_videopress_copy( 'invalid' )['errors'] ); - $this->assertArrayHasKey( 'videopress_copy_forbidden', XMLRPC::init()->authorize_videopress_copy( 'unknown1' )['errors'] ); - $this->assertEmpty( Posts::init()->posts ); - } /** * Sets up the test environment before the class tests begin. @@ -164,230 +78,38 @@ public function test_create_media_item_falls_back_to_file_name() { } /** - * Copy retries return the same attachment without resetting its completed metadata. + * The VideoPress callbacks are added without dropping the methods Jetpack already registered. */ - public function test_copy_request_reuses_the_attachment() { - $request_id = 'source12:32457391-3ebf-4c67-ac58-a34dd71399bf'; - $media = array( - array( - 'title' => 'New video', - 'videopress_copy_request_id' => $request_id, - ), - ); - $first = XMLRPC::init()->create_videopress_copy( $media ); - $post_id = $first['media'][0]['post']->ID; - $metadata = array( - 'videopress' => array( - 'guid' => 'newcopy1', - 'finished' => true, - ), - ); - wp_update_attachment_metadata( $post_id, $metadata ); - $second = XMLRPC::init()->create_videopress_copy( $media ); + public function test_xmlrpc_methods_registers_videopress_callbacks() { + $methods = XMLRPC::init()->xmlrpc_methods( array( 'existing.method' => '__return_true' ), array(), new \WP_User( 0 ) ); - $this->assertSame( $post_id, $second['media'][0]['post']->ID ); - $this->assertSame( $request_id, $second['media'][0]['videopress_copy_request_id_ack'] ); - $this->assertSame( $request_id, get_post_meta( $post_id, '_videopress_copy_request_id', true ) ); - $this->assertSame( $metadata, wp_get_attachment_metadata( $post_id ) ); - } - - /** New copies belong to the authenticated creator, independently of the ambient current user. */ - public function test_copy_attachment_belongs_to_its_connected_creator() { - $other_id = wp_insert_user( - array( - 'user_login' => 'other-copy-author', - 'user_pass' => 'password', - 'role' => 'author', - ) - ); - wp_set_current_user( $other_id ); - $result = XMLRPC::init()->create_videopress_copy( - array( - array( - 'title' => 'Copied by the connected author', - 'videopress_copy_request_id' => 'source12:32457391-3ebf-4c67-ac58-a34dd71399b4', - 'post_author' => $other_id, - ), - ) - ); - $post = $result['media'][0]['post']; - $this->assertSame( $this->author_id, (int) $post->post_author ); - $this->assertTrue( user_can( $this->author_id, 'edit_post', $post->ID ) ); - $this->assertFalse( user_can( $other_id, 'edit_post', $post->ID ) ); + $this->assertSame( '__return_true', $methods['existing.method'] ); + $this->assertIsCallable( $methods['jetpack.createMediaItem'] ); + $this->assertIsCallable( $methods['jetpack.updateVideoPressMediaItem'] ); + $this->assertIsCallable( $methods['jetpack.updateVideoPressPosterImage'] ); } /** - * @dataProvider unauthorized_copy_users - * @param string $role The callback signer's role, or empty for no user. + * A callback signed by a user creates the attachment as that user. */ - #[DataProvider( 'unauthorized_copy_users' )] - public function test_copy_requires_an_authenticated_uploader( $role ) { - $user_id = $role ? wp_insert_user( + public function test_create_media_item_runs_as_the_signing_user() { + $user_id = wp_insert_user( array( - 'user_login' => 'copy-subscriber', + 'user_login' => 'videopress-uploader', 'user_pass' => 'password', - 'role' => $role, + 'role' => 'author', ) - ) : 0; - XMLRPC::init()->xmlrpc_methods( array(), array(), new \WP_User( $user_id ) ); - $request_id = 'source12:32457391-3ebf-4c67-ac58-a34dd71399b5'; - $result = XMLRPC::init()->create_videopress_copy( array( array( 'videopress_copy_request_id' => $request_id ) ) ); - $this->assertArrayHasKey( 'videopress_copy_forbidden', $result['errors'] ); - $this->assertFalse( get_option( 'videopress_copy_attachment_' . hash( 'sha256', $request_id ) ) ); - $this->assertEmpty( Posts::init()->posts ); - } - - /** @return array Signers that cannot create attachments. */ - public static function unauthorized_copy_users() { - return array( - 'No connected user' => array( '' ), - 'Subscriber' => array( 'subscriber' ), ); - } - - /** - * An in-flight or uncertain creation is never replaced by a duplicate attachment. - */ - public function test_copy_request_keeps_an_uncertain_reservation() { - $request_id = 'source12:32457391-3ebf-4c67-ac58-a34dd71399b0'; - $option = 'videopress_copy_attachment_' . hash( 'sha256', $request_id ); - add_option( $option, 0, '', false ); - $result = XMLRPC::init()->create_videopress_copy( array( array( 'videopress_copy_request_id' => $request_id ) ) ); - - $this->assertArrayHasKey( 'videopress_copy_attachment_pending', $result['errors'] ); - $this->assertSame( 0, (int) get_option( $option ) ); - } - - /** - * A deleted copy is not recreated by replaying an old request. - */ - public function test_copy_request_does_not_recreate_a_deleted_attachment() { - $request_id = 'source12:32457391-3ebf-4c67-ac58-a34dd71399b1'; - $media = array( - array( - 'title' => 'New video', - 'videopress_copy_request_id' => $request_id, - ), - ); - $first = XMLRPC::init()->create_videopress_copy( $media ); - wp_delete_post( $first['media'][0]['post']->ID, true ); - $second = XMLRPC::init()->create_videopress_copy( $media ); - - $this->assertArrayHasKey( 'videopress_copy_attachment_unavailable', $second['errors'] ); - } - - /** - * Malformed copy identifiers are rejected before creating a media item. - */ - public function test_copy_request_rejects_an_invalid_identifier() { - $result = XMLRPC::init()->create_videopress_copy( array( array( 'videopress_copy_request_id' => '../../invalid' ) ) ); - - $this->assertArrayHasKey( 'videopress_copy_invalid_request', $result['errors'] ); - } - - /** - * The copy-only method rejects ordinary uploads before creating an attachment. - */ - public function test_copy_method_requires_its_idempotency_identifier() { - $result = XMLRPC::init()->create_videopress_copy( array( array( 'title' => 'Incomplete copy request' ) ) ); - - $this->assertArrayHasKey( 'videopress_copy_invalid_request', $result['errors'] ); - } - - /** - * The dedicated XML-RPC method preserves existing registrations and rejects incomplete copies. - */ - public function test_copy_method_is_registered() { - $existing = array( 'existing.method' => '__return_true' ); - $methods = XMLRPC::init()->xmlrpc_methods( $existing, array(), new \WP_User( 0 ) ); - - $this->assertSame( $existing['existing.method'], $methods['existing.method'] ); - $this->assertIsCallable( $methods['jetpack.createVideoPressCopy'] ); - $this->assertIsCallable( $methods['jetpack.authorizeVideoPressCopy'] ); - $result = call_user_func( $methods['jetpack.createVideoPressCopy'], array() ); - $this->assertArrayHasKey( 'videopress_copy_invalid_request', $result['errors'] ); - $this->assertEmpty( Posts::init()->posts ); - } - - /** - * A failed insert keeps its reservation so a retry cannot allocate another attachment. - */ - public function test_copy_request_keeps_reservation_when_insertion_fails() { - $request_id = 'source12:32457391-3ebf-4c67-ac58-a34dd71399b2'; - $option = 'videopress_copy_attachment_' . hash( 'sha256', $request_id ); - $media = array( - array( - 'title' => 'New video', - 'videopress_copy_request_id' => $request_id, - ), - ); - $result = array(); - add_filter( 'wp_insert_post_empty_content', '__return_true' ); + XMLRPC::init()->xmlrpc_methods( array(), array(), new \WP_User( $user_id ) ); try { - $result = XMLRPC::init()->create_videopress_copy( $media ); - } finally { - remove_filter( 'wp_insert_post_empty_content', '__return_true' ); - } - - $this->assertArrayHasKey( 'videopress_copy_attachment_failed', $result['errors'] ); - $this->assertArrayNotHasKey( 'media', $result ); - $this->assertSame( 0, (int) get_option( $option ) ); - $retry = XMLRPC::init()->create_videopress_copy( $media ); - $this->assertArrayHasKey( 'videopress_copy_attachment_pending', $retry['errors'] ); - $this->assertEmpty( Posts::init()->posts ); - } + $post = XMLRPC::init()->create_media_item( array( array( 'url' => 'https://videopress.com/v/file.mp4' ) ) )['media'][0]['post']; - /** - * A created attachment without a committed checkpoint is never acknowledged or duplicated. - * - * @dataProvider copy_checkpoint_failures - * @param string $failure The checkpoint write that fails. - */ - #[DataProvider( 'copy_checkpoint_failures' )] - public function test_copy_request_keeps_reservation_when_checkpoint_fails( $failure ) { - $request_id = 'source12:32457391-3ebf-4c67-ac58-a34dd71399b3'; - $option = 'videopress_copy_attachment_' . hash( 'sha256', $request_id ); - $media = array( - array( - 'title' => 'New video', - 'videopress_copy_request_id' => $request_id, - ), - ); - $attachment_id = 0; - $on_attachment = static function ( $post_id ) use ( &$attachment_id, $failure ) { - $attachment_id = $post_id; - if ( 'metadata' === $failure ) { - add_post_meta( $post_id, '_videopress_copy_request_id', 'another-request', true ); - } - }; - $on_option = static function ( $value, $old_value ) use ( $failure ) { - return 'option' === $failure ? $old_value : $value; - }; - add_action( 'add_attachment', $on_attachment ); - add_filter( 'pre_update_option_' . $option, $on_option, 10, 2 ); - $result = array(); - try { - $result = XMLRPC::init()->create_videopress_copy( $media ); + $this->assertSame( $user_id, get_current_user_id() ); + $this->assertSame( $user_id, (int) $post->post_author ); } finally { - remove_action( 'add_attachment', $on_attachment ); - remove_filter( 'pre_update_option_' . $option, $on_option ); + // The singleton keeps the signer, so reset it for later tests. + XMLRPC::init()->xmlrpc_methods( array(), array(), new \WP_User( 0 ) ); + wp_set_current_user( 0 ); } - - $this->assertArrayHasKey( 'videopress_copy_attachment_failed', $result['errors'] ); - $this->assertArrayNotHasKey( 'media', $result ); - $this->assertSame( 0, (int) get_option( $option ) ); - $this->assertSame( 'attachment', get_post_type( $attachment_id ) ); - $this->assertSame( 'metadata' === $failure ? 'another-request' : $request_id, get_post_meta( $attachment_id, '_videopress_copy_request_id', true ) ); - $retry = XMLRPC::init()->create_videopress_copy( $media ); - $this->assertArrayHasKey( 'videopress_copy_attachment_pending', $retry['errors'] ); - $this->assertCount( 1, Posts::init()->posts ); - } - - /** @return array Checkpoint failure cases. */ - public static function copy_checkpoint_failures() { - return array( - 'Conflicting attachment metadata' => array( 'metadata' ), - 'Failed reservation update' => array( 'option' ), - ); } } diff --git a/projects/plugins/jetpack/changelog/add-videopress-trim-cut b/projects/plugins/jetpack/changelog/add-videopress-trim-cut index 8d90ed180eb1..6d3147122351 100644 --- a/projects/plugins/jetpack/changelog/add-videopress-trim-cut +++ b/projects/plugins/jetpack/changelog/add-videopress-trim-cut @@ -1,4 +1,4 @@ Significance: minor Type: enhancement -VideoPress: Add an optional trim and cut editor with preview, undo, original video restoration, and a choice to update or save a new video. Keep the editor available during processing and resume pending copies when returning to the page. Reduce background status checks, refresh delayed timeline thumbnails, and allow retrying failed edits without creating another video. +VideoPress: Add an optional trim and cut editor with preview, undo, and original video restoration. Keep the editor available during processing, reduce background status checks, refresh delayed timeline thumbnails, and allow retrying failed edits. diff --git a/projects/plugins/videopress/changelog/add-videopress-trim-cut b/projects/plugins/videopress/changelog/add-videopress-trim-cut index 965fbeac9cce..1482c2006e66 100644 --- a/projects/plugins/videopress/changelog/add-videopress-trim-cut +++ b/projects/plugins/videopress/changelog/add-videopress-trim-cut @@ -1,4 +1,4 @@ Significance: minor Type: added -Add an optional trim and cut editor with preview, undo, original video restoration, and a choice to update or save a new video. Keep the editor available during processing and resume pending copies when returning to the page. Reduce background status checks, refresh delayed timeline thumbnails, and allow retrying failed edits without creating another video. +Add an optional trim and cut editor with preview, undo, and original video restoration. Keep the editor available during processing, reduce background status checks, refresh delayed timeline thumbnails, and allow retrying failed edits.