Fix loading states when report errors

.storybook/fetch-mocks.js

@@ -54,6 +54,20 @@ export function fetchMockCatchAll() {
 	fetchMock.catch( ( url, opts ) => {
 		global.console.warn( 'fetch', opts.method, url, opts );
 
+		if (
+			url.startsWith(
+				'/google-site-kit/v1/modules/search-console/data/searchanalytics'
+			) ||
+			url.startsWith(
+				'/google-site-kit/v1/modules/analytics/data/report'
+			)
+		) {
+			return {
+				status: 200,
+				body: '[]',
+			};
+		}
+
 		return {
 			status: 200,
 			body: '{}',

assets/js/modules/analytics/datastore/report.js

@@ -310,14 +310,24 @@ const baseSelectors = {
 			args.url = url;
 		}
 
+		// Disable reason: select needs to be called here or it will never run.
+		// eslint-disable-next-line @wordpress/no-unused-vars-before-return
 		const report = select( MODULES_ANALYTICS ).getReport( args );
-		if ( report === undefined ) {
+		const hasResolvedReport = select(
+			MODULES_ANALYTICS
+		).hasFinishedResolution( 'getReport', [ args ] );
+
+		if ( ! hasResolvedReport ) {
 			return undefined;
 		}
 
+		if ( ! Array.isArray( report ) ) {
+			return false;
+		}
+
 		if (
-			! Array.isArray( report?.[ 0 ]?.data?.rows ) ||
-			report?.[ 0 ]?.data?.rows?.length === 0
+			! Array.isArray( report[ 0 ]?.data?.rows ) ||
+			report[ 0 ]?.data?.rows?.length === 0
 		) {
 			return true;
 		}
@@ -349,13 +359,26 @@ const baseSelectors = {
 			args.url = url;
 		}
 
-		const report = select( MODULES_ANALYTICS ).getReport( args );
 		const isGatheringData = select( MODULES_ANALYTICS ).isGatheringData();
+		if ( isGatheringData === undefined ) {
+			return undefined;
+		}
+
+		// Disable reason: select needs to be called here or it will never run.
+		// eslint-disable-next-line @wordpress/no-unused-vars-before-return
+		const report = select( MODULES_ANALYTICS ).getReport( args );
+		const hasResolvedReport = select(
+			MODULES_ANALYTICS
+		).hasFinishedResolution( 'getReport', [ args ] );
 
-		if ( report === undefined || isGatheringData === undefined ) {
+		if ( ! hasResolvedReport ) {
 			return undefined;
 		}
 
+		if ( ! Array.isArray( report ) ) {
+			return false;
+		}
+
 		const hasZeroReport = isZeroReport( report );
 		if ( isGatheringData === false && hasZeroReport === false ) {
 			return false;

assets/js/modules/analytics/datastore/report.test.js

@@ -466,7 +466,7 @@ describe( 'modules/analytics report', () => {
 					/^\/google-site-kit\/v1\/modules\/analytics\/data\/report/,
 					// When `rows` is `null` it means we're still gathering data for
 					// this report.
-					{ body: { data: { rows: null } } }
+					{ body: [ { data: { rows: null } } ] }
 				);
 
 				const hasZeroData = registry
@@ -502,7 +502,7 @@ describe( 'modules/analytics report', () => {
 			it( 'should return TRUE if isZeroReport is true', async () => {
 				fetchMock.getOnce(
 					/^\/google-site-kit\/v1\/modules\/analytics\/data\/report/,
-					{ body: { data: { rows: [] } } }
+					{ body: [ { data: { rows: [] } } ] }
 				);
 
 				const hasZeroData = registry

assets/js/modules/search-console/components/dashboard/SearchFunnelWidget/index.js

@@ -293,11 +293,6 @@ const SearchFunnelWidget = ( {
 		analyticsStatsLoading ||
 		analyticsVisitorsOverviewAndStatsLoading ||
 		analyticsGoalsLoading ||
-		searchConsoleData === undefined ||
-		analyticsOverviewData === undefined ||
-		analyticsStatsData === undefined ||
-		analyticsVisitorsOverviewAndStatsData === undefined ||
-		analyticsGoalsData === undefined ||
 		isAnalyticsGatheringData === undefined ||
 		isSearchConsoleGatheringData === undefined
 	) {

assets/js/modules/search-console/datastore/report.js

@@ -155,12 +155,22 @@ const baseSelectors = {
 			args.url = url;
 		}
 
+		// Disable reason: select needs to be called here or it will never run.
+		// eslint-disable-next-line @wordpress/no-unused-vars-before-return
 		const report = select( MODULES_SEARCH_CONSOLE ).getReport( args );
-		if ( report === undefined ) {
+		const hasResolvedReport = select(
+			MODULES_SEARCH_CONSOLE
+		).hasFinishedResolution( 'getReport', [ args ] );
+
+		if ( ! hasResolvedReport ) {
 			return undefined;
 		}
 
-		if ( ! Array.isArray( report ) || ! report.length ) {
+		if ( ! Array.isArray( report ) ) {
+			return false;
+		}
+
+		if ( ! report.length ) {
 			return true;
 		}
 
@@ -195,15 +205,29 @@ const baseSelectors = {
 			args.url = url;
 		}
 
-		const report = select( MODULES_SEARCH_CONSOLE ).getReport( args );
 		const isGatheringData = select(
 			MODULES_SEARCH_CONSOLE
 		).isGatheringData();
+		if ( isGatheringData === undefined ) {
+			return undefined;
+		}
 
-		if ( report === undefined || isGatheringData === undefined ) {
+		// Disable reason: select needs to be called here or it will never run.
+		// eslint-disable-next-line @wordpress/no-unused-vars-before-return
+		const report = select( MODULES_SEARCH_CONSOLE ).getReport( args );
+
+		const hasResolvedReport = select(
+			MODULES_SEARCH_CONSOLE
+		).hasFinishedResolution( 'getReport', [ args ] );
+
+		if ( ! hasResolvedReport ) {
 			return undefined;
 		}
 
+		if ( ! Array.isArray( report ) ) {
+			return false;
+		}
+
 		const hasZeroReport = isZeroReport( report );
 		if ( isGatheringData === false && hasZeroReport === false ) {
 			return false;

assets/js/modules/search-console/datastore/report.test.js

@@ -31,6 +31,23 @@ import * as fixtures from './__fixtures__';
 
 describe( 'modules/search-console report', () => {
 	const searchAnalyticsRegexp = /^\/google-site-kit\/v1\/modules\/search-console\/data\/searchanalytics/;
+	const errorResponse = {
+		status: 403,
+		body: {
+			code: 403,
+			message:
+				'User does not have sufficient permissions for this profile.',
+			data: { status: 403, reason: 'forbidden' },
+		},
+	};
+	const consoleError = [
+		'Google Site Kit API Error',
+		'method:GET',
+		'datapoint:searchanalytics',
+		'type:modules',
+		'identifier:search-console',
+		'error:"User does not have sufficient permissions for this profile."',
+	];
 
 	let registry;
 
@@ -152,46 +169,58 @@ describe( 'modules/search-console report', () => {
 				expect( isGatheringData ).toBeUndefined();
 			} );
 
-			it.each( [
-				[ 'an empty array', [] ],
-				[ 'not an array', null ],
-			] )(
-				'should return TRUE if the returned report is %s',
-				async ( _, body ) => {
-					fetchMock.getOnce( searchAnalyticsRegexp, { body } );
+			it( 'should return TRUE if the returned report is an empty array', async () => {
+				fetchMock.getOnce( searchAnalyticsRegexp, { body: [] } );
 
-					const isGatheringData = registry
-						.select( MODULES_SEARCH_CONSOLE )
-						.isGatheringData();
+				const { isGatheringData } = registry.select(
+					MODULES_SEARCH_CONSOLE
+				);
+
+				expect( isGatheringData() ).toBeUndefined();
+
+				await subscribeUntil(
+					registry,
+					() =>
+						registry
+							.select( MODULES_SEARCH_CONSOLE )
+							.isGatheringData() !== undefined
+				);
 
-					expect( isGatheringData ).toBeUndefined();
+				expect( isGatheringData() ).toBe( true );
+			} );
 
-					await subscribeUntil(
-						registry,
-						() =>
-							registry
-								.select( MODULES_SEARCH_CONSOLE )
-								.isGatheringData() !== undefined
-					);
+			it( 'should return FALSE if the report API returns error', async () => {
+				fetchMock.getOnce( searchAnalyticsRegexp, errorResponse );
 
-					const isNotGathered = registry
-						.select( MODULES_SEARCH_CONSOLE )
-						.isGatheringData();
+				const { isGatheringData } = registry.select(
+					MODULES_SEARCH_CONSOLE
+				);
 
-					expect( isNotGathered ).toBe( true );
-				}
-			);
+				expect( isGatheringData() ).toBeUndefined();
+
+				await subscribeUntil(
+					registry,
+					() =>
+						registry
+							.select( MODULES_SEARCH_CONSOLE )
+							.isGatheringData() !== undefined
+				);
+
+				expect( console ).toHaveErroredWith( ...consoleError );
+
+				expect( isGatheringData() ).toBe( false );
+			} );
 
 			it( 'should return FALSE if the returned report has rows', async () => {
 				fetchMock.getOnce( searchAnalyticsRegexp, {
 					body: fixtures.report,
 				} );
 
-				const isGatheringData = registry
-					.select( MODULES_SEARCH_CONSOLE )
-					.isGatheringData();
+				const { isGatheringData } = registry.select(
+					MODULES_SEARCH_CONSOLE
+				);
 
-				expect( isGatheringData ).toBeUndefined();
+				expect( isGatheringData() ).toBeUndefined();
 
 				await subscribeUntil(
 					registry,
@@ -201,11 +230,7 @@ describe( 'modules/search-console report', () => {
 							.isGatheringData() !== undefined
 				);
 
-				const isNotGathered = registry
-					.select( MODULES_SEARCH_CONSOLE )
-					.isGatheringData();
-
-				expect( isNotGathered ).toBe( false );
+				expect( isGatheringData() ).toBe( false );
 			} );
 		} );
 
@@ -220,35 +245,55 @@ describe( 'modules/search-console report', () => {
 				expect( hasZeroData ).toBeUndefined();
 			} );
 
-			it.each( [
-				[ 'an empty array', [] ],
-				[ 'not an array', null ],
-			] )(
-				'should return TRUE if report data in isGatheringData OR isZeroReport is %s',
-				async ( _, body ) => {
-					fetchMock.getOnce( searchAnalyticsRegexp, { body } );
+			it( 'should return TRUE if report data in isGatheringData OR isZeroReport is an empty array', async () => {
+				fetchMock.getOnce( searchAnalyticsRegexp, { body: [] } );
 
-					const hasZeroData = registry
-						.select( MODULES_SEARCH_CONSOLE )
-						.hasZeroData();
+				const hasZeroData = registry
+					.select( MODULES_SEARCH_CONSOLE )
+					.hasZeroData();
+
+				expect( hasZeroData ).toBeUndefined();
+
+				await subscribeUntil(
+					registry,
+					() =>
+						registry
+							.select( MODULES_SEARCH_CONSOLE )
+							.hasZeroData() !== undefined
+				);
 
-					expect( hasZeroData ).toBeUndefined();
+				const zeroData = registry
+					.select( MODULES_SEARCH_CONSOLE )
+					.hasZeroData();
 
-					await subscribeUntil(
-						registry,
-						() =>
-							registry
-								.select( MODULES_SEARCH_CONSOLE )
-								.hasZeroData() !== undefined
-					);
+				expect( zeroData ).toBe( true );
+			} );
 
-					const zeroData = registry
-						.select( MODULES_SEARCH_CONSOLE )
-						.hasZeroData();
+			it( 'should return FALSE if report API returns error', async () => {
+				fetchMock.getOnce( searchAnalyticsRegexp, errorResponse );
 
-					expect( zeroData ).toBe( true );
-				}
-			);
+				const hasZeroData = registry
+					.select( MODULES_SEARCH_CONSOLE )
+					.hasZeroData();
+
+				expect( hasZeroData ).toBeUndefined();
+
+				await subscribeUntil(
+					registry,
+					() =>
+						registry
+							.select( MODULES_SEARCH_CONSOLE )
+							.hasZeroData() !== undefined
+				);
+
+				const zeroData = registry
+					.select( MODULES_SEARCH_CONSOLE )
+					.hasZeroData();
+
+				expect( console ).toHaveErroredWith( ...consoleError );
+
+				expect( zeroData ).toBe( false );
+			} );
 
 			it( 'should return false if isGatheringData and isZeroReport return false', async () => {
 				fetchMock.getOnce( searchAnalyticsRegexp, {

stories/wp-dashboard.stories.js

@@ -279,12 +279,12 @@ storiesOf( 'WordPress', module )
 			// For <WPDashboardImpressions />
 			registry
 				.dispatch( MODULES_SEARCH_CONSOLE )
-				.receiveGetReport( {}, { options: impressionsArgs } );
+				.receiveGetReport( [], { options: impressionsArgs } );
 
 			// For <WPDashboardClicks />
 			registry
 				.dispatch( MODULES_SEARCH_CONSOLE )
-				.receiveGetReport( {}, { options: clicksOptions } );
+				.receiveGetReport( [], { options: clicksOptions } );
 
 			return (
 				<div id="dashboard-widgets">