alex-poor commented on code in PR #40679:
URL: https://github.com/apache/superset/pull/40679#discussion_r4034720003
##########
superset/models/dashboard.py:
##########
@@ -329,13 +349,27 @@ def data(self) -> dict[str, Any]:
positions = self.position_json
if positions:
positions = json.loads(positions)
+ # Resolve every chart name in one shot; the per-slice
``localized_name``
+ # lookups below then read from the request memo instead of hitting the
+ # translation hook once per chart. Gated so a disabled deployment does
+ # not pay for the extra pass over the slices.
+ if is_asset_translation_enabled():
+ translate_many(
+ (slc.slice_name for slc in self.slices),
+ model_name="Slice",
+ field_name="slice_name",
+ )
return {
"id": self.id,
"metadata": self.params_dict,
"certified_by": self.certified_by,
"certification_details": self.certification_details,
"css": self.css,
+ # ``dashboard_title`` stays canonical: the layout header seeds
+ # ``meta.text`` from it and persists it on save. The localized
value
+ # is exposed separately for display only.
"dashboard_title": self.dashboard_title,
+ "localized_title": self.localized_title,
Review Comment:
Removed in fa68898 — you're right, and the earlier fix was the wrong shape:
popping the key in one test left every other caller to do the same, and
`dao_tests` feeds `dashboard.data` into `set_dash_metadata` the same way.
Checked the consumers before removing, and those two tests are the only ones
left, so the key and its prefetch are gone. The localized title still reaches
the frontend from `DashboardGetResponseSchema`, which is where it was already
being read, and the test workaround is reverted.
##########
superset/jinja_context.py:
##########
@@ -1108,6 +1109,20 @@ def set_context(self, **kwargs: Any) -> None:
}
)
+ def i18n_with_cache_key(default_text: str) -> str:
+ # The rendered value varies by locale, so it has to vary the query
+ # cache key too -- otherwise one viewer's translated SQL result is
+ # served to a viewer in another locale within the cache timeout.
+ # Keyed on the resolved text rather than the locale so locales that
+ # resolve alike still share a cache entry.
+ return extra_cache.cache_key_wrapper(i18n_macro(default_text))
+
+ # Registered only when a deployment has not already bound this name
+ # through JINJA_CONTEXT_ADDONS: the macro is new, so an existing addon
+ # called ``i18n`` has to keep working across the upgrade.
+ if "i18n" not in self._context:
Review Comment:
Fixed in fa68898 — the guard now tests `context_addons()` alone. Traced it:
`models/helpers.py` merges `template_params_dict` into the kwargs that become
`_context`, so a dataset parameter named `i18n` landed there before the macro
registration ran and would have left a non-callable bound to the name. Addons
keep precedence, per-query parameters no longer displace a core macro, and
there is a test for the template_param case.
##########
superset/dashboards/schemas.py:
##########
@@ -280,6 +284,9 @@ class DashboardGetResponseSchema(Schema):
dashboard_title = fields.String(
metadata={"description": dashboard_title_description}
)
+ localized_title = fields.String(
Review Comment:
Fixed in fa68898 — `allow_none=True` on the schema fields and `string |
null` in the TypeScript declarations. Applied the same treatment to
`localized_name`: `slice_name` is nullable too, so the chart and Explore
schemas had the identical gap. The regenerated spec picks up `nullable: true`
in the four affected places.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
