Gutenberg: Taxonomy terms are gone in 4.2 rc-1

[slimmilkduds]

Describe the bug
For non admin users all taxonomies are now empty in the editor. It throws one (403) Forbidden error per taxonomy.

This is an issue with this specific version of GB, reverting back to 4.1.1 and everything is back to normal

To Reproduce
Steps to reproduce the behavior:

1. Open or create a new post
2. Try to expand the taxonomy tabs (I have only tried custom Taxonomies)
3. Nothing happens (except errors in console)

If these steps to reproduce doesn’t work I’ll add the console log tomorrow as I don’t have a computer available right now.

Desktop (please complete the following information):
Chrome

Additional context
GB 4.2 rc-1

[danielbachhuber]

Hi @slimmilkduds,

Thanks for the report. I wasn't able to reproduce this particular issue.

First, I registered a custom taxonomy called topic:

add_action( 'init', function() {
    register_taxonomy( 'topic', array( 'post' ), array(
        'hierarchical'      => false,
        'public'            => true,
        'show_in_nav_menus' => true,
        'show_ui'           => true,
        'show_admin_column' => false,
        'query_var'         => true,
        'rewrite'           => true,
        'capabilities'      => array(
            'manage_terms'  => 'edit_posts',
            'edit_terms'    => 'edit_posts',
            'delete_terms'  => 'edit_posts',
            'assign_terms'  => 'edit_posts',
        ),
        'labels'            => array(
            'name'                       => __( 'Topics', 'YOUR-TEXTDOMAIN' ),
            'singular_name'              => _x( 'Topic', 'taxonomy general name', 'YOUR-TEXTDOMAIN' ),
        ),
        'show_in_rest'      => true,
        'rest_base'         => 'topic',
        'rest_controller_class' => 'WP_REST_Terms_Controller',
    ) );
});

Next, I signed in as a user with the author role and was able to observe the taxonomy's Post Settings Panel:

Can you share:

1. The code you're using to register your custom taxonomy?
2. Any modifications you've made to your roles?
3. The contents of the Network response in your Chrome developer tools?
4. Any plugins you're running?

[designsimply]

Tested and confirmed for the Categories panel (not custom) using both the author role and the contributor role. (33s)

^{Seen at http://alittletestblog.com/wp-admin/post.php?post=15405&action=edit running WordPress 4.9.8 and Gutenberg 4.2.0-rc.1 and logged in as the contributor role using Firefox 63.0 on macOS 10.13.6.}

In the Network panel in Developer Tools, I see a 403 error for a call to http://alittletestblog.com/wp-json/wp/v2/categories?per_page=100&orderby=name&order=asc&context=edit&_locale=user that looks like this:

{"code":"rest_forbidden_context","message":"Sorry, you are not allowed to edit terms in this taxonomy.","data":{"status":403}}

^{Seen at http://alittletestblog.com/wp-admin/post.php?post=15405&action=edit running WordPress 4.9.8 and Gutenberg 4.2.0-rc.1 and logged in as the contributor role using Firefox 63.0 on macOS 10.13.6.}

In the Console panel, this error appears when on page load when I open a post for editing but I am not sure if it's related:

Unhandled promise rejection Error: "[object Object]"
wp-polyfill-ecmascript.min.2ae96136.js:2:29718

^{Seen at http://alittletestblog.com/wp-admin/post.php?post=15405&action=edit running WordPress 4.9.8 and Gutenberg 4.2.0-rc.1 and logged in as the contributor role using Firefox 63.0 on macOS 10.13.6.}

I tested using the contributor role on Firefox 63.0 on macOS 10.13.6 as well as the author role on Chrome 70.0.3538.77 on macOS 10.13.6. The alittletestblog.com site is hosted at WP Engine. Gutenberg 4.2.0-rc.1 was the only plugin active at the time of testing.

[danielbachhuber]

Tested and confirmed for the Categories panel (not custom) using both the author role and the contributor role.

I can confirm with Categories and Contributor. Looking further.

[danielbachhuber]

The immediate problem is that this request URL has context=edit:

http://alittletestblog.com/wp-json/wp/v2/categories?per_page=100&orderby=name&order=asc&context=edit&_locale=user 

The request URL should have context=view, because authors and contributors don't have permission to edit terms. Tracking down where this was introduced.

[danielbachhuber]

I believe the problem originated with #10089, specifically the change to use getEntityRecords( 'taxonomy', slug, DEFAULT_QUERY );, which then inadvertently means the request URL uses context=edit.

[adamsilverstein]

Can the e2e tests be extended to run as multiple roles so we can catch these types of regressions?

[danielbachhuber]

Can the e2e tests be extended to run as multiple roles so we can catch these types of regressions?

Yes. We could create a plugin that creates an author user on load and switches to it.

---

@youknowriad Just so it's stated, I think reverting #10089 is the wrong approach for this.

The data module will eventually need to handle context=view vs. context=edit. There are plenty of scenarios in the WordPress backend where a WordPress User only has read permission for an entity, and can't access the full shape of its contex=edit data. For instance, an Editor can assign authors to a Post but shouldn't be able to view their capabilities data. And, in this specific scenario, an Author should be able to assign Terms to a Post without editing them.

You might disagree with the underlying context abstraction but:

1. It's what we have.
2. It isn't going away or changing substantially.
3. It's is probably what you'd end up designing if you had to solve the problem originally.

To revert #10089 instead of solving for context=view vs. context=edit in the data module is to simply re-introduce the bugs that it fixes and kick the problems further down the road.

cc @aduth

[youknowriad]

The data module will eventually need to handle context=view vs. context=edit.

As discussed in DM. I think the data module shouldn't have to think about context at all. It's not abstraction on the REST API, it's an abstraction to retrieve WordPress Data and the REST API is an implementation detail.

We can't end up with entities in the store having different shapes depending on how they were fetched.

To revert #10089 instead of solving for context=view vs. context=edit in the data module is to simply re-introduce the bugs that it fixes and kick the problems further down the road.

What bugs we will be reintroducing? #10089 is supposed to be a refactoring. If it fixes a bug by inadvertance, we can still fix in the old implementation of the component.

[danielbachhuber]

I think the data module shouldn't have to think about context at all. It's not abstraction on the REST API, it's an abstraction to retrieve WordPress Data and the REST API is an implementation detail.

Ok. Where should the distinction between context=view and context=edit live then? The editor _must_ be able to distinguish between the two, and handle both interchangably.

What bugs we will be reintroducing?

Maybe I misunderstood the implementation. I thought it was the path to eventually solving #10873 #7266 #7565

[youknowriad]

Ok. Where should the distinction between context=view and context=edit live then? The editor must be able to distinguish between the two, and handle both.

Why it must?

[danielbachhuber]

Why it must?

As I mentioned before:

There are plenty of scenarios in the WordPress backend where a WordPress User only has read permission for an entity, and can't access the full shape of its contex=edit data.

Is there an alternative implementation that you'd suggest?

[youknowriad]

Is there an alternative implementation that you'd suggest?

Yes, always return all the fields the user can access to in the context=edit

[youknowriad]

I created this trac ticket to discuss it https://core.trac.wordpress.org/ticket/45252

[youknowriad]

Actually, it could be a new "special" context if we don't want to break backwards compatibility.

context=all or something.

[danielbachhuber]

Yes, always return all the fields the user can access to in the context=edit

But then you'd have two different shapes of data in the store?

Are you expecting that we'd change the implementation of context at this point in the release cycle?

[youknowriad]

But then you'd have two different shapes of data in the store?

No, because the shape won't change for the same user.

Are you expecting that we'd change the implementation of context at this point in the release cycle?

I'm not. I already talked about it a long time and I know It's not going to change now. I do think context=all or something is not that impactful as it's just a new context but I'm more pragmatic than that and thinking of reverting the component to use apiFetch directly and call it a day.

[danielbachhuber]

I'm more pragmatic than that and thinking of reverting the component to use apiFetch directly and call it a day.

How do we refresh the component then?

[youknowriad]

How do we refresh the component then?

I don't know, a hook maybe (even if it's not my favorite option in term of extensibility). Why would you refresh it though?

[danielbachhuber]

Why would you refresh it though?

Because of #7565 #7266?

[youknowriad]

For #7565 even hacking with the Data Module seems like the wrong way to deal with those issues, it probably deserves its own component (Sidebar API or other).

7266 These kind of issues is exactly what the data module is supposed to fix but it also highlights something I was talking about, if we do sepparate view and edit requests we can't automatically refresh the entities as well because we'd need a request on the other context as well.

[danielbachhuber]

if we do sepparate view and edit requests we can't automatically refresh the entities as well because we'd need a request on the other context as well.

Good point.

[youknowriad]

I'll wait a little bit, it looks like an important discussion, so I'm fine waiting for other opinions on how to solve these issues. I can revert at anytime.

[danielbachhuber]

Per #core-restapi conversation today, @kadamwhite will create a PR that partially reverts the original PR.