Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move embed API call out of block and into data #6678

Merged
merged 15 commits into from
Aug 7, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
441 changes: 240 additions & 201 deletions core-blocks/embed/index.js

Large diffs are not rendered by default.

11 changes: 8 additions & 3 deletions core-blocks/embed/test/index.js
Original file line number Diff line number Diff line change
@@ -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();
} );
Expand Down
17 changes: 17 additions & 0 deletions packages/core-data/src/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,3 +96,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,
};
}
21 changes: 21 additions & 0 deletions packages/core-data/src/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,10 +197,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,
} );
17 changes: 17 additions & 0 deletions packages/core-data/src/resolvers.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
receiveUserQuery,
receiveEntityRecords,
receiveThemeSupportsFromIndex,
receiveEmbedPreview,
} from './actions';
import { getKindEntities } from './entities';

Expand Down Expand Up @@ -84,3 +85,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 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.
yield receiveEmbedPreview( url, false );
}
}
48 changes: 47 additions & 1 deletion packages/core-data/src/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
}

/**
Expand Down Expand Up @@ -76,6 +76,19 @@ export function isRequestingCategories() {
return isResolving( 'getCategories' );
}

/**
* Returns true if a request is in progress for embed preview data, or false
* otherwise.
*
* @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.
*/
export function isRequestingEmbedPreview( state, url ) {
return isResolving( 'getEmbedPreview', url );
}

/**
* Returns all available authors.
*
Expand Down Expand Up @@ -171,3 +184,36 @@ export function getEntityRecords( state, kind, name, query ) {
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, otherwise, the preview fetched from the embed preview API.
*/
export function getEmbedPreview( state, url ) {
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 ( ! preview ) {
return false;
}
return preview.html === oEmbedLinkCheck;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems fragile to test the html. WP can decide to change the fallback right? Can't we add an explicit attribute to the embed endpoint when it's a fallback: isFallback: true?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The embed fallbacks are currently done by a filter in embed_maybe_make_link, and that only deals with HTML, you can't change any of the data structure. We'd have to change the core embed code itself to do that.

}
23 changes: 22 additions & 1 deletion packages/core-data/src/test/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -96,3 +96,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 },
} );
} );
} );
37 changes: 35 additions & 2 deletions packages/core-data/src/test/resolvers.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,16 @@
*/
import apiFetch from '@wordpress/api-fetch';

/**
* External dependencies
*/
import { addQueryArgs } from '@wordpress/url';

/**
* 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', () => jest.fn() );

Expand Down Expand Up @@ -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( () => {
apiFetch.mockImplementation( ( options ) => {
if ( options.path === addQueryArgs( '/oembed/1.0/proxy', { 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 ) );
} );
} );
35 changes: 34 additions & 1 deletion packages/core-data/src/test/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,14 @@ import deepFreeze from 'deep-freeze';
/**
* Internal dependencies
*/
import { getTerms, isRequestingCategories, getEntityRecord, getEntityRecords } from '../selectors';
import {
getTerms,
isRequestingCategories,
getEntityRecord,
getEntityRecords,
getEmbedPreview,
isPreviewEmbedFallback,
} from '../selectors';
import { select } from '@wordpress/data';

jest.mock( '@wordpress/data', () => {
Expand Down Expand Up @@ -144,3 +151,29 @@ 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 } );
} );
} );

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( isPreviewEmbedFallback( state, 'http://example.com/' ) ).toEqual( true );
} );
} );