WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 2 months ago

Last modified 4 weeks ago

#43517 closed defect (bug) (fixed)

Adding support of default category term for custom taxonomies

Reported by: enrico.sorcinelli Owned by: whyisjake
Milestone: 5.5 Priority: normal
Severity: normal Version:
Component: Taxonomy Keywords: has-patch has-dev-note
Focuses: Cc:

Description

This patch aims to add the support for default category term in custom taxonomies in the same way that is done for built-in taxonomies.

The new default_term argument is added to register_taxonomy() allowing to define the default term name and optionally slug and description. For example:

register_taxonomy( 'custom-tax', 'my-cpt', array(
   'default_term' => array( 'name' => 'My default category', 'slug' => 'default-category' ),
));

This way, by inserting a new my-cpt object without setting any custom-tax terms, the default term 'My default category' will be used for that taxonomy.

The default_taxonomy_{$axonomy} value is used as option_name in order to save default terms id in wp_options.

PS: Apparently, I haven't found any related ticket other than this old one.

Attachments (8)

43517.patch (6.9 KB) - added by enrico.sorcinelli 3 years ago.
43517.2.diff (7.2 KB) - added by davidbaumwald 12 months ago.
Refreshed patch
43517.2.patch (7.0 KB) - added by enrico.sorcinelli 4 months ago.
43517.3.patch (7.0 KB) - added by enrico.sorcinelli 3 months ago.
43517.diff (6.8 KB) - added by whyisjake 3 months ago.
43517.4.patch (8.2 KB) - added by enrico.sorcinelli 3 months ago.
43517.5.patch (2.6 KB) - added by enrico.sorcinelli 3 months ago.
43517.6.patch (4.1 KB) - added by enrico.sorcinelli 3 months ago.

Download all attachments as: .zip

Change History (50)

#1 @mukesh27
3 years ago

  • Keywords has-patch added

#2 @enrico.sorcinelli
3 years ago

  • Keywords has-unit-tests added

This ticket was mentioned in Slack in #core by enrico.sorcinelli. View the logs.


3 years ago

#4 @SergeyBiryukov
3 years ago

  • Milestone changed from Awaiting Review to 5.0

#5 @SergeyBiryukov
2 years ago

  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#7 @pento
2 years ago

  • Milestone changed from 5.0 to 5.1

#9 @pento
21 months ago

  • Milestone changed from 5.1 to 5.2

#10 @SergeyBiryukov
19 months ago

  • Milestone changed from 5.2 to 5.3

Missed the 5.2 Beta 1 deadline, moving to 5.3.

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


13 months ago

#12 @desrosj
13 months ago

  • Keywords needs-refresh added

43517.patch needs a refresh against current trunk. @SergeyBiryukov are you still up for reviewing this one?

@davidbaumwald
12 months ago

Refreshed patch

#13 @davidbaumwald
12 months ago

  • Keywords needs-refresh removed

@SergeyBiryukov I refreshed the patch an cleaned up a couple of things. Is this something that can be reviewed in time for 5.3 Beta 1 tomorrow or should this have more time for testing?

#14 @davidbaumwald
12 months ago

  • Milestone changed from 5.3 to Future Release

The deadline for version 5.3 Beta 1 enhancements is now passed, so this is being moved to Future Release.

#15 @enrico.sorcinelli
4 months ago

I just refreshed my patch for current trunk.

This ticket was mentioned in Slack in #core by enrico.sorcinelli. View the logs.


4 months ago

#17 @whyisjake
4 months ago

  • Milestone changed from Future Release to 5.5

#18 @whyisjake
4 months ago

  • Owner changed from SergeyBiryukov to whyisjake
  • Status changed from reviewing to accepted

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


3 months ago

#20 @davidbaumwald
3 months ago

@whyisjake Is this still on your list to review prior to Beta 1 for 5.5 due to release next week?

#21 @enrico.sorcinelli
3 months ago

I uploaded a refreshed patch in order to apply cleanly to current trunk.
@davidbaumwald as far i know, I think that the ticket is on the @SergeyBiryukov list

Last edited 3 months ago by enrico.sorcinelli (previous) (diff)

@whyisjake
3 months ago

#22 @whyisjake
3 months ago

43517.diff runs the changes through composer format.

#23 @whyisjake
3 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 48356:

Taxonomy: Add support for default terms for custom taxonomies.

The new default_term argument is added to register_taxonomy() allowing a user to define the default term name and optionally slug and description.

Fixes #43517.

Props enrico.sorcinelli, SergeyBiryukov, desrosj, davidbaumwald, whyisjake.

#24 @TimothyBlynJacobs
3 months ago

What is the performance impact of this change? AFAICT, this is adding a term_exists query to every page load. Can we check for the option first before checking the term?

#25 @whyisjake
3 months ago

  • Keywords dev-feedback added; has-patch has-unit-tests removed
  • Resolution fixed deleted
  • Status changed from closed to reopened
  • Type changed from enhancement to defect (bug)

Going to reopen to look into adding a check for the option. @enricosorcinelli, can you check?

#26 @johnbillion
3 months ago

  • Keywords needs-patch added
  • Version 5.1 deleted

Can we also avoid the mixed type and just go with an array? It's not clear from the parameter description whether this field accepts a slug, name, or ID, and we might as well be explicit about it.

#27 @johnbillion
3 months ago

Sorry I meant the array|string type, not mixed.

#28 @enrico.sorcinelli
3 months ago

The term_exists call is made only for custom taxonomies for which has been defined a default_term (and the default value in null).
However term_exists() doesn't cache results anyway and adding it could be an interesting new functionality.

Moreover I'd like to add some missing features like:

  • remove default_taxonomy_{$taxonomy} option on unregister_taxonomy()
  • check and prevent to delete the default term for custom taxonomies like wp_delete_term() does for category taxonomy.

So I refreshed the patch by updating also default_term parameter documentation (I leaved string|array type).

Last edited 3 months ago by enrico.sorcinelli (previous) (diff)

#29 @TimothyBlynJacobs
3 months ago

The term_exists call is made only for custom taxonomies for which has been defined a default_term (and the default value in null).
However term_exists() doesn't cache results anyway and adding it could be an interesting new functionality.

Right, which is why I think we should first check if the default taxonomy option is already set so we can avoid that uncached query.

So I refreshed the patch by updating also default_term parameter documentation (I leaved string|array type).

The patch has been committed already, so it needs to be refreshed against trunk.

A separate thing. With the current version, is it possible to completely remove all terms from a post? Looking at the code it seems like the default term will always get added back.

#30 @enrico.sorcinelli
3 months ago

I've just uploaded a refreshed patch.
@TimothyBlynJacobs If I understood your question, as far I know, it is possible with wp_delete_object_term_relationships() that that uses wp_remove_object_terms() behind the scenes.
The default term is added with wp_set_object_terms() instead.

#31 @TimothyBlynJacobs
3 months ago

Hm, previously though calling wp_set_object_terms() with an empty array would work though, no? It does the diffing and then calls wp_remove_object_terms. That is how the REST API sets the terms for a post and it looks like wp_insert_post as well.

#32 @enrico.sorcinelli
3 months ago

  • Keywords dev-feedback needs-patch removed

@TimothyBlynJacobs My fault, and I agree with you that wp_set_object_terms() shouldn't assign the default taxonomy term so I moved related code into wp_insert_post().

PS: I discovered a strange/different behavior between different editors API described in #50648

#33 @davidbaumwald
3 months ago

  • Keywords dev-feedback has-patch added

#34 @SergeyBiryukov
3 months ago

In 48480:

Taxonomy: Make some adjustments to handling default terms for custom taxonomies:

  • Move default term assignment from wp_set_object_terms() to wp_insert_post().
  • Make sure the passed taxonomy list overwrites the existing list if not empty.
  • Remove the default term option on unregister_taxonomy().
  • Prevent deletion of the default term in wp_delete_term().

Props enrico.sorcinelli, TimothyBlynJacobs.
See #43517.

#35 @whyisjake
2 months ago

@SergeyBiryukov can we close this out, or do we want to track a few more changes?

#36 @whyisjake
2 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

I think with the changes in [48480] we are good to close this out.

#37 @SergeyBiryukov
2 months ago

In 48664:

Coding Standards: Reformat a condifion for default taxonomy terms in map_meta_cap() for better readability.

Follow-up to [48356], [48480].

See #43517.

#38 @SergeyBiryukov
2 months ago

In 48665:

Taxonomy: Rename the default_taxonomy_$taxonomy option key to default_term_$taxonomy.

This better reflects the purpose of the option.

Follow-up to [48356], [48480].

See #43517.

#39 @SergeyBiryukov
2 months ago

  • Keywords needs-dev-note added; dev-feedback removed

#41 @helgatheviking
4 weeks ago

Is the default term meant to be saved for the taxonomy in the absence of other chosen terms? With posts and categories, if you don't select any other category "uncategorized" is checked the next time the post editor is loaded. This doesn't happen with the default term in a custom taxonomy. But if it's meant to be equivalent to "uncategorized" shouldn't this term be saved to a post when no other terms are selected?

#42 @enrico.sorcinelli
4 weeks ago

Hi @helgatheviking,
in order to auto-add the default custom taxonomy term when saving or updating post without custom taxonomy terms you have to explicitly define default_term when registering the custom taxonomy, for example:

register_taxonomy(
        'custom-tax',
        'post',
        array(
                'labels'=> array(
                        'name' => 'Custom tax'
                ),
                'hierarchical' => true,
                'public'       => true,
                'show_ui'      => true,
                'show_admin_column' => true,
                'show_in_nav_menus' => true,
                'show_in_menu'      => true,
                'show_in_rest'      => true,
                'default_term' => array(
                        'name' => 'Default category',
                        'slug' => 'default-category',
                ),
        )
);

However note that with block editor this doesn't happen in the same manner (for example if you remove all category or custom taxonomy terms, the default term will not be auto re-added to the post). See #50648

Note: See TracTickets for help on using tickets.