From 1c5090e2a88f895937b86829dddd88284ca3e57c Mon Sep 17 00:00:00 2001
From: Nicola Heald <nicola@notnowlewis.com>
Date: Thu, 28 Jun 2018 12:53:43 +0100
Subject: [PATCH 01/12] Move embed API call out of block and into data

---
 core-blocks/embed/index.js               | 138 ++++++++++++++---------
 packages/core-data/src/actions.js        |  17 +++
 packages/core-data/src/reducer.js        |  21 ++++
 packages/core-data/src/resolvers.js      |  22 ++++
 packages/core-data/src/selectors.js      |  25 ++++
 packages/core-data/src/test/reducer.js   |  23 +++-
 packages/core-data/src/test/resolvers.js |  37 +++++-
 packages/core-data/src/test/selectors.js |  26 ++++-
 8 files changed, 249 insertions(+), 60 deletions(-)

diff --git a/core-blocks/embed/index.js b/core-blocks/embed/index.js
index 9d34bb91bc40cc..14f321ed4a5c19 100644
--- a/core-blocks/embed/index.js
+++ b/core-blocks/embed/index.js
@@ -3,19 +3,17 @@
  */
 import { parse } from 'url';
 import { includes, kebabCase, toLower } from 'lodash';
-import { stringify } from 'querystring';
-import memoize from 'memize';
 import classnames from 'classnames';
 
 /**
  * WordPress dependencies
  */
 import { __, sprintf } from '@wordpress/i18n';
-import { Component, Fragment, renderToString } from '@wordpress/element';
+import { Component, compose, Fragment, renderToString } from '@wordpress/element';
 import { Button, Placeholder, Spinner, SandBox, IconButton, Toolbar } from '@wordpress/components';
 import { createBlock } from '@wordpress/blocks';
 import { RichText, BlockControls } from '@wordpress/editor';
-import apiFetch from '@wordpress/api-fetch';
+import { withSelect } from '@wordpress/data';
 
 /**
  * Internal dependencies
@@ -26,9 +24,6 @@ import './editor.scss';
 // These embeds do not work in sandboxes
 const HOSTS_NO_PREVIEWS = [ 'facebook.com' ];
 
-// Caches the embed API calls, so if blocks get transformed, or deleted and added again, we don't spam the API.
-const wpEmbedAPI = memoize( ( url ) => apiFetch( { path: `/oembed/1.0/proxy?${ stringify( { url } ) }` } ) );
-
 const matchesPatterns = ( url, patterns = [] ) => {
 	return patterns.some( ( pattern ) => {
 		return url.match( pattern );
@@ -77,12 +72,23 @@ function getEmbedBlockSettings( { title, description, icon, category = 'embed',
 
 		transforms,
 
-		edit: class extends Component {
+		edit: compose(
+			withSelect( ( select, ownProps ) => {
+				const { url } = ownProps.attributes;
+				// Preview is undefined if we don't know the status of it, false if it failed,
+				// otherwise it will be an object containing the embed response.
+				const preview = url ? select( 'core' ).getEmbedPreview( url ) : undefined;
+				return {
+					preview,
+				};
+			} )
+		)( class extends Component {
 			constructor() {
 				super( ...arguments );
 
-				this.doServerSideRender = this.doServerSideRender.bind( this );
 				this.switchBackToURLInput = this.switchBackToURLInput.bind( this );
+				this.setUrl = this.setUrl.bind( this );
+				this.processPreview = this.processPreview.bind( this );
 
 				this.state = {
 					html: '',
@@ -90,11 +96,19 @@ function getEmbedBlockSettings( { title, description, icon, category = 'embed',
 					error: false,
 					fetching: false,
 					providerName: '',
+					url: '',
 				};
 			}
 
-			componentDidMount() {
-				this.doServerSideRender();
+			componentWillMount() {
+				if ( this.props.attributes.url ) {
+					// Loading from a saved block, set the state up and display the fetching UI.
+					this.setState( { fetching: true, url: this.props.attributes.url } );
+					if ( this.props.preview ) {
+						// Preview must have already been fetched prior to loading this block, so process it.
+						this.processPreview( this.props.preview, this.props.attributes.url );
+					}
+				}
 			}
 
 			componentWillUnmount() {
@@ -102,6 +116,13 @@ function getEmbedBlockSettings( { title, description, icon, category = 'embed',
 				this.unmounting = true;
 			}
 
+			componentWillReceiveProps( nextProps ) {
+				const hasPreview = undefined !== nextProps.preview;
+				if ( hasPreview ) {
+					this.processPreview( nextProps.preview, nextProps.attributes.url );
+				}
+			}
+
 			getPhotoHtml( photo ) {
 				// 100% width for the preview so it fits nicely into the document, some "thumbnails" are
 				// acually the full size photo.
@@ -109,13 +130,31 @@ function getEmbedBlockSettings( { title, description, icon, category = 'embed',
 				return renderToString( photoPreview );
 			}
 
-			doServerSideRender( event ) {
+			setUrl( event ) {
 				if ( event ) {
 					event.preventDefault();
 				}
-				const { url } = this.props.attributes;
+				const { url } = this.state;
+				const { setAttributes } = this.props;
+				if ( url === this.props.attributes.url ) {
+					// Don't change anything, otherwise we go into the 'fetching' state but never
+					// get new props, because the url has not changed.
+					return;
+				}
+				setAttributes( { url } );
+				this.setState( { fetching: true, error: false } );
+			}
+
+			processPreview( obj, url ) {
 				const { setAttributes } = this.props;
 
+				if ( false === obj ) {
+					// If the preview is false (not falsey, but actually false) then the embed request failed,
+					// so we cannot embed it.
+					this.setState( { fetching: false, error: true } );
+					return;
+				}
+
 				if ( undefined === url ) {
 					return;
 				}
@@ -132,44 +171,31 @@ function getEmbedBlockSettings( { title, description, icon, category = 'embed',
 					}
 				}
 
-				this.setState( { error: false, fetching: true } );
-				wpEmbedAPI( url )
-					.then(
-						( obj ) => {
-							if ( this.unmounting ) {
-								return;
-							}
-							// Some plugins only return HTML with no type info, so default this to 'rich'.
-							let { type = 'rich' } = obj;
-							// If we got a provider name from the API, use it for the slug, otherwise we use the title,
-							// because not all embed code gives us a provider name.
-							const { html, provider_name: providerName } = obj;
-							const providerNameSlug = kebabCase( toLower( '' !== providerName ? providerName : title ) );
-							// This indicates it's a WordPress embed, there aren't a set of URL patterns we can use to match WordPress URLs.
-							if ( includes( html, 'class="wp-embedded-content" data-secret' ) ) {
-								type = 'wp-embed';
-								// If this is not the WordPress embed block, transform it into one.
-								if ( this.props.name !== 'core-embed/wordpress' ) {
-									this.props.onReplace( createBlock( 'core-embed/wordpress', { url } ) );
-									return;
-								}
-							}
-							if ( html ) {
-								this.setState( { html, type, providerNameSlug } );
-								setAttributes( { type, providerNameSlug } );
-							} else if ( 'photo' === type ) {
-								this.setState( { html: this.getPhotoHtml( obj ), type, providerNameSlug } );
-								setAttributes( { type, providerNameSlug } );
-							} else {
-								// No html, no custom type that we support, so show the error state.
-								this.setState( { error: true } );
-							}
-							this.setState( { fetching: false } );
-						},
-						() => {
-							this.setState( { fetching: false, error: true } );
-						}
-					);
+				// Some plugins only return HTML with no type info, so default this to 'rich'.
+				let { type = 'rich' } = obj;
+				// If we got a provider name from the API, use it for the slug, otherwise we use the title,
+				// because not all embed code gives us a provider name.
+				const { html, provider_name: providerName } = obj;
+				const providerNameSlug = kebabCase( toLower( '' !== providerName ? providerName : title ) );
+
+				// This indicates it's a WordPress embed, there aren't a set of URL patterns we can use to match WordPress URLs.
+				if ( includes( html, 'class="wp-embedded-content" data-secret' ) ) {
+					type = 'wp-embed';
+					// If this is not the WordPress embed block, transform it into one.
+					if ( this.props.name !== 'core-embed/wordpress' ) {
+						this.props.onReplace( createBlock( 'core-embed/wordpress', { url } ) );
+						return;
+					}
+				}
+
+				if ( html ) {
+					this.setState( { html, type, providerNameSlug } );
+					setAttributes( { type, providerNameSlug } );
+				} else if ( 'photo' === type ) {
+					this.setState( { html: this.getPhotoHtml( obj ), type, providerNameSlug } );
+					setAttributes( { type, providerNameSlug } );
+				}
+				this.setState( { fetching: false } );
 			}
 
 			switchBackToURLInput() {
@@ -177,8 +203,8 @@ function getEmbedBlockSettings( { title, description, icon, category = 'embed',
 			}
 
 			render() {
-				const { html, type, error, fetching } = this.state;
-				const { url, caption } = this.props.attributes;
+				const { html, type, error, fetching, url } = this.state;
+				const { caption } = this.props.attributes;
 				const { setAttributes, isSelected, className } = this.props;
 				const controls = (
 					<BlockControls>
@@ -208,14 +234,14 @@ function getEmbedBlockSettings( { title, description, icon, category = 'embed',
 
 					return (
 						<Placeholder icon={ icon } label={ label } className="wp-block-embed">
-							<form onSubmit={ this.doServerSideRender }>
+							<form onSubmit={ this.setUrl }>
 								<input
 									type="url"
 									value={ url || '' }
 									className="components-placeholder__input"
 									aria-label={ label }
 									placeholder={ __( 'Enter URL to embed here…' ) }
-									onChange={ ( event ) => setAttributes( { url: event.target.value } ) } />
+									onChange={ ( event ) => this.setState( { url: event.target.value } ) } />
 								<Button
 									isLarge
 									type="submit">
@@ -269,7 +295,7 @@ function getEmbedBlockSettings( { title, description, icon, category = 'embed',
 					</Fragment>
 				);
 			}
-		},
+		} ),
 
 		save( { attributes } ) {
 			const { url, caption, type, providerNameSlug } = attributes;
diff --git a/packages/core-data/src/actions.js b/packages/core-data/src/actions.js
index 31107d8daef102..7d007681c3e952 100644
--- a/packages/core-data/src/actions.js
+++ b/packages/core-data/src/actions.js
@@ -81,3 +81,20 @@ export function receiveThemeSupportsFromIndex( index ) {
 		themeSupports: index.theme_supports,
 	};
 }
+
+/**
+ * Returns an action object used in signalling that the preview data for
+ * a given URl has been received.
+ *
+ * @param {string}  url      URL to preview the embed for.
+ * @param {Mixed}   preview  Preview data.
+ *
+ * @return {Object} Action object.
+ */
+export function receiveEmbedPreview( url, preview ) {
+	return {
+		type: 'RECEIVE_EMBED_PREVIEW',
+		url,
+		preview,
+	};
+}
diff --git a/packages/core-data/src/reducer.js b/packages/core-data/src/reducer.js
index 5f582890b44e4d..2828a9b8f30afb 100644
--- a/packages/core-data/src/reducer.js
+++ b/packages/core-data/src/reducer.js
@@ -201,10 +201,31 @@ export const entities = ( state = {}, action ) => {
 	};
 };
 
+/**
+ * Reducer managing embed preview data.
+ *
+ * @param {Object} state  Current state.
+ * @param {Object} action Dispatched action.
+ *
+ * @return {Object} Updated state.
+ */
+export function embedPreviews( state = {}, action ) {
+	switch ( action.type ) {
+		case 'RECEIVE_EMBED_PREVIEW':
+			const { url, preview } = action;
+			return {
+				...state,
+				[ url ]: preview,
+			};
+	}
+	return state;
+}
+
 export default combineReducers( {
 	terms,
 	users,
 	taxonomies,
 	themeSupports,
 	entities,
+	embedPreviews,
 } );
diff --git a/packages/core-data/src/resolvers.js b/packages/core-data/src/resolvers.js
index a105997bfedd68..7e3221e06e99f9 100644
--- a/packages/core-data/src/resolvers.js
+++ b/packages/core-data/src/resolvers.js
@@ -8,6 +8,11 @@ import { find } from 'lodash';
  */
 import apiFetch from '@wordpress/api-fetch';
 
+/**
+ * External dependencies
+ */
+import { stringify } from 'querystring';
+
 /**
  * Internal dependencies
  */
@@ -16,6 +21,7 @@ import {
 	receiveUserQuery,
 	receiveEntityRecords,
 	receiveThemeSupportsFromIndex,
+	receiveEmbedPreview,
 } from './actions';
 import { getKindEntities } from './entities';
 
@@ -78,3 +84,19 @@ export async function* getThemeSupports() {
 	const index = await apiFetch( { path: '/' } );
 	yield receiveThemeSupportsFromIndex( index );
 }
+
+/**
+ * Requests a preview from the from the Embed API.
+ *
+ * @param {Object} state State tree
+ * @param {string} url   URL to get the preview for.
+ */
+export async function* getEmbedPreview( state, url ) {
+	try {
+		const embedProxyResponse = await apiRequest( { path: `/oembed/1.0/proxy?${ stringify( { url } ) }` } );
+		yield receiveEmbedPreview( url, embedProxyResponse );
+	} catch ( error ) {
+		// Embed API 404s if the URL cannot be embedded, so we have to catch the error from the apiRequest here.
+		yield receiveEmbedPreview( url, false );
+	}
+}
diff --git a/packages/core-data/src/selectors.js b/packages/core-data/src/selectors.js
index 7d36a4c65558a5..377c474ba7f5c5 100644
--- a/packages/core-data/src/selectors.js
+++ b/packages/core-data/src/selectors.js
@@ -168,3 +168,28 @@ export const getEntityRecords = createSelector(
 export function getThemeSupports( state ) {
 	return state.themeSupports;
 }
+
+/**
+ * Returns the embed preview for the given URL.
+ *
+ * @param {Object} state    Data state.
+ * @param {string} url      Embedded URL.
+ *
+ * @return {*} Undefined if the preview has not been fetched, false if the URL cannot be embedded, array of embed preview data if the preview has been fetched.
+ */
+export function getEmbedPreview( state, url ) {
+	const preview = state.embedPreviews[ url ];
+
+	if ( ! preview ) {
+		return preview;
+	}
+
+	const oEmbedLinkCheck = '<a href="' + url + '">' + url + '</a>';
+
+	if ( oEmbedLinkCheck === preview.html ) {
+		// just a link to the url, it's oEmbed being helpful and creating a link for us, not actually embedding content
+		return false;
+	}
+
+	return preview;
+}
diff --git a/packages/core-data/src/test/reducer.js b/packages/core-data/src/test/reducer.js
index 781056e89009ed..4cee14debb6e75 100644
--- a/packages/core-data/src/test/reducer.js
+++ b/packages/core-data/src/test/reducer.js
@@ -7,7 +7,7 @@ import { filter } from 'lodash';
 /**
  * Internal dependencies
  */
-import { terms, entities } from '../reducer';
+import { terms, entities, embedPreviews } from '../reducer';
 
 describe( 'terms()', () => {
 	it( 'returns an empty object by default', () => {
@@ -93,3 +93,24 @@ describe( 'entities', () => {
 		] );
 	} );
 } );
+
+describe( 'embedPreviews()', () => {
+	it( 'returns an empty object by default', () => {
+		const state = embedPreviews( undefined, {} );
+
+		expect( state ).toEqual( {} );
+	} );
+
+	it( 'returns with received preview', () => {
+		const originalState = deepFreeze( {} );
+		const state = embedPreviews( originalState, {
+			type: 'RECEIVE_EMBED_PREVIEW',
+			url: 'http://twitter.com/notnownikki',
+			preview: { data: 42 },
+		} );
+
+		expect( state ).toEqual( {
+			'http://twitter.com/notnownikki': { data: 42 },
+		} );
+	} );
+} );
diff --git a/packages/core-data/src/test/resolvers.js b/packages/core-data/src/test/resolvers.js
index 29f0cf68bf2712..7448dbc45967e1 100644
--- a/packages/core-data/src/test/resolvers.js
+++ b/packages/core-data/src/test/resolvers.js
@@ -3,11 +3,16 @@
  */
 import apiFetch from '@wordpress/api-fetch';
 
+/**
+ * External dependencies
+ */
+import { stringify } from 'querystring';
+
 /**
  * Internal dependencies
  */
-import { getCategories, getEntityRecord, getEntityRecords } from '../resolvers';
-import { receiveTerms, receiveEntityRecords, addEntities } from '../actions';
+import { getCategories, getEntityRecord, getEntityRecords, getEmbedPreview } from '../resolvers';
+import { receiveTerms, receiveEntityRecords, addEntities, receiveEmbedPreview } from '../actions';
 
 jest.mock( '@wordpress/api-fetch' );
 
@@ -105,3 +110,31 @@ describe( 'getEntityRecords', () => {
 		expect( received ).toEqual( receiveEntityRecords( 'root', 'postType', Object.values( POST_TYPES ) ) );
 	} );
 } );
+
+describe( 'getEmbedPreview', () => {
+	const SUCCESSFUL_EMBED_RESPONSE = { data: '<p>some html</p>' };
+	const UNEMBEDDABLE_RESPONSE = false;
+	const EMBEDDABLE_URL = 'http://twitter.com/notnownikki';
+	const UNEMBEDDABLE_URL = 'http://example.com/';
+
+	beforeAll( () => {
+		apiRequest.mockImplementation( ( options ) => {
+			if ( options.path === `/oembed/1.0/proxy?${ stringify( { url: EMBEDDABLE_URL } ) }` ) {
+				return Promise.resolve( SUCCESSFUL_EMBED_RESPONSE );
+			}
+			throw 404;
+		} );
+	} );
+
+	it( 'yields with fetched embed preview', async () => {
+		const fulfillment = getEmbedPreview( {}, EMBEDDABLE_URL );
+		const received = ( await fulfillment.next() ).value;
+		expect( received ).toEqual( receiveEmbedPreview( EMBEDDABLE_URL, SUCCESSFUL_EMBED_RESPONSE ) );
+	} );
+
+	it( 'yields false if the URL cannot be embedded', async () => {
+		const fulfillment = getEmbedPreview( {}, UNEMBEDDABLE_URL );
+		const received = ( await fulfillment.next() ).value;
+		expect( received ).toEqual( receiveEmbedPreview( UNEMBEDDABLE_URL, UNEMBEDDABLE_RESPONSE ) );
+	} );
+} );
diff --git a/packages/core-data/src/test/selectors.js b/packages/core-data/src/test/selectors.js
index ce3b68cf1b4d64..b8c30caa016fee 100644
--- a/packages/core-data/src/test/selectors.js
+++ b/packages/core-data/src/test/selectors.js
@@ -6,7 +6,7 @@ import deepFreeze from 'deep-freeze';
 /**
  * Internal dependencies
  */
-import { getTerms, isRequestingCategories, getEntityRecord, getEntityRecords } from '../selectors';
+import { getTerms, isRequestingCategories, getEntityRecord, getEntityRecords, getEmbedPreview } from '../selectors';
 import { select } from '@wordpress/data';
 
 jest.mock( '@wordpress/data', () => {
@@ -138,3 +138,27 @@ describe( 'getEntityRecords', () => {
 	} );
 } );
 
+describe( 'getEmbedPreview()', () => {
+	it( 'returns preview stored for url', () => {
+		let state = deepFreeze( {
+			embedPreviews: {},
+		} );
+		expect( getEmbedPreview( state, 'http://example.com/' ) ).toBe( undefined );
+
+		state = deepFreeze( {
+			embedPreviews: {
+				'http://example.com/': { data: 42 },
+			},
+		} );
+		expect( getEmbedPreview( state, 'http://example.com/' ) ).toEqual( { data: 42 } );
+	} );
+
+	it( 'returns false if the preview html is just a single link', () => {
+		const state = deepFreeze( {
+			embedPreviews: {
+				'http://example.com/': { html: '<a href="http://example.com/">http://example.com/</a>' },
+			},
+		} );
+		expect( getEmbedPreview( state, 'http://example.com/' ) ).toEqual( false );
+	} );
+} );

From a78196b4ac955d20118187275930870fdee61bfc Mon Sep 17 00:00:00 2001
From: Nicola Heald <nicola@notnowlewis.com>
Date: Mon, 23 Jul 2018 11:48:49 +0100
Subject: [PATCH 02/12] Update for apiFetch

---
 packages/core-data/src/resolvers.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/core-data/src/resolvers.js b/packages/core-data/src/resolvers.js
index 7e3221e06e99f9..db6a8b170d7841 100644
--- a/packages/core-data/src/resolvers.js
+++ b/packages/core-data/src/resolvers.js
@@ -93,7 +93,7 @@ export async function* getThemeSupports() {
  */
 export async function* getEmbedPreview( state, url ) {
 	try {
-		const embedProxyResponse = await apiRequest( { path: `/oembed/1.0/proxy?${ stringify( { url } ) }` } );
+		const embedProxyResponse = await apiFetch( { path: `/oembed/1.0/proxy?${ stringify( { url } ) }` } );
 		yield receiveEmbedPreview( url, embedProxyResponse );
 	} catch ( error ) {
 		// Embed API 404s if the URL cannot be embedded, so we have to catch the error from the apiRequest here.

From c9d9bc5421fc449d6acbf355e4ed2eb0042d1242 Mon Sep 17 00:00:00 2001
From: Nicola Heald <nicola@notnowlewis.com>
Date: Mon, 23 Jul 2018 12:10:41 +0100
Subject: [PATCH 03/12] fix tests

---
 packages/core-data/src/test/resolvers.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/core-data/src/test/resolvers.js b/packages/core-data/src/test/resolvers.js
index 7448dbc45967e1..884e08ea2bf343 100644
--- a/packages/core-data/src/test/resolvers.js
+++ b/packages/core-data/src/test/resolvers.js
@@ -118,7 +118,7 @@ describe( 'getEmbedPreview', () => {
 	const UNEMBEDDABLE_URL = 'http://example.com/';
 
 	beforeAll( () => {
-		apiRequest.mockImplementation( ( options ) => {
+		apiFetch.mockImplementation( ( options ) => {
 			if ( options.path === `/oembed/1.0/proxy?${ stringify( { url: EMBEDDABLE_URL } ) }` ) {
 				return Promise.resolve( SUCCESSFUL_EMBED_RESPONSE );
 			}

From e2f4f0a85bbfc62b07ee2957f29551a821f19401 Mon Sep 17 00:00:00 2001
From: Nicola Heald <nicola@notnowlewis.com>
Date: Mon, 23 Jul 2018 15:48:29 +0100
Subject: [PATCH 04/12] Move the edit component out of settings for easier
 testing

---
 core-blocks/embed/index.js               | 418 +++++++++++------------
 core-blocks/embed/test/index.js          |  11 +-
 packages/core-data/src/selectors.js      |  30 +-
 packages/core-data/src/test/selectors.js |  15 +-
 4 files changed, 239 insertions(+), 235 deletions(-)

diff --git a/core-blocks/embed/index.js b/core-blocks/embed/index.js
index 14f321ed4a5c19..caf4c3aacb566d 100644
--- a/core-blocks/embed/index.js
+++ b/core-blocks/embed/index.js
@@ -9,7 +9,8 @@ import classnames from 'classnames';
  * WordPress dependencies
  */
 import { __, sprintf } from '@wordpress/i18n';
-import { Component, compose, Fragment, renderToString } from '@wordpress/element';
+import { compose } from '@wordpress/compose';
+import { Component, Fragment, renderToString } from '@wordpress/element';
 import { Button, Placeholder, Spinner, SandBox, IconButton, Toolbar } from '@wordpress/components';
 import { createBlock } from '@wordpress/blocks';
 import { RichText, BlockControls } from '@wordpress/editor';
@@ -39,6 +40,198 @@ const findBlock = ( url ) => {
 	return 'core/embed';
 };
 
+export function getEmbedEdit( title, icon ) {
+	return class extends Component {
+		constructor() {
+			super( ...arguments );
+
+			this.setUrl = this.setUrl.bind( this );
+			this.processPreview = this.processPreview.bind( this );
+
+			this.state = {
+				html: this.props.preview ? this.props.preview.html : '',
+				type: this.props.attributes.type,
+				error: false,
+				fetching: !! this.props.attributes.url && ! this.props.preview,
+				providerName: '',
+				url: this.props.attributes.url,
+			};
+
+			if ( this.props.preview ) {
+				this.processPreview();
+			}
+		}
+
+		componentWillUnmount() {
+			// can't abort the fetch promise, so let it know we will unmount
+			this.unmounting = true;
+		}
+
+		componentDidUpdate( prevProps ) {
+			const hasPreview = undefined !== this.props.preview;
+			const hadPreview = undefined !== prevProps.preview;
+			if ( hasPreview && ! hadPreview ) {
+				this.processPreview();
+			}
+		}
+
+		getPhotoHtml( photo ) {
+			// 100% width for the preview so it fits nicely into the document, some "thumbnails" are
+			// acually the full size photo.
+			const photoPreview = <p><img src={ photo.thumbnail_url } alt={ photo.title } width="100%" /></p>;
+			return renderToString( photoPreview );
+		}
+
+		setUrl( event ) {
+			if ( event ) {
+				event.preventDefault();
+			}
+			const { url } = this.state;
+			const { setAttributes } = this.props;
+			if ( url === this.props.attributes.url ) {
+				// Don't change anything, otherwise we go into the 'fetching' state but never
+				// get new props, because the url has not changed.
+				return;
+			}
+			setAttributes( { url } );
+			this.setState( { fetching: true, error: false } );
+		}
+
+		processPreview() {
+			const { setAttributes, preview, previewIsFallback } = this.props;
+			const { url } = this.props.attributes;
+
+			if ( previewIsFallback ) {
+				// If the preview is false (not falsey, but actually false) then the embed request failed,
+				// so we cannot embed it.
+				this.setState( { fetching: false, error: true } );
+				return;
+			}
+
+			if ( undefined === url ) {
+				return;
+			}
+
+			const matchingBlock = findBlock( url );
+
+			// WordPress blocks can work on multiple sites, and so don't have patterns,
+			// so if we're in a WordPress block, assume the user has chosen it for a WordPress URL.
+			if ( 'core-embed/wordpress' !== this.props.name && 'core/embed' !== matchingBlock ) {
+				// At this point, we have discovered a more suitable block for this url, so transform it.
+				if ( this.props.name !== matchingBlock ) {
+					this.props.onReplace( createBlock( matchingBlock, { url } ) );
+					return;
+				}
+			}
+
+			// Some plugins only return HTML with no type info, so default this to 'rich'.
+			let { type = 'rich' } = preview;
+			// If we got a provider name from the API, use it for the slug, otherwise we use the title,
+			// because not all embed code gives us a provider name.
+			const { html, provider_name: providerName } = preview;
+			const providerNameSlug = kebabCase( toLower( '' !== providerName ? providerName : title ) );
+
+			// This indicates it's a WordPress embed, there aren't a set of URL patterns we can use to match WordPress URLs.
+			if ( includes( html, 'class="wp-embedded-content" data-secret' ) ) {
+				type = 'wp-embed';
+				// If this is not the WordPress embed block, transform it into one.
+				if ( this.props.name !== 'core-embed/wordpress' ) {
+					this.props.onReplace( createBlock( 'core-embed/wordpress', { url } ) );
+					return;
+				}
+			}
+
+			if ( html ) {
+				this.setState( { html, type, providerNameSlug } );
+				setAttributes( { type, providerNameSlug } );
+			} else if ( 'photo' === type ) {
+				this.setState( { html: this.getPhotoHtml( preview ), type, providerNameSlug } );
+				setAttributes( { type, providerNameSlug } );
+			}
+			this.setState( { fetching: false } );
+		}
+
+		render() {
+			const { html, type, error, fetching, url } = this.state;
+			const { caption } = this.props.attributes;
+			const { setAttributes, isSelected, className } = this.props;
+
+			if ( fetching ) {
+				return (
+					<div className="wp-block-embed is-loading">
+						<Spinner />
+						<p>{ __( 'Embedding…' ) }</p>
+					</div>
+				);
+			}
+
+			if ( ! html ) {
+				// translators: %s: type of embed e.g: "YouTube", "Twitter", etc. "Embed" is used when no specific type exists
+				const label = sprintf( __( '%s URL' ), title );
+
+				return (
+					<Placeholder icon={ icon } label={ label } className="wp-block-embed">
+						<form onSubmit={ this.setUrl }>
+							<input
+								type="url"
+								value={ url || '' }
+								className="components-placeholder__input"
+								aria-label={ label }
+								placeholder={ __( 'Enter URL to embed here…' ) }
+								onChange={ ( event ) => this.setState( { url: event.target.value } ) } />
+							<Button
+								isLarge
+								type="submit">
+								{ __( 'Embed' ) }
+							</Button>
+							{ error && <p className="components-placeholder__error">{ __( 'Sorry, we could not embed that content.' ) }</p> }
+						</form>
+					</Placeholder>
+				);
+			}
+
+			const parsedUrl = parse( url );
+			const cannotPreview = includes( HOSTS_NO_PREVIEWS, parsedUrl.host.replace( /^www\./, '' ) );
+			// translators: %s: host providing embed content e.g: www.youtube.com
+			const iframeTitle = sprintf( __( 'Embedded content from %s' ), parsedUrl.host );
+			const embedWrapper = 'wp-embed' === type ? (
+				<div
+					className="wp-block-embed__wrapper"
+					dangerouslySetInnerHTML={ { __html: html } }
+				/>
+			) : (
+				<div className="wp-block-embed__wrapper">
+					<SandBox
+						html={ html }
+						title={ iframeTitle }
+						type={ type }
+					/>
+				</div>
+			);
+
+			return (
+				<figure className={ classnames( className, 'wp-block-embed', { 'is-video': 'video' === type } ) }>
+					{ ( cannotPreview ) ? (
+						<Placeholder icon={ icon } label={ __( 'Embed URL' ) }>
+							<p className="components-placeholder__error"><a href={ url }>{ url }</a></p>
+							<p className="components-placeholder__error">{ __( 'Previews for this are unavailable in the editor, sorry!' ) }</p>
+						</Placeholder>
+					) : embedWrapper }
+					{ ( caption && caption.length > 0 ) || isSelected ? (
+						<RichText
+							tagName="figcaption"
+							placeholder={ __( 'Write caption…' ) }
+							value={ caption }
+							onChange={ ( value ) => setAttributes( { caption: value } ) }
+							inlineToolbar
+						/>
+					) : null }
+				</figure>
+			);
+		}
+	};
+}
+
 function getEmbedBlockSettings( { title, description, icon, category = 'embed', transforms, keywords = [] } ) {
 	// translators: %s: Name of service (e.g. VideoPress, YouTube)
 	const blockDescription = description || sprintf( __( 'Add a block that displays content pulled from other sites, like Twitter, Instagram or YouTube.' ), title );
@@ -75,227 +268,16 @@ function getEmbedBlockSettings( { title, description, icon, category = 'embed',
 		edit: compose(
 			withSelect( ( select, ownProps ) => {
 				const { url } = ownProps.attributes;
-				// Preview is undefined if we don't know the status of it, false if it failed,
-				// otherwise it will be an object containing the embed response.
-				const preview = url ? select( 'core' ).getEmbedPreview( url ) : undefined;
+				const core = select( 'core' );
+				const { getEmbedPreview, isPreviewEmbedFallback } = core;
+				const preview = getEmbedPreview( url );
+				const previewIsFallback = isPreviewEmbedFallback( url );
 				return {
 					preview,
+					previewIsFallback,
 				};
 			} )
-		)( class extends Component {
-			constructor() {
-				super( ...arguments );
-
-				this.switchBackToURLInput = this.switchBackToURLInput.bind( this );
-				this.setUrl = this.setUrl.bind( this );
-				this.processPreview = this.processPreview.bind( this );
-
-				this.state = {
-					html: '',
-					type: '',
-					error: false,
-					fetching: false,
-					providerName: '',
-					url: '',
-				};
-			}
-
-			componentWillMount() {
-				if ( this.props.attributes.url ) {
-					// Loading from a saved block, set the state up and display the fetching UI.
-					this.setState( { fetching: true, url: this.props.attributes.url } );
-					if ( this.props.preview ) {
-						// Preview must have already been fetched prior to loading this block, so process it.
-						this.processPreview( this.props.preview, this.props.attributes.url );
-					}
-				}
-			}
-
-			componentWillUnmount() {
-				// can't abort the fetch promise, so let it know we will unmount
-				this.unmounting = true;
-			}
-
-			componentWillReceiveProps( nextProps ) {
-				const hasPreview = undefined !== nextProps.preview;
-				if ( hasPreview ) {
-					this.processPreview( nextProps.preview, nextProps.attributes.url );
-				}
-			}
-
-			getPhotoHtml( photo ) {
-				// 100% width for the preview so it fits nicely into the document, some "thumbnails" are
-				// acually the full size photo.
-				const photoPreview = <p><img src={ photo.thumbnail_url } alt={ photo.title } width="100%" /></p>;
-				return renderToString( photoPreview );
-			}
-
-			setUrl( event ) {
-				if ( event ) {
-					event.preventDefault();
-				}
-				const { url } = this.state;
-				const { setAttributes } = this.props;
-				if ( url === this.props.attributes.url ) {
-					// Don't change anything, otherwise we go into the 'fetching' state but never
-					// get new props, because the url has not changed.
-					return;
-				}
-				setAttributes( { url } );
-				this.setState( { fetching: true, error: false } );
-			}
-
-			processPreview( obj, url ) {
-				const { setAttributes } = this.props;
-
-				if ( false === obj ) {
-					// If the preview is false (not falsey, but actually false) then the embed request failed,
-					// so we cannot embed it.
-					this.setState( { fetching: false, error: true } );
-					return;
-				}
-
-				if ( undefined === url ) {
-					return;
-				}
-
-				const matchingBlock = findBlock( url );
-
-				// WordPress blocks can work on multiple sites, and so don't have patterns,
-				// so if we're in a WordPress block, assume the user has chosen it for a WordPress URL.
-				if ( 'core-embed/wordpress' !== this.props.name && 'core/embed' !== matchingBlock ) {
-					// At this point, we have discovered a more suitable block for this url, so transform it.
-					if ( this.props.name !== matchingBlock ) {
-						this.props.onReplace( createBlock( matchingBlock, { url } ) );
-						return;
-					}
-				}
-
-				// Some plugins only return HTML with no type info, so default this to 'rich'.
-				let { type = 'rich' } = obj;
-				// If we got a provider name from the API, use it for the slug, otherwise we use the title,
-				// because not all embed code gives us a provider name.
-				const { html, provider_name: providerName } = obj;
-				const providerNameSlug = kebabCase( toLower( '' !== providerName ? providerName : title ) );
-
-				// This indicates it's a WordPress embed, there aren't a set of URL patterns we can use to match WordPress URLs.
-				if ( includes( html, 'class="wp-embedded-content" data-secret' ) ) {
-					type = 'wp-embed';
-					// If this is not the WordPress embed block, transform it into one.
-					if ( this.props.name !== 'core-embed/wordpress' ) {
-						this.props.onReplace( createBlock( 'core-embed/wordpress', { url } ) );
-						return;
-					}
-				}
-
-				if ( html ) {
-					this.setState( { html, type, providerNameSlug } );
-					setAttributes( { type, providerNameSlug } );
-				} else if ( 'photo' === type ) {
-					this.setState( { html: this.getPhotoHtml( obj ), type, providerNameSlug } );
-					setAttributes( { type, providerNameSlug } );
-				}
-				this.setState( { fetching: false } );
-			}
-
-			switchBackToURLInput() {
-				this.setState( { html: undefined } );
-			}
-
-			render() {
-				const { html, type, error, fetching, url } = this.state;
-				const { caption } = this.props.attributes;
-				const { setAttributes, isSelected, className } = this.props;
-				const controls = (
-					<BlockControls>
-						<Toolbar>
-							{ ( html && <IconButton
-								className="components-toolbar__control"
-								label={ __( 'Edit URL' ) }
-								icon="edit"
-								onClick={ this.switchBackToURLInput }
-							/> ) }
-						</Toolbar>
-					</BlockControls>
-				);
-
-				if ( fetching ) {
-					return (
-						<div className="wp-block-embed is-loading">
-							<Spinner />
-							<p>{ __( 'Embedding…' ) }</p>
-						</div>
-					);
-				}
-
-				if ( ! html ) {
-					// translators: %s: type of embed e.g: "YouTube", "Twitter", etc. "Embed" is used when no specific type exists
-					const label = sprintf( __( '%s URL' ), title );
-
-					return (
-						<Placeholder icon={ icon } label={ label } className="wp-block-embed">
-							<form onSubmit={ this.setUrl }>
-								<input
-									type="url"
-									value={ url || '' }
-									className="components-placeholder__input"
-									aria-label={ label }
-									placeholder={ __( 'Enter URL to embed here…' ) }
-									onChange={ ( event ) => this.setState( { url: event.target.value } ) } />
-								<Button
-									isLarge
-									type="submit">
-									{ __( 'Embed' ) }
-								</Button>
-								{ error && <p className="components-placeholder__error">{ __( 'Sorry, we could not embed that content.' ) }</p> }
-							</form>
-						</Placeholder>
-					);
-				}
-
-				const parsedUrl = parse( url );
-				const cannotPreview = includes( HOSTS_NO_PREVIEWS, parsedUrl.host.replace( /^www\./, '' ) );
-				// translators: %s: host providing embed content e.g: www.youtube.com
-				const iframeTitle = sprintf( __( 'Embedded content from %s' ), parsedUrl.host );
-				const embedWrapper = 'wp-embed' === type ? (
-					<div
-						className="wp-block-embed__wrapper"
-						dangerouslySetInnerHTML={ { __html: html } }
-					/>
-				) : (
-					<div className="wp-block-embed__wrapper">
-						<SandBox
-							html={ html }
-							title={ iframeTitle }
-							type={ type }
-						/>
-					</div>
-				);
-
-				return (
-					<Fragment>
-						{ controls }
-						<figure className={ classnames( className, 'wp-block-embed', { 'is-video': 'video' === type } ) }>
-							{ ( cannotPreview ) ? (
-								<Placeholder icon={ icon } label={ __( 'Embed URL' ) }>
-									<p className="components-placeholder__error"><a href={ url }>{ url }</a></p>
-									<p className="components-placeholder__error">{ __( 'Previews for this are unavailable in the editor, sorry!' ) }</p>
-								</Placeholder>
-							) : embedWrapper }
-							{ ( caption && caption.length > 0 ) || isSelected ? (
-								<RichText
-									tagName="figcaption"
-									placeholder={ __( 'Write caption…' ) }
-									value={ caption }
-									onChange={ ( value ) => setAttributes( { caption: value } ) }
-									inlineToolbar
-								/>
-							) : null }
-						</figure>
-					</Fragment>
-				);
-			}
-		} ),
+		)( getEmbedEdit( title, icon ) ),
 
 		save( { attributes } ) {
 			const { url, caption, type, providerNameSlug } = attributes;
diff --git a/core-blocks/embed/test/index.js b/core-blocks/embed/test/index.js
index 0b8a1985b06aa4..2ca6d72649755e 100644
--- a/core-blocks/embed/test/index.js
+++ b/core-blocks/embed/test/index.js
@@ -1,12 +1,17 @@
+/**
+ * External dependencies
+ */
+import { render } from 'enzyme';
+
 /**
  * Internal dependencies
  */
-import { name, settings } from '../';
-import { blockEditRender } from '../../test/helpers';
+import { getEmbedEdit } from '../';
 
 describe( 'core/embed', () => {
 	test( 'block edit matches snapshot', () => {
-		const wrapper = blockEditRender( name, settings );
+		const EmbedEdit = getEmbedEdit( 'Embed', 'embed-generic' );
+		const wrapper = render( <EmbedEdit attributes={ {} } /> );
 
 		expect( wrapper ).toMatchSnapshot();
 	} );
diff --git a/packages/core-data/src/selectors.js b/packages/core-data/src/selectors.js
index 377c474ba7f5c5..89f498b5aca9f2 100644
--- a/packages/core-data/src/selectors.js
+++ b/packages/core-data/src/selectors.js
@@ -175,21 +175,29 @@ export function getThemeSupports( state ) {
  * @param {Object} state    Data state.
  * @param {string} url      Embedded URL.
  *
- * @return {*} Undefined if the preview has not been fetched, false if the URL cannot be embedded, array of embed preview data if the preview has been fetched.
+ * @return {*} Undefined if the preview has not been fetched, otherwise, the preview fetched from the embed preview API.
  */
 export function getEmbedPreview( state, url ) {
-	const preview = state.embedPreviews[ url ];
-
-	if ( ! preview ) {
-		return preview;
-	}
+	return state.embedPreviews[ url ];
+}
 
+/**
+ * Determines if the returned preview is an oEmbed link fallback.
+ *
+ * WordPress can be configured to return a simple link to a URL if it is not embeddable.
+ * We need to be able to determine if a URL is embeddable or not, based on what we
+ * get back from the oEmbed preview API.
+ *
+ * @param {Object} state    Data state.
+ * @param {string} url      Embedded URL.
+ *
+ * @return {booleans} Is the preview for the URL an oEmbed link fallback.
+ */
+export function isPreviewEmbedFallback( state, url ) {
+	const preview = state.embedPreviews[ url ];
 	const oEmbedLinkCheck = '<a href="' + url + '">' + url + '</a>';
-
-	if ( oEmbedLinkCheck === preview.html ) {
-		// just a link to the url, it's oEmbed being helpful and creating a link for us, not actually embedding content
+	if ( ! preview ) {
 		return false;
 	}
-
-	return preview;
+	return preview.html === oEmbedLinkCheck;
 }
diff --git a/packages/core-data/src/test/selectors.js b/packages/core-data/src/test/selectors.js
index b8c30caa016fee..b6fe540c61c3b6 100644
--- a/packages/core-data/src/test/selectors.js
+++ b/packages/core-data/src/test/selectors.js
@@ -6,7 +6,14 @@ import deepFreeze from 'deep-freeze';
 /**
  * Internal dependencies
  */
-import { getTerms, isRequestingCategories, getEntityRecord, getEntityRecords, getEmbedPreview } from '../selectors';
+import {
+	getTerms,
+	isRequestingCategories,
+	getEntityRecord,
+	getEntityRecords,
+	getEmbedPreview,
+	isPreviewEmbedFallback,
+} from '../selectors';
 import { select } from '@wordpress/data';
 
 jest.mock( '@wordpress/data', () => {
@@ -152,13 +159,15 @@ describe( 'getEmbedPreview()', () => {
 		} );
 		expect( getEmbedPreview( state, 'http://example.com/' ) ).toEqual( { data: 42 } );
 	} );
+} );
 
-	it( 'returns false if the preview html is just a single link', () => {
+describe( 'isPreviewEmbedFallback()', () => {
+	it( 'returns true if the preview html is just a single link', () => {
 		const state = deepFreeze( {
 			embedPreviews: {
 				'http://example.com/': { html: '<a href="http://example.com/">http://example.com/</a>' },
 			},
 		} );
-		expect( getEmbedPreview( state, 'http://example.com/' ) ).toEqual( false );
+		expect( isPreviewEmbedFallback( state, 'http://example.com/' ) ).toEqual( true );
 	} );
 } );

From 14b7e755f2de41fc539a820588634b2a0e6055c9 Mon Sep 17 00:00:00 2001
From: Nicola Heald <nicola@notnowlewis.com>
Date: Tue, 31 Jul 2018 11:28:01 +0100
Subject: [PATCH 05/12] Reinstate edit controls

---
 core-blocks/embed/index.js | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/core-blocks/embed/index.js b/core-blocks/embed/index.js
index caf4c3aacb566d..010a8ae5d3202a 100644
--- a/core-blocks/embed/index.js
+++ b/core-blocks/embed/index.js
@@ -10,7 +10,7 @@ import classnames from 'classnames';
  */
 import { __, sprintf } from '@wordpress/i18n';
 import { compose } from '@wordpress/compose';
-import { Component, Fragment, renderToString } from '@wordpress/element';
+import { Component, renderToString } from '@wordpress/element';
 import { Button, Placeholder, Spinner, SandBox, IconButton, Toolbar } from '@wordpress/components';
 import { createBlock } from '@wordpress/blocks';
 import { RichText, BlockControls } from '@wordpress/editor';
@@ -45,6 +45,7 @@ export function getEmbedEdit( title, icon ) {
 		constructor() {
 			super( ...arguments );
 
+			this.switchBackToURLInput = this.switchBackToURLInput.bind( this );
 			this.setUrl = this.setUrl.bind( this );
 			this.processPreview = this.processPreview.bind( this );
 
@@ -70,7 +71,9 @@ export function getEmbedEdit( title, icon ) {
 		componentDidUpdate( prevProps ) {
 			const hasPreview = undefined !== this.props.preview;
 			const hadPreview = undefined !== prevProps.preview;
-			if ( hasPreview && ! hadPreview ) {
+			// We had a preview, and the URL was edited, and the new URL already has a preview fetched.
+			const switchedPreview = this.props.preview && this.props.attributes.url !== prevProps.attributes.url;
+			if ( ( hasPreview && ! hadPreview ) || switchedPreview ) {
 				this.processPreview();
 			}
 		}
@@ -89,6 +92,12 @@ export function getEmbedEdit( title, icon ) {
 			const { url } = this.state;
 			const { setAttributes } = this.props;
 			if ( url === this.props.attributes.url ) {
+				if ( this.props.preview ) {
+					// Form has been submitted without changing the url, and we already have a preview,
+					// so process it again. Happens when the user clicks edit and then immediately
+					// clicks embed.
+					this.processPreview();
+				}
 				// Don't change anything, otherwise we go into the 'fetching' state but never
 				// get new props, because the url has not changed.
 				return;
@@ -151,10 +160,26 @@ export function getEmbedEdit( title, icon ) {
 			this.setState( { fetching: false } );
 		}
 
+		switchBackToURLInput() {
+			this.setState( { html: undefined } );
+		}
+
 		render() {
 			const { html, type, error, fetching, url } = this.state;
 			const { caption } = this.props.attributes;
 			const { setAttributes, isSelected, className } = this.props;
+			const controls = (
+				<BlockControls>
+					<Toolbar>
+						{ ( html && <IconButton
+							className="components-toolbar__control"
+							label={ __( 'Edit URL' ) }
+							icon="edit"
+							onClick={ this.switchBackToURLInput }
+						/> ) }
+					</Toolbar>
+				</BlockControls>
+			);
 
 			if ( fetching ) {
 				return (
@@ -211,6 +236,7 @@ export function getEmbedEdit( title, icon ) {
 
 			return (
 				<figure className={ classnames( className, 'wp-block-embed', { 'is-video': 'video' === type } ) }>
+					{ controls }
 					{ ( cannotPreview ) ? (
 						<Placeholder icon={ icon } label={ __( 'Embed URL' ) }>
 							<p className="components-placeholder__error"><a href={ url }>{ url }</a></p>

From 3db8a042e0d9a419198044e5acf4c602a4fd91fb Mon Sep 17 00:00:00 2001
From: Nicola Heald <nicola@notnowlewis.com>
Date: Fri, 3 Aug 2018 09:44:24 +0100
Subject: [PATCH 06/12] switch to `addQueryArgs`

---
 packages/core-data/src/resolvers.js      | 4 ++--
 packages/core-data/src/test/resolvers.js | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/packages/core-data/src/resolvers.js b/packages/core-data/src/resolvers.js
index db6a8b170d7841..eeda4561d838df 100644
--- a/packages/core-data/src/resolvers.js
+++ b/packages/core-data/src/resolvers.js
@@ -11,7 +11,7 @@ import apiFetch from '@wordpress/api-fetch';
 /**
  * External dependencies
  */
-import { stringify } from 'querystring';
+import { addQueryArgs } from '@wordpress/url';
 
 /**
  * Internal dependencies
@@ -93,7 +93,7 @@ export async function* getThemeSupports() {
  */
 export async function* getEmbedPreview( state, url ) {
 	try {
-		const embedProxyResponse = await apiFetch( { path: `/oembed/1.0/proxy?${ stringify( { url } ) }` } );
+		const embedProxyResponse = await apiFetch( { path: addQueryArgs( '/oembed/1.0/proxy', { url } ) } );
 		yield receiveEmbedPreview( url, embedProxyResponse );
 	} catch ( error ) {
 		// Embed API 404s if the URL cannot be embedded, so we have to catch the error from the apiRequest here.
diff --git a/packages/core-data/src/test/resolvers.js b/packages/core-data/src/test/resolvers.js
index 414babdf9cbdb4..f14ec8abeb50c3 100644
--- a/packages/core-data/src/test/resolvers.js
+++ b/packages/core-data/src/test/resolvers.js
@@ -6,7 +6,7 @@ import apiFetch from '@wordpress/api-fetch';
 /**
  * External dependencies
  */
-import { stringify } from 'querystring';
+import { addQueryArgs } from '@wordpress/url';
 
 /**
  * Internal dependencies
@@ -119,7 +119,7 @@ describe( 'getEmbedPreview', () => {
 
 	beforeAll( () => {
 		apiFetch.mockImplementation( ( options ) => {
-			if ( options.path === `/oembed/1.0/proxy?${ stringify( { url: EMBEDDABLE_URL } ) }` ) {
+			if ( options.path === addQueryArgs( '/oembed/1.0/proxy', { url: EMBEDDABLE_URL } ) ) {
 				return Promise.resolve( SUCCESSFUL_EMBED_RESPONSE );
 			}
 			throw 404;

From 796bf22b48b449c033485248a3352e1c3f1ba153 Mon Sep 17 00:00:00 2001
From: Nicola Heald <nicola@notnowlewis.com>
Date: Fri, 3 Aug 2018 10:35:28 +0100
Subject: [PATCH 07/12] Get rid of a lot of the embed edit component's state

---
 core-blocks/embed/index.js | 26 +++++++++-----------------
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/core-blocks/embed/index.js b/core-blocks/embed/index.js
index 010a8ae5d3202a..04bf2356723619 100644
--- a/core-blocks/embed/index.js
+++ b/core-blocks/embed/index.js
@@ -50,11 +50,7 @@ export function getEmbedEdit( title, icon ) {
 			this.processPreview = this.processPreview.bind( this );
 
 			this.state = {
-				html: this.props.preview ? this.props.preview.html : '',
-				type: this.props.attributes.type,
-				error: false,
 				fetching: !! this.props.attributes.url && ! this.props.preview,
-				providerName: '',
 				url: this.props.attributes.url,
 			};
 
@@ -103,7 +99,7 @@ export function getEmbedEdit( title, icon ) {
 				return;
 			}
 			setAttributes( { url } );
-			this.setState( { fetching: true, error: false } );
+			this.setState( { fetching: true } );
 		}
 
 		processPreview() {
@@ -111,9 +107,7 @@ export function getEmbedEdit( title, icon ) {
 			const { url } = this.props.attributes;
 
 			if ( previewIsFallback ) {
-				// If the preview is false (not falsey, but actually false) then the embed request failed,
-				// so we cannot embed it.
-				this.setState( { fetching: false, error: true } );
+				this.setState( { fetching: false } );
 				return;
 			}
 
@@ -150,11 +144,7 @@ export function getEmbedEdit( title, icon ) {
 				}
 			}
 
-			if ( html ) {
-				this.setState( { html, type, providerNameSlug } );
-				setAttributes( { type, providerNameSlug } );
-			} else if ( 'photo' === type ) {
-				this.setState( { html: this.getPhotoHtml( preview ), type, providerNameSlug } );
+			if ( html || 'photo' === type ) {
 				setAttributes( { type, providerNameSlug } );
 			}
 			this.setState( { fetching: false } );
@@ -165,9 +155,9 @@ export function getEmbedEdit( title, icon ) {
 		}
 
 		render() {
-			const { html, type, error, fetching, url } = this.state;
+			const { fetching, url } = this.state;
 			const { caption } = this.props.attributes;
-			const { setAttributes, isSelected, className } = this.props;
+			const { setAttributes, isSelected, className, preview, previewIsFallback } = this.props;
 			const controls = (
 				<BlockControls>
 					<Toolbar>
@@ -190,7 +180,7 @@ export function getEmbedEdit( title, icon ) {
 				);
 			}
 
-			if ( ! html ) {
+			if ( ! preview || previewIsFallback ) {
 				// translators: %s: type of embed e.g: "YouTube", "Twitter", etc. "Embed" is used when no specific type exists
 				const label = sprintf( __( '%s URL' ), title );
 
@@ -209,12 +199,14 @@ export function getEmbedEdit( title, icon ) {
 								type="submit">
 								{ __( 'Embed' ) }
 							</Button>
-							{ error && <p className="components-placeholder__error">{ __( 'Sorry, we could not embed that content.' ) }</p> }
+							{ previewIsFallback && <p className="components-placeholder__error">{ __( 'Sorry, we could not embed that content.' ) }</p> }
 						</form>
 					</Placeholder>
 				);
 			}
 
+			const { type } = preview;
+			const html = 'photo' === type ? this.getPhotoHtml( preview ) : preview.html;
 			const parsedUrl = parse( url );
 			const cannotPreview = includes( HOSTS_NO_PREVIEWS, parsedUrl.host.replace( /^www\./, '' ) );
 			// translators: %s: host providing embed content e.g: www.youtube.com

From 741b2afdd073333e535ffd56ef765902065117a9 Mon Sep 17 00:00:00 2001
From: Nicola Heald <nicola@notnowlewis.com>
Date: Fri, 3 Aug 2018 10:49:49 +0100
Subject: [PATCH 08/12] Fixed editing url state

---
 core-blocks/embed/index.js | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/core-blocks/embed/index.js b/core-blocks/embed/index.js
index 04bf2356723619..355113ae15e972 100644
--- a/core-blocks/embed/index.js
+++ b/core-blocks/embed/index.js
@@ -50,6 +50,7 @@ export function getEmbedEdit( title, icon ) {
 			this.processPreview = this.processPreview.bind( this );
 
 			this.state = {
+				editingURL: false,
 				fetching: !! this.props.attributes.url && ! this.props.preview,
 				url: this.props.attributes.url,
 			};
@@ -87,6 +88,7 @@ export function getEmbedEdit( title, icon ) {
 			}
 			const { url } = this.state;
 			const { setAttributes } = this.props;
+			this.setState( { editingURL: false } );
 			if ( url === this.props.attributes.url ) {
 				if ( this.props.preview ) {
 					// Form has been submitted without changing the url, and we already have a preview,
@@ -107,7 +109,7 @@ export function getEmbedEdit( title, icon ) {
 			const { url } = this.props.attributes;
 
 			if ( previewIsFallback ) {
-				this.setState( { fetching: false } );
+				this.setState( { fetching: false, editingURL: true } );
 				return;
 			}
 
@@ -151,17 +153,17 @@ export function getEmbedEdit( title, icon ) {
 		}
 
 		switchBackToURLInput() {
-			this.setState( { html: undefined } );
+			this.setState( { editingURL: true } );
 		}
 
 		render() {
-			const { fetching, url } = this.state;
-			const { caption } = this.props.attributes;
+			const { fetching, url, editingURL } = this.state;
+			const { caption, type } = this.props.attributes;
 			const { setAttributes, isSelected, className, preview, previewIsFallback } = this.props;
 			const controls = (
 				<BlockControls>
 					<Toolbar>
-						{ ( html && <IconButton
+						{ ( preview && ! previewIsFallback && <IconButton
 							className="components-toolbar__control"
 							label={ __( 'Edit URL' ) }
 							icon="edit"
@@ -180,7 +182,7 @@ export function getEmbedEdit( title, icon ) {
 				);
 			}
 
-			if ( ! preview || previewIsFallback ) {
+			if ( ! preview || previewIsFallback || editingURL ) {
 				// translators: %s: type of embed e.g: "YouTube", "Twitter", etc. "Embed" is used when no specific type exists
 				const label = sprintf( __( '%s URL' ), title );
 
@@ -205,7 +207,6 @@ export function getEmbedEdit( title, icon ) {
 				);
 			}
 
-			const { type } = preview;
 			const html = 'photo' === type ? this.getPhotoHtml( preview ) : preview.html;
 			const parsedUrl = parse( url );
 			const cannotPreview = includes( HOSTS_NO_PREVIEWS, parsedUrl.host.replace( /^www\./, '' ) );

From 35db192b32be0d3ee8c61369338c7a777f424342 Mon Sep 17 00:00:00 2001
From: Nicola Heald <nicola@notnowlewis.com>
Date: Mon, 6 Aug 2018 12:03:12 +0100
Subject: [PATCH 09/12] Fix previous merge mess-up

---
 packages/core-data/src/resolvers.js | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/packages/core-data/src/resolvers.js b/packages/core-data/src/resolvers.js
index b43657b990fe93..7d939de0fac4a6 100644
--- a/packages/core-data/src/resolvers.js
+++ b/packages/core-data/src/resolvers.js
@@ -9,11 +9,6 @@ import { find } from 'lodash';
 import apiFetch from '@wordpress/api-fetch';
 import { addQueryArgs } from '@wordpress/url';
 
-/**
- * External dependencies
- */
-import { addQueryArgs } from '@wordpress/url';
-
 /**
  * Internal dependencies
  */

From 1a451d8dba5c8884059206f9f1da05aa0ff2d644 Mon Sep 17 00:00:00 2001
From: Nicola Heald <nicola@notnowlewis.com>
Date: Tue, 7 Aug 2018 10:48:22 +0100
Subject: [PATCH 10/12] Trying to make fetching happen

---
 core-blocks/embed/index.js          | 18 +++++++-----------
 packages/core-data/src/selectors.js | 12 ++++++++++++
 2 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/core-blocks/embed/index.js b/core-blocks/embed/index.js
index 355113ae15e972..36adfbb9bc56a9 100644
--- a/core-blocks/embed/index.js
+++ b/core-blocks/embed/index.js
@@ -14,7 +14,7 @@ import { Component, renderToString } from '@wordpress/element';
 import { Button, Placeholder, Spinner, SandBox, IconButton, Toolbar } from '@wordpress/components';
 import { createBlock } from '@wordpress/blocks';
 import { RichText, BlockControls } from '@wordpress/editor';
-import { withSelect } from '@wordpress/data';
+import { withSelect, isRequesting } from '@wordpress/data';
 
 /**
  * Internal dependencies
@@ -51,7 +51,6 @@ export function getEmbedEdit( title, icon ) {
 
 			this.state = {
 				editingURL: false,
-				fetching: !! this.props.attributes.url && ! this.props.preview,
 				url: this.props.attributes.url,
 			};
 
@@ -96,12 +95,8 @@ export function getEmbedEdit( title, icon ) {
 					// clicks embed.
 					this.processPreview();
 				}
-				// Don't change anything, otherwise we go into the 'fetching' state but never
-				// get new props, because the url has not changed.
-				return;
 			}
 			setAttributes( { url } );
-			this.setState( { fetching: true } );
 		}
 
 		processPreview() {
@@ -109,7 +104,7 @@ export function getEmbedEdit( title, icon ) {
 			const { url } = this.props.attributes;
 
 			if ( previewIsFallback ) {
-				this.setState( { fetching: false, editingURL: true } );
+				this.setState( { editingURL: true } );
 				return;
 			}
 
@@ -149,7 +144,6 @@ export function getEmbedEdit( title, icon ) {
 			if ( html || 'photo' === type ) {
 				setAttributes( { type, providerNameSlug } );
 			}
-			this.setState( { fetching: false } );
 		}
 
 		switchBackToURLInput() {
@@ -157,9 +151,9 @@ export function getEmbedEdit( title, icon ) {
 		}
 
 		render() {
-			const { fetching, url, editingURL } = this.state;
+			const { url, editingURL } = this.state;
 			const { caption, type } = this.props.attributes;
-			const { setAttributes, isSelected, className, preview, previewIsFallback } = this.props;
+			const { fetching, setAttributes, isSelected, className, preview, previewIsFallback } = this.props;
 			const controls = (
 				<BlockControls>
 					<Toolbar>
@@ -288,12 +282,14 @@ function getEmbedBlockSettings( { title, description, icon, category = 'embed',
 			withSelect( ( select, ownProps ) => {
 				const { url } = ownProps.attributes;
 				const core = select( 'core' );
-				const { getEmbedPreview, isPreviewEmbedFallback } = core;
+				const { getEmbedPreview, isPreviewEmbedFallback, isRequestingEmbedPreview } = core;
 				const preview = getEmbedPreview( url );
 				const previewIsFallback = isPreviewEmbedFallback( url );
+				const fetching = isRequestingEmbedPreview( url );
 				return {
 					preview,
 					previewIsFallback,
+					fetching,
 				};
 			} )
 		)( getEmbedEdit( title, icon ) ),
diff --git a/packages/core-data/src/selectors.js b/packages/core-data/src/selectors.js
index 47723b9e114fc9..9e583112e66e2a 100644
--- a/packages/core-data/src/selectors.js
+++ b/packages/core-data/src/selectors.js
@@ -76,6 +76,18 @@ export function isRequestingCategories() {
 	return isResolving( 'getCategories' );
 }
 
+/**
+ * Returns true if a request is in progress for embed preview data, or false
+ * otherwise.
+ *
+ * @param {string} url URL the preview would be for.
+ *
+ * @return {boolean} Whether a request is in progress for an embed preview.
+ */
+export function isRequestingEmbedPreview( state, url ) {
+	return isResolving( 'getEmbedPreview', url );
+}
+
 /**
  * Returns all available authors.
  *

From ad831ec7bf038116711ead91157ea87c07f40e5f Mon Sep 17 00:00:00 2001
From: Riad Benguella <benguella@gmail.com>
Date: Tue, 7 Aug 2018 11:12:41 +0100
Subject: [PATCH 11/12] Fix isResolving core data selector

---
 packages/core-data/src/selectors.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/core-data/src/selectors.js b/packages/core-data/src/selectors.js
index 9e583112e66e2a..6b9dc0bfee4eb9 100644
--- a/packages/core-data/src/selectors.js
+++ b/packages/core-data/src/selectors.js
@@ -25,7 +25,7 @@ import { getQueriedItems } from './queried-data';
  * @return {boolean} Whether resolution is in progress.
  */
 function isResolving( selectorName, ...args ) {
-	return select( 'core/data' ).isResolving( REDUCER_KEY, selectorName, ...args );
+	return select( 'core/data' ).isResolving( REDUCER_KEY, selectorName, args );
 }
 
 /**

From 142d263499d063db01d3d43bed03b1d23c912fe7 Mon Sep 17 00:00:00 2001
From: Nicola Heald <nicola@notnowlewis.com>
Date: Tue, 7 Aug 2018 14:03:24 +0100
Subject: [PATCH 12/12] Split `processPreview` into methods with distinct
 responsibilities

---
 core-blocks/embed/index.js          | 80 ++++++++++++++++++-----------
 packages/core-data/src/selectors.js |  3 +-
 2 files changed, 51 insertions(+), 32 deletions(-)

diff --git a/core-blocks/embed/index.js b/core-blocks/embed/index.js
index 36adfbb9bc56a9..8e5f72ff38869c 100644
--- a/core-blocks/embed/index.js
+++ b/core-blocks/embed/index.js
@@ -14,7 +14,7 @@ import { Component, renderToString } from '@wordpress/element';
 import { Button, Placeholder, Spinner, SandBox, IconButton, Toolbar } from '@wordpress/components';
 import { createBlock } from '@wordpress/blocks';
 import { RichText, BlockControls } from '@wordpress/editor';
-import { withSelect, isRequesting } from '@wordpress/data';
+import { withSelect } from '@wordpress/data';
 
 /**
  * Internal dependencies
@@ -47,16 +47,15 @@ export function getEmbedEdit( title, icon ) {
 
 			this.switchBackToURLInput = this.switchBackToURLInput.bind( this );
 			this.setUrl = this.setUrl.bind( this );
-			this.processPreview = this.processPreview.bind( this );
+			this.maybeSwitchBlock = this.maybeSwitchBlock.bind( this );
+			this.setAttributesFromPreview = this.setAttributesFromPreview.bind( this );
 
 			this.state = {
 				editingURL: false,
 				url: this.props.attributes.url,
 			};
 
-			if ( this.props.preview ) {
-				this.processPreview();
-			}
+			this.maybeSwitchBlock();
 		}
 
 		componentWillUnmount() {
@@ -69,8 +68,18 @@ export function getEmbedEdit( title, icon ) {
 			const hadPreview = undefined !== prevProps.preview;
 			// We had a preview, and the URL was edited, and the new URL already has a preview fetched.
 			const switchedPreview = this.props.preview && this.props.attributes.url !== prevProps.attributes.url;
+			const switchedURL = this.props.attributes.url !== prevProps.attributes.url;
+
+			if ( switchedURL && this.maybeSwitchBlock() ) {
+				return;
+			}
+
 			if ( ( hasPreview && ! hadPreview ) || switchedPreview ) {
-				this.processPreview();
+				if ( this.props.previewIsFallback ) {
+					this.setState( { editingURL: true } );
+					return;
+				}
+				this.setAttributesFromPreview();
 			}
 		}
 
@@ -88,28 +97,21 @@ export function getEmbedEdit( title, icon ) {
 			const { url } = this.state;
 			const { setAttributes } = this.props;
 			this.setState( { editingURL: false } );
-			if ( url === this.props.attributes.url ) {
-				if ( this.props.preview ) {
-					// Form has been submitted without changing the url, and we already have a preview,
-					// so process it again. Happens when the user clicks edit and then immediately
-					// clicks embed.
-					this.processPreview();
-				}
-			}
 			setAttributes( { url } );
 		}
 
-		processPreview() {
-			const { setAttributes, preview, previewIsFallback } = this.props;
+		/***
+		 * Maybe switches to a different embed block type, based on the URL
+		 * and the HTML in the preview.
+		 *
+		 * @return {boolean} Whether the block was switched.
+		 */
+		maybeSwitchBlock() {
+			const { preview } = this.props;
 			const { url } = this.props.attributes;
 
-			if ( previewIsFallback ) {
-				this.setState( { editingURL: true } );
-				return;
-			}
-
-			if ( undefined === url ) {
-				return;
+			if ( ! url ) {
+				return false;
 			}
 
 			const matchingBlock = findBlock( url );
@@ -120,10 +122,32 @@ export function getEmbedEdit( title, icon ) {
 				// At this point, we have discovered a more suitable block for this url, so transform it.
 				if ( this.props.name !== matchingBlock ) {
 					this.props.onReplace( createBlock( matchingBlock, { url } ) );
-					return;
+					return true;
+				}
+			}
+
+			if ( preview ) {
+				const { html } = preview;
+
+				// This indicates it's a WordPress embed, there aren't a set of URL patterns we can use to match WordPress URLs.
+				if ( includes( html, 'class="wp-embedded-content" data-secret' ) ) {
+					// If this is not the WordPress embed block, transform it into one.
+					if ( this.props.name !== 'core-embed/wordpress' ) {
+						this.props.onReplace( createBlock( 'core-embed/wordpress', { url } ) );
+						return true;
+					}
 				}
 			}
 
+			return false;
+		}
+
+		/***
+		 * Sets block attributes based on the preview data.
+		 */
+		setAttributesFromPreview() {
+			const { setAttributes, preview } = this.props;
+
 			// Some plugins only return HTML with no type info, so default this to 'rich'.
 			let { type = 'rich' } = preview;
 			// If we got a provider name from the API, use it for the slug, otherwise we use the title,
@@ -131,14 +155,8 @@ export function getEmbedEdit( title, icon ) {
 			const { html, provider_name: providerName } = preview;
 			const providerNameSlug = kebabCase( toLower( '' !== providerName ? providerName : title ) );
 
-			// This indicates it's a WordPress embed, there aren't a set of URL patterns we can use to match WordPress URLs.
 			if ( includes( html, 'class="wp-embedded-content" data-secret' ) ) {
 				type = 'wp-embed';
-				// If this is not the WordPress embed block, transform it into one.
-				if ( this.props.name !== 'core-embed/wordpress' ) {
-					this.props.onReplace( createBlock( 'core-embed/wordpress', { url } ) );
-					return;
-				}
 			}
 
 			if ( html || 'photo' === type ) {
@@ -285,7 +303,7 @@ function getEmbedBlockSettings( { title, description, icon, category = 'embed',
 				const { getEmbedPreview, isPreviewEmbedFallback, isRequestingEmbedPreview } = core;
 				const preview = getEmbedPreview( url );
 				const previewIsFallback = isPreviewEmbedFallback( url );
-				const fetching = isRequestingEmbedPreview( url );
+				const fetching = undefined !== url && isRequestingEmbedPreview( url );
 				return {
 					preview,
 					previewIsFallback,
diff --git a/packages/core-data/src/selectors.js b/packages/core-data/src/selectors.js
index 6b9dc0bfee4eb9..1e0fd66f63f11a 100644
--- a/packages/core-data/src/selectors.js
+++ b/packages/core-data/src/selectors.js
@@ -80,7 +80,8 @@ export function isRequestingCategories() {
  * Returns true if a request is in progress for embed preview data, or false
  * otherwise.
  *
- * @param {string} url URL the preview would be for.
+ * @param {Object} state Data state.
+ * @param {string} url   URL the preview would be for.
  *
  * @return {boolean} Whether a request is in progress for an embed preview.
  */