media state: create fetchNextMediaPage thunk and replace usages of …

…`MediaActions.fetchNextPage` (#44023)

* create 'isFetchingNextPage' selector

* create 'fetchNextPage' thunk and prepare data layer

* replace usages of `MediaActions.fetchNextPage` with `fetchNextPage` thunk

* fix tests

* make flux removal comment more prominent

* fetch-next-page: fix jsdoc description

* media state: rename `fetchNextPage` to `fetchNextMediaPage`

client/components/data/media-list-data/index.jsx

@@ -5,6 +5,7 @@
 import { assign, isEqual } from 'lodash';
 import PropTypes from 'prop-types';
 import React from 'react';
+import { connect } from 'react-redux';
 
 /**
  * Internal dependencies
@@ -13,6 +14,7 @@ import MediaActions from 'lib/media/actions';
 import MediaListStore from 'lib/media/list-store';
 import passToChildren from 'lib/react-pass-to-children';
 import utils from './utils';
+import { fetchNextMediaPage } from 'state/media/thunks';
 
 function getStateData( siteId ) {
 	return {
@@ -22,7 +24,7 @@ function getStateData( siteId ) {
 	};
 }
 
-export default class extends React.Component {
+export class MediaListData extends React.Component {
 	static displayName = 'MediaListData';
 
 	static propTypes = {
@@ -87,7 +89,7 @@ export default class extends React.Component {
 	};
 
 	fetchData = () => {
-		MediaActions.fetchNextPage( this.props.siteId );
+		this.props.fetchNextMediaPage( this.props.siteId );
 	};
 
 	updateStateData = () => {
@@ -103,3 +105,5 @@ export default class extends React.Component {
 		);
 	}
 }
+
+export default connect( null, { fetchNextMediaPage } )( MediaListData );

client/components/data/media-list-data/test/index.jsx

@@ -12,7 +12,7 @@ import React from 'react';
 /**
  * Internal dependencies
  */
-import MediaListData from 'components/data/media-list-data';
+import { MediaListData } from 'components/data/media-list-data';
 
 jest.mock( 'lib/media/actions', () => ( { setQuery: () => {}, fetchNextPage: () => {} } ) );
 jest.mock( 'lib/media/list-store', () => ( {

client/lib/media/actions.js

@@ -9,16 +9,9 @@ const debug = debugFactory( 'calypso:media' );
  * Internal dependencies
  */
 import Dispatcher from 'dispatcher';
-import wpcom from 'lib/wp';
 import { reduxDispatch } from 'lib/redux-bridge';
 import MediaStore from './store';
-import MediaListStore from './list-store';
-import {
-	changeMediaSource,
-	failMediaRequest,
-	receiveMedia,
-	successMediaRequest,
-} from 'state/media/actions';
+import { changeMediaSource } from 'state/media/actions';
 
 /**
  * @typedef IMediaActions
@@ -43,43 +36,6 @@ MediaActions.setQuery = function ( siteId, query ) {
 	} );
 };
 
-MediaActions.fetchNextPage = function ( siteId ) {
-	if ( MediaListStore.isFetchingNextPage( siteId ) ) {
-		return;
-	}
-
-	Dispatcher.handleViewAction( {
-		type: 'FETCH_MEDIA_ITEMS',
-		siteId: siteId,
-	} );
-
-	const query = MediaListStore.getNextPageQuery( siteId );
-
-	const mediaReceived = ( error, data ) => {
-		Dispatcher.handleServerAction( {
-			type: 'RECEIVE_MEDIA_ITEMS',
-			error: error,
-			siteId: siteId,
-			data: data,
-			query: query,
-		} );
-		if ( error ) {
-			reduxDispatch( failMediaRequest( siteId, query, error ) );
-		} else {
-			reduxDispatch( successMediaRequest( siteId, query ) );
-			reduxDispatch( receiveMedia( siteId, data.media, data.found, query ) );
-		}
-	};
-
-	debug( 'Fetching media for %d using query %o', siteId, query );
-
-	if ( ! query.source ) {
-		wpcom.site( siteId ).mediaList( query, mediaReceived );
-	} else {
-		wpcom.undocumented().externalMediaList( query, mediaReceived );
-	}
-};
-
 MediaActions.edit = function ( siteId, item ) {
 	const newItem = assign( {}, MediaStore.get( siteId, item.ID ), item );
 

client/lib/media/test/actions.js

@@ -44,11 +44,10 @@ jest.mock( 'lib/redux-bridge', () => ( {
 } ) );
 
 describe( 'MediaActions', () => {
-	let MediaActions, sandbox, Dispatcher, MediaListStore, savedCreateObjectURL;
+	let MediaActions, sandbox, Dispatcher, savedCreateObjectURL;
 
 	beforeAll( function () {
 		Dispatcher = require( 'dispatcher' );
-		MediaListStore = require( '../list-store' );
 		MediaActions = require( '../actions' );
 	} );
 
@@ -90,53 +89,6 @@ describe( 'MediaActions', () => {
 		} );
 	} );
 
-	describe( '#fetchNextPage()', () => {
-		test( 'should call to the internal WordPress.com REST API', () => {
-			return new Promise( ( done ) => {
-				const query = MediaListStore.getNextPageQuery( DUMMY_SITE_ID );
-
-				MediaActions.fetchNextPage( DUMMY_SITE_ID );
-
-				expect( stubs.mediaList ).to.have.been.calledOn( DUMMY_SITE_ID );
-				process.nextTick( function () {
-					expect( Dispatcher.handleServerAction ).to.have.been.calledWithMatch( {
-						type: 'RECEIVE_MEDIA_ITEMS',
-						error: null,
-						siteId: DUMMY_SITE_ID,
-						data: DUMMY_API_RESPONSE,
-						query: query,
-					} );
-
-					done();
-				} );
-			} );
-		} );
-
-		test( 'should call to the external WordPress.com REST API', () => {
-			return new Promise( ( done ) => {
-				MediaListStore._activeQueries[ DUMMY_SITE_ID ] = { query: { source: 'external' } };
-
-				const query = MediaListStore.getNextPageQuery( DUMMY_SITE_ID );
-
-				MediaActions.fetchNextPage( DUMMY_SITE_ID );
-
-				expect( stubs.mediaListExternal ).to.have.been.calledWithMatch( { source: 'external' } );
-
-				process.nextTick( function () {
-					expect( Dispatcher.handleServerAction ).to.have.been.calledWithMatch( {
-						type: 'RECEIVE_MEDIA_ITEMS',
-						error: null,
-						siteId: DUMMY_SITE_ID,
-						data: DUMMY_API_RESPONSE,
-						query: query,
-					} );
-
-					done();
-				} );
-			} );
-		} );
-	} );
-
 	describe( '#edit()', () => {
 		const item = { ID: 100, description: 'Example' };
 

client/my-sites/media-library/external-media-header.jsx

@@ -17,7 +17,7 @@ import { Card, Button } from '@automattic/components';
 import MediaActions from 'lib/media/actions';
 import MediaListStore from 'lib/media/list-store';
 import StickyPanel from 'components/sticky-panel';
-import { addExternalMedia } from 'state/media/thunks';
+import { addExternalMedia, fetchNextMediaPage } from 'state/media/thunks';
 
 const DEBOUNCE_TIME = 250;
 
@@ -91,7 +91,7 @@ class MediaLibraryExternalHeader extends React.Component {
 		const { ID } = this.props.site;
 
 		MediaActions.sourceChanged( ID );
-		MediaActions.fetchNextPage( ID );
+		this.props.fetchNextMediaPage( ID );
 	}
 
 	onCopy = () => {
@@ -159,4 +159,6 @@ class MediaLibraryExternalHeader extends React.Component {
 	}
 }
 
-export default connect( null, { addExternalMedia } )( localize( MediaLibraryExternalHeader ) );
+export default connect( null, { addExternalMedia, fetchNextMediaPage } )(
+	localize( MediaLibraryExternalHeader )
+);

client/state/data-layer/wpcom/sites/media/index.js

@@ -33,6 +33,7 @@ import {
 	dispatchFluxRemoveMediaItemError,
 	dispatchFluxRequestMediaItemSuccess,
 	dispatchFluxRequestMediaItemError,
+	dispatchFluxRequestMediaItemsSuccess,
 } from 'state/media/utils/flux-adapter';
 
 import { registerHandlers } from 'state/data-layer/handler-registry';
@@ -87,24 +88,33 @@ export const editMedia = ( action ) => {
 export function requestMedia( action ) {
 	log( 'Request media for site %d using query %o', action.siteId, action.query );
 
+	const { siteId, query } = action;
+
+	const path = query.source ? `/meta/external-media/${ query.source }` : `/sites/${ siteId }/media`;
+
 	return [
 		http(
 			{
 				method: 'GET',
-				path: `/sites/${ action.siteId }/media`,
+				path,
 				apiVersion: '1.1',
+				query,
 			},
 			action
 		),
 	];
 }
 
-export const requestMediaSuccess = ( { siteId, query }, { media, found } ) => [
-	receiveMedia( siteId, media, found, query ),
-	successMediaRequest( siteId, query ),
-];
+export const requestMediaSuccess = ( { siteId, query }, data ) => ( dispatch ) => {
+	dispatch( receiveMedia( siteId, data.media, data.found, query ) );
+	dispatch( successMediaRequest( siteId, query ) );
 
-export const requestMediaError = ( { siteId, query } ) => failMediaRequest( siteId, query );
+	dispatchFluxRequestMediaItemsSuccess( siteId, data, query );
+};
+
+export const requestMediaError = ( { siteId, query } ) => ( dispatch ) => {
+	dispatch( failMediaRequest( siteId, query ) );
+};
 
 export function requestMediaItem( action ) {
 	const { mediaId, query, siteId } = action;

client/state/data-layer/wpcom/sites/media/test/index.js

@@ -21,23 +21,25 @@ import {
 
 describe( 'media request', () => {
 	test( 'should dispatch SUCCESS action when request completes', () => {
-		expect(
-			requestMediaSuccess(
-				{ siteId: 2916284, query: 'a=b' },
-				{ media: { ID: 10, title: 'media title' }, found: true }
-			)
-		).toEqual(
-			expect.arrayContaining( [
-				successMediaRequest( 2916284, 'a=b' ),
-				receiveMedia( 2916284, { ID: 10, title: 'media title' }, true, 'a=b' ),
-			] )
+		const dispatch = jest.fn();
+
+		requestMediaSuccess(
+			{ siteId: 2916284, query: 'a=b' },
+			{ media: { ID: 10, title: 'media title' }, found: true }
+		)( dispatch );
+
+		expect( dispatch ).toHaveBeenCalledWith( successMediaRequest( 2916284, 'a=b' ) );
+		expect( dispatch ).toHaveBeenCalledWith(
+			receiveMedia( 2916284, { ID: 10, title: 'media title' }, true, 'a=b' )
 		);
 	} );
 
 	test( 'should dispatch FAILURE action when request fails', () => {
-		expect( requestMediaError( { siteId: 2916284, query: 'a=b' } ) ).toEqual(
-			failMediaRequest( 2916284, 'a=b' )
-		);
+		const dispatch = jest.fn();
+
+		requestMediaError( { siteId: 2916284, query: 'a=b' } )( dispatch );
+
+		expect( dispatch ).toHaveBeenCalledWith( failMediaRequest( 2916284, 'a=b' ) );
 	} );
 
 	test( 'should dispatch http request', () => {
@@ -48,6 +50,7 @@ describe( 'media request', () => {
 						method: 'GET',
 						path: '/sites/2916284/media',
 						apiVersion: '1.1',
+						query: 'a=b',
 					},
 					{ siteId: 2916284, query: 'a=b' }
 				),

client/state/media/thunks/fetch-next-media-page.js

@@ -0,0 +1,38 @@
+/**
+ * External dependencies
+ */
+import debugFactory from 'debug';
+
+/**
+ * Internal dependencies
+ */
+import isFetchingNextPage from 'state/selectors/is-fetching-next-page';
+import MediaListStore from 'lib/media/list-store';
+import { dispatchFluxFetchMediaItems } from 'state/media/utils/flux-adapter';
+import { requestMedia } from 'state/media/actions';
+
+const debug = debugFactory( 'calypso:media' );
+
+/**
+ * Redux thunk to fetch next page of media items
+ *
+ * @param {number} siteId site identifier
+ */
+export const fetchNextMediaPage = ( siteId ) => ( dispatch, getState ) => {
+	if ( isFetchingNextPage( getState(), siteId ) ) {
+		return;
+	}
+
+	/*
+	 * @TODO: Temporarily this action will dispatch to the flux store, until
+	 * the flux store is removed. After we completely removed the flux store
+	 * it should be enough to just call the `requestMedia( ... )` action
+	 */
+	dispatchFluxFetchMediaItems( siteId );
+
+	const query = MediaListStore.getNextPageQuery( siteId );
+
+	debug( 'Fetching media for %d using query %o', siteId, query );
+
+	dispatch( requestMedia( siteId, query ) );
+};

client/state/media/thunks/index.js

@@ -6,3 +6,4 @@ export { updateMedia } from './update-media';
 export { deleteMedia } from './delete-media';
 export { fetchMediaItem } from './fetch-media-item';
 export { addWoocommerceProductImage } from './add-woocommerce-product-image';
+export { fetchNextMediaPage } from './fetch-next-media-page';

client/state/media/utils/flux-adapter.js

@@ -99,5 +99,21 @@ export const dispatchFluxFetchMediaItem = ( siteId, mediaId ) => {
 	} );
 };
 
+export const dispatchFluxFetchMediaItems = ( siteId ) => {
+	Dispatcher.handleViewAction( {
+		type: 'FETCH_MEDIA_ITEMS',
+		siteId: siteId,
+	} );
+};
+
+export const dispatchFluxRequestMediaItemsSuccess = ( siteId, data, query ) => {
+	Dispatcher.handleServerAction( {
+		type: 'RECEIVE_MEDIA_ITEMS',
+		siteId,
+		data,
+		query,
+	} );
+};
+
 export const dispatchFluxFetchMediaLimits = ( siteId ) =>
 	Dispatcher.handleServerAction( { type: 'FETCH_MEDIA_LIMITS', siteId } );

client/state/selectors/is-fetching-next-page.js

@@ -0,0 +1,15 @@
+/**
+ * Internal dependencies
+ */
+import { get } from 'lodash';
+
+/**
+ * Returns true if media is being requested for a specified site ID and query.
+ *
+ * @param  {object}  state  Global state tree
+ * @param  {number}  siteId Site ID
+ * @returns {boolean}           True if media is being requested
+ */
+export default function isFetchingNextPage( state, siteId ) {
+	return get( state.media.fetching, [ siteId, 'nextPage' ], false );
+}