From f1d90737d36946add9431078ab58741e38e53b44 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Wed, 22 Aug 2018 16:10:01 +0800 Subject: [PATCH 1/9] Add MediaUploadCheck component and unit tests - Hardcode hasUploadPermissions value to true for now until implementation of server-side and redux state has been finalised Co-authored-by: imath --- packages/editor/src/components/index.js | 1 + .../src/components/media-upload/check.js | 10 +++ .../src/components/media-upload/test/check.js | 62 +++++++++++++++++++ 3 files changed, 73 insertions(+) create mode 100644 packages/editor/src/components/media-upload/check.js create mode 100644 packages/editor/src/components/media-upload/test/check.js diff --git a/packages/editor/src/components/index.js b/packages/editor/src/components/index.js index 5505e6f67f6e8..d8e42ae2eec1b 100644 --- a/packages/editor/src/components/index.js +++ b/packages/editor/src/components/index.js @@ -22,6 +22,7 @@ export { default as RichText } from './rich-text'; export { default as ServerSideRender } from './server-side-render'; export { default as MediaPlaceholder } from './media-placeholder'; export { default as MediaUpload } from './media-upload'; +export { default as MediaUploadCheck } from './media-upload/check'; export { default as URLInput } from './url-input'; export { default as URLInputButton } from './url-input/button'; diff --git a/packages/editor/src/components/media-upload/check.js b/packages/editor/src/components/media-upload/check.js new file mode 100644 index 0000000000000..80ad2b20d2c87 --- /dev/null +++ b/packages/editor/src/components/media-upload/check.js @@ -0,0 +1,10 @@ +// Hardcode hasUploadPermissions to true for now - in future this value will be +// correctly populated from application state. +// See github issue for more information: +// https://github.com/WordPress/gutenberg/issues/3672 +export function MediaUploadCheck( { hasUploadPermissions = true, fallback, children } ) { + const optionalFallback = fallback || null; + return hasUploadPermissions ? children : optionalFallback; +} + +export default MediaUploadCheck; diff --git a/packages/editor/src/components/media-upload/test/check.js b/packages/editor/src/components/media-upload/test/check.js new file mode 100644 index 0000000000000..0fb81670965bc --- /dev/null +++ b/packages/editor/src/components/media-upload/test/check.js @@ -0,0 +1,62 @@ +/** + * External dependencies + */ +import { shallow } from 'enzyme'; + +/** + * Internal dependencies + */ +import { MediaUploadCheck } from '../check.js'; + +describe( 'MediaUploadCheck', () => { + it( 'renders its child if hasUploadPermissions is true', () => { + const wrapper = shallow( + +
Child
+ + ); + + expect( wrapper.children() ).toHaveLength( 1 ); + expect( wrapper.children().first().text() ).toBe( 'Child' ); + } ); + + it( 'renders its child if hasUploadPermissions is true and a fallback is provided', () => { + const fallback = ( +
Fallback
+ ); + + const wrapper = shallow( + +
Child
+ + ); + + expect( wrapper.children() ).toHaveLength( 1 ); + expect( wrapper.children().first().text() ).toBe( 'Child' ); + } ); + + it( 'renders the fallback if hasUploadPermissions is false and a fallback is provided', () => { + const fallback = ( +
Fallback
+ ); + + const wrapper = shallow( + +
Child
+ + ); + + expect( wrapper.children() ).toHaveLength( 1 ); + expect( wrapper.children().first().text() ).toBe( 'Fallback' ); + } ); + + it( 'renders nothing if hasUploadPermissions is false and no fallback is provided', () => { + const wrapper = shallow( + +
Child
+ + ); + + expect( wrapper.children() ).toHaveLength( 0 ); + } ); +} ); From 592137d04206730953d06b7347128abd7ed4fc34 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Wed, 22 Aug 2018 16:30:26 +0800 Subject: [PATCH 2/9] Use MediaUploadCheck in MediaPlaceholder MediaUploadCheck component will conditionally show/hide parts of the UI related to file uploads dependent on whether the user has the capability to upload files. Co-authored-by: imath --- .../src/components/media-placeholder/index.js | 55 ++++++++++--------- 1 file changed, 30 insertions(+), 25 deletions(-) diff --git a/packages/editor/src/components/media-placeholder/index.js b/packages/editor/src/components/media-placeholder/index.js index b0912c46c9184..73a3056fa2ee3 100644 --- a/packages/editor/src/components/media-placeholder/index.js +++ b/packages/editor/src/components/media-placeholder/index.js @@ -20,6 +20,7 @@ import { Component } from '@wordpress/element'; * Internal dependencies */ import MediaUpload from '../media-upload'; +import MediaUploadCheck from '../media-upload/check'; import { mediaUpload } from '../../utils/'; class MediaPlaceholder extends Component { @@ -96,10 +97,12 @@ class MediaPlaceholder extends Component { className={ classnames( 'editor-media-placeholder', className ) } notices={ notices } > - + + + { onSelectURL && (
) } - - { __( 'Upload' ) } - - ( - - ) } - /> + + + { __( 'Upload' ) } + + ( + + ) } + /> + ); } From 36d7d58c2cab1a1496cd09214753a1eae01ae120 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Wed, 22 Aug 2018 16:52:58 +0800 Subject: [PATCH 3/9] Simplify rendering logic of PostFeaturedImage Use early return when no featuredImageId is set. Reduce boolean checks around component rendering. Co-authored-by: imath --- .../components/post-featured-image/index.js | 88 ++++++++++--------- 1 file changed, 46 insertions(+), 42 deletions(-) diff --git a/packages/editor/src/components/post-featured-image/index.js b/packages/editor/src/components/post-featured-image/index.js index 849e49ec2d713..632bcdd6cf080 100644 --- a/packages/editor/src/components/post-featured-image/index.js +++ b/packages/editor/src/components/post-featured-image/index.js @@ -17,12 +17,33 @@ import { withSelect, withDispatch } from '@wordpress/data'; */ import PostFeaturedImageCheck from './check'; import MediaUpload from '../media-upload'; +import MediaUploadCheck from '../media-upload/check'; // Used when labels from post type were not yet loaded or when they are not present. const DEFAULT_SET_FEATURE_IMAGE_LABEL = __( 'Set featured image' ); const DEFAULT_REMOVE_FEATURE_IMAGE_LABEL = __( 'Remove image' ); function PostFeaturedImage( { currentPostId, featuredImageId, onUpdateImage, onRemoveImage, media, postType } ) { + if ( ! featuredImageId ) { + return ( + +
+ ( + + ) } + /> +
+ + ); + } + const postLabel = get( postType, [ 'labels' ], {} ); let mediaWidth, mediaHeight, mediaSourceUrl; @@ -42,60 +63,43 @@ function PostFeaturedImage( { currentPostId, featuredImageId, onUpdateImage, onR return (
- { !! featuredImageId && - ( - - ) } - /> - } - { !! featuredImageId && media && ! media.isLoading && ( - ) } /> + { media && ! media.isLoading && + ( + + ) } + /> } - { ! featuredImageId && -
- ( - - ) } - /> -
- } - { !! featuredImageId && + - } +
); From 4d234ed91a4d6c17bbd593c0c04d2e2190754e53 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Wed, 22 Aug 2018 17:25:24 +0800 Subject: [PATCH 4/9] Hard code hasUploadPermissions in redux state instead of MediaUploadCheck component Co-authored-by: imath --- docs/data/data-core.md | 8 ++++++++ packages/core-data/src/selectors.js | 13 +++++++++++++ .../editor/src/components/media-upload/check.js | 17 +++++++++++------ 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/docs/data/data-core.md b/docs/data/data-core.md index 0fe8caa27d6f2..79bf5e3a37d54 100644 --- a/docs/data/data-core.md +++ b/docs/data/data-core.md @@ -136,6 +136,14 @@ get back from the oEmbed preview API. Is the preview for the URL an oEmbed link fallback. +### hasUploadPermissions + +Return whether the user has media upload permissions. + +*Returns* + +Does the user have media upload permissions? + ## Actions ### receiveUserQuery diff --git a/packages/core-data/src/selectors.js b/packages/core-data/src/selectors.js index 57e8a758ca392..25ac90c82e7dd 100644 --- a/packages/core-data/src/selectors.js +++ b/packages/core-data/src/selectors.js @@ -169,3 +169,16 @@ export function isPreviewEmbedFallback( state, url ) { } return preview.html === oEmbedLinkCheck; } + +/* + * Return whether the user has media upload permissions. + * + * @return {boolean} Does the user have media upload permissions? + */ +export function hasUploadPermissions() { + // Hardcode hasUploadPermissions to true for now - in future this value will be + // correctly populated from a REST API call that will store user permissions + // See github issue for more information: + // https://github.com/WordPress/gutenberg/issues/3672 + return true; +} diff --git a/packages/editor/src/components/media-upload/check.js b/packages/editor/src/components/media-upload/check.js index 80ad2b20d2c87..df9bab268a1d5 100644 --- a/packages/editor/src/components/media-upload/check.js +++ b/packages/editor/src/components/media-upload/check.js @@ -1,10 +1,15 @@ -// Hardcode hasUploadPermissions to true for now - in future this value will be -// correctly populated from application state. -// See github issue for more information: -// https://github.com/WordPress/gutenberg/issues/3672 -export function MediaUploadCheck( { hasUploadPermissions = true, fallback, children } ) { +/** + * WordPress dependencies + */ +import { withSelect } from '@wordpress/data'; + +export function MediaUploadCheck( { hasUploadPermissions, fallback, children } ) { const optionalFallback = fallback || null; return hasUploadPermissions ? children : optionalFallback; } -export default MediaUploadCheck; +export default withSelect( ( select ) => { + return { + hasUploadPermissions: select( 'core' ).hasUploadPermissions(), + }; +} )( MediaUploadCheck ); From 27fa4fc0ef96ec381d5c5e33fb42c2b02ea1f1ee Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Wed, 22 Aug 2018 18:42:24 +0800 Subject: [PATCH 5/9] Implement MediaUploadCheck wherever a user might interact with file uploads Co-authored-by: imath --- .../block-library/src/cover-image/index.js | 33 ++--- packages/block-library/src/file/edit.js | 33 ++--- packages/block-library/src/gallery/edit.js | 63 ++++----- packages/block-library/src/image/edit.js | 29 +++-- .../src/components/media-placeholder/index.js | 25 +++- .../components/post-featured-image/index.js | 123 +++++++++++------- 6 files changed, 181 insertions(+), 125 deletions(-) diff --git a/packages/block-library/src/cover-image/index.js b/packages/block-library/src/cover-image/index.js index 63000909d5806..56080ce65eb12 100644 --- a/packages/block-library/src/cover-image/index.js +++ b/packages/block-library/src/cover-image/index.js @@ -17,6 +17,7 @@ import { BlockAlignmentToolbar, MediaPlaceholder, MediaUpload, + MediaUploadCheck, AlignmentToolbar, RichText, } from '@wordpress/editor'; @@ -154,21 +155,23 @@ export const settings = { setAttributes( { contentAlign: nextAlign } ); } } /> - - ( - - ) } - /> - + + + ( + + ) } + /> + + { !! url && ( diff --git a/packages/block-library/src/file/edit.js b/packages/block-library/src/file/edit.js index 9a7eb417345b6..e70933e4d396f 100644 --- a/packages/block-library/src/file/edit.js +++ b/packages/block-library/src/file/edit.js @@ -18,6 +18,7 @@ import { withSelect } from '@wordpress/data'; import { Component, Fragment } from '@wordpress/element'; import { MediaUpload, + MediaUploadCheck, MediaPlaceholder, BlockControls, RichText, @@ -171,21 +172,23 @@ class FileEdit extends Component { } } /> - - ( - - ) } - /> - + + + ( + + ) } + /> + +
diff --git a/packages/block-library/src/gallery/edit.js b/packages/block-library/src/gallery/edit.js index 5190799474f11..4a592da36f5f5 100644 --- a/packages/block-library/src/gallery/edit.js +++ b/packages/block-library/src/gallery/edit.js @@ -22,6 +22,7 @@ import { import { BlockControls, MediaUpload, + MediaUploadCheck, MediaPlaceholder, InspectorControls, mediaUpload, @@ -165,23 +166,25 @@ class GalleryEdit extends Component { const controls = ( { !! images.length && ( - - img.id ) } - render={ ( { open } ) => ( - - ) } - /> - + + + img.id ) } + render={ ( { open } ) => ( + + ) } + /> + + ) } ); @@ -252,18 +255,20 @@ class GalleryEdit extends Component { ) ) } { isSelected && -
  • - - { __( 'Upload an image' ) } - -
    • + +
  • + + { __( 'Upload an image' ) } + +
    • +     } diff --git a/packages/block-library/src/image/edit.js b/packages/block-library/src/image/edit.js index 77bd51c8c8174..d8b5c233fffc6 100644 --- a/packages/block-library/src/image/edit.js +++ b/packages/block-library/src/image/edit.js @@ -35,6 +35,7 @@ import { InspectorControls, MediaPlaceholder, MediaUpload, + MediaUploadCheck, BlockAlignmentToolbar, mediaUpload, } from '@wordpress/editor'; @@ -219,19 +220,21 @@ class ImageEdit extends Component { /> - ( - - ) } - /> + + ( + + ) } + /> + ); diff --git a/packages/editor/src/components/media-placeholder/index.js b/packages/editor/src/components/media-placeholder/index.js index 73a3056fa2ee3..b961013f6982f 100644 --- a/packages/editor/src/components/media-placeholder/index.js +++ b/packages/editor/src/components/media-placeholder/index.js @@ -15,6 +15,7 @@ import { } from '@wordpress/components'; import { __, sprintf } from '@wordpress/i18n'; import { Component } from '@wordpress/element'; +import { withSelect } from '@wordpress/data'; /** * Internal dependencies @@ -23,7 +24,7 @@ import MediaUpload from '../media-upload'; import MediaUploadCheck from '../media-upload/check'; import { mediaUpload } from '../../utils/'; -class MediaPlaceholder extends Component { +export class MediaPlaceholder extends Component { constructor() { super( ...arguments ); this.state = { @@ -86,14 +87,24 @@ class MediaPlaceholder extends Component { onHTMLDrop = noop, multiple = false, notices, + hasUploadPermissions, } = this.props; + let instructions; + if ( hasUploadPermissions ) { + instructions = sprintf( __( 'Drag %s, upload a new one, or select a file from your library.' ), labels.name ); + } else if ( onSelectURL ) { + instructions = sprintf( __( 'Given your current role, you can only link %s, you cannot upload.' ), labels.name ); + } else { + instructions = __( 'To edit this block, you need permission to upload media.' ); + } + return ( @@ -148,4 +159,12 @@ class MediaPlaceholder extends Component { } } -export default MediaPlaceholder; +export default withSelect( ( select ) => { + const { + hasUploadPermissions, + } = select( 'core' ); + + return { + hasUploadPermissions: hasUploadPermissions(), + }; +} )( MediaPlaceholder ); diff --git a/packages/editor/src/components/post-featured-image/index.js b/packages/editor/src/components/post-featured-image/index.js index 632bcdd6cf080..47fb7c44a3fe9 100644 --- a/packages/editor/src/components/post-featured-image/index.js +++ b/packages/editor/src/components/post-featured-image/index.js @@ -11,6 +11,7 @@ import { applyFilters } from '@wordpress/hooks'; import { Button, Spinner, ResponsiveWrapper, withFilters } from '@wordpress/components'; import { compose } from '@wordpress/compose'; import { withSelect, withDispatch } from '@wordpress/data'; +import { Fragment } from '@wordpress/element'; /** * Internal dependencies @@ -23,29 +24,45 @@ import MediaUploadCheck from '../media-upload/check'; const DEFAULT_SET_FEATURE_IMAGE_LABEL = __( 'Set featured image' ); const DEFAULT_REMOVE_FEATURE_IMAGE_LABEL = __( 'Remove image' ); +const PostFeaturedImageContainer = ( { mediaUploadCheckFallback, children } ) => ( + + +
    + { children } +
    +     +     +); + function PostFeaturedImage( { currentPostId, featuredImageId, onUpdateImage, onRemoveImage, media, postType } ) { + const postLabel = get( postType, [ 'labels' ], {} ); + if ( ! featuredImageId ) { + const fallback = ( +

    { __( 'To edit the featured image, you need permission to upload media.' ) }

    + ); + return ( - -
    - ( - - ) } - /> -
    -     + + ( + + ) } + /> + ); } - const postLabel = get( postType, [ 'labels' ], {} ); - let mediaWidth, mediaHeight, mediaSourceUrl; if ( media ) { const mediaSize = applyFilters( 'editor.PostFeaturedImage.imageSize', 'post-thumbnail', media.id, currentPostId ); @@ -60,48 +77,54 @@ function PostFeaturedImage( { currentPostId, featuredImageId, onUpdateImage, onR } } + const imagePreview = ( + + { media && + + { + + } + { ! media && } + + ); + return ( - -
    + + ( + + ) } + /> + { media && ! media.isLoading && ( - ) } /> - { media && ! media.isLoading && - ( - - ) } - /> - } - - - -
    -     + } + + + + ); } From de838cce6ece7f0f67278ef3b88b8ffc21e62612 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Wed, 22 Aug 2018 19:17:21 +0800 Subject: [PATCH 6/9] Remove tests that can no longer be run due to use of withSelect in nested component --- .../audio/test/__snapshots__/index.js.snap | 107 ------------------ .../block-library/src/audio/test/index.js | 13 --- .../test/__snapshots__/index.js.snap | 92 --------------- .../src/cover-image/test/index.js | 13 --- .../gallery/test/__snapshots__/index.js.snap | 93 --------------- .../block-library/src/gallery/test/index.js | 13 --- .../video/test/__snapshots__/index.js.snap | 107 ------------------ .../block-library/src/video/test/index.js | 13 --- 8 files changed, 451 deletions(-) delete mode 100644 packages/block-library/src/audio/test/__snapshots__/index.js.snap delete mode 100644 packages/block-library/src/audio/test/index.js delete mode 100644 packages/block-library/src/cover-image/test/__snapshots__/index.js.snap delete mode 100644 packages/block-library/src/cover-image/test/index.js delete mode 100644 packages/block-library/src/gallery/test/__snapshots__/index.js.snap delete mode 100644 packages/block-library/src/gallery/test/index.js delete mode 100644 packages/block-library/src/video/test/__snapshots__/index.js.snap delete mode 100644 packages/block-library/src/video/test/index.js diff --git a/packages/block-library/src/audio/test/__snapshots__/index.js.snap b/packages/block-library/src/audio/test/__snapshots__/index.js.snap deleted file mode 100644 index 71c38018c590f..0000000000000 --- a/packages/block-library/src/audio/test/__snapshots__/index.js.snap +++ /dev/null @@ -1,107 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`core/audio block edit matches snapshot 1`] = ` -
    -
    - - Audio -
    -
    - Drag an audio, upload a new one or select a file from your library. -
    -
    -
    -
    - - - Drop files to upload - -
    -
    -
    - - -
    -
    - - -
    -
    -
    -`; diff --git a/packages/block-library/src/audio/test/index.js b/packages/block-library/src/audio/test/index.js deleted file mode 100644 index a5611a3dd8ce6..0000000000000 --- a/packages/block-library/src/audio/test/index.js +++ /dev/null @@ -1,13 +0,0 @@ -/** - * Internal dependencies - */ -import { name, settings } from '../'; -import { blockEditRender } from '../../test/helpers'; - -describe( 'core/audio', () => { - test( 'block edit matches snapshot', () => { - const wrapper = blockEditRender( name, settings ); - - expect( wrapper ).toMatchSnapshot(); - } ); -} ); diff --git a/packages/block-library/src/cover-image/test/__snapshots__/index.js.snap b/packages/block-library/src/cover-image/test/__snapshots__/index.js.snap deleted file mode 100644 index f6690fdae6fe9..0000000000000 --- a/packages/block-library/src/cover-image/test/__snapshots__/index.js.snap +++ /dev/null @@ -1,92 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`core/cover-image block edit matches snapshot 1`] = ` -
    -
    - - Cover Image -
    -
    - Drag an image, upload a new one or select a file from your library. -
    -
    -
    -
    - - - Drop files to upload - -
    -
    -
    - - -
    -
    -
    -`; diff --git a/packages/block-library/src/cover-image/test/index.js b/packages/block-library/src/cover-image/test/index.js deleted file mode 100644 index ceb0bc9d5123c..0000000000000 --- a/packages/block-library/src/cover-image/test/index.js +++ /dev/null @@ -1,13 +0,0 @@ -/** - * Internal dependencies - */ -import { name, settings } from '../'; -import { blockEditRender } from '../../test/helpers'; - -describe( 'core/cover-image', () => { - test( 'block edit matches snapshot', () => { - const wrapper = blockEditRender( name, settings ); - - expect( wrapper ).toMatchSnapshot(); - } ); -} ); diff --git a/packages/block-library/src/gallery/test/__snapshots__/index.js.snap b/packages/block-library/src/gallery/test/__snapshots__/index.js.snap deleted file mode 100644 index fa8820564390d..0000000000000 --- a/packages/block-library/src/gallery/test/__snapshots__/index.js.snap +++ /dev/null @@ -1,93 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`core/gallery block edit matches snapshot 1`] = ` -
    -
    - - Gallery -
    -
    - Drag images, upload a new one or select a file from your library. -
    -
    -
    -
    - - - Drop files to upload - -
    -
    -
    - - -
    -
    -
    -`; diff --git a/packages/block-library/src/gallery/test/index.js b/packages/block-library/src/gallery/test/index.js deleted file mode 100644 index 5731cc00ba483..0000000000000 --- a/packages/block-library/src/gallery/test/index.js +++ /dev/null @@ -1,13 +0,0 @@ -/** - * Internal dependencies - */ -import { name, settings } from '../'; -import { blockEditRender } from '../../test/helpers'; - -describe( 'core/gallery', () => { - test( 'block edit matches snapshot', () => { - const wrapper = blockEditRender( name, settings ); - - expect( wrapper ).toMatchSnapshot(); - } ); -} ); diff --git a/packages/block-library/src/video/test/__snapshots__/index.js.snap b/packages/block-library/src/video/test/__snapshots__/index.js.snap deleted file mode 100644 index 33d0d470ec7da..0000000000000 --- a/packages/block-library/src/video/test/__snapshots__/index.js.snap +++ /dev/null @@ -1,107 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`core/video block edit matches snapshot 1`] = ` -
    -
    - - Video -
    -
    - Drag a video, upload a new one or select a file from your library. -
    -
    -
    -
    - - - Drop files to upload - -
    -
    -
    - - -
    -
    - - -
    -
    -
    -`; diff --git a/packages/block-library/src/video/test/index.js b/packages/block-library/src/video/test/index.js deleted file mode 100644 index a947f278a5882..0000000000000 --- a/packages/block-library/src/video/test/index.js +++ /dev/null @@ -1,13 +0,0 @@ -/** - * Internal dependencies - */ -import { name, settings } from '../'; -import { blockEditRender } from '../../test/helpers'; - -describe( 'core/video', () => { - test( 'block edit matches snapshot', () => { - const wrapper = blockEditRender( name, settings ); - - expect( wrapper ).toMatchSnapshot(); - } ); -} ); From 790a6438ad38607a3bef58a61c5115a3dd09eee2 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Thu, 23 Aug 2018 18:46:51 +0800 Subject: [PATCH 7/9] Add state handling for upload permissions Co-authored-by: imath --- docs/data/data-core.md | 14 ++++- packages/core-data/src/actions.js | 14 +++++ packages/core-data/src/reducer.js | 18 +++++++ packages/core-data/src/resolvers.js | 22 +++++++- packages/core-data/src/selectors.js | 10 ++-- packages/core-data/src/test/reducer.js | 28 +++++++++- packages/core-data/src/test/resolvers.js | 65 +++++++++++++++++++++++- packages/core-data/src/test/selectors.js | 25 +++++++++ 8 files changed, 185 insertions(+), 11 deletions(-) diff --git a/docs/data/data-core.md b/docs/data/data-core.md index 79bf5e3a37d54..f7990ee3321ee 100644 --- a/docs/data/data-core.md +++ b/docs/data/data-core.md @@ -140,6 +140,10 @@ Is the preview for the URL an oEmbed link fallback. Return whether the user has media upload permissions. +*Parameters* + + * state: Data state. + *Returns* Does the user have media upload permissions? @@ -190,4 +194,12 @@ a given URl has been received. *Parameters* * url: URL to preview the embed for. - * preview: Preview data. \ No newline at end of file + * preview: Preview data. + +### receiveUploadPermissions + +Returns an action object used in signalling that Upload permissions have been received. + +*Parameters* + + * hasUploadPermissions: Does the user have permission to upload files? \ No newline at end of file diff --git a/packages/core-data/src/actions.js b/packages/core-data/src/actions.js index b3b08fb138bf6..290d0ebaf740b 100644 --- a/packages/core-data/src/actions.js +++ b/packages/core-data/src/actions.js @@ -96,3 +96,17 @@ export function receiveEmbedPreview( url, preview ) { preview, }; } + +/** + * Returns an action object used in signalling that Upload permissions have been received. + * + * @param {boolean} hasUploadPermissions Does the user have permission to upload files? + * + * @return {Object} Action object. + */ +export function receiveUploadPermissions( hasUploadPermissions ) { + return { + type: 'RECEIVE_UPLOAD_PERMISSIONS', + hasUploadPermissions, + }; +} diff --git a/packages/core-data/src/reducer.js b/packages/core-data/src/reducer.js index 9a8d6a37d5cf0..9cd15384ab45e 100644 --- a/packages/core-data/src/reducer.js +++ b/packages/core-data/src/reducer.js @@ -101,6 +101,23 @@ export function themeSupports( state = {}, action ) { return state; } +/** + * Reducer managing Upload permissions. + * + * @param {Object} state Current state. + * @param {Object} action Dispatched action. + * + * @return {Object} Updated state. + */ +export function hasUploadPermissions( state = false, action ) { + switch ( action.type ) { + case 'RECEIVE_UPLOAD_PERMISSIONS': + return action.hasUploadPermissions; + } + + return state; +} + /** * Higher Order Reducer for a given entity config. It supports: * @@ -222,6 +239,7 @@ export default combineReducers( { users, taxonomies, themeSupports, + hasUploadPermissions, entities, embedPreviews, } ); diff --git a/packages/core-data/src/resolvers.js b/packages/core-data/src/resolvers.js index fc101110818a5..01a57d1c1a8db 100644 --- a/packages/core-data/src/resolvers.js +++ b/packages/core-data/src/resolvers.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { find } from 'lodash'; +import { find, includes, get, hasIn } from 'lodash'; /** * WordPress dependencies @@ -16,6 +16,7 @@ import { receiveEntityRecords, receiveThemeSupportsFromIndex, receiveEmbedPreview, + receiveUploadPermissions, } from './actions'; import { getKindEntities } from './entities'; import { apiFetch } from './controls'; @@ -88,3 +89,22 @@ export function* getEmbedPreview( url ) { yield receiveEmbedPreview( url, false ); } } + +/** + * Requests Upload Permissions from the REST API. + */ +export function* hasUploadPermissions() { + const response = yield apiFetch( { path: '/wp/v2/media', method: 'OPTIONS', parse: false } ); + + let allowHeader; + if ( hasIn( response, [ 'headers', 'get' ] ) ) { + // If the request is fetched using the fetch api, the header can be + // retrieved using the 'get' method. + allowHeader = response.headers.get( 'allow' ); + } else { + // If the request was preloaded server-side and is returned by the + // preloading middleware, the header will be a simple property. + allowHeader = get( response, [ 'headers', 'Allow' ], '' ); + } + yield receiveUploadPermissions( includes( allowHeader, 'POST' ) ); +} diff --git a/packages/core-data/src/selectors.js b/packages/core-data/src/selectors.js index 25ac90c82e7dd..861fd087ad20a 100644 --- a/packages/core-data/src/selectors.js +++ b/packages/core-data/src/selectors.js @@ -173,12 +173,10 @@ export function isPreviewEmbedFallback( state, url ) { /* * Return whether the user has media upload permissions. * + * @param {Object} state Data state. + * * @return {boolean} Does the user have media upload permissions? */ -export function hasUploadPermissions() { - // Hardcode hasUploadPermissions to true for now - in future this value will be - // correctly populated from a REST API call that will store user permissions - // See github issue for more information: - // https://github.com/WordPress/gutenberg/issues/3672 - return true; +export function hasUploadPermissions( state ) { + return !! state.hasUploadPermissions; } diff --git a/packages/core-data/src/test/reducer.js b/packages/core-data/src/test/reducer.js index 90f9b21f1a8cf..819646b8946c9 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, embedPreviews } from '../reducer'; +import { terms, entities, embedPreviews, hasUploadPermissions } from '../reducer'; describe( 'terms()', () => { it( 'returns an empty object by default', () => { @@ -97,6 +97,32 @@ describe( 'entities', () => { } ); } ); +describe( 'hasUploadPermissions()', () => { + it( 'returns false by default', () => { + const state = hasUploadPermissions( undefined, {} ); + + expect( state ).toBe( false ); + } ); + + it( 'returns true for an action with the hasUploadPermissions property with the value true', () => { + const state = hasUploadPermissions( false, { + type: 'RECEIVE_UPLOAD_PERMISSIONS', + hasUploadPermissions: true, + } ); + + expect( state ).toBe( true ); + } ); + + it( 'returns false for an action with the hasUploadPermissions property with the value false', () => { + const state = hasUploadPermissions( true, { + type: 'RECEIVE_UPLOAD_PERMISSIONS', + hasUploadPermissions: false, + } ); + + expect( state ).toBe( false ); + } ); +} ); + describe( 'embedPreviews()', () => { it( 'returns an empty object by default', () => { const state = embedPreviews( undefined, {} ); diff --git a/packages/core-data/src/test/resolvers.js b/packages/core-data/src/test/resolvers.js index 8b008bc1cad68..c4feb4fd5cabe 100644 --- a/packages/core-data/src/test/resolvers.js +++ b/packages/core-data/src/test/resolvers.js @@ -1,8 +1,8 @@ /** * Internal dependencies */ -import { getEntityRecord, getEntityRecords, getEmbedPreview } from '../resolvers'; -import { receiveEntityRecords, receiveEmbedPreview } from '../actions'; +import { getEntityRecord, getEntityRecords, getEmbedPreview, hasUploadPermissions } from '../resolvers'; +import { receiveEntityRecords, receiveEmbedPreview, receiveUploadPermissions } from '../actions'; describe( 'getEntityRecord', () => { const POST_TYPE = { slug: 'post' }; @@ -68,3 +68,64 @@ describe( 'getEmbedPreview', () => { expect( received ).toEqual( receiveEmbedPreview( UNEMBEDDABLE_URL, UNEMBEDDABLE_RESPONSE ) ); } ); } ); + +describe( 'hasUploadPermissions', () => { + describe( 'a normal fetch request for a user with upload_file capabilities', () => { + // When retrieving a non-preloaded request, the header is an object of the type Header. + const ADMIN_FETCH_RESPONSE = { + headers: { + get: () => 'GET,POST,PUT,OPTIONS', + }, + }; + + it( 'yields true if the response header allow parameters contains POST', async () => { + const fulfillment = hasUploadPermissions(); + + // Trigger generator + fulfillment.next(); + + // Provide valid response + const received = ( await fulfillment.next( ADMIN_FETCH_RESPONSE ) ).value; + expect( received ).toEqual( receiveUploadPermissions( true ) ); + } ); + } ); + + describe( 'a preloaded request for a user with upload_file capabilities', () => { + // For a preloaded request, the header is an object with simple properties. + const ADMIN_PRELOADED_RESPONSE = { + headers: { + Allow: 'GET,POST,PUT,OPTIONS', + }, + }; + + it( 'yields true if the response header allow parameters contains POST', async () => { + const fulfillment = hasUploadPermissions(); + + // Trigger generator + fulfillment.next(); + + // Provide valid response + const received = ( await fulfillment.next( ADMIN_PRELOADED_RESPONSE ) ).value; + expect( received ).toEqual( receiveUploadPermissions( true ) ); + } ); + } ); + + describe( 'a user without upload_file capabilities', () => { + const CONTRIBUTOR_RESPONSE = { + headers: { + get: () => 'GET', + }, + }; + + it( 'yields false if the response header allow parameters do not contain POST', async () => { + const fulfillment = hasUploadPermissions(); + + // Trigger generator + fulfillment.next(); + + // Provide response indicating user does not have permissions + const received = ( await fulfillment.next( CONTRIBUTOR_RESPONSE ) ).value; + expect( received ).toEqual( receiveUploadPermissions( false ) ); + } ); + } ); +} ); diff --git a/packages/core-data/src/test/selectors.js b/packages/core-data/src/test/selectors.js index b982ada4f3a9d..14da0f64010ea 100644 --- a/packages/core-data/src/test/selectors.js +++ b/packages/core-data/src/test/selectors.js @@ -11,6 +11,7 @@ import { getEntityRecords, getEmbedPreview, isPreviewEmbedFallback, + hasUploadPermissions, } from '../selectors'; describe( 'getEntityRecord', () => { @@ -117,3 +118,27 @@ describe( 'isPreviewEmbedFallback()', () => { expect( isPreviewEmbedFallback( state, 'http://example.com/' ) ).toEqual( true ); } ); } ); + +describe( 'hasUploadPermissions', () => { + it( 'returns the true when the hasUploadPermissions property is true', () => { + const state = deepFreeze( { + hasUploadPermissions: true, + } ); + + expect( hasUploadPermissions( state ) ).toBe( true ); + } ); + + it( 'returns the false when the hasUploadPermissions property is false', () => { + const state = deepFreeze( { + hasUploadPermissions: false, + } ); + + expect( hasUploadPermissions( state ) ).toBe( false ); + } ); + + it( 'returns the false when the hasUploadPermissions property is not defined', () => { + const state = deepFreeze( {} ); + + expect( hasUploadPermissions( state ) ).toBe( false ); + } ); +} ); From 1131d6a17727c98c42cd2153d88c381a4e9a8720 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Thu, 23 Aug 2018 19:06:24 +0800 Subject: [PATCH 8/9] Preload OPTIONS request to media endpoint used to determine if user has upload permissions Co-authored-by: imath --- lib/client-assets.php | 54 ++++++++++++++----- .../api-fetch/src/middlewares/preloading.js | 5 +- .../src/middlewares/test/preloading.js | 6 ++- 3 files changed, 47 insertions(+), 18 deletions(-) diff --git a/lib/client-assets.php b/lib/client-assets.php index 08e575def38c5..0abccc2c6905c 100644 --- a/lib/client-assets.php +++ b/lib/client-assets.php @@ -762,11 +762,11 @@ function gutenberg_register_scripts_and_styles() { * data to be attached to the page. Expected to be called in the context of * `array_reduce`. * - * @param array $memo Reduce accumulator. - * @param string $path REST API path to preload. - * @return array Modified reduce accumulator. + * @param array $memo Reduce accumulator. + * @param array $args REST API path and method to preload. + * @return array Modified reduce accumulator. */ -function gutenberg_preload_api_request( $memo, $path ) { +function gutenberg_preload_api_request( $memo, $args ) { // array_reduce() doesn't support passing an array in PHP 5.2 // so we need to make sure we start with one. @@ -774,16 +774,16 @@ function gutenberg_preload_api_request( $memo, $path ) { $memo = array(); } - if ( empty( $path ) ) { + if ( empty( $args ) || empty( $args['path'] ) || empty( $args['method'] ) ) { return $memo; } - $path_parts = parse_url( $path ); + $path_parts = parse_url( $args['path'] ); if ( false === $path_parts ) { return $memo; } - $request = new WP_REST_Request( 'GET', $path_parts['path'] ); + $request = new WP_REST_Request( $args['method'], $path_parts['path'] ); if ( ! empty( $path_parts['query'] ) ) { parse_str( $path_parts['query'], $query_params ); $request->set_query_params( $query_params ); @@ -802,7 +802,11 @@ function gutenberg_preload_api_request( $memo, $path ) { $data['_links'] = $links; } - $memo[ $path ] = array( + if ( 'OPTIONS' === $args['method'] ) { + $response = rest_send_allow_header( $response, $server, $request ); + } + + $memo[ $args['method'] ][ $args['path'] ] = array( 'body' => $data, 'headers' => $response->headers, ); @@ -1278,12 +1282,34 @@ function gutenberg_editor_scripts_and_styles( $hook ) { // Preload common data. $preload_paths = array( - '/', - '/wp/v2/types?context=edit', - '/wp/v2/taxonomies?per_page=-1&context=edit', - sprintf( '/wp/v2/%s/%s?context=edit', $rest_base, $post->ID ), - sprintf( '/wp/v2/types/%s?context=edit', $post_type ), - sprintf( '/wp/v2/users/me?post_type=%s&context=edit', $post_type ), + array( + 'path' => '/', + 'method' => 'GET', + ), + array( + 'path' => '/wp/v2/types?context=edit', + 'method' => 'GET', + ), + array( + 'path' => '/wp/v2/taxonomies?per_page=-1&context=edit', + 'method' => 'GET', + ), + array( + 'path' => sprintf( '/wp/v2/%s/%s?context=edit', $rest_base, $post->ID ), + 'method' => 'GET', + ), + array( + 'path' => sprintf( '/wp/v2/types/%s?context=edit', $post_type ), + 'method' => 'GET', + ), + array( + 'path' => sprintf( '/wp/v2/users/me?post_type=%s&context=edit', $post_type ), + 'method' => 'GET', + ), + array( + 'path' => '/wp/v2/media', + 'method' => 'OPTIONS', + ), ); // Ensure the global $post remains the same after diff --git a/packages/api-fetch/src/middlewares/preloading.js b/packages/api-fetch/src/middlewares/preloading.js index 78f431a984d98..14659557701ca 100644 --- a/packages/api-fetch/src/middlewares/preloading.js +++ b/packages/api-fetch/src/middlewares/preloading.js @@ -32,8 +32,9 @@ const createPreloadingMiddleware = ( preloadedData ) => ( options, next ) => { const method = options.method || 'GET'; const path = getStablePath( options.path ); - if ( 'GET' === method && preloadedData[ path ] ) { - return Promise.resolve( preloadedData[ path ].body ); + if ( preloadedData[ method ] && preloadedData[ method ][ path ] ) { + const response = 'OPTIONS' === method ? preloadedData[ method ][ path ] : preloadedData[ method ][ path ].body; + return Promise.resolve( response ); } } diff --git a/packages/api-fetch/src/middlewares/test/preloading.js b/packages/api-fetch/src/middlewares/test/preloading.js index d697af3b42949..00467b7b5de47 100644 --- a/packages/api-fetch/src/middlewares/test/preloading.js +++ b/packages/api-fetch/src/middlewares/test/preloading.js @@ -9,8 +9,10 @@ describe( 'Preloading Middleware', () => { status: 'this is the preloaded response', }; const preloadedData = { - 'wp/v2/posts': { - body, + GET: { + 'wp/v2/posts': { + body, + }, }, }; const prelooadingMiddleware = createPreloadingMiddleware( preloadedData ); From 4b4b4d67928cb7b07d37468d9fb52412cd85e3f0 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Fri, 24 Aug 2018 16:50:21 +0800 Subject: [PATCH 9/9] Add test for OPTIONS requests in preloading middleware --- .../src/middlewares/test/preloading.js | 45 +++++++++++++++---- 1 file changed, 36 insertions(+), 9 deletions(-) diff --git a/packages/api-fetch/src/middlewares/test/preloading.js b/packages/api-fetch/src/middlewares/test/preloading.js index 00467b7b5de47..50cd643a713c6 100644 --- a/packages/api-fetch/src/middlewares/test/preloading.js +++ b/packages/api-fetch/src/middlewares/test/preloading.js @@ -4,26 +4,53 @@ import createPreloadingMiddleware from '../preloading'; describe( 'Preloading Middleware', () => { - it( 'should return the preloaded data if provided', () => { - const body = { - status: 'this is the preloaded response', + it( 'should return the preloaded response body if provided when using a GET request', () => { + const postsResponse = { + body: { + status: 'this is the preloaded response', + }, }; const preloadedData = { GET: { - 'wp/v2/posts': { - body, - }, + 'wp/v2/posts': postsResponse, }, }; - const prelooadingMiddleware = createPreloadingMiddleware( preloadedData ); + const preloadingMiddleware = createPreloadingMiddleware( preloadedData ); const requestOptions = { method: 'GET', path: 'wp/v2/posts', }; - const response = prelooadingMiddleware( requestOptions ); + const response = preloadingMiddleware( requestOptions ); + return response.then( ( value ) => { + expect( value ).toEqual( postsResponse.body ); + } ); + } ); + + it( 'should return the preloaded full response data if provided when using an OPTIONS request', () => { + const postsResponse = { + headers: { + test: 'this is a preloaded header', + }, + body: { + status: 'this is the preloaded response', + }, + }; + + const preloadedData = { + OPTIONS: { + 'wp/v2/posts': postsResponse, + }, + }; + const preloadingMiddleware = createPreloadingMiddleware( preloadedData ); + const requestOptions = { + method: 'OPTIONS', + path: 'wp/v2/posts', + }; + + const response = preloadingMiddleware( requestOptions ); return response.then( ( value ) => { - expect( value ).toEqual( body ); + expect( value ).toEqual( postsResponse ); } ); } );