In get_edit_term_link: $term validation

[tmatsuur]

$term_id is sometimes invalid, so isn't an error checking necessary?

Proposal:

$term = get_term( $term_id, $taxonomy );
if ( is_wp_error() )
	return;

[MikeHansenMe]

32786.diff​ checks term for WP_Error or null which are the 2 values get_term can return if it is not valid.

[tmatsuur]

Thank you for reply.
This has been careless. I think that your patch is better.

[boonebgorges]

Pretty lame that we return null from this function instead of a string, and pretty lame that we return null here but WP_Error objects in get_term_link(). We can't easily change the latter, I guess.

Can we get two unit tests demonstrating the null and WP_Error cases? Like, one that includes an empty $term and one that includes an invalid $taxonomy.

[MikeHansenMe]

32786.2.diff​ has 4 unit tests.

1. Valid Scenario
2. Invalid Term ID
3. Empty Term ID
4. Bad Taxonomy

When writing the tests I realized that $tax had the same problem as $term. The fourth test checks this. The patch corrects this.

[tmatsuur]

When a consistency with source cord of the other functions is considered, isn't the next source cord appropriate?
(See: get_term_feed_link)

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

And, can't get_term be called first?

	$term = get_term( $term_id, $taxonomy );
	if ( empty( $term ) || is_wp_error( $term ) )
		return false;

	$tax = get_taxonomy( $taxonomy );
	if ( !current_user_can( $tax->cap->edit_terms ) )
		return;

I'm sorry, but I don't understand how to use the diff.
I hope that my intention is transmitted correctly.

[boonebgorges]

Thanks all. As a rule, I think what's being encouraged here is not very good practice. Functions that are supposed to return a string (in this case, a URL) should not simply return null when bad input is provided. Ideally, they'd return a meaningful error object. But, in the absence of that, a PHP notice can be a helpful debugging tool for developers. If you're passing a bad term ID or a bad taxonomy name to get_edit_term_link(), you should know about it.

All this being said, the function is already odd in the sense that it bails with null when the cap check is not met. And we can't change the function to return WP_Error objects, as plugins (and WP itself) sometimes checks ! $link = get_edit_term_link() for failure. So, for the sake of consistency, it shouldn't do any harm to patch as suggested.

[boonebgorges]

In 32954:

Improve error checking in get_edit_term_link().

The function should not throw notices when an improper term or taxonomy is
passed.

Props tmatsuur, MikeHansenMe.
Fixes #32786.