Cleanup/font library view error handling (#57926)

* Add batchInstallFontFaces function and related plumbing. * Fix resolver name. * Add embedding and rebuild theme.json settings for fontFamily. * Handle responses directly, add to collection before activating. Remove unused test. * Remove getIntersectingFontFaces. * Check for existing font family before installing. * Reference src, not uploadedFile key. Co-authored-by: Matias Benedetto <matias.benedetto@gmail.com> * Check for existing font family using GET /font-families?slug=. * Filter already installed font faces (determined by matching fontWeight AND fontStyle) * moved response processing into the resolver for fetchGetFontFamilyBySlug * Moved response processing for font family installation to the resolver * Refactored font face installation process to handle errors more cleanly * Cleanup error handling for font library view * Add i18n function to error messages * Add TODO comment for uninstall notice --------- Co-authored-by: Jeff Ong <jonger4@gmail.com> Co-authored-by: Matias Benedetto <matias.benedetto@gmail.com> Co-authored-by: Sarah Norris <sarah@sekai.co.uk>
WordPress · Jan 17, 2024 · 9ebc25f · 9ebc25f
1 parent 492a3ee
commit 9ebc25f
Show file tree

Hide file tree

Showing 9 changed files with 143 additions and 485 deletions.
diff --git a/packages/edit-site/src/components/global-styles/font-library-modal/context.js b/packages/edit-site/src/components/global-styles/font-library-modal/context.js
@@ -9,6 +9,7 @@ import {
 	useEntityRecords,
 	store as coreStore,
 } from '@wordpress/core-data';
+import { __ } from '@wordpress/i18n';
 
 /**
  * Internal dependencies
@@ -30,6 +31,7 @@ import {
 	makeFontFacesFormData,
 	makeFontFamilyFormData,
 	batchInstallFontFaces,
+	checkFontFaceInstalled,
 } from './utils';
 import { toggleFont } from './utils/toggleFont';
 
@@ -201,109 +203,94 @@ function FontLibraryProvider( { children } ) {
 		return getActivatedFontsOutline( source )[ slug ] || [];
 	};
 
-	async function installFont( font ) {
+	async function installFont( fontFamilyToInstall ) {
 		setIsInstalling( true );
 		try {
-			// Get the ID of the font family post, if it is already installed.
+			// Get the font family if it already exists.
 			let installedFontFamily = await fetchGetFontFamilyBySlug(
-				font.slug
-			)
-				.then( ( response ) => {
-					if ( ! response || response.length === 0 ) {
-						return null;
-					}
-					const fontFamilyPost = response[ 0 ];
-					return {
-						id: fontFamilyPost.id,
-						...fontFamilyPost.font_family_settings,
-						fontFace:
-							fontFamilyPost?._embedded?.font_faces.map(
-								( face ) => face.font_face_settings
-							) || [],
-					};
-				} )
-				.catch( ( e ) => {
-					// eslint-disable-next-line no-console
-					console.error( e );
-					return null;
-				} );
+				fontFamilyToInstall.slug
+			);
 
-			// Otherwise, install it.
+			// Otherwise create it.
 			if ( ! installedFontFamily ) {
-				const fontFamilyFormData = makeFontFamilyFormData( font );
 				// Prepare font family form data to install.
 				installedFontFamily = await fetchInstallFontFamily(
-					fontFamilyFormData
-				)
-					.then( ( response ) => {
-						return {
-							id: response.id,
-							...response.font_face_settings,
-							fontFace: [],
-						};
-					} )
-					.catch( ( e ) => {
-						throw Error( e.message );
-					} );
+					makeFontFamilyFormData( fontFamilyToInstall )
+				);
 			}
 
-			// Filter Font Faces that have already been installed
-			// We determine that by comparing the fontWeight and fontStyle
-			font.fontFace = font.fontFace.filter( ( fontFaceToInstall ) => {
-				return (
-					-1 ===
-					installedFontFamily.fontFace.findIndex(
-						( installedFontFace ) => {
-							return (
-								installedFontFace.fontWeight ===
-									fontFaceToInstall.fontWeight &&
-								installedFontFace.fontStyle ===
-									fontFaceToInstall.fontStyle
-							);
-						}
+			// Collect font faces that have already been installed (to be activated later)
+			const alreadyInstalledFontFaces =
+				installedFontFamily.fontFace.filter( ( fontFaceToInstall ) =>
+					checkFontFaceInstalled(
+						fontFaceToInstall,
+						fontFamilyToInstall.fontFace
 					)
 				);
-			} );
-
-			if ( font.fontFace.length === 0 ) {
-				// Looks like we're only trying to install fonts that are already installed.
-				// Let's not do that.
-				// TODO: Exit with an error message?
-				return {
-					errors: [ 'All font faces are already installed' ],
-				};
-			}
 
-			// Prepare font faces form data to install.
-			const fontFacesFormData = makeFontFacesFormData( font );
+			// Filter out Font Faces that have already been installed (so that they are not re-installed)
+			fontFamilyToInstall.fontFace = fontFamilyToInstall.fontFace.filter(
+				( fontFaceToInstall ) =>
+					! checkFontFaceInstalled(
+						fontFaceToInstall,
+						installedFontFamily.fontFace
+					)
+			);
 
 			// Install the fonts (upload the font files to the server and create the post in the database).
-			const response = await batchInstallFontFaces(
-				installedFontFamily.id,
-				fontFacesFormData
+			let sucessfullyInstalledFontFaces = [];
+			let unsucessfullyInstalledFontFaces = [];
+			if ( fontFamilyToInstall.fontFace.length > 0 ) {
+				const response = await batchInstallFontFaces(
+					installedFontFamily.id,
+					makeFontFacesFormData( fontFamilyToInstall )
+				);
+				sucessfullyInstalledFontFaces = response?.successes;
+				unsucessfullyInstalledFontFaces = response?.errors;
+			}
+
+			const detailedErrorMessage = unsucessfullyInstalledFontFaces.reduce(
+				( errorMessageCollection, error ) => {
+					return `${ errorMessageCollection } ${ error.message }`;
+				},
+				''
 			);
 
-			const fontFacesInstalled = response?.successes || [];
+			// If there were no successes and nothing already installed then we don't need to activate anything and can bounce now.
+			if (
+				sucessfullyInstalledFontFaces.length === 0 &&
+				alreadyInstalledFontFaces.length === 0
+			) {
+				throw new Error(
+					__( 'No font faces were installed. ' ) +
+						detailedErrorMessage
+				);
+			}
 
-			// Rebuild fontFace settings
-			font.fontFace =
-				fontFacesInstalled.map( ( face ) => {
-					return face.font_face_settings;
-				} ) || [];
+			// Use the sucessfully installed font faces
+			// As well as any font faces that were already installed (those will be activated)
+			fontFamilyToInstall.fontFace = [
+				...sucessfullyInstalledFontFaces,
+				...alreadyInstalledFontFaces,
+			];
 
 			// Activate the font family (add the font family to the global styles).
-			activateCustomFontFamilies( [ font ] );
+			activateCustomFontFamilies( [ fontFamilyToInstall ] );
+
 			// Save the global styles to the database.
 			saveSpecifiedEntityEdits( 'root', 'globalStyles', globalStylesId, [
 				'settings.typography.fontFamilies',
 			] );
+
 			refreshLibrary();
 
-			return response;
-		} catch ( error ) {
-			return {
-				errors: [ error ],
-			};
+			if ( unsucessfullyInstalledFontFaces.length > 0 ) {
+				throw new Error(
+					__(
+						'Some font faces were installed. There were some errors. '
+					) + detailedErrorMessage
+				);
+			}
 		} finally {
 			setIsInstalling( false );
 		}
@@ -324,7 +311,7 @@ function FontLibraryProvider( { children } ) {
 					[ 'settings.typography.fontFamilies' ]
 				);
 			}
-			// Refresh the library (the the library font families from database).
+			// Refresh the library (the library font families from database).
 			refreshLibrary();
 			return response;
 		} catch ( error ) {

diff --git a/packages/edit-site/src/components/global-styles/font-library-modal/font-collection.js b/packages/edit-site/src/components/global-styles/font-library-modal/font-collection.js
@@ -30,7 +30,6 @@ import CollectionFontDetails from './collection-font-details';
 import { toggleFont } from './utils/toggleFont';
 import { getFontsOutline } from './utils/fonts-outline';
 import GoogleFontsConfirmDialog from './google-fonts-confirm-dialog';
-import { getNoticeFromInstallResponse } from './utils/get-notice-from-response';
 import { downloadFontFaceAsset } from './utils';
 
 const DEFAULT_CATEGORY = {
@@ -182,9 +181,18 @@ function FontCollection( { id } ) {
 			return;
 		}
 
-		const response = await installFont( fontFamily );
-		const installNotice = getNoticeFromInstallResponse( response );
-		setNotice( installNotice );
+		try {
+			await installFont( fontFamily );
+			setNotice( {
+				type: 'success',
+				message: __( 'Fonts were installed successfully.' ),
+			} );
+		} catch ( error ) {
+			setNotice( {
+				type: 'error',
+				message: error.message,
+			} );
+		}
 		resetFontsToInstall();
 	};
 

diff --git a/packages/edit-site/src/components/global-styles/font-library-modal/installed-fonts.js b/packages/edit-site/src/components/global-styles/font-library-modal/installed-fonts.js
@@ -22,7 +22,6 @@ import FontsGrid from './fonts-grid';
 import LibraryFontDetails from './library-font-details';
 import LibraryFontCard from './library-font-card';
 import ConfirmDeleteDialog from './confirm-delete-dialog';
-import { getNoticeFromUninstallResponse } from './utils/get-notice-from-response';
 import { unlock } from '../../../lock-unlock';
 const { ProgressBar } = unlock( componentsPrivateApis );
 
@@ -50,8 +49,9 @@ function InstalledFonts() {
 
 	const handleConfirmUninstall = async () => {
 		const response = await uninstallFont( libraryFontSelected );
-		const uninstallNotice = getNoticeFromUninstallResponse( response );
-		setNotice( uninstallNotice );
+		// TODO: Refactor uninstall notices
+		// const uninstallNotice = getNoticeFromUninstallResponse( response );
+		// setNotice( uninstallNotice );
 		// If the font was succesfully uninstalled it is unselected
 		if ( ! response?.errors?.length ) {
 			handleUnselectFont();

diff --git a/packages/edit-site/src/components/global-styles/font-library-modal/local-fonts.js b/packages/edit-site/src/components/global-styles/font-library-modal/local-fonts.js
@@ -23,7 +23,6 @@ import { FontLibraryContext } from './context';
 import { Font } from '../../../../lib/lib-font.browser';
 import makeFamiliesFromFaces from './utils/make-families-from-faces';
 import { loadFontFaceInBrowser } from './utils';
-import { getNoticeFromInstallResponse } from './utils/get-notice-from-response';
 import { unlock } from '../../../lock-unlock';
 
 const { ProgressBar } = unlock( componentsPrivateApis );
@@ -161,12 +160,23 @@ function LocalFonts() {
 					'Variants from only one font family can be uploaded at a time.'
 				),
 			} );
+			setIsUploading( false );
 			return;
 		}
 
-		const response = await installFont( fontFamilies[ 0 ] );
-		const installNotice = getNoticeFromInstallResponse( response );
-		setNotice( installNotice );
+		try {
+			await installFont( fontFamilies[ 0 ] );
+			setNotice( {
+				type: 'success',
+				message: __( 'Fonts were installed successfully.' ),
+			} );
+		} catch ( error ) {
+			setNotice( {
+				type: 'error',
+				message: error,
+			} );
+		}
+
 		setIsUploading( false );
 	};
 

diff --git a/packages/edit-site/src/components/global-styles/font-library-modal/resolvers.js b/packages/edit-site/src/components/global-styles/font-library-modal/resolvers.js
@@ -13,7 +13,13 @@ export async function fetchInstallFontFamily( data ) {
 		method: 'POST',
 		body: data,
 	};
-	return apiFetch( config );
+	return apiFetch( config ).then( ( response ) => {
+		return {
+			id: response.id,
+			...response.font_face_settings,
+			fontFace: [],
+		};
+	} );
 }
 
 export async function fetchInstallFontFace( fontFamilyId, data ) {
@@ -22,15 +28,33 @@ export async function fetchInstallFontFace( fontFamilyId, data ) {
 		method: 'POST',
 		body: data,
 	};
-	return apiFetch( config );
+	return apiFetch( config ).then( ( response ) => {
+		return {
+			id: response.id,
+			...response.font_face_settings,
+		};
+	} );
 }
 
 export async function fetchGetFontFamilyBySlug( slug ) {
 	const config = {
 		path: `/wp/v2/font-families?slug=${ slug }&_embed=true`,
 		method: 'GET',
 	};
-	return apiFetch( config );
+	return apiFetch( config ).then( ( response ) => {
+		if ( ! response || response.length === 0 ) {
+			return null;
+		}
+		const fontFamilyPost = response[ 0 ];
+		return {
+			id: fontFamilyPost.id,
+			...fontFamilyPost.font_family_settings,
+			fontFace:
+				fontFamilyPost?._embedded?.font_faces.map(
+					( face ) => face.font_face_settings
+				) || [],
+		};
+	} );
 }
 
 export async function fetchUninstallFonts( fonts ) {

diff --git a/...site/src/components/global-styles/font-library-modal/utils/get-intersecting-font-faces.js b/...site/src/components/global-styles/font-library-modal/utils/get-intersecting-font-faces.js