Merge pull request #3978 from google/bug/3752-failed-url-2

google · Sep 2, 2021 · 0c40e3a · 0c40e3a
2 parents 0ea4475 + d17face
commit 0c40e3a
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 15 deletions.
diff --git a/assets/js/modules/analytics/components/dashboard/DashboardAllTrafficWidget/index.js b/assets/js/modules/analytics/components/dashboard/DashboardAllTrafficWidget/index.js
@@ -21,6 +21,7 @@
  */
 import { _x } from '@wordpress/i18n';
 import { useState, useEffect } from '@wordpress/element';
+import { isURL } from '@wordpress/url';
 
 /**
  * Internal dependencies
@@ -183,7 +184,7 @@ function DashboardAllTrafficWidget( {
 		compareEndDate,
 	} );
 
-	if ( entityURL ) {
+	if ( isURL( entityURL ) ) {
 		reportArgs[ 'explorer-table.plotKeys' ] = '[]';
 		reportArgs[ '_r.drilldown' ] = `analytics.pagePath:${ getURLPath(
 			entityURL

diff --git a/assets/js/modules/analytics/components/dashboard/DashboardBounceRateWidget.js b/assets/js/modules/analytics/components/dashboard/DashboardBounceRateWidget.js
@@ -20,6 +20,7 @@
  * WordPress dependencies
  */
 import { __, _x } from '@wordpress/i18n';
+import { isURL } from '@wordpress/url';
 
 /**
  * Internal dependencies
@@ -67,18 +68,20 @@ function DashboardBounceRateWidget( { WidgetReportZero, WidgetReportError } ) {
 			],
 		};
 
+		let drilldown;
+
 		const url = select( CORE_SITE ).getCurrentEntityURL();
-		if ( url ) {
+		if ( isURL( url ) ) {
 			args.url = url;
+			drilldown = `analytics.pagePath:${ getURLPath( url ) }`;
 		}
+
 		return {
 			data: store.getReport( args ),
 			error: store.getErrorForSelector( 'getReport', [ args ] ),
 			loading: ! store.hasFinishedResolution( 'getReport', [ args ] ),
 			serviceURL: store.getServiceReportURL( 'visitors-overview', {
-				'_r.drilldown': url
-					? `analytics.pagePath:${ getURLPath( url ) }`
-					: undefined,
+				'_r.drilldown': drilldown,
 				...generateDateRangeArgs( {
 					startDate,
 					endDate,

diff --git a/assets/js/modules/analytics/components/dashboard/DashboardSearchVisitorsWidget.js b/assets/js/modules/analytics/components/dashboard/DashboardSearchVisitorsWidget.js
@@ -20,6 +20,7 @@
  * WordPress dependencies
  */
 import { __, _x } from '@wordpress/i18n';
+import { isURL } from '@wordpress/url';
 
 /**
  * Internal dependencies
@@ -114,7 +115,7 @@ function DashboardSearchVisitorsWidget( {
 		};
 
 		const drilldowns = [ 'analytics.trafficChannel:Organic Search' ];
-		if ( url ) {
+		if ( isURL( url ) ) {
 			drilldowns.push( `analytics.pagePath:${ getURLPath( url ) }` );
 		}
 

diff --git a/assets/js/util/urls.js b/assets/js/util/urls.js
@@ -25,7 +25,11 @@
  * @return {string} The URL path.
  */
 export function getURLPath( url ) {
-	return new URL( url ).pathname;
+	try {
+		return new URL( url ).pathname;
+	} catch {}
+
+	return null;
 }
 
 /**
@@ -38,7 +42,14 @@ export function getURLPath( url ) {
  * @return {string} The URL path.
  */
 export function getFullURL( siteURL, path ) {
-	return new URL( path, siteURL ).href;
+	try {
+		return new URL( path, siteURL ).href;
+	} catch {}
+
+	return (
+		( typeof siteURL === 'string' ? siteURL : '' ) +
+		( typeof path === 'string' ? path : '' )
+	);
 }
 
 /**

diff --git a/assets/js/util/urls.test.js b/assets/js/util/urls.test.js
@@ -40,9 +40,9 @@ describe( 'getURLPath', () => {
 		[ 'an empty string', '' ],
 		[ 'incomplete URL', 'foo.com/test' ],
 	] )(
-		'should throw an error if "%s" is passed instead of a valid URL',
+		'should return NULL if "%s" is passed instead of a valid URL',
 		( _, val ) => {
-			expect( () => getURLPath( val ) ).toThrow( 'Invalid URL' );
+			expect( getURLPath( val ) ).toBeNull();
 		}
 	);
 } );
@@ -79,16 +79,20 @@ describe( 'getFullURL', () => {
 	);
 
 	it.each( [
-		[ 'falsy site URL and falsy path are passed', false, false ],
-		[ 'incomplete URL is passed', '/slug', '/path' ],
+		[ 'falsy site URL and falsy path are passed', false, false, '' ],
+		[ 'incomplete URL is passed', '/slug', '/path', '/slug/path' ],
 		[
 			'site URL is passed as path parameter',
 			'',
 			'https://www.example.com',
+			'https://www.example.com',
 		],
-	] )( 'should throw an error if %s', ( _, siteURL, path ) => {
-		expect( () => getFullURL( siteURL, path ) ).toThrow();
-	} );
+	] )(
+		'should return the concatenated URL if %s',
+		( _, siteURL, path, expected ) => {
+			expect( getFullURL( siteURL, path ) ).toBe( expected );
+		}
+	);
 } );
 
 describe( 'normalizeURL', () => {