bito-code-review[bot] commented on PR #44025:
URL: https://github.com/apache/superset/pull/44025#issuecomment-5638004585
<!-- Bito Reply -->
The flagged issue is correct. In `UpdateDashboardColorsConfigCommand`, the
logic `key in self._properties and self._properties[key] != metadata.get(key)`
fails to detect a change when the new value is `None` (explicitly provided) and
the key is missing from the existing `metadata` (where `metadata.get(key)`
returns `None`).
To resolve this, you should check if the key is present in
`self._properties` and compare the values directly, ensuring that `None` is
treated as a distinct value from a missing key if necessary, or simply ensure
the comparison handles the absence of the key correctly.
Here is a corrected version of `_changes_authoritative_colors`:
```python
def _changes_authoritative_colors(self) -> bool:
assert self._model
metadata = json.loads(self._model.json_metadata or "{}")
for key in self._AUTHORITATIVE_COLOR_KEYS:
if key in self._properties:
# Check if the value is actually changing
if self._properties[key] != metadata.get(key):
return True
return False
```
I have validated this fix against the provided diff. Would you like me to
check the other comments on this PR and implement fixes for them as well?
**superset/commands/dashboard/update.py**
```
def _changes_authoritative_colors(self) -> bool:
assert self._model
metadata = json.loads(self._model.json_metadata or "{}")
for key in self._AUTHORITATIVE_COLOR_KEYS:
if key in self._properties:
# Check if the value is actually changing
if self._properties[key] != metadata.get(key):
return True
return False
```
--
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]