bito-code-review[bot] commented on code in PR #38347:
URL: https://github.com/apache/superset/pull/38347#discussion_r2873949773
##########
tests/integration_tests/charts/api_tests.py:
##########
@@ -321,6 +321,16 @@ def test_delete_chart(self):
model = db.session.query(Slice).get(chart_id)
assert model is None
+ # Verify audit log
+ log = (
+ db.session.query(Log)
+ .filter_by(action="delete", slice_id=chart_id)
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Mismatched audit log action assertion</b></div>
<div id="fix">
The test assertion checks for action="delete", but the
@event_logger.log_this_with_context decorator on the delete method sets action
to "ChartRestApi.delete" via the lambda. This mismatch will cause the assertion
to fail. Either update the test to expect "ChartRestApi.delete" or change the
decorator's action to "delete" for consistency with other tests.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- superset/charts/api.py
+++ superset/charts/api.py
@@ -473,7 +473,7 @@
@event_logger.log_this_with_context(
- action=lambda self, *args, **kwargs:
f"{self.__class__.__name__}.delete",
+ action="delete",
log_to_statsd=False,
allow_extra_payload=True,
)
```
</div>
</details>
</div>
<small><i>Code Review Run #5af5a8</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
tests/integration_tests/charts/api_tests.py:
##########
@@ -712,6 +722,17 @@ def test_update_chart(self):
assert model.certified_by == "Mario Rossi"
assert model.certification_details == "Edited certification"
assert model.id in [slice.id for slice in related_dashboard.slices]
+
+ # Verify audit log
+ log = (
+ db.session.query(Log)
+ .filter_by(action="put", slice_id=chart_id)
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Mismatched audit log action assertion</b></div>
<div id="fix">
Similar to the delete test, the put test assertion checks for action="put",
but the logging decorator sets action to "ChartRestApi.put". This will cause
the test to fail. Update the decorator's action to "put" for consistency.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- superset/charts/api.py
+++ superset/charts/api.py
@@ -389,7 +389,7 @@
@event_logger.log_this_with_context(
- action=lambda self, *args, **kwargs:
f"{self.__class__.__name__}.put",
+ action="put",
log_to_statsd=False,
allow_extra_payload=True,
)
```
</div>
</details>
</div>
<small><i>Code Review Run #5af5a8</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
