You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Spun out from #939, where both Copilot and ask-bonk flagged this.
When handleMenuSetItems rewrites a menu's items, it currently writes translation_group = NULL on every new row. That silently breaks the cross-locale linkage the rest of the menu system relies on. Before shipping a fix I would like want input on which approach to take, especially since one of them would not be a bug fix in my opinion as it would expand the public API.
Some context
Menu items carry both locale and translation_group so that a single nav entry retains identity across translations, that way the EN "Home" and the FR "Accueil" share a translation_group and are recognizable as the same logical item.
Three code paths insert into _emdash_menu_items, but only two of them respect this convention.
handleMenuItemCreate sets translation_group to the new item's own id. handleMenuCreate, when cloning items as part of a translationOf call, inherits each source item's translation_group so the clones stay linked to their originals. handleMenuSetItems is the odd one out as it leaves translation_group null on every insert.
User-visible consequence
Create main in en, add items via handleMenuItemCreate — items get translation_group = id.
Create the FR translation via menu_create({ translationOf }) — FR items inherit each EN item's translation_group, so EN/FR are linked.
Update the FR items via menu_set_items — every item is rewritten with translation_group = NULL. The linkage from step 2 is gone.
After step 3 there's no way to reconnect "Accueil" with "Home" without manually backfilling.
Options to fix that
I see two options to fix that, but maybe I'm missing something.
Always assign translation_group = id. Smallest change, matches handleMenuItemCreate. Every menu_set_items call still breaks pre-existing linkage, but at least the column is never null. Treats the call as a blank-slate replacement.
Accept optional translationGroup per item in the input schema. Lets callers explicitly thread cross-locale identity. Opt-in and unsurprising, but expands the public MCP/REST API surface.
My instinct would be to go for option 2. It's the most impact but it's the solution I think would live best on the long run (and avoid that issue to come back later on). But as this is touching external surfaces, maybe it's not such a good idea.
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
Uh oh!
There was an error while loading. Please reload this page.
-
Spun out from #939, where both Copilot and ask-bonk flagged this.
When handleMenuSetItems rewrites a menu's items, it currently writes translation_group = NULL on every new row. That silently breaks the cross-locale linkage the rest of the menu system relies on. Before shipping a fix I would like want input on which approach to take, especially since one of them would not be a bug fix in my opinion as it would expand the public API.
Some context
Menu items carry both locale and translation_group so that a single nav entry retains identity across translations, that way the EN "Home" and the FR "Accueil" share a translation_group and are recognizable as the same logical item.
Three code paths insert into _emdash_menu_items, but only two of them respect this convention.
handleMenuItemCreate sets translation_group to the new item's own id.
handleMenuCreate, when cloning items as part of a translationOf call, inherits each source item's translation_group so the clones stay linked to their originals.
handleMenuSetItems is the odd one out as it leaves translation_group null on every insert.
User-visible consequence
After step 3 there's no way to reconnect "Accueil" with "Home" without manually backfilling.
Options to fix that
I see two options to fix that, but maybe I'm missing something.
My instinct would be to go for option 2. It's the most impact but it's the solution I think would live best on the long run (and avoid that issue to come back later on). But as this is touching external surfaces, maybe it's not such a good idea.
Beta Was this translation helpful? Give feedback.
All reactions