codeant-ai-for-open-source[bot] commented on code in PR #42142:
URL: https://github.com/apache/superset/pull/42142#discussion_r3697446412
##########
superset/commands/dashboard/update.py:
##########
@@ -83,11 +83,23 @@ def run(self) -> Model:
json.loads(position_json)
)
- dashboard = DashboardDAO.update(self._model, self._properties)
- if self._properties.get("json_metadata"):
+ # ``set_dash_metadata`` merges the incoming metadata against
+ # ``dashboard.params_dict`` (the *stored* ``json_metadata``) to
+ # preserve fields the caller omitted. Routing ``json_metadata``
+ # through the generic attribute update below would overwrite
+ # that stored value before the merge ever sees it, silently
+ # collapsing the merge into a no-op and resetting any omitted
+ # field to its default -- so it is excluded here and applied
+ # exclusively via ``set_dash_metadata``.
+ json_metadata = self._properties.get("json_metadata")
+ dashboard = DashboardDAO.update(
+ self._model,
+ {k: v for k, v in self._properties.items() if k !=
"json_metadata"},
+ )
+ if json_metadata:
DashboardDAO.set_dash_metadata(
dashboard,
- data=json.loads(self._properties.get("json_metadata",
"{}")),
+ data=json.loads(json_metadata),
)
Review Comment:
Agreed. Since `json_metadata` is excluded from the generic update, the
command must distinguish an omitted field from an explicitly empty/null value.
The fix should check key presence and preserve the previous clearing semantics:
```python
json_metadata_present = "json_metadata" in self._properties
json_metadata = self._properties.get("json_metadata")
dashboard = DashboardDAO.update(
self._model,
{k: v for k, v in self._properties.items() if k != "json_metadata"},
)
if json_metadata_present:
if json_metadata:
DashboardDAO.set_dash_metadata(
dashboard,
data=json.loads(json_metadata),
)
else:
# Preserve the API's prior behavior for explicit null/empty values.
dashboard.json_metadata = json_metadata
```
This keeps partial non-empty metadata updates routed through
`set_dash_metadata`, while `json_metadata: null` or `json_metadata: ""`
explicitly clears the stored value. An omitted `json_metadata` key remains
untouched. A regression test should cover both explicit clearing values and
omission.
--
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]
