$deleted_term in wp_delete_term() has incorrect `count` value

[danielbachhuber]

It's assigned after all of the term relationships are removed. When accessing it through the delete_term action, I'd expect it to persist the original count.

[boonebgorges]

Yeah, it hardly seems worth passing count if it's always 0. At the very least, I'd expect 'delete_term_taxonomy' to get the pre-delete count.

[nicholas_io]

@boonebgorges, Do you mean to pass a count parameter to the delete_term_taxonomy action? I checked the source code and I saw that delete_term_taxonomy only receives the term_id.

[boonebgorges]

Replying to nicholas_io:

@boonebgorges, Do you mean to pass a count parameter to the delete_term_taxonomy action? I checked the source code and I saw that delete_term_taxonomy only receives the term_id.

No, I mean that the $deleted_term variable should be initialized before its term relationships are deleted. That way, $deleted_term->count will reflect the pre-delete state of the term. I assume that you can just switch the blocks of code around, but this needs testing (and possibly a unit test).

[nicholas_io]

Replying to boonebgorges:

Replying to nicholas_io:

@boonebgorges, Do you mean to pass a count parameter to the delete_term_taxonomy action? I checked the source code and I saw that delete_term_taxonomy only receives the term_id.

No, I mean that the $deleted_term variable should be initialized before its term relationships are deleted. That way, $deleted_term->count will reflect the pre-delete state of the term. I assume that you can just switch the blocks of code around, but this needs testing (and possibly a unit test).

Ok, I got it, I'll try to make patch.

[nicholas_io]

A Patch to fix the expected behavior

[boonebgorges]

Thanks, nicholas_io! Patch looks good. I'm going to reconfigure the unit test a little bit to (a) make it compatible with PHP 5.2, and (b) keep the assertions in the scope of the main test method rather than the 'wp_delete_term' callback (which is how we do it elsewhere in the test suite).

[boonebgorges]

In 33711:

In wp_delete_term(), the $deleted_term object passed to filters should be generated before term relationships are deleted.

This allows the count property to reflect the pre-delete state of affairs,
rather than always being 0.

Props nicholas_io.
Fixes #33485.

[nicholas_io]

Replying to boonebgorges:

Thanks, nicholas_io! Patch looks good. I'm going to reconfigure the unit test a little bit to (a) make it compatible with PHP 5.2, and (b) keep the assertions in the scope of the main test method rather than the 'wp_delete_term' callback (which is how we do it elsewhere in the test suite).

Great, I was in doubt about how do test inside callbacks, now I understood how is done.

[boonebgorges]

Replying to nicholas_io:

Replying to boonebgorges:

Thanks, nicholas_io! Patch looks good. I'm going to reconfigure the unit test a little bit to (a) make it compatible with PHP 5.2, and (b) keep the assertions in the scope of the main test method rather than the 'wp_delete_term' callback (which is how we do it elsewhere in the test suite).

Great, I was in doubt about how do test inside callbacks, now I understood how is done.

Your technique was good too, I'm just trying to be consistent :) Thanks again for the patch!

[nicholas_io]

Replying to boonebgorges:

Replying to nicholas_io:

Replying to boonebgorges:

Thanks, nicholas_io! Patch looks good. I'm going to reconfigure the unit test a little bit to (a) make it compatible with PHP 5.2, and (b) keep the assertions in the scope of the main test method rather than the 'wp_delete_term' callback (which is how we do it elsewhere in the test suite).

Great, I was in doubt about how do test inside callbacks, now I understood how is done.

Your technique was good too, I'm just trying to be consistent :) Thanks again for the patch!

All good! No problem :)