Saving a Post Breaks in Gutenberg if you have a custom taxonomy named Status

[kingkool68]

Steps to reproduce:

Register a taxonomy called Status:

function custom_taxonomy() {

	$labels = array(
		'name'                       => 'Status',
	);
	$args = array(
		'labels'                     => $labels,
		'hierarchical'               => false,
		'public'                     => true,
		'show_ui'                    => true,
		'show_admin_column'          => true,
		'show_in_nav_menus'          => true,
		'show_tagcloud'              => true,
		'show_in_rest'               => true,
	);
	register_taxonomy( 'status', array( 'post' ), $args );

}
add_action( 'init', 'custom_taxonomy', 0 );

Create a new Post using the Gutenberg editor
Try and Publish the post
Publishing should fail.
Add rest_base => 'status_tax' and publishing a post should work as expected.

The issue is if the rest_base argument isn't specified register_taxonomy() will use the taxonomy key as the rest base. In this case that would be status which is already a REST API route.

There should be a check to make sure a reserved REST API base isn't used. Ideally it would modify the default used to avoid the conflict such as appending _tax. Or atleast an error or warning should be thrown somewhere.

There is some confusion out there that this is a conflict between different plugins as documented here --> ​https://wordpress.org/support/topic/conflict-between-cpt-ui-and-acf-pro-in-the-block-editor/

[SergeyBiryukov]

Thanks for the ticket!

Apparently status should be added to the list of ​reserved terms (I thought it's already there).

A _doing_it_wrong() notice could also be added to avoid the conflict.

[TimothyBlynJacobs]

I agree a _doing_it_wrong would be a good addition. Perhaps in get_item_schem issue a warning if a property is already set with the same name on the schema?

[fgiannar]

Hi @TimothyBlynJacobs ,

I have added a patch that issues a warning if a property is already set with the same name on the schema, as you mention.
I also included the corresponding unit test.

Please make sure to double check the version in _doing_it_wrong as I was uncertain what the correct value should be.

Also, I was wondering whether the same warning that we issue to get_item_schema for taxonomies should be also issued in add_additional_fields_schema.

Finally, I was unsure whether I should unregister the taxonomy I use in the unit test.

This is one of my first tickets so your feedback would be more than welcome.

Thanks,

[TimothyBlynJacobs]

Thanks for working on this @fgiannar, this looks great! That version number is correct, and the tests look good.

Instead of setting the function on _doing_it_wrong to the current method, I think it might make more sense for the function to be register_taxonomy as that is where the developer would have to make their adjustments. I think it would also be helpful to add a bit more context to the message. Something along the lines of...

The %s taxonomy name conflicts with an existing property on the REST API Posts Controller. Specify a custom "rest_base" when registering the taxonomy to avoid this error.

[fgiannar]

Thanks a lot for your feedback @TimothyBlynJacobs !

Just a question please on your suggested message:
I think that the _doing_it_wrong warning could be issued not only if the taxonomy name conflicts with an existing property, but also the rest_base, as the code to decide on the base is:
$base = ! empty( $taxonomy->rest_base ) ? $taxonomy->rest_base : $taxonomy->name;

So, if for example, one had set 'status' as the rest_base the same warning would be produced.
Maybe if we only mention the name on the warning it could be misleading.

What do you think?

[TimothyBlynJacobs]

That's a great point! I think including the rest base would be a good idea.

[fgiannar]

@TimothyBlynJacobs I have added a new patch with the discussed updates.
While testing this I also noticed that it is possible to register two different taxonomies with the same rest_base.
eg

register_taxonomy( 'tax_1', array( 'post' ), array( 'show_in_rest' => true, 'rest_base' => 'common' ) );
register_taxonomy( 'tax_2', array( 'post' ), array( 'show_in_rest' => true, 'rest_base' => 'common' ) );

This also causes the warning to be displayed, but this time because the common custom taxonomy rest_base of tax_2 conflicts with the already registered property of the tax_1 taxonomy rest_base.
I think it's alright to also show the warning in this case.
Do you believe it would also make sense to open another ticket for preventing registering two taxonomies with the same custom rest_base or the current one should be considered enough?

[TimothyBlynJacobs]

Thanks @fgiannar!

Interesting, I think we can keep showing the warning in the same place we are now. Could we easily detect that the conflict is coming from another taxonomy and include that taxonomy in the message if that is the case?

[fgiannar]

@TimothyBlynJacobs
I guess we could keep an array of the taxonomy bases and check against it or keep a copy of the schema properties before starting the taxonomy loop, but I wouldn't like the idea of adding extra code outside the conditional check that produces the warning only for utilising it there...

Technically the message is correct, since if the error is produced because of 2 taxonomies sharing a common rest_base, the first one is already defined as an existing schema property.

Nevertheless, if you believe it's best to differentiate the warnings let me know and I will submit another patch.

[TimothyBlynJacobs]

Ok, let's stick with the single warning message for now!

[fgiannar]

@TimothyBlynJacobs Sure, please let me know if I need to do anything in addition, eg update ticket details etc (I'm new to this, so a bit unsure what the proper process is).
Thanks a lot for your time and guidance!

[TimothyBlynJacobs]

No problem! At this point, nothing further is needed. Another committer will review the patch, and assuming all is good we'll get it committed.

[TimothyBlynJacobs]

Uploaded a revised version of the patch to add a translators comment to the warning message. Translator comments are always required when using sprintf placeholders within a translatable string.

[kadamwhite]

In 47037:

REST API: Issue doing_it_wrong if a taxonomy's specified rest_base is already in use by a different resource.

Props fgiannar, TimothyBlynJacobs, kingkool68, SergeyBiryukov.
Fixes #48401.

[SergeyBiryukov]

In 47045:

REST API: Display the taxonomy name in the _doing_it_wrong() message for a conflicting rest_base or name property, to match the translator comment.

Follow-up to [47037].

See #48401.

[SergeyBiryukov]

In 47046:

REST API: Display the actual conflicting value in addition to the taxonomy name in the _doing_it_wrong() message for a conflicting rest_base or name property.

Follow-up to [47037] and [47045].

Props TimothyBlynJacobs.
See #48401.