fix(taxonomies): respect active locale in admin surfaces - #2342
fix(taxonomies): respect active locale in admin surfaces#2342fbartolitsch wants to merge 2 commits into
Conversation
Group translated definitions by logical identity before rendering admin navigation and editor choices. Scope visible term counts and cache entries to the resolved content locale.
🦋 Changeset detectedLatest commit: f488ff5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is a focused bug fix with the right shape: normalize localized taxonomy definitions to one logical row per locale, then use that resolved locale in admin labels, editor choices, manifest metadata, and visible term counts. I read the full diff, the changed source files, and the relevant route/handler call sites.
Headline: the approach is sound, but the sidebar navigation only respects the active locale for the label — the taxonomy-management link it produces drops the ?locale= parameter, so clicking a DE item lands on the default-locale page. That undermines the stated goal and should be fixed before merge.
I also found a smaller edge where the API term-list/term-get handlers still scope visible counts off the lowest-locale taxonomy definition rather than the active-locale one. That is safe when translations share collections, but it is inconsistent with the public taxonomies/index.ts helpers and worth aligning.
No SQL-injection issues: the dynamic locale predicate is interpolated as a Kysely parameter. No new logged-out round-trips are introduced; the count query just gains a locale predicate and a locale-aware cache key. Tests cover the new resolver, editor filtering, manifest identity, and locale-scoped counts.
Findings
-
[needs fixing]
packages/admin/src/components/Sidebar.tsx:254The sidebar resolves the active-locale taxonomy label using
routeLocale, but the generated taxonomy nav items omit the locale, so the link navigates to e.g./taxonomies/coursewithout?locale=de. Clicking a German taxonomy in the sidebar will open the taxonomy-management route in the configured default/locale-less fallback, which partially defeats the PR's goal of respecting the active content locale on admin surfaces.Extend
NavItemto carry optional query params and append them inresolveItemPathso taxonomy links preserve the active locale:// in the manageItems construction ...getSidebarTaxonomies(manifest.taxonomies, routeLocale, manifest.i18n?.defaultLocale).map( (tax) => ({ to: "/taxonomies/$taxonomy" as const, label: tax.label, icon: getTaxonomyNavIcon(tax.name), params: { taxonomy: tax.name }, search: routeLocale ? { locale: routeLocale } : undefined, minRole: ROLE_EDITOR, }), ), // interface NavItem { ... search?: Record<string, string>; ... } function resolveItemPath(item: NavItem): string { let path = item.to; if (item.params) { for (const [key, value] of Object.entries(item.params)) { path = path.replace(`$${key}`, value); } } if (item.search && Object.keys(item.search).length > 0) { const q = new URLSearchParams(item.search).toString(); path += `?${q}`; } return path; }
-
[suggestion]
packages/core/src/api/handlers/taxonomies.ts:393handleTermListresolves the taxonomy withrequireTaxonomyDef(db, taxonomyName)(no locale), which returns the lowest-locale row, and then scopes visible counts todefCollections(lookup.def). If a taxonomy's translations declare differentcollections, the active-locale term list will count the wrong collections. The public helpergetTaxonomyDef(name, options)already resolves the active-locale definition.Suggestion: pass the resolved
localetorequireTaxonomyDefhere (and inhandleTermGetat line 659) so counts use the same definition the user is actually viewing:const lookup = await requireTaxonomyDef(db, taxonomyName, locale);
github-actions
Bot
added
review/needs-rereview
Overlapping PRsThis PR modifies files that are also changed by other open PRs:
This may cause merge conflicts or duplicated work. A maintainer will coordinate. |
1 participant
What does this PR do?
Summary
Scope
id,locale, andtranslationGroup).@emdash-cms/adminandemdash.Validation
pnpm formatandpnpm format:checkpnpm typecheckpnpm lintpnpm buildgit diff --checkRisk and rollback
translationGroup; legacy definitions without that metadata fall back to their taxonomy name.mainadvanced by two commits after local validation. A Git merge-tree check reports no conflict; CI remains the final integration check.Security/privacy impact
No new external calls, credentials, permissions, or personal-data processing are introduced.
Fixes #2224
Fixes #2338
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges are included.AI-generated code disclosure
Screenshots / test output
Targeted automated tests:
Manual demo verification used one consistent multilingual taxonomy fixture across DE, EN, and FR. The sidebar and editor each displayed one active-locale definition per logical taxonomy; visible term counts were DE 1, EN 2, and FR 3.