Improve "wp_update_term" documentation

[ruud@…]

Hi all,

When using the wp_update_term function and reading through its inline docs, it struck me that some part of the inline docs is kind of vague (highly subjective, I know).
I will add a patch with a proposal for a more specific inline docs.

Thanks,
Ruud

[ruud@…]

Improved docs proposal

[boonebgorges]

Hi @ruud@joyo - Agreed 100% that this bit of the documentation is unclear. Introduced in [6378].

IMHO, the paragraph you've edited should be removed altogether:

1. It's not actually accurate that the terms/term_taxonomy table schemas give any information about $args.
2. $args should be documented as a @param anyway. It already is, though it a lackluster and apparently incorrect way. I don't think these $args have anything to do with get_terms().

I think we need a new patch that removes the paragraph in question, and updates the @param documentation to be correct. You should be able to figure out each argument's purpose by looking at the way the $parsed_args parameter is used throughout the function (and $defaults will show you the default values).

[mazzomaz]

Patch uploaded in 38310.diff​

[ruud@…]

Hi Boone and Mazzomaz,

@boonebgorges, Thanks for thorough description and the clear instructions on how to move this ticket forward
@mazzomaz, Thanks for the patch, removing the paragraph is for the better.

However, while digging through this lengthy function, and thinking of the double 'warning' in the inline-docs =>

1. 'Care must be taken to not override important information need to update or update will fail (or perhaps create a new term, neither would be acceptable).'
2. 'check the term scheme can contain and stay away from the term keys.'

I thought why bother with all these 'warnings' while we can just circumvent (un)intentional by removing the term_id from the $args.

Example: not removing the 'term_id' from $args will result in getting a slug from the wrong Term

$slug = wp_unique_term_slug($slug, (object) $args) => $query = $wpdb->prepare( "SELECT slug FROM $wpdb->terms WHERE slug = %s AND term_id != %d", $slug, $term->term_id );

If the 'term_id' is missing and/or removed the sanitize_term() function will correct this anyway to its default, but after the array_merge( $term, $args ) which I think is important for not overwriting the original term_id.

Disclaimer: I do not know any of the background of this function, or it's use in the broader WP eco-system. So please let me know if this patch doesn't make sense at all.

Thanks,
Ruud

[boonebgorges]

Hi @ruud@joyo - From what I can see, passing term_id as part of the $args array will *already* be ignored. The only $args values that do anything are: 'name', 'description', 'parent', 'term_group'.

The "Care must be taken..." line is another one that doesn't make much sense for modern WordPress. We use $args from a whitelist. It's not possible to modify existing terms in way other than these whitelisted arguments. Maybe what's meant is: if you don't want to change the 'description' field, don't pass a 'description' key, etc. If so, this could use some rewording. Originally added in [8164].

[ruud@…]

Hi @boonebgorges,

Thanks for your reply.

From what I can see, passing term_id as part of the $args array will *already* be ignored. The only $args values that do anything are: 'name', 'description', 'parent', 'term_group'.

I think your mostly right, except that the entire $args array is also passed to wp_unique_term_slug on line 2908:
$slug = wp_unique_term_slug($slug, (object) $args);

None of the other $args values are filtered out and 'survive' the entire function to this point and further along.

The "Care must be taken..." line is another one that doesn't make much sense for modern WordPress.

Indeed.

We use $args from a whitelist. It's not possible to modify existing terms in way other than these whitelisted arguments.

Not entirely true, I think it is possible to alter the slug via a slug from another term (via a different term_id via $args), see remark about the wp_unique_term_slug() function in use.

Maybe what's meant is: if you don't want to change the 'description' field, don't pass a 'description' key, etc. If so, this could use some rewording.

Indeed, I'm not a big fan of mentioning things in the docs which might/could happen if used in some way etc. Stating what a function does do, seems a much more prudent way. Adding some more clarification in the @params could help?

I'm curious if I'm missing the obvious here with the possibility of mis-using a 'term_id' in $args here.

Thanks
Ruud

[DrewAPicture]

Assigning ownership to mark the good-first-bug as "claimed".

[DrewAPicture]

Replying to ruud@…:

Indeed, I'm not a big fan of mentioning things in the docs which might/could happen if used in some way etc. Stating what a function does do, seems a much more prudent way. Adding some more clarification in the @params could help?

The value in occasionally describing what could happen is when one piece of functionality is commonly used with another and discrepancies (that we can't necessarily f ix) cause behavior perceived as unexpected.

[boonebgorges]

Hi @ruud@joyo - Apologies for the silence on this ticket.

You're right about the args that are being passed through to wp_unique_term_slug(). The long description in the doc block should probably make this more explicit.

The unset( $args['term_id'] ) change does seem like it might fix a bug, but I think we need more explicit description of the bug before dropping in this change. I think it would be a unit test that: 1. sets up terms A and B, 2. updates term A ($term_id) and but pass $args['term_id'] = B, 3. show how the generated slug is incorrect. It may turn out that $args['term_id'] = $term_id is a more appropriate fix than simply unsetting.

[flixos90]

Adding appropriate keywords (as far as I understand the discussion) - please adjust if I chose the wrong ones. :)

[johnnyb]

Jumping in here because inaccurate docs lost me some time yesterday.

Can we add a list of what will actually get updated if passed to $args? For example, the codex says (​https://codex.wordpress.org/Function_Reference/wp_update_term) that we can update the taxonomy that a term is part of by passing $args['taxonomy'] = 'new-taxonomy';, but this is not the case. An explicit list of allowed $args would be great for developers.

I'm off to correct the codex now.