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

Editor: Refactor Post Author component #33695

Merged
merged 9 commits into from
Jul 27, 2021
41 changes: 17 additions & 24 deletions packages/editor/src/components/post-author/check.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,31 @@ import { get } from 'lodash';
/**
* WordPress dependencies
*/
import { withInstanceId, compose } from '@wordpress/compose';
import { withSelect } from '@wordpress/data';
import { useSelect } from '@wordpress/data';
import { store as coreStore } from '@wordpress/core-data';

/**
* Internal dependencies
*/
import PostTypeSupportCheck from '../post-type-support-check';
import { store as editorStore } from '../../store';
import { AUTHORS_QUERY } from './constants';

export function PostAuthorCheck( {
hasAssignAuthorAction,
authors,
children,
} ) {
if ( ! hasAssignAuthorAction || ! authors || 1 >= authors.length ) {
export default function PostAuthorCheck( { children } ) {
const { hasAssignAuthorAction, hasAuthors } = useSelect( ( select ) => {
const post = select( editorStore ).getCurrentPost();
const authors = select( coreStore ).getUsers( AUTHORS_QUERY );
return {
hasAssignAuthorAction: get(
post,
[ '_links', 'wp:action-assign-author' ],
false
),
hasAuthors: authors?.length >= 1,
};
}, [] );

if ( ! hasAssignAuthorAction || ! hasAuthors ) {
return null;
}

Expand All @@ -31,19 +40,3 @@ export function PostAuthorCheck( {
</PostTypeSupportCheck>
);
}

export default compose( [
withSelect( ( select ) => {
const post = select( editorStore ).getCurrentPost();
return {
hasAssignAuthorAction: get(
post,
[ '_links', 'wp:action-assign-author' ],
false
),
postType: select( editorStore ).getCurrentPostType(),
Copy link
Member Author

Choose a reason for hiding this comment

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

We weren't using this prop in PostAuthorCheck.

authors: select( coreStore ).getAuthors(),
};
} ),
withInstanceId,
] )( PostAuthorCheck );
41 changes: 20 additions & 21 deletions packages/editor/src/components/post-author/combobox.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,36 +6,40 @@ import { debounce } from 'lodash';
/**
* WordPress dependencies
*/
import { useState, useMemo, useEffect } from '@wordpress/element';
import { useState, useMemo } from '@wordpress/element';
import { useSelect, useDispatch } from '@wordpress/data';
import { __ } from '@wordpress/i18n';
import { ComboboxControl } from '@wordpress/components';
import { decodeEntities } from '@wordpress/html-entities';
import { store as coreStore } from '@wordpress/core-data';

/**
* Internal dependencies
*/
import { store as editorStore } from '../../store';
import { AUTHORS_QUERY } from './constants';

function PostAuthorCombobox() {
const [ fieldValue, setFieldValue ] = useState();

const { authorId, isLoading, authors, postAuthor } = useSelect(
( select ) => {
const { __unstableGetAuthor, getAuthors, isResolving } = select(
coreStore
);
const { getUser, getUsers, isResolving } = select( coreStore );
const { getEditedPostAttribute } = select( editorStore );
const author = __unstableGetAuthor(
getEditedPostAttribute( 'author' )
);
const query =
! fieldValue || '' === fieldValue ? {} : { search: fieldValue };
const author = getUser( getEditedPostAttribute( 'author' ), {
context: 'view',
} );
const query = { ...AUTHORS_QUERY };

if ( fieldValue ) {
query.search = fieldValue;
}

return {
authorId: getEditedPostAttribute( 'author' ),
postAuthor: author,
authors: getAuthors( query ),
isLoading: isResolving( 'core', 'getAuthors', [ query ] ),
authors: getUsers( query ),
isLoading: isResolving( 'core', 'getUsers', [ query ] ),
};
},
[ fieldValue ]
Expand All @@ -46,7 +50,7 @@ function PostAuthorCombobox() {
const fetchedAuthors = ( authors ?? [] ).map( ( author ) => {
return {
value: author.id,
label: author.name,
label: decodeEntities( author.name ),
Copy link
Member Author

Choose a reason for hiding this comment

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

Added missing entity decoding. We were already doing this for select control.

};
} );

Expand All @@ -57,22 +61,17 @@ function PostAuthorCombobox() {

if ( foundAuthor < 0 && postAuthor ) {
return [
{ value: postAuthor.id, label: postAuthor.name },
{
value: postAuthor.id,
label: decodeEntities( postAuthor.name ),
},
...fetchedAuthors,
];
}

return fetchedAuthors;
}, [ authors, postAuthor ] );

// Initializes the post author properly
// Also ensures external changes are reflected.
useEffect( () => {
if ( postAuthor ) {
setFieldValue( postAuthor.name );
}
}, [ postAuthor ] );

Comment on lines -68 to -75
Copy link
Member Author

Choose a reason for hiding this comment

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

I couldn't track down why this effect was added precisely, but I don't think it's needed anymore.

It also initializes an unnecessary search query on the mount.

/**
* Handle author selection.
*
Expand Down
6 changes: 6 additions & 0 deletions packages/editor/src/components/post-author/constants.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export const AUTHORS_QUERY = {
who: 'authors',
per_page: 50,
Copy link
Member Author

@Mamaduka Mamaduka Jul 27, 2021

Choose a reason for hiding this comment

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

I changed the "per_page" argument from 100 to 50. However, I don't think this number makes a big difference when used with ComboboxControl. It just needs to be higher than 25 and reasonable :)

Happy to revert it if needed.

_fields: 'id,name',
context: 'view', // Allows non-admins to perform requests.
};
5 changes: 3 additions & 2 deletions packages/editor/src/components/post-author/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,14 @@ import { store as coreStore } from '@wordpress/core-data';
*/
import PostAuthorCombobox from './combobox';
import PostAuthorSelect from './select';
import { AUTHORS_QUERY } from './constants';

const minimumUsersForCombobox = 25;

function PostAuthor() {
const showCombobox = useSelect( ( select ) => {
// Not using `getUsers()` because it requires `list_users` capability.
const authors = select( coreStore ).getAuthors();
youknowriad marked this conversation as resolved.
Show resolved Hide resolved
const authors = select( coreStore ).getUsers( AUTHORS_QUERY );

return authors?.length >= minimumUsersForCombobox;
}, [] );

Expand Down
4 changes: 3 additions & 1 deletion packages/editor/src/components/post-author/select.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@ import { store as coreStore } from '@wordpress/core-data';
* Internal dependencies
*/
import { store as editorStore } from '../../store';
import { AUTHORS_QUERY } from './constants';

function PostAuthorSelect() {
const { editPost } = useDispatch( editorStore );
const { postAuthor, authors } = useSelect( ( select ) => {
const authorsFromAPI = select( coreStore ).getAuthors();
const authorsFromAPI = select( coreStore ).getUsers( AUTHORS_QUERY );

return {
postAuthor: select( editorStore ).getEditedPostAttribute(
'author'
Expand Down
83 changes: 29 additions & 54 deletions packages/editor/src/components/post-author/test/check.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,75 +3,50 @@
*/
import { shallow } from 'enzyme';

/**
* WordPress dependencies
*/
import { useSelect } from '@wordpress/data';

/**
* Internal dependencies
*/
import { PostAuthorCheck } from '../check';
import PostAuthorCheck from '../check';

describe( 'PostAuthorCheck', () => {
const users = {
data: [
{
id: 1,
name: 'admin',
capabilities: {
level_1: true,
},
},
{
id: 2,
name: 'subscriber',
capabilities: {
level_0: true,
},
},
{
id: 3,
name: 'andrew',
capabilities: {
level_1: true,
},
},
],
};
jest.mock( '@wordpress/data/src/components/use-select', () => {
// This allows us to tweak the returned value on each test
const mock = jest.fn();
return mock;
} );

it( 'should not render anything if users unknown', () => {
const wrapper = shallow(
<PostAuthorCheck authors={ [] } hasAssignAuthorAction={ true }>
authors
</PostAuthorCheck>
);
expect( wrapper.type() ).toBe( null );
} );
describe( 'PostAuthorCheck', () => {
it( 'should not render anything if has no authors', () => {
useSelect.mockImplementation( () => ( {
hasAuthors: false,
hasAssignAuthorAction: true,
} ) );

it( 'should not render anything if single user', () => {
const wrapper = shallow(
<PostAuthorCheck
authors={ users.data.slice( 0, 1 ) }
hasAssignAuthorAction={ true }
>
authors
</PostAuthorCheck>
);
const wrapper = shallow( <PostAuthorCheck>authors</PostAuthorCheck> );
expect( wrapper.type() ).toBe( null );
} );

it( "should not render anything if doesn't have author action", () => {
const wrapper = shallow(
<PostAuthorCheck authors={ users } hasAssignAuthorAction={ false }>
authors
</PostAuthorCheck>
);
useSelect.mockImplementation( () => ( {
hasAuthors: true,
hasAssignAuthorAction: false,
} ) );

const wrapper = shallow( <PostAuthorCheck>authors</PostAuthorCheck> );
expect( wrapper.type() ).toBe( null );
} );

it( 'should render control', () => {
const wrapper = shallow(
<PostAuthorCheck authors={ users } hasAssignAuthorAction={ true }>
authors
</PostAuthorCheck>
);
useSelect.mockImplementation( () => ( {
hasAuthors: true,
hasAssignAuthorAction: true,
} ) );

const wrapper = shallow( <PostAuthorCheck>authors</PostAuthorCheck> );
expect( wrapper.type() ).not.toBe( null );
} );
} );