get_edit_term_link can technically accept also WP_Term and object for $term_id

[david.binda]

The doc block for get_edit_term_link currently says it can only accept the term_id, but the $term_id param is being passed to the get_term function right away, which, IMHO, technically means it can be anything what the get_term function accepts, which is int, WP_Term, object.

I'm really able to figure out why the documentation limits the param to term_id only. I wonder if I perhaps missed something?

However, the get_term behaves differently when it's being passed different values/types. From the documentation:

(int|WP_Term|object) (Required) If integer, term data will be fetched from the database, or from the cache if available. If stdClass object (as in the results of a database query), will apply filters and return a WP_Term object corresponding to the $term data. If WP_Term, will return $term.

I have noticed that in many places where the code is passing just the term_id to get_edit_term_link, the code might go over some unnecessarily complicated logic, eventually causing some minor inefficiencies.

For instance, the wp_tag_cloud. The function first collects all the terms, which means collecting whole lines of terms entries in database, populating the terms, and then loops over those passing in just the `term_id` to the `get_edit_term_link` (as well as get_term_link, tho).

Which means that the get_edit_term_link calls the get_term just with the term_id, which in turn, collects the very same data from local cache, in ideal situation, despite the data have all been already collected by get_terms and are ready in the get_edit_term_link function.

However, there's also a situation in which passing in just the term_id actually triggers unnecessary SQL queries or prompts some remote cache get requests.

In case the term is not being used elsewhere on the page, and in case a persistent object cache is in use, the get_terms may end early if it was cached previously, there is an early return, only triggering the WP_Term::populate_terms, skipping the populate_term_caches call, which otherwise happens later.

So, the terms are well populated, but they are not in local object cache, and need to be brought from the object cache server, and if not found there, the get_term function, when passed in just the term_id, fetches the data from the database again.

That said, I believe that the possibility of passing in the WP_Term (or object) to get_edit_term_link should be documented, and used by WordPress itself on respective places.

It, IMHO, would also be beneficial, if we could go further, and pass in the WP_Term or object to any similar calls, eg: `get_term_feed_link`, which can in theory also accept the WP_Term object, and which is being called repeatedly from `Walker_Category::start_el`, which is being called from wp_list_categories which previously collected the WP_Term objects via `get_categories` (which is calling the get_terms)

The same goes for WP_List_Table.

I'm attaching a patch, with those adjustments.

[SergeyBiryukov]

Didn't get a chance to review this one in time for 5.5 RC, let's handle this in 5.6.

[Hareesh Pillai]

Refreshed patch. Fixed typo.

[johnbillion]

Note that the type of the $term_id parameter that's passed to the filters cannot change. This affects get_edit_tag_link and get_edit_term_link. The parameter needs to remain an integer for backwards compatibility, but a new parameter can be added.

[hellofromTonya]

With 5.6 RC1 tomorrow, it's too late in the cycle. Punting this ticket.

If any maintainer or committer feels this can be resolved in time, or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

[audrasjb]

DocBlock changes

[audrasjb]

The last patch adds some DocBlocks/coding standards issues.

[johnbillion]

It looks like this patch still changes the type of the value passed to the filters as mentioned in comment 6. Can you check that please?

https://core.trac.wordpress.org/ticket/50225

[peterwilsoncc]

In my ​linked pull request:

• Added a bunch of tests for edit_term_link(), get_edit_term_link()
• Tests ensure an integer is sent to the filter in the hooks get_edit_term_link and edit_term_link
• Tests ensure a WP_Term object is sent to the filter in term_link
• Fixed filters per comment 6 and comment 11
• Changed edit_term_link() to also accept a term ID or object in line with get_edit_term_link() -- it seems potentially confusing for the equivalent parameters to differ
• There was accidental revert of an earlier commit in the patch, removed that

[hellofromTonya]

With 5.7 RC1 minutes away, ran out of time for this one. Moving to 5.8 for it to go through a beta cycle.

[chaion07]

Thanks to @davidbinda for reporting this issue. We reviewed this ticket during a recent [bug-scrub session]​https://wordpress.slack.com/archives/C02RQBWTW/p1623096669357200. With Beta 1 coming in a day, checking with @SergeyBiryukov to kindly draw some attention to this. Thank you! Otherwise it is headed for a punt to Future Release if that patch isn't solid enough to land in time. Thanks

Props to @jeffpaul

[hellofromTonya]

Today is 5.8 Beta 1. Ran out of time for this ticket to land.

Though this ticket has been punted several times, ​PR 50225 looks close to being ready for testing. Punting to 5.9 (instead of Future Release).

Hey @SergeyBiryukov reassigning ownership to me and added to my code/test review and testing queue.

[hellofromTonya]

Upgraded and improved the unit tests. Then manually tested. No regressions. Works the same in 5.8 as it does with this patch. No errors/notices/warnings in the error or debug log.

Ready for commit.

[hellofromTonya]

In 52180:

Taxonomy: Allow get_*_*_link() and edit_term_link() functions to accept a term ID, WP_Term, or term object.

get_term() accepts a term ID, instance of WP_Term, or an object (i.e. stdClass as a result of a db query). Functions that use get_term() also now allow for the same data types.

Why? For consistency, removing extra processing code in consuming functions, and performance.

Functions changed in this commit are:

• get_category_feed_link()
• get_term_feed_link()
• get_tag_feed_link()
• get_edit_tag_link()
• get_edit_term_link()
• edit_term_link()

For each of consumer of these functions, changes to pass the object instead of the term ID.

Includes unit/integration tests for test coverage of these changes.

Follow-up to [6365], [9136], [9340], [14711], [15792], [15800], [18827], [32606], [36646], [37252].

Props davidbinda, johnbillion, peterwilsoncc, hellofromTonya, sergeybiryukov, mista-flo, hareesh-pillai, audrasjb, jeffpaul, chaion07.
Fixes #50225.

Committed via changeset https://core.trac.wordpress.org/changeset/52180.

[hellofromTonya]

Thank you everyone for contributing! The patch has been committed and will ship with 5.9 Beta 1 today.

[hellofromTonya]

Docs will need to be updated for these functions. May be worth a mention in Misc dev note.

[hellofromTonya]

In 52188:

Formatting: Add additional support for single and nestable tags in force_balance_tags().

Adds track and wbr support for single tags.

Adds article, aside, details, figure, and section for nestable tags.

Updates tests.

Follow-up to [5805], [21828], [45929].

Props glendaviesnz, costdev, talldanwp, ramonopoly, sergeybiryukov.
Fixes #50225.

[hellofromTonya]

Whoopsie, [52188] is not for this ticket. It's for #54130. Moving it there.

## Problem

Backward compatibility is broken since WordPress 5.9-alpha because get_term_feed_link() doesn't honor the $taxonomy parameter anymore. See ​https://github.com/WordPress/wordpress-develop/blob/a8485376d27b5b3348a68963f6fd38200b6e64cf/src/wp-includes/link-template.php#L925-L926

For instance, given a post tag term, get_term_feed_link( $term_id, 'post_tag' ) returns false now.

## Changes

Don't use category as default taxonomy when passing a term ID.
As the WP_Term object is retrieved later, use this to get the taxonomy.

Trac ticket: https://core.trac.wordpress.org/ticket/50225

[hugod]

Hi,

In my opinion, the way get_term_feed_link() has been modified brokes backward compatibility.
I described it in my PR ​https://github.com/WordPress/wordpress-develop/pull/1945.

[audrasjb]

Thanks for reopening this @hugod, and for the PR. Self-assigning for proper review/test.

[hugod]

Replying to audrasjb:

Thanks for reopening this @hugod, and for the PR. Self-assigning for proper review/test.

Hi @audrasjb! Do you prefer I upload a patch here instead of a PR on GitHub ?

[audrasjb]

@hugod a PR is fine, thank you :)

[audrasjb]

Thanks @hugod, the PR looks good to me, marking this for commit.

[hugod]

Thanks for the review @audrasjb !

[audrasjb]

In 52255:

Taxonomy: Use WP_Term object to retrieve the taxonomy in get_term_feed_link().

This change fixes a backward compatibility issue introduced in [52180] where get_term_feed_link() did not honor the $taxonomy parameter anymore. Rather than using the default category taxonomy when passing a term ID in get_term_feed_link(), use the WP_Term object to get the taxonomy.

Follow-up to [52180].

Props hugod.
Fixes #50225.

Committed in https://core.trac.wordpress.org/changeset/52255

[dlh]

I'm sorry to reopen this again, but [52255] retrieves the $term->taxonomy property before the type safety check on $term:

$taxonomy = $term->taxonomy;

if ( empty( $term ) || is_wp_error( $term ) ) {
	return false;
}

Wouldn't it be preferable to reverse these statements?

Also, the documentation for $taxonomy:

Defaults to 'category' if term ID or non WP_Term object is passed.

seems to no longer be accurate, since $taxonomy is now passed as-is to get_term() regardless of the type of $term.

Trac ticket: https://core.trac.wordpress.org/ticket/50225

[audrasjb]

Thanks for spotting those two issues @dlh!
I think they are fixed in the above PR. I'd appreciate second pair of eyes on this :)

[dlh]

That PR looks good to me with respect to the two items I noticed, but other folks who have been following the effort in this ticket as a whole might also want to take a look.

[audrasjb]

In 52258:

Taxonomy: Check $term object before using it in get_term_feed_link().

This change performs the type safety check on $term before it is used to retrieve the taxonomy in get_term_feed_link(). It also updates the related DocBlock after [52255].

Follow up to [52255].

Props dlh.
Fixes #50225.

https://core.trac.wordpress.org/changeset/52258

[SergeyBiryukov]

In 52305:

Docs: Fix typo in some get_edit_term_link() test DocBlocks.

Follow-up to [52180], [52255].

See #50225, #53399.

[audrasjb]

Dev note published: ​https://make.wordpress.org/core/2022/01/04/miscellaneous-core-changes-in-wordpress-5-9/