Media: Reduxify addExternal into addExternalMedia thunk (#43703)

* Media: Split uploading files out from adding new media

* Media: Add `addExternalMedia` thunk

* Media: Add `serially` wrapper to upload lists of media

* Media: Wrap `uploadMedia` with `serially` and update consumers

* Media: Simplify API and allow for single files to still throw errors

client/state/media/thunks/add-external-media.js

@@ -0,0 +1,24 @@
+/**
+ * Internal dependencies
+ */
+import { uploadMedia, uploadSingleMedia } from './upload-media';
+import wpcom from 'lib/wp';
+
+const getExternalUploader = ( service ) => ( file, siteId ) => {
+	return wpcom.undocumented().site( siteId ).uploadExternalMedia( service, [ file.guid ] );
+};
+
+/**
+ * Add a single external media file.
+ *
+ * @param {object} site The site for which to upload the file(s)
+ * @param {object|object[]} file The media file or files to upload
+ * @param {object} service The external media service used
+ */
+export const addExternalMedia = ( site, file, service ) => ( dispatch ) => {
+	const uploader = getExternalUploader( service );
+
+	const action = Array.isArray( file ) ? uploadMedia : uploadSingleMedia;
+
+	return dispatch( action( file, site, uploader ) );
+};

client/state/media/thunks/add-media.js

@@ -1,94 +1,19 @@
 /**
  * Internal dependencies
  */
-import { createTransientMedia, getFileUploader, validateMediaItem } from 'lib/media/utils';
-import { getTransientDate } from 'state/media/utils/transient-date';
-import {
-	dispatchFluxCreateMediaItem,
-	dispatchFluxFetchMediaLimits,
-	dispatchFluxReceiveMediaItemError,
-	dispatchFluxReceiveMediaItemSuccess,
-} from 'state/media/utils/flux-adapter';
-import {
-	createMediaItem,
-	receiveMedia,
-	successMediaItemRequest,
-	failMediaItemRequest,
-	setMediaItemErrors,
-} from 'state/media/actions';
+import { uploadMedia, uploadSingleMedia } from './upload-media';
+import { getFileUploader } from 'lib/media/utils';
 
 /**
- * Add a single media item. Allow passing in the transient date so
- * that consumers can upload in series. Use a safe default for when
- * only a single item is being uploaded.
+ * Upload a single media item
  *
- * Restrict this function to purely a single media item.
- *
- * Note: Temporarily this action will dispatch to the flux store, until
- * the flux store is removed.
- *
- * @param {object} site The site to add the media to
- * @param {object} file The file to upload
- * @param {string?} transientDate Date for the transient item
- * @returns {import('redux-thunk').ThunkAction<Promise<object>, any, any, any>} A thunk resolving with the uploaded media item
+ * @param {object} site The site for which to upload the file(s)
+ * @param {object|object[]} file The file or files to upload
  */
-export const addMedia = ( site, file, transientDate = getTransientDate() ) => async (
-	dispatch,
-	getState
-) => {
-	const uploader = getFileUploader( getState(), site, file );
-
-	const transientMedia = {
-		date: transientDate,
-		...createTransientMedia( file ),
-	};
-
-	if ( file.ID ) {
-		transientMedia.ID = file.ID;
-	}
-
-	const { ID: siteId } = site;
-
-	dispatchFluxCreateMediaItem( transientMedia, site );
-
-	const errors = validateMediaItem( site, transientMedia );
-	if ( errors?.length ) {
-		dispatch( setMediaItemErrors( siteId, transientMedia.ID, errors ) );
-		// throw rather than silently escape so consumers know the upload failed based on Promise resolution rather than state having to re-derive the failure themselves from state
-		throw errors;
-	}
-
-	dispatch( createMediaItem( site, transientMedia ) );
-
-	try {
-		const {
-			media: [ uploadedMedia ],
-			found,
-		} = await uploader( file, siteId );
-
-		dispatchFluxReceiveMediaItemSuccess( transientMedia.ID, siteId, uploadedMedia );
-
-		dispatch( successMediaItemRequest( siteId, transientMedia.ID ) );
-		dispatch(
-			receiveMedia(
-				siteId,
-				{
-					...uploadedMedia,
-					transientId: transientMedia.ID,
-				},
-				found
-			)
-		);
-
-		dispatchFluxFetchMediaLimits( siteId );
+export const addMedia = ( site, file ) => ( dispatch ) => {
+	const uploader = getFileUploader();
 
-		return uploadedMedia;
-	} catch ( error ) {
-		dispatchFluxReceiveMediaItemError( transientMedia.ID, siteId, error );
+	const action = Array.isArray( file ) ? uploadMedia : uploadSingleMedia;
 
-		dispatch( failMediaItemRequest( siteId, transientMedia.ID, error ) );
-		// no need to dispatch `deleteMedia` as `createMediaItem` won't have added it to the MediaQueryManager which tracks instances.
-		// rethrow so consumers know the upload failed
-		throw error;
-	}
+	return dispatch( action( file, site, uploader ) );
 };

client/state/media/thunks/index.js

@@ -1,2 +1,3 @@
 export { addMedia } from './add-media';
 export { uploadSiteIcon } from './upload-site-icon';
+export { addExternalMedia } from './add-external-media';

client/state/media/thunks/serially.js

@@ -0,0 +1,26 @@
+/**
+ * Internal dependencies
+ */
+import { getBaseTime, getTransientDate } from 'state/media/utils/transient-date';
+
+/**
+ * Creates a function that serially uploads a list of media files using
+ * the passed in thunk.
+ *
+ * @param {Function} mediaAddingAction Dispatchable action accepting a file as a first argument and date as the last argument
+ */
+export const serially = ( mediaAddingAction ) => ( files, ...extraArgs ) => ( dispatch ) => {
+	const baseTime = getBaseTime();
+	const fileCount = files.length;
+
+	return files.reduce( async ( previousUpload, file, index ) => {
+		await previousUpload;
+		const transientDate = getTransientDate( baseTime, index, fileCount );
+		try {
+			return await dispatch( mediaAddingAction( file, ...extraArgs, transientDate ) );
+		} catch {
+			// Swallow the error because inner `mediaAddingAction` will have already handled it
+			return Promise.resolve();
+		}
+	}, Promise.resolve() );
+};

client/state/media/thunks/test/add-external-media.js

@@ -0,0 +1,32 @@
+/**
+ * Internal dependencies
+ */
+import { addExternalMedia as addExternalMediaThunk } from 'state/media/thunks/add-external-media';
+import { uploadMedia, uploadSingleMedia } from 'state/media/thunks/upload-media';
+
+jest.mock( 'state/media/thunks/upload-media', () => ( {
+	uploadMedia: jest.fn(),
+	uploadSingleMedia: jest.fn(),
+} ) );
+
+describe( 'media - thunks - addExternalMedia', () => {
+	const site = Symbol( 'site' );
+	const file = Symbol( 'file' );
+	const service = Symbol( 'service' );
+	const dispatch = jest.fn();
+	const getState = jest.fn();
+
+	const addExternalMedia = ( ...args ) => addExternalMediaThunk( ...args )( dispatch, getState );
+
+	it( 'should dispatch to uploadSingleMedia with the file uploader', async () => {
+		await addExternalMedia( site, file, service );
+
+		expect( uploadSingleMedia ).toHaveBeenCalledWith( file, site, expect.any( Function ) );
+	} );
+
+	it( 'should dispatch to uploadMedia with the file uploader', async () => {
+		await addExternalMedia( site, [ file, file ], service );
+
+		expect( uploadMedia ).toHaveBeenCalledWith( [ file, file ], site, expect.any( Function ) );
+	} );
+} );