fix(term-propagation): two corruption bugs uncovered by ConfessIt backfill#204
Merged
JohnRDOrazio merged 1 commit intoJun 11, 2026
Merged
Conversation
…kfill Both observed on production 2026-06-08 when backfilling project_tag terms on the ConfessIt community_project translations (referral predated PR #201, so its 5 non-EN siblings had zero tags). Toggling status to fire the propagation hook on each non-EN sibling produced: - DE post 1501: 4 correctly-localized tags (Beichte/Examen/Prüfung/ Versöhnung) — German distinguishes "examen" from "examination", no collision, all clean. - IT/PT posts (1497/1500): only 3 tags each — lost "examination" because OpenAI translates both EN "examen" and EN "examination" to the same target word (esame/exame). - ES/FR posts (1498/1499): 4 tags listed but one entry is the raw EN term 171 ("examen"), and querying term 171 directly now shows language=fr — the EN source term itself got language-flipped. - EN source 1485 still references term 171 in its term_relationships, so its `projectTags.nodes[1].language.slug` is now "fr". Cross-class corruption: an EN post serving a French-classified tag. Two distinct bugs in cdcf_get_or_create_translated_term, both fixed here: BUG 1 — pll_get_term self-fallback treated as "found a sibling". Some Polylang versions return the INPUT term_id from pll_get_term when no target-language sibling exists (vs returning 0/false). The handler's `if ($existing) return (int) $existing;` then returns the EN term_id, which propagates up to wp_set_object_terms($post, [..., EN_TERM_ID, ...]) — assigning an English term to a non-English post. Subsequent runs with the same EN term then call pll_set_term_language($new_id, $target_lang) on the EN term id (because `pll_get_term` would return the just-promoted-with-incorrect-language sibling), flipping its language out from under the EN post that still references it. Fix: guard with `if ($existing && (int) $existing !== (int) $en_term->term_id)`. BUG 2 — slug-collision adoption rewrites the colliding term's polylang group. When wp_insert_term hits term_exists (two distinct EN terms translating to the same target word), the old code adopted the existing term_id AND called both pll_set_term_language AND pll_save_term_translations on it. The save_term_translations call REASSIGNS the adopted term from its original EN sibling to ours — orphaning the original link. The set_term_language re-asserts language on a term that already had one, which in pathological cases (interacting with bug 1) flipped the language outright. Fix: on term_exists, look up the EN term's current polylang group: - If the colliding term IS already our sibling in this language, return its id (idempotent re-runs are safe). - Otherwise, log a warning and return null. Don't adopt; don't rewrite. The post ends up with fewer tags than the source had, which is information loss but NOT corruption. The alternative (rewriting the group) was a silent data-integrity violation. Test churn: - REMOVED test_get_or_create_adopts_existing_term_on_slug_collision — its assertion encoded the buggy adoption + rewrite behavior. - ADDED test_get_or_create_treats_pll_get_term_self_fallback_as_no_sibling (regression guard for bug 1; stubs pll_get_term to return the input term_id and asserts the handler creates a NEW term instead of reusing the EN id). - ADDED test_get_or_create_reuses_collision_term_when_already_my_polylang_sibling (idempotent re-run path on collision). - ADDED test_get_or_create_returns_null_when_collision_term_belongs_to_different_sibling (the corruption case — returns null, never calls pll_set_term_language or pll_save_term_translations). Production cleanup of the ConfessIt term state (restoring term 171's language to 'en', un-leaking it from ES/FR posts) is deferred to a follow-up manual step once this fix deploys — re-toggling status while the buggy code is live would just re-corrupt. All 9 existing tests for the function (the ones unrelated to the buggy adoption path) continue to pass unchanged. 546/546 theme suite green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR hardens ChangesTerm Propagation Polylang Robustness
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Member
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
JohnRDOrazio
deleted the
fix/term-propagation-slug-collision-and-pll-fallback
branch
3 tasks
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
2 participants
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
Fixes two distinct bugs in
cdcf_get_or_create_translated_term(from #201) that surfaced when backfilling tags on the ConfessIt community_project translations on production 2026-06-08. Both bugs corrupt Polylang term state silently — they were masked on the Interior Castle App test (contemplation/prayer/wisdom-of-the-saints — no translation collision, no corruption) and only became visible when EN tagsexamen+examinationboth translated to the same Romance-language word.Observed corruption on ConfessIt backfill
fr— EN post serving anfr-classified tagTwo bugs, two fixes
Bug 1 —
pll_get_termself-fallback treated as "found a sibling"Some Polylang versions return the input
term_idfrompll_get_termwhen no target-language sibling exists (vs. returning0/false). The old code:treats the fallback as a real sibling, returns the EN id, and
wp_set_object_terms($post, [..., EN_ID, ...])assigns an EN term to a non-EN post. Subsequent runs then callpll_set_term_language(EN_ID, $target_lang)on the EN term itself, flipping its language out from under the EN post that still references it.Fix: guard
if ($existing && (int) $existing !== (int) $en_term->term_id).Bug 2 — slug-collision adoption rewrites the colliding term's Polylang group
When two distinct EN terms translate to the same target word (
examen+examination→examenin IT/ES/FR/PT),wp_insert_termreturnsterm_existswith the existing term's id indata. The old code adopted that id and called:The save call REASSIGNS the adopted term from its original EN sibling to ours — orphaning the original link. The set-language call re-asserts language on a term that already had one and (interacting with bug 1) can flip the language entirely.
Fix: on
term_exists, look up the EN term's current Polylang group:$target_lang, return its id (idempotent re-runs stay safe).null. The post ends up with fewer tags than the source had — that's information loss but not corruption. The alternative (rewriting the group) was a silent data-integrity violation.Test churn
test_get_or_create_adopts_existing_term_on_slug_collision— its assertion encoded the buggy adoption+rewrite behavior.test_get_or_create_treats_pll_get_term_self_fallback_as_no_sibling— regression guard for bug 1; stubspll_get_termto return the input id and asserts the handler creates a new term instead of reusing the EN id.test_get_or_create_reuses_collision_term_when_already_my_polylang_sibling— idempotent re-run path on collision.test_get_or_create_returns_null_when_collision_term_belongs_to_different_sibling— the corruption case is now refused with a logged warning; verifiespll_set_term_languageandpll_save_term_translationsare never called.Net: -1 + 3 = +2 new tests. The other 9 existing tests for the function continue to pass unchanged. 546/546 theme suite green.
Production data cleanup — deferred
Restoring ConfessIt's term state (term 171's language back to
en, un-leaking it from ES/FR posts, optionally adding proper ES/FR/IT/PT siblings for "examination") is deferred to a follow-up manual step once this fix deploys. Re-toggling status while the buggy code is still live would just re-corrupt.Deploy
Backend (theme) change — ship via
gh workflow run deploy.yml -f environment=productionafter merge. A baredeploy.ymlrun defaults to staging and skips the WP theme steps.Related
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes