Protect: Update use fixers query error prop handling (#39498)

* Update FixerStatus type to account for potential error props * changelog * Handle error responses * Only handle top level errors in useFixersQuery, threat level errors are handled in the completion block * Add clarifying comment * Improve fixer types * Fix prop handling * Update useFixers hook for compatibility with type updates * Remove optional chaining where applicable, update logic to rely on TS types * Update types * Fix type error --------- Co-authored-by: Nate Weller <[email protected]>
Automattic · Oct 2, 2024 · fdcd55e · fdcd55e
1 parent 738e909
commit fdcd55e
Show file tree

Hide file tree

Showing 5 changed files with 102 additions and 38 deletions.
diff --git a/projects/plugins/protect/changelog/update-protect-use-fixers-response-error-prop-handling b/projects/plugins/protect/changelog/update-protect-use-fixers-response-error-prop-handling
@@ -0,0 +1,4 @@
+Significance: patch
+Type: changed
+
+Adds handling for FixerStatus error props
diff --git a/projects/plugins/protect/src/js/data/scan/use-fixers-mutation.ts b/projects/plugins/protect/src/js/data/scan/use-fixers-mutation.ts
@@ -16,6 +16,18 @@ export default function useFixersMutation(): UseMutationResult {
 	return useMutation( {
 		mutationFn: API.fixThreats,
 		onSuccess: data => {
+			// Handle a top level error
+			if ( data.ok === false ) {
+				throw new Error( data.error );
+			}
+
+			const isThreatLevelError = Object.values( data.threats ).every( threat => 'error' in threat );
+
+			// Handle a threat level error
+			if ( isThreatLevelError ) {
+				throw new Error();
+			}
+
 			// The data returned from the API is the same as the data we need to update the cache.
 			queryClient.setQueryData( [ QUERY_FIXERS_KEY ], data );
 

diff --git a/projects/plugins/protect/src/js/data/scan/use-fixers-query.ts b/projects/plugins/protect/src/js/data/scan/use-fixers-query.ts
@@ -6,7 +6,7 @@ import API from '../../api';
 import { QUERY_FIXERS_KEY, QUERY_HISTORY_KEY, QUERY_SCAN_STATUS_KEY } from '../../constants';
 import { fixerTimestampIsStale } from '../../hooks/use-fixers';
 import useNotices from '../../hooks/use-notices';
-import { FixersStatus } from '../../types/fixers';
+import { FixersStatus, ThreatFixStatus } from '../../types/fixers';
 
 const initialData: FixersStatus = window.jetpackProtectInitialState?.fixerStatus || {
 	ok: true,
@@ -74,33 +74,37 @@ export default function useFixersQuery( {
 				| FixersStatus
 				| undefined;
 
+			// Handle a top level error
+			if ( data.ok === false ) {
+				throw new Error( data.error );
+			}
+
 			const successes: string[] = [];
 			const failures: string[] = [];
 
-			Object.keys( data?.threats || {} ).forEach( threatId => {
+			Object.keys( data.threats || {} ).forEach( threatId => {
 				const threat = data.threats[ threatId ];
-				const cachedThreat = cachedData?.threats?.[ threatId ];
-
-				if ( cachedThreat?.status === 'in_progress' ) {
-					// If still in progress
-					if ( threat.status === 'in_progress' ) {
-						if (
-							! fixerTimestampIsStale( cachedThreat.last_updated ) &&
-							fixerTimestampIsStale( threat.last_updated )
-						) {
-							failures.push( threatId );
-						}
-					}
 
-					// Handle completion of fixers
-					if ( threat.status !== 'in_progress' ) {
-						queryClient.invalidateQueries( { queryKey: [ QUERY_SCAN_STATUS_KEY ] } );
-						queryClient.invalidateQueries( { queryKey: [ QUERY_HISTORY_KEY ] } );
-
-						if ( threat.status === 'fixed' ) {
-							successes.push( threatId );
+				if ( cachedData.ok === true ) {
+					const cachedThreat = cachedData.threats?.[ threatId ];
+
+					if ( cachedThreat && cachedThreat.status === 'in_progress' ) {
+						if ( threat.status === 'in_progress' ) {
+							if (
+								! fixerTimestampIsStale( cachedThreat.last_updated ) &&
+								fixerTimestampIsStale( threat.last_updated )
+							) {
+								failures.push( threatId );
+							}
 						} else {
-							failures.push( threatId );
+							queryClient.invalidateQueries( { queryKey: [ QUERY_SCAN_STATUS_KEY ] } );
+							queryClient.invalidateQueries( { queryKey: [ QUERY_HISTORY_KEY ] } );
+
+							if ( threat.status === 'fixed' ) {
+								successes.push( threatId );
+							} else {
+								failures.push( threatId );
+							}
 						}
 					}
 				}
@@ -117,15 +121,21 @@ export default function useFixersQuery( {
 				return false;
 			}
 
-			const inProgressNotStale = Object.values( query.state.data.threats ).some(
-				( threat: { status: string; last_updated: string } ) =>
-					threat.status === 'in_progress' && ! fixerTimestampIsStale( threat.last_updated )
-			);
+			const data = query.state.data;
+
+			if ( data.ok === true ) {
+				const inProgressNotStale = Object.values( data.threats ).some(
+					( threat: ThreatFixStatus ) =>
+						'status' in threat &&
+						threat.status === 'in_progress' &&
+						! fixerTimestampIsStale( threat.last_updated )
+				);
 
-			// Refetch while any threats are still in progress and not stale.
-			if ( inProgressNotStale ) {
-				// Refetch on a shorter interval first, then slow down if it is taking a while.
-				return query.state.dataUpdateCount < 5 ? 5000 : 15000;
+				// Refetch while any threats are still in progress and not stale.
+				if ( inProgressNotStale ) {
+					// Refetch on a shorter interval first, then slow down if it is taking a while.
+					return query.state.dataUpdateCount < 5 ? 5000 : 15000;
+				}
 			}
 
 			return false;

diff --git a/projects/plugins/protect/src/js/hooks/use-fixers.ts b/projects/plugins/protect/src/js/hooks/use-fixers.ts
@@ -13,7 +13,11 @@ export const fixerTimestampIsStale = ( lastUpdatedTimestamp: string ) => {
 };
 
 export const fixerStatusIsStale = ( fixerStatus: ThreatFixStatus ) => {
-	return fixerStatus.status === 'in_progress' && fixerTimestampIsStale( fixerStatus.last_updated );
+	return (
+		'status' in fixerStatus &&
+		fixerStatus.status === 'in_progress' &&
+		fixerTimestampIsStale( fixerStatus.last_updated )
+	);
 };
 
 type UseFixersResult = {
@@ -40,13 +44,20 @@ export default function useFixers(): UseFixersResult {
 
 	const isThreatFixInProgress = useCallback(
 		( threatId: number ) => {
-			return fixersStatus?.threats?.[ threatId ]?.status === 'in_progress';
+			if ( fixersStatus.ok === false ) {
+				return false;
+			}
+			const threatFix = fixersStatus.threats?.[ threatId ];
+			return threatFix && 'status' in threatFix && threatFix.status === 'in_progress';
 		},
 		[ fixersStatus ]
 	);
 
 	const isThreatFixStale = useCallback(
 		( threatId: number ) => {
+			if ( fixersStatus.ok === false ) {
+				return false;
+			}
 			const threatFixStatus = fixersStatus?.threats?.[ threatId ];
 			return threatFixStatus ? fixerStatusIsStale( threatFixStatus ) : false;
 		},

diff --git a/projects/plugins/protect/src/js/types/fixers.ts b/projects/plugins/protect/src/js/types/fixers.ts
@@ -1,13 +1,40 @@
 export type FixerStatus = 'not_started' | 'in_progress' | 'fixed' | 'not_fixed';
 
-export type FixersStatus = {
-	ok: boolean;
-	threats: {
-		[ key: number ]: ThreatFixStatus;
-	};
+/**
+ * Threat Fix Status
+ *
+ * Individual fixer status for a threat.
+ */
+export type ThreatFixStatusError = {
+	error: string;
 };
 
-export type ThreatFixStatus = {
+export type ThreatFixStatusSuccess = {
 	status: FixerStatus;
 	last_updated: string;
 };
+
+export type ThreatFixStatus = ThreatFixStatusError | ThreatFixStatusSuccess;
+
+/**
+ * Fixers Status
+ *
+ * Overall status of all fixers.
+ */
+type FixersStatusBase = {
+	ok: boolean; // Discriminator for overall success
+};
+
+export type FixersStatusError = FixersStatusBase & {
+	ok: false;
+	error: string;
+};
+
+export type FixersStatusSuccess = FixersStatusBase & {
+	ok: true;
+	threats: {
+		[ key: number ]: ThreatFixStatus;
+	};
+};
+
+export type FixersStatus = FixersStatusSuccess | FixersStatusError;