From 456f28adeccc16f708a8004740c40f3a8bfcdb3d Mon Sep 17 00:00:00 2001
From: Andrew Duthie <aduthie7@gmail.com>
Date: Sat, 20 Jan 2018 17:13:37 -0500
Subject: [PATCH] Components: Ensure isLoading set to false after request error
 (#4525)

---
 .../higher-order/with-api-data/index.js       | 22 ++++++++-----
 .../higher-order/with-api-data/test/index.js  | 31 +++++++++++++++++--
 2 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/components/higher-order/with-api-data/index.js b/components/higher-order/with-api-data/index.js
index 36c95165c1ea85..19c4e4910c19f4 100644
--- a/components/higher-order/with-api-data/index.js
+++ b/components/higher-order/with-api-data/index.js
@@ -139,16 +139,24 @@ export default ( mapPropsToData ) => ( WrappedComponent ) => {
 				[ this.getPendingKey( method ) ]: true,
 			} );
 
-			request( { path, method } ).then( ( response ) => {
-				this.setIntoDataProp( propName, {
-					[ this.getPendingKey( method ) ]: false,
+			request( { path, method } )
+				// [Success] Set the data prop:
+				.then( ( response ) => ( {
 					[ this.getResponseDataKey( method ) ]: response.body,
-				} );
-			} ).catch( ( error ) => {
-				this.setIntoDataProp( propName, {
+				} ) )
+
+				// [Failure] Set the error prop:
+				.catch( ( error ) => ( {
 					[ this.getErrorResponseKey( method ) ]: error,
+				} ) )
+
+				// Always reset loading prop:
+				.then( ( nextDataProp ) => {
+					this.setIntoDataProp( propName, {
+						[ this.getPendingKey( method ) ]: false,
+						...nextDataProp,
+					} );
 				} );
-			} );
 		}
 
 		applyMapping( props ) {
diff --git a/components/higher-order/with-api-data/test/index.js b/components/higher-order/with-api-data/test/index.js
index cd42a1cd82fcf1..e6036ef99d81f4 100644
--- a/components/higher-order/with-api-data/test/index.js
+++ b/components/higher-order/with-api-data/test/index.js
@@ -10,9 +10,17 @@ import { identity, fromPairs } from 'lodash';
 import withAPIData from '../';
 
 jest.mock( '../request', () => {
-	const request = jest.fn( () => Promise.resolve( {
-		body: {},
-	} ) );
+	const request = jest.fn( ( { path } ) => {
+		if ( /\/users$/.test( path ) ) {
+			return Promise.reject( {
+				code: 'rest_forbidden_context',
+				message: 'Sorry, you are not allowed to list users.',
+				data: { status: 403 },
+			} );
+		}
+
+		return Promise.resolve( { body: {} } );
+	} );
 
 	request.getCachedResponse = ( { method, path } ) => {
 		return method === 'GET' && '/wp/v2/pages/10' === path ?
@@ -29,6 +37,9 @@ describe( 'withAPIData()', () => {
 			'/wp/v2/pages/(?P<parent>[\\d]+)/revisions': {
 				methods: [ 'GET' ],
 			},
+			'/wp/v2/users': {
+				methods: [ 'GET' ],
+			},
 			'/wp/v2/pages/(?P<id>[\\d]+)': {
 				methods: [
 					'GET',
@@ -80,6 +91,20 @@ describe( 'withAPIData()', () => {
 		} );
 	} );
 
+	it( 'should handle error response', ( done ) => {
+		const wrapper = getWrapper( () => ( {
+			users: '/wp/v2/users',
+		} ) );
+
+		process.nextTick( () => {
+			expect( wrapper.state( 'dataProps' ).users.isLoading ).toBe( false );
+			expect( wrapper.state( 'dataProps' ).users ).not.toHaveProperty( 'data' );
+			expect( wrapper.state( 'dataProps' ).users.error.code ).toBe( 'rest_forbidden_context' );
+
+			done();
+		} );
+	} );
+
 	it( 'should preassign cached data', ( done ) => {
 		const wrapper = getWrapper( () => ( {
 			page: '/wp/v2/pages/10',