Append shadow terms instead of replacing on associate and restore - #67
Open
jeremyfelt wants to merge 2 commits into
Open
Append shadow terms instead of replacing on associate and restore#67jeremyfelt wants to merge 2 commits into
jeremyfelt wants to merge 2 commits into
Conversation
`wp_set_object_terms()` defaults to `$append = false`, which replaces every
term an object has in the target taxonomy. Because a single `{post_type}_connect`
taxonomy holds a term for every shadow post, one connected post can legitimately
carry several terms in it. Three call sites managed a single shadow term without
appending, so they silently wiped the connected post's other associations:
- sync.php:73 — restore loop when a shadow post returns to publish
- sync.php:110 — recovery loop when a published post's term went missing
- taxonomy.php:170 — REST `associate` endpoint for a published shadow post
Pass `$append = true` at all three sites and add regression coverage in
tests/test-multi-association.php that fails without the fix.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
✅ Plugin Check passedNo errors or warnings from |
When the target shadow post is published but its term cannot be resolved (`API\get_term_id()` returns 0 — e.g. the term was deleted directly and the post has not been re-saved), `wp_set_object_terms()` skips the non-existent term ID and the handler still returned `success: true` with no association actually made. Recreate the missing term before associating (mirroring the recovery branch in sync.php) and, if a term still cannot be resolved, return `success: false` instead of a misleading success. Reuse the already-resolved taxonomy slug and term ID for the response query. Adds a regression test that fails without the fix (the connected post gains no term while the response reports success). Closes #68 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
wp_set_object_terms()defaults to$append = false, which replaces every term an object already has in the target taxonomy. Because a single{post_type}_connecttaxonomy holds one term for every shadow post, a connected post can legitimately carry several terms in that one taxonomy. Three call sites managed a single shadow term without appending, so they silently wiped the connected post's other shadow-term associations.Sites fixed (pass
$append = true):includes/sync.php:73— restore loop when a shadow post returns to publishincludes/sync.php:110— recovery loop when a published post's shadow term went missingincludes/taxonomy.php:170— RESTassociateendpoint for a published shadow postWhy it's a real bug
The shadow taxonomy is shared across all posts of a type (e.g.
example_connectholdsacme,globex, …) and is attached to the connected post type. A connected post associated with two shadow posts holds two terms in that one taxonomy. Re-attaching one term without$append = truereplaces the full set, dropping the rest.Tests
Adds
tests/test-multi-association.phpwith one regression test per call site. Each attaches a connected post to two shadow posts, exercises the relevant path, and asserts both associations survive.Verified:
phpunit23/23,phpcsclean,phpstanlevel 7 reports no errors.Also addresses #68
This branch now also fixes a pre-existing edge case in the REST
associatehandler: when a published shadow post'''s term cannot be resolved (e.g. the term was deleted and the post not re-saved),get_term_id()returns 0,wp_set_object_terms()skips the non-existent ID, and the handler previously returnedsuccess: truewith nothing attached. It now recreates the missing term (mirroringsync.php'''s recovery branch) and returnssuccess: falseonly if a term still cannot be resolved. Covered by an added regression test. Closes #68.🤖 Generated with Claude Code