Proposed new function: wp_unset_object_terms

[simonwheatley]

In order to remove a single term associated with an object, leaving it's other terms in the same taxonomy, you have to call wp_get_object_terms, remove the terms you don't require from the array, then call wp_set_object_terms. I propose a single function, wp_unset_object_terms, which does this, applying relevant actions as it does.

Use case: A "starring" system whereby a user can (un)star a post to (un)favourite it. Each user has their own term where their stars are stored. You can use wp_set_object_terms with the append parameter (thanks for pointing that out Nacin) to "star" posts, but there's no simple way to "unstar" (and be sure that appropriate actions are run).

The attached patch adds the proposed function. Feedback, obviously, is welcome.

[simonwheatley]

Adds wp_unset_object_terms

[nacin]

Can you svn up before diffing so the last commit doesn't leak through?

[scribu]

Instead of a new function, maybe it would be better to add a $term_ids parameter to wp_delete_object_term_relationships() ?

[ericmann]

After some discussion on Twitter, it is apparent that this function will be needed sooner rather than later, though it might be a bit late for 3.2.

I'm attaching a patch (written primarily by @trepmal) that extends this functionality to WordPress as wp_unset_object_terms(). It accepts the same style of parameters as wp_set_object_terms() and has all the appropriate actions as well.

[ericmann]

Patch to add wp_unset_object_terms()

[scribu]

I think it would be better called wp_remove_object_terms() to avoid confusion with it's counterpart.

[ericmann]

Replying to scribu:

I think it would be better called wp_remove_object_terms() to avoid confusion with it's counterpart.

Fair enough. That was my original inclination when I opened #17878, but it was closed as a dup so I just took the naming schema from this ticket. New alternate patch uses wp_remove_object_terms() instead.

[ericmann]

Alternate patch uses wp_remove_object_terms()

[ericmann]

So what more needs to be done to this patch to make it commit ready? I know we're not pushing things until the upcoming scope discussion for 3.3 ... but I want to make sure this is ready to be on the table.

[scribu]

Coding standards:

Avoid intermediary variables:

$tt_id = $term_info['term_taxonomy_id']; 
$tt_ids[] = $tt_id;

Add missing whitespace:

foreach ( (array) $terms as $term) {

return new WP_Error('invalid_taxonomy', __('Invalid Taxonomy'));

wp_update_term_count($delete_terms, $taxonomy);

etc.

[jeremyclarke]

Just want to +1 this idea. Very useful and necessary since wp_set_object_terms() is not very user-friendly with it's need of array_mapped integers etc.

Glad to see it's already almost done :)

[iandunn]

+1

[nacin]

Since we have wp_set_object_terms(), I would agree the counterpart should be wp_unset_object_terms().

Whitespace issues aside, I'm curious of some structural aspects.

The filter in wp_set_object_terms() (and wp_delete_object_term_relationships()) is called delete_term_relationships, not delete_term_relationship.

The code at large needs to be abstracted and, overall, DRY. You can see at how relatively concise wp_delete_object_term_relationships() is -- it uses wp_get_object_terms() and then does the simple delete. The patch tries to do too much by emulating wp_get_object_terms(), rather than calling it.

Likewise, I would think that we could probably get away with wp_delete_object_term_relationships() becoming a wrapper for wp_unset_object_terms(), and wp_set_object_terms() calling wp_unset_object_terms() when it is time to delete the terms that need to be deleted. Ideally, delete_term_relationships should only fire once, anywhere, not in three places. It demonstrates that our API is properly structured.

Since we are passing around term IDs and need TTIDs, then this full level of abstraction may not be necessary or efficient. But maybe someone can take a crack at it?

[nacin]

Since we have wp_set_object_terms(), I would agree the counterpart should be wp_unset_object_terms().

I am willing to change my mind on that. Since set() does do replacements, adds, and removals, I understand the desire for remove() rather than unset(). Likewise, if we are to add remove(), we should probably add add(), which would simply wrap set() with $append = true.

[maxcutler]

DRY patch for wp_remove_object_terms

[maxcutler]

I added a fresh patch based on ericmann's but implementing the improvements recommended by nacin.

I had to add another DB query to wp_set_object_terms in the append=true case to get the term IDs from the tt IDs. The only reason is for back-compat because $old_tt_ids is used in the do_action at the end of the method (otherwise I would just change $old_tt_ids to $old_term_ids). There may be a better way to handle that, but I couldn't find any examples in taxonomy.php.

[simonwheatley]

Tested patch: applies cleanly and wp_remove_object_terms does what it says on the tin.

[ericmann]

Refresh patch and update docs

[ericmann]

Rounds out tag manipulation by adding wp_add_object_terms()

[markjaquith]

In 23398:

Introduce wp_remove_object_terms() and wp_add_object_terms(). No more fetch-alter-update nonsense. Use wp_remove_object_terms() in a few places internally.

props simonwheatley, scribu, ericmann, maxcutler. fixes #15475

[nacin]

Could we have some unit tests for this?

[kovshenin]

Hey there! 15475.2.diff​ makes sure that false is returned if nothing has been deleted, 15475.tests.diff​ covers the two new functions.

[SergeyBiryukov]

In 1227/tests:

Tests for wp_add_object_terms() and wp_remove_object_terms(). props kovshenin. see #15475.

[SergeyBiryukov]

In 23552:

Make wp_remove_object_terms() return false if nothing has been deleted. props kovshenin. fixes #15475.