Switch `post-author` component to use `/wp/v2/users/?who=authors` #6515

danielbachhuber · 2018-05-01T01:46:33Z

Description

Switches the post-author component to use /wp/v2/users/?who=authors and its underling REST API query parameters.

See https://core.trac.wordpress.org/ticket/42202
See #3010, #6361

Screenshots

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.

danielbachhuber · 2018-05-01T01:49:13Z

@aduth This isn't quite working how I'd expect it to. Mind checking it out locally and giving it a look?

danielbachhuber · 2018-05-01T02:02:39Z

This isn't quite working how I'd expect it to.

To clarify, I can confirm the authors are entering the store, but the rendered component isn't displaying them.

youknowriad · 2018-05-01T08:51:02Z

core-data/reducer.js

+			return [
+				...state,
+				...action.authors,
+			];


I believe this would create duplicate authors if we trigger the request twice (it's not happening at the moment though :)). Maybe we can store them by Id :).

Also, why do we have an authors reducer instead of a users reducer. I believe we should have a users reducer and use the getAuthors selector to filter the data according to the user's object. This way we won't be duplicating data when we introduce the users fetching.

This looks more complex at first sight, but it's the way to go. Just imagine how you'd update the authors list if you create a new author from the JS side for instance? If it's all handled in a generic way in a users reducer, the selector will update its changes without any specific behavior to trigger the request again.

I believe this would create duplicate authors if we trigger the request twice (it's not happening at the moment though :)). Maybe we can store them by Id :).

Fixed in 97a7a35

Also, why do we have an authors reducer instead of a users reducer. I believe we should have a users reducer and use the getAuthors selector to filter the data according to the user's object.

Because of #6361. With this approach, we keep the definition of "which user is an author" server-side.

Because of #6361. With this approach, we keep the definition of "which user is an author" server-side.

Well! Can I suggest that it's not a good compromise going forward. This means that our data module would consider authors as a separate entity than users. While this may be ok shortterm, I don't think it's good because we have to think of the core-data module as a generic module to handle client data in any WP Admin context. It means another page could create users, which could also be "authors" but since this knowledge is only available server-side, the UI won't update once those users are created.

How can we move this knowledge client-side? what property do we need in the user's object to say that it's an author or not?

While this may be ok shortterm, I don't think it's good because we have to think of the core-data module as a generic module to handle client data in any WP Admin context.

Would you like me to move it to editor data then?

How can we move this knowledge client-side? what property do we need in the user's object to say that it's an author or not?

We'd need to run a second query client-side for every user collection request. First, a WP_User_Query( [ 'who' => 'authors' ] ) to get all authors, and the second WP_User_Query for the full query. When preparing the results for return, we'd inspect whether the user is a part of the first query and include an author attribute correspondingly.

My preference is to keep this current implementation.

In fact, this current implementation can be progressively enhanced to a better implementation in the future, should one arise.

So we discussed this with @aduth and I believe there's a compromise to find here if we can't move the "author" knowledge to the client.

we can track the ?who=authors response as a set of IDs pointing to the canonical users state.

Also this relates to #6395 helper since ?who=authors can be considered just one query on the user model.

I'm fine using a compromise for now and generalizing this as a query in #6395 or similar.

Can you clarify what changes you'd like me to make to this PR? Or, open a PR against this PR with your suggested changes?

Sure, give me some minutes

Here #6522 It's not tested etc... just to get the idea.

youknowriad · 2018-05-01T08:51:39Z

editor/components/post-author/check.js

 		};
 	} ),
 	withAPIData( ( props ) => {
-		const { postType } = props;
+		const { postType, authors } = props;


Passing the props down is useless, it's done automatically in withAPIData

aduth · 2018-05-01T19:54:51Z

Re: #6522 would either of you care to review #6395 so we can track queried data properly?

…ors to users (#6522)

youknowriad · 2018-05-02T08:50:02Z

I've merged my native users query implementation here. I believe it conflicts a bit with #6395 but we can easily refactor once one of the PRs hit master.

danielbachhuber · 2018-05-02T13:55:10Z

@aduth @youknowriad Can we land this, and handle the conflict in the other PR? I'd like to move on with #6361

youknowriad

LGTM 👍 Thanks for all these REST API improvements @danielbachhuber

Switch post-author component to use /wp/v2/users/?who=authors

f112000

danielbachhuber self-assigned this May 1, 2018

danielbachhuber added 3 commits April 30, 2018 19:15

Ensure state is always an array, properly

80d0500

Update tests for 80d0500

e2c7f4e

Remove unused variable

ee7ac14

danielbachhuber added this to the 2.8 milestone May 1, 2018

danielbachhuber requested a review from a team May 1, 2018 02:22

danielbachhuber removed their assignment May 1, 2018

youknowriad reviewed May 1, 2018

View reviewed changes

danielbachhuber added 2 commits May 1, 2018 07:05

Remove unnecessary prop pass-through

de5b7f0

Ensure no duplicate authors are listed

97a7a35

youknowriad mentioned this pull request May 1, 2018

Use a "users" reducer combined with a "queries" sub state to map authors to users #6522

Merged

danielbachhuber mentioned this pull request May 1, 2018

Avoid direct use of user capabilities in client-side code #6361

Closed

danielbachhuber mentioned this pull request May 1, 2018

Ability to select authors has vanished #3010

Closed

Use a "users" reducer combined with a "queries" sub state to map auth…

1e3205b

…ors to users (#6522)

youknowriad approved these changes May 2, 2018

View reviewed changes

danielbachhuber merged commit d904911 into master May 2, 2018

danielbachhuber deleted the 3010-switch-who branch May 2, 2018 14:26

danielbachhuber mentioned this pull request May 2, 2018

Further harden who=authors check by author support for post type #6548

Merged

danielbachhuber mentioned this pull request May 18, 2018

PostAuthor component can cause a crash for some users #4437

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch `post-author` component to use `/wp/v2/users/?who=authors` #6515

Switch `post-author` component to use `/wp/v2/users/?who=authors` #6515

danielbachhuber commented May 1, 2018 •

edited

Loading

danielbachhuber commented May 1, 2018

danielbachhuber commented May 1, 2018

youknowriad May 1, 2018

danielbachhuber May 1, 2018

youknowriad May 1, 2018

danielbachhuber May 1, 2018

danielbachhuber May 1, 2018

youknowriad May 1, 2018

danielbachhuber May 1, 2018

youknowriad May 1, 2018

youknowriad May 1, 2018 •

edited

Loading

youknowriad May 1, 2018

danielbachhuber May 1, 2018

aduth commented May 1, 2018

youknowriad commented May 2, 2018

danielbachhuber commented May 2, 2018

youknowriad left a comment

Switch post-author component to use /wp/v2/users/?who=authors #6515

Switch post-author component to use /wp/v2/users/?who=authors #6515

Conversation

danielbachhuber commented May 1, 2018 • edited Loading

Description

Screenshots

Checklist:

danielbachhuber commented May 1, 2018

danielbachhuber commented May 1, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youknowriad May 1, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aduth commented May 1, 2018

youknowriad commented May 2, 2018

danielbachhuber commented May 2, 2018

youknowriad left a comment

Choose a reason for hiding this comment

Switch `post-author` component to use `/wp/v2/users/?who=authors` #6515

Switch `post-author` component to use `/wp/v2/users/?who=authors` #6515

danielbachhuber commented May 1, 2018 •

edited

Loading

youknowriad May 1, 2018 •

edited

Loading