[6.x] Fix deleted terms reappearing after clearing the Stache - #15426
Open
duncanmcclean wants to merge 5 commits into
Open
duncanmcclean wants to merge 5 commits into
duncanmcclean wants to merge 5 commits into
Conversation
`TaxonomyTermsStore::getItem` overrides `BasicStore::getItem` without syncing the term's original state, so `UpdateTermReferences` saw a null original slug on deletion and left the term's references in entries. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XzL1xhLTSn6kRxAi7UTcm8
Syncing on every getItem() mutated the shared cached instance, which clobbered pending changes mid-save: TaxonomyTermsStore::save() looks up the old term to delete its file, and Term::save() looks the term up for its isNew check, both of which re-entered getItem() and reset the original. Renaming a term retrieved from the Stache stopped updating entry references, and isDirty() was false inside TermSaving. Sync in makeItemFromFile() instead, restoring what #5502 removed, and once more on the term built in getItem() before it's cached. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cache items are written with forever(), so terms cached by a version that didn't sync their original state persist across an upgrade and come back through the cache hit branch without one. Sync those, but only when it's missing, so a term already in the middle of being saved keeps the original state its pending changes are measured against. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
getItem() already syncs every term it builds, and terms cached in bulk by paths() get their original state backfilled when they're read, so this was only ever duplicating work. Co-Authored-By: Claude Opus 5 (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
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.
Deleting a taxonomy term left its slug behind in any entries referencing it, so the term reappeared as a "ghost" the next time the Stache was rebuilt.
UpdateTermReferencesis what strips the slug from entries, but it bails when$term->getOriginal('slug')is empty — so the delete silently did nothing, and the leftover reference regenerated the term.The original state was empty because
TaxonomyTermsStoreoverridesBasicStore::getItem(to support terms which only exist in entry data), and #5502 removed thesyncOriginal()that used to live inmakeItemFromFile. Every other store still gets it fromBasicStore.This syncs the original state when a term is hydrated, before it's cached. The cache-hit branch backfills too, since items are cached with
forever()and terms cached by an older version would otherwise never get one — but only when it's missing. Syncing unconditionally there mutates the shared cached instance, and bothTaxonomyTermsStore::save()andTerm::save()re-entergetItem()mid-save, which would reset the original and break renaming.Fixes #11264
Caused by #5502
Related: #11058