From 24faa5e143120c6f11a1329eab2d2522a2661b26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krist=C3=B3fer=20R?= Date: Fri, 19 Jul 2019 00:49:09 +0000 Subject: [PATCH] Add `showTransactionsPlaceholder` selector Used to indicate whether the `TableCard` in the transactions list should show the loading placeholder view or not. --- .../api-spec/transactions/selectors.js | 27 +++++++--- .../test/api-spec/transactions/selectors.js | 51 +++++++++++++++---- client/transactions/index.js | 7 ++- 3 files changed, 65 insertions(+), 20 deletions(-) diff --git a/client/payments-api/api-spec/transactions/selectors.js b/client/payments-api/api-spec/transactions/selectors.js index ba08de4dcbc..f3a701509cc 100644 --- a/client/payments-api/api-spec/transactions/selectors.js +++ b/client/payments-api/api-spec/transactions/selectors.js @@ -1,3 +1,9 @@ +/** @format */ + +/** + * External dependencies. + */ +import { isNil } from 'lodash'; /** * Internal dependencies. @@ -11,20 +17,29 @@ const getTransactions = ( getResource, requireResource ) => ( return requireResource( requirement, resourceName ).data || {}; } -const getTransactionsIsLoading = ( getResource ) => { +const transactionsInitStatus = ( getResource ) => () => { const resourceName = 'transactions-list'; const transactionsResource = getResource( resourceName ); - const transactions = transactionsResource.data || {}; - // If no transactions are available, assume the request is loading. - if ( ! transactions.data ) { - return true; - } + return ! ( isNil( transactionsResource.lastRequested ) || isNil( transactionsResource.lastReceived ) ); +} + +const getTransactionsIsLoading = ( getResource ) => () => { + const resourceName = 'transactions-list'; + const transactionsResource = getResource( resourceName ); return transactionsResource.lastRequested > transactionsResource.lastReceived; } +const showTransactionsPlaceholder = ( getResource ) => () => { + const isInitialized = transactionsInitStatus( getResource )(); + + return ! isInitialized; +} + export default { getTransactions, getTransactionsIsLoading, + transactionsInitStatus, + showTransactionsPlaceholder, }; diff --git a/client/payments-api/test/api-spec/transactions/selectors.js b/client/payments-api/test/api-spec/transactions/selectors.js index 77f1b809aa6..08a288f1798 100644 --- a/client/payments-api/test/api-spec/transactions/selectors.js +++ b/client/payments-api/test/api-spec/transactions/selectors.js @@ -42,18 +42,17 @@ describe( 'Transactions selectors', () => { } ); } ); - describe( 'getTransactionsIsLoading', () => { + describe( 'getTransactionsIsLoading()', () => { it( "getTransactionsIsLoading returns false when a read operation isn't in flight", () => { const expected = false; const mockGetResource = jest.fn(); mockGetResource.mockReturnValue( { - data: { data: {} }, lastRequested: 0, lastReceived: 1, } ); - const isLoading = transactionsSelectors.getTransactionsIsLoading( mockGetResource ); + const isLoading = transactionsSelectors.getTransactionsIsLoading( mockGetResource )(); expect( mockGetResource ).toHaveBeenCalledTimes( 1 ); expect( mockGetResource ).toHaveBeenCalledWith( expectedResourceName ); @@ -66,33 +65,65 @@ describe( 'Transactions selectors', () => { const mockGetResource = jest.fn(); mockGetResource.mockReturnValue( { - data: { data: {} }, lastRequested: 1, lastReceived: 0, } ); - const isLoading = transactionsSelectors.getTransactionsIsLoading( mockGetResource ); + const isLoading = transactionsSelectors.getTransactionsIsLoading( mockGetResource )(); expect( mockGetResource ).toHaveBeenCalledTimes( 1 ); expect( mockGetResource ).toHaveBeenCalledWith( expectedResourceName ); expect( isLoading ).toStrictEqual( expected ); } ); + } ); + + describe( 'transactionsInitStatus()', () => { + it( 'Returns true when transactions are initialized', () => { + const expected = true; + + const mockGetResource = jest.fn(); + mockGetResource.mockReturnValue( { + lastRequested: 0, + lastReceived: 1, + } ); + + const initStatus = transactionsSelectors.transactionsInitStatus( mockGetResource )(); + + expect( mockGetResource ).toHaveBeenCalledTimes( 1 ); + expect( mockGetResource ).toHaveBeenCalledWith( expectedResourceName ); + expect( initStatus ).toBe( expected ); + } ); + + it( "Returns false when transactions aren't initialized", () => {} ); + } ); - it( "getTransactionsIsLoading returns true when the requested data doesn't exist", () => { + describe( 'showTransactionsPlaceholder()', () => { + it( "Returns true when transactions aren't initialized", () => { const expected = true; const mockGetResource = jest.fn(); + mockGetResource.mockReturnValue( {} ); + + const showPlaceholder = transactionsSelectors.showTransactionsPlaceholder( mockGetResource )(); - // Note that it's important here that lastRequested < lastReceived. + expect( mockGetResource ).toHaveBeenCalledTimes( 1 ); + expect( mockGetResource ).toHaveBeenCalledWith( expectedResourceName ); + expect( showPlaceholder ).toBe( expected ); + } ); + + it( "Returns false when transactions are initialized", () => { + const expected = false; + + const mockGetResource = jest.fn(); mockGetResource.mockReturnValue( { - data: {}, lastRequested: 0, lastReceived: 1, } ); - const isLoading = transactionsSelectors.getTransactionsIsLoading( mockGetResource ); + + const showPlaceholder = transactionsSelectors.showTransactionsPlaceholder( mockGetResource )(); expect( mockGetResource ).toHaveBeenCalledTimes( 1 ); expect( mockGetResource ).toHaveBeenCalledWith( expectedResourceName ); - expect( isLoading ).toStrictEqual( expected ); + expect( showPlaceholder ).toBe( expected ); } ); } ); } ); diff --git a/client/transactions/index.js b/client/transactions/index.js index 042faa42acf..8b679268d1c 100644 --- a/client/transactions/index.js +++ b/client/transactions/index.js @@ -34,7 +34,6 @@ class TransactionsList extends Component { const { transactions, isLoading } = this.props; const transactionsData = transactions.data || []; // Do not display table loading view if data is already available. - const loadingStatus = ( transactionsData.length <= 0 ) && isLoading; const rows = transactionsData.map( ( txn ) => { const charge = txn.source.object === 'charge' ? txn.source : null; @@ -69,7 +68,7 @@ class TransactionsList extends Component { return ( { - const { getTransactions, getTransactionsIsLoading } = select( 'wc-payments-api' ); + const { getTransactions, showTransactionsPlaceholder } = select( 'wc-payments-api' ); const transactions = getTransactions(); - const isLoading = getTransactionsIsLoading; + const isLoading = showTransactionsPlaceholder(); return { transactions, isLoading }; } )