Post publish upload media dialog: handle more block types (#65122)

* Post publish upload media dialog: handle more block types

* Deduplicate uploads and media filenames

* Correctly create File object for fetched media

* media-util.js: review suggestions

* Add more JSDoc

* Fix wrong types

* Allow all uploads to finish, even if some fail

Co-authored-by: sgomes <[email protected]>
Co-authored-by: swissspidy <[email protected]>
Co-authored-by: mtias <[email protected]>
Co-authored-by: youknowriad <[email protected]>
Co-authored-by: ntsekouras <[email protected]>

packages/editor/src/components/post-publish-panel/maybe-upload-media.js

@@ -14,6 +14,11 @@ import { store as blockEditorStore } from '@wordpress/block-editor';
 import { useState } from '@wordpress/element';
 import { isBlobURL } from '@wordpress/blob';
 
+/**
+ * Internal dependencies
+ */
+import { fetchMedia } from './media-util';
+
 function flattenBlocks( blocks ) {
 	const result = [];
 
@@ -25,25 +30,71 @@ function flattenBlocks( blocks ) {
 	return result;
 }
 
-function Image( block ) {
+/**
+ * Determine whether a block has external media.
+ *
+ * Different blocks use different attribute names (and potentially
+ * different logic as well) in determining whether the media is
+ * present, and whether it's external.
+ *
+ * @param {{name: string, attributes: Object}} block The block.
+ * @return {boolean?} Whether the block has external media
+ */
+function hasExternalMedia( block ) {
+	if ( block.name === 'core/image' || block.name === 'core/cover' ) {
+		return block.attributes.url && ! block.attributes.id;
+	}
+
+	if ( block.name === 'core/media-text' ) {
+		return block.attributes.mediaUrl && ! block.attributes.mediaId;
+	}
+
+	return undefined;
+}
+
+/**
+ * Retrieve media info from a block.
+ *
+ * Different blocks use different attribute names, so we need this
+ * function to normalize things into a consistent naming scheme.
+ *
+ * @param {{name: string, attributes: Object}} block The block.
+ * @return {{url: ?string, alt: ?string, id: ?number}} The media info for the block.
+ */
+function getMediaInfo( block ) {
+	if ( block.name === 'core/image' || block.name === 'core/cover' ) {
+		const { url, alt, id } = block.attributes;
+		return { url, alt, id };
+	}
+
+	if ( block.name === 'core/media-text' ) {
+		const { mediaUrl: url, mediaAlt: alt, mediaId: id } = block.attributes;
+		return { url, alt, id };
+	}
+
+	return {};
+}
+
+// Image component to represent a single image in the upload dialog.
+function Image( { clientId, alt, url } ) {
 	const { selectBlock } = useDispatch( blockEditorStore );
 	return (
 		<motion.img
 			tabIndex={ 0 }
 			role="button"
 			aria-label={ __( 'Select image block.' ) }
 			onClick={ () => {
-				selectBlock( block.clientId );
+				selectBlock( clientId );
 			} }
 			onKeyDown={ ( event ) => {
 				if ( event.key === 'Enter' || event.key === ' ' ) {
-					selectBlock( block.clientId );
+					selectBlock( clientId );
 					event.preventDefault();
 				}
 			} }
-			key={ block.clientId }
-			alt={ block.attributes.alt }
-			src={ block.attributes.url }
+			key={ clientId }
+			alt={ alt }
+			src={ url }
 			animate={ { opacity: 1 } }
 			exit={ { opacity: 0, scale: 0 } }
 			style={ {
@@ -58,7 +109,7 @@ function Image( block ) {
 	);
 }
 
-export default function PostFormatPanel() {
+export default function MaybeUploadMediaPanel() {
 	const [ isUploading, setIsUploading ] = useState( false );
 	const [ isAnimating, setIsAnimating ] = useState( false );
 	const [ hadUploadError, setHadUploadError ] = useState( false );
@@ -69,15 +120,14 @@ export default function PostFormatPanel() {
 		} ),
 		[]
 	);
-	const externalImages = flattenBlocks( editorBlocks ).filter(
-		( block ) =>
-			block.name === 'core/image' &&
-			block.attributes.url &&
-			! block.attributes.id
+
+	// Get a list of blocks with external media.
+	const blocksWithExternalMedia = flattenBlocks( editorBlocks ).filter(
+		( block ) => hasExternalMedia( block )
 	);
 	const { updateBlockAttributes } = useDispatch( blockEditorStore );
 
-	if ( ! mediaUpload || ! externalImages.length ) {
+	if ( ! mediaUpload || ! blocksWithExternalMedia.length ) {
 		return null;
 	}
 
@@ -88,43 +138,86 @@ export default function PostFormatPanel() {
 		</span>,
 	];
 
+	/**
+	 * Update an individual block to point to newly-added library media.
+	 *
+	 * Different blocks use different attribute names, so we need this
+	 * function to ensure we modify the correct attributes for each type.
+	 *
+	 * @param {{name: string, attributes: Object}} block The block.
+	 * @param {{id: number, url: string}}          media Media library file info.
+	 */
+	function updateBlockWithUploadedMedia( block, media ) {
+		if ( block.name === 'core/image' || block.name === 'core/cover' ) {
+			updateBlockAttributes( block.clientId, {
+				id: media.id,
+				url: media.url,
+			} );
+		}
+
+		if ( block.name === 'core/media-text' ) {
+			updateBlockAttributes( block.clientId, {
+				mediaId: media.id,
+				mediaUrl: media.url,
+			} );
+		}
+	}
+
+	// Handle fetching and uploading all external media in the post.
 	function uploadImages() {
 		setIsUploading( true );
 		setHadUploadError( false );
-		Promise.all(
-			externalImages.map( ( image ) =>
-				window
-					.fetch(
-						image.attributes.url.includes( '?' )
-							? image.attributes.url
-							: image.attributes.url + '?'
-					)
-					.then( ( response ) => response.blob() )
-					.then( ( blob ) =>
-						new Promise( ( resolve, reject ) => {
-							mediaUpload( {
-								filesList: [ blob ],
-								onFileChange: ( [ media ] ) => {
-									if ( isBlobURL( media.url ) ) {
-										return;
-									}
-
-									updateBlockAttributes( image.clientId, {
-										id: media.id,
-										url: media.url,
-									} );
-									resolve();
-								},
-								onError() {
-									reject();
-								},
-							} );
-						} ).then( () => setIsAnimating( true ) )
-					)
-					.catch( () => {
-						setHadUploadError( true );
-					} )
+
+		// Multiple blocks can be using the same URL, so we
+		// should ensure we only fetch and upload each of them once.
+		const mediaUrls = new Set(
+			blocksWithExternalMedia.map( ( block ) => {
+				const { url } = getMediaInfo( block );
+				return url;
+			} )
+		);
+
+		// Create an upload promise for each URL, that we can wait for in all
+		// blocks that make use of that media.
+		const uploadPromises = Object.fromEntries(
+			Object.entries( fetchMedia( [ ...mediaUrls ] ) ).map(
+				( [ url, filePromise ] ) => {
+					const uploadPromise = filePromise.then(
+						( blob ) =>
+							new Promise( ( resolve, reject ) => {
+								mediaUpload( {
+									filesList: [ blob ],
+									onFileChange: ( [ media ] ) => {
+										if ( isBlobURL( media.url ) ) {
+											return;
+										}
+
+										resolve( media );
+									},
+									onError() {
+										reject();
+									},
+								} );
+							} )
+					);
+
+					return [ url, uploadPromise ];
+				}
 			)
+		);
+
+		// Wait for all blocks to be updated with library media.
+		Promise.allSettled(
+			blocksWithExternalMedia.map( ( block ) => {
+				const { url } = getMediaInfo( block );
+
+				return uploadPromises[ url ]
+					.then( ( media ) =>
+						updateBlockWithUploadedMedia( block, media )
+					)
+					.then( () => setIsAnimating( true ) )
+					.catch( () => setHadUploadError( true ) );
+			} )
 		).finally( () => {
 			setIsUploading( false );
 		} );
@@ -147,8 +240,16 @@ export default function PostFormatPanel() {
 				<AnimatePresence
 					onExitComplete={ () => setIsAnimating( false ) }
 				>
-					{ externalImages.map( ( image ) => {
-						return <Image key={ image.clientId } { ...image } />;
+					{ blocksWithExternalMedia.map( ( block ) => {
+						const { url, alt } = getMediaInfo( block );
+						return (
+							<Image
+								key={ block.clientId }
+								clientId={ block.clientId }
+								url={ url }
+								alt={ alt }
+							/>
+						);
 					} ) }
 				</AnimatePresence>
 				{ isUploading || isAnimating ? (

packages/editor/src/components/post-publish-panel/media-util.js

@@ -0,0 +1,87 @@
+/**
+ * External dependencies
+ */
+import { v4 as uuid } from 'uuid';
+
+/**
+ * WordPress dependencies
+ */
+import { getFilename } from '@wordpress/url';
+
+/**
+ * Generate a list of unique basenames given a list of URLs.
+ *
+ * We want all basenames to be unique, since sometimes the extension
+ * doesn't reflect the mime type, and may end up getting changed by
+ * the server, on upload.
+ *
+ * @param {string[]} urls The list of URLs
+ * @return {Record< string, string >} A URL => basename record.
+ */
+export function generateUniqueBasenames( urls ) {
+	const basenames = new Set();
+
+	return Object.fromEntries(
+		urls.map( ( url ) => {
+			// We prefer to match the remote filename, if possible.
+			const filename = getFilename( url );
+			let basename = '';
+
+			if ( filename ) {
+				const parts = filename.split( '.' );
+				if ( parts.length > 1 ) {
+					// Assume the last part is the extension.
+					parts.pop();
+				}
+				basename = parts.join( '.' );
+			}
+
+			if ( ! basename ) {
+				// It looks like we don't have a basename, so let's use a UUID.
+				basename = uuid();
+			}
+
+			if ( basenames.has( basename ) ) {
+				// Append a UUID to deduplicate the basename.
+				// The server will try to deduplicate on its own if we don't do this,
+				// but it may run into a race condition
+				// (see https://github.com/WordPress/gutenberg/issues/64899).
+				// Deduplicating the filenames before uploading is safer.
+				basename = `${ basename }-${ uuid() }`;
+			}
+
+			basenames.add( basename );
+
+			return [ url, basename ];
+		} )
+	);
+}
+
+/**
+ * Fetch a list of URLs, turning those into promises for files with
+ * unique filenames.
+ *
+ * @param {string[]} urls The list of URLs
+ * @return {Record< string, Promise< File > >} A URL => File promise record.
+ */
+export function fetchMedia( urls ) {
+	return Object.fromEntries(
+		Object.entries( generateUniqueBasenames( urls ) ).map(
+			( [ url, basename ] ) => {
+				const filePromise = window
+					.fetch( url.includes( '?' ) ? url : url + '?' )
+					.then( ( response ) => response.blob() )
+					.then( ( blob ) => {
+						// The server will reject the upload if it doesn't have an extension,
+						// even though it'll rewrite the file name to match the mime type.
+						// Here we provide it with a safe extension to get it past that check.
+						return new File( [ blob ], `${ basename }.png`, {
+							type: blob.type,
+						} );
+					} );
+
+				return [ url, filePromise ];
+			}
+		)
+	);
+}