#36514 closed defect (bug) (fixed)
posting with custom taxes
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 5.1 | Priority: | normal |
Severity: | normal | Version: | 4.2 |
Component: | Taxonomy | Keywords: | has-unit-tests has-dev-note |
Focuses: | administration | Cc: |
Description
Sorry for my bad english.
Seems there is a bug in wp-admin/includes/post.php on line 348.
Need for use
'include' => $term,
instead
'name' => $term,
Now it causes errors when saving posts within custom non-hierarchical taxonomies - WordPress by mistake spontaneously creates new items which has names like id's cheked items.
Attachments (5)
Change History (39)
#2
follow-up:
↓ 4
@
9 years ago
- Component changed from General to Taxonomy
- Focuses administration added
#4
in reply to:
↑ 2
@
9 years ago
Replying to boonebgorges:
By default, non-hierarchical taxonomies do not use a checkbox interface. I assume you are overriding this behavior with a custom
meta_box_cb
parameter when registering the taxonomy. Can you please share the code you're using to register the taxonomy, and any code you may be using to render the metabox (assuming it's not one of the core functions)?
Right, here is the code
function create_custom_taxes(){
$labels = array(
'name' => 'Слайдеры',
'singular_name' => 'Слайдер',
'search_items' => 'Слайдеры',
'all_items' => 'Слайдеры',
'parent_item' => '',
'parent_item_colon' => '',
'edit_item' => 'Редактировать слайдер',
'update_item' => 'Обновить слайдер',
'add_new_item' => 'Добавить слайдер',
'new_item_name' => 'Новый слайдер',
'menu_name' => 'Слайдеры'
);
register_taxonomy( 'slider', array('slide'), array(
'hierarchical' => false,
'labels' => $labels,
'show_ui' => true,
'show_admin_column' => true,
'query_var' => true,
'rewrite' => array( 'slug' => 'slider' ),
'meta_box_cb' => 'post_categories_meta_box',
));
};
add_action( 'init', 'create_custom_taxes' );
#5
@
9 years ago
- Keywords 2nd-opinion has-patch added
- Milestone changed from Awaiting Review to 4.6
Thanks for the details, @hokku. As I'd guessed, you're specifying post_categories_meta_box
for a non-hierarchical taxonomy, which is confusing the logic introduced in [31359].
We need something more specific, which will tell edit_post()
when it ought to be converting 'tax_input' values to term IDs. It's hard to think of a very elegant way to do this. 36514.diff is my best idea: a new meta_box_expect_ids
param for register_taxonomy()
, which defaults to true
when the meta_box_cb is 'post_categories_meta_box' (either explicitly or as fallback, based on hierarchical
). Then, in edit_post()
, we check this value instead of is_taxonomy_hierarchical()
.
This technique is not a beautiful piece of software craftsmanship, but it will only ever matter for people who are providing a meta_box_cb
other than the defaults from core, which is hardly anyone.
#6
@
9 years ago
I found that the original patch left the array of term IDs as a strings. These are interpreted to be slugs (so creating a new term also created a term with the name matching the original term's ID).
The only difference to the original patch is the line which casts all the values in the array to integers.
This fixes the bug for me.
#8
@
9 years ago
- Keywords 4.7-early added; 2nd-opinion removed
- Milestone changed from 4.6 to Future Release
I think we missed the 4.6 deadline for this improvement, but let's do right away once 4.7 kicks off.
This ticket was mentioned in Slack in #core by helen. View the logs.
8 years ago
#10
follow-up:
↓ 11
@
8 years ago
Related/possibly a duplicate: #14877
I think it'd be perfectly reasonable to get what exists working with both IDs and slugs as appropriate in this ticket and then using that to model new callbacks after. And I'd love to see that happen.
#11
in reply to:
↑ 10
;
follow-up:
↓ 12
@
8 years ago
- Keywords needs-patch 2nd-opinion added; has-patch 4.7-early removed
- Milestone changed from Future Release to 4.7
Replying to helen:
Related/possibly a duplicate: #14877
I think it'd be perfectly reasonable to get what exists working with both IDs and slugs as appropriate in this ticket and then using that to model new callbacks after. And I'd love to see that happen.
If we're headed for a future where WP provides more than just two meta box callbacks for taxonomies, then we may want to opt for something a bit more sophisticated here than an if/else toggle. I'm thinking something like a meta_box_save_cb
alongside meta_box_cb
. We'll have an internal matrix that can be used to provide default mappings of meta_box_cb
to meta_box_save_cb
:
// register_taxonomy() if ( null === $args['meta_box_save_cb'] ) ) { switch ( $args['meta_box_cb'] ) { case 'post_categories_meta_box' : $args['meta_box_save_cb'] = 'meta_box_save_cb_checkboxes'; break; case 'post_tags_meta_box' : default: $args['meta_box_save_cb'] = 'meta_box_save_cb_input'; break; } }
The format of tax_input
and the corresponding validation required will look a bit different for select
, multiselect
, radio
, so they should each probably get their own meta_box_save_cb
. This provides that framework. Thoughts?
#12
in reply to:
↑ 11
@
8 years ago
Replying to boonebgorges:
The format of
tax_input
and the corresponding validation required will look a bit different forselect
,multiselect
,radio
, so they should each probably get their ownmeta_box_save_cb
. This provides that framework. Thoughts?
Ah, interesting. Do we break out much of the insert/update post process right now? I feel like the last time I looked it was one giant procedural call.
In practice, I think it really ends up as three callbacks - single (select, radio), multi (checkboxes, multi-select), and freeform input (currently tags).
#13
@
8 years ago
- Keywords has-patch added; needs-patch removed
Ah, interesting. Do we break out much of the insert/update post process right now? I feel like the last time I looked it was one giant procedural call.
Yes, the way it's saved is a bit of a mess, and not easily separated from wp_insert_post()
without changing the order in which things happen. We can simplify the improvement by asking for a sanitization callback instead. See 36514.2.diff for a proof-of-concept.
In practice, I think it really ends up as three callbacks - single (select, radio), multi (checkboxes, multi-select), and freeform input (currently tags).
Yes, this covers the native HTML elements, but who knows what wacky stuff people are doing inside of a meta_box_cb
. In any case, the sanitize callbacks are good for more than just differentiating between the *shape* of the payload, they can also do specific validation on the *contents* of the payload. Imagine, for instance, that you allowed users to provide a comma-separated list of term IDs (not sure why you'd do this specifically, but you get the idea). A generic meta_box_sanitize_cb
allows you to do whatever you want with your 'tax_input'.
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
8 years ago
This ticket was mentioned in Slack in #core by stevenkword. View the logs.
8 years ago
#18
@
8 years ago
- Keywords 4.8-early added
- Milestone changed from 4.7 to Future Release
- Owner changed from boonebgorges to helen
Revisiting this: I think my approach is the right one, but it's an API enhancement masquerading as a bug fix, and at this point in the release cycle I don't have the stomach for more enhancements. Let's do it for 4.8, at which point I will gratefully accept any thoughts from @helen.
36514.3.diff is a refresh after the introduction of WP_Taxonomy
.
#21
@
8 years ago
- Keywords has-unit-tests added; needs-unit-tests removed
I've submitted a unit test for the proposed patch. The unit test failed before applying the patch, and passed once the patch was applied.
I was able to manually reproduced the issue via the WP Admin. I then created a unit test that;
- registers a given taxonomy based on the reporters criteria,
- creates a post and term for the newly registered taxonomy,
- sets the current user to editor role,
- uses
edit_post()
to apply the newly created term to the given post,
I verified that WordPress did create a new term with the term ID as the term, vs. adding the term to the post.
I then applied the patch, re-run the test, and the test passed.
One item to note is that to fully simulate the process of editing a post from the WP Admin, the $term_id
had to be type casted as (string) $term_id
.
#22
@
8 years ago
Hi @obenland looking for feedback on the unit test submitted. Also wanted to know if there's any other movement with this ticket.
#24
@
8 years ago
- Milestone changed from 4.8 to 4.8.1
No movement on this ticket, punting to 4.8.1. Any feedback here, @helen, @boonebgorges or @obenland?
#25
@
8 years ago
- Keywords 2nd-opinion 4.8-early removed
- Milestone changed from 4.8.1 to 4.9
- Owner changed from helen to boonebgorges
This is not viable for 4.8.1, but let's do it for 4.9. 36514.4.diff looks very close - I will review.
This ticket was mentioned in Slack in #core by melchoyce. View the logs.
7 years ago
#27
@
7 years ago
- Milestone changed from 4.9 to 5.0
Apologies for not reviewing this in time. Let's shoot for the next release.
#30
@
6 years ago
- Keywords needs-patch added; has-patch removed
- Milestone changed from 5.0 to 5.1
- Resolution fixed deleted
- Status changed from closed to reopened
@since
needs updating
#32
@
6 years ago
- Keywords has-dev-note added; needs-patch removed
Dev note has been published: https://make.wordpress.org/core/2019/01/23/improved-taxonomy-metabox-sanitization-in-5-1/
By default, non-hierarchical taxonomies do not use a checkbox interface. I assume you are overriding this behavior with a custom
meta_box_cb
parameter when registering the taxonomy. Can you please share the code you're using to register the taxonomy, and any code you may be using to render the metabox (assuming it's not one of the core functions)?