bito-code-review[bot] commented on code in PR #44148:
URL: https://github.com/apache/superset/pull/44148#discussion_r4140636094
##########
tests/unit_tests/mcp_service/chart/test_compile.py:
##########
@@ -766,3 +766,29 @@ def test_aggregation_ambiguity_returns_validation_errors()
-> None:
)
assert len(errors) == 1
assert errors[0].error_code == "AMBIGUOUS_DATASET_REFERENCE"
+
+
+@patch("superset.commands.chart.data.get_data_command.ChartDataCommand")
+@patch("superset.common.query_context_factory.QueryContextFactory")
+def test_compile_chart_returns_structured_error_for_malformed_gantt_form_data(
+ mock_factory, mock_cmd_cls
+):
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing test type annotations</b></div>
<div id="fix">
New test omits the return type annotation and mock parameter types mandated
by BITO.md adaptive rules 14234/7819 and 12787. Sibling tests here are also
untyped, but these rules apply to new code; annotate `mock_factory: Mock`,
`mock_cmd_cls: Mock` (`Mock` is already imported) and add `-> None` to satisfy
mypy and the org typing standard.
</div>
</div>
<small><i>Code Review Run #fd76c7</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
##########
superset-frontend/plugins/plugin-chart-country-map/src/CountryMap.ts:
##########
@@ -178,6 +182,11 @@ function CountryMap(element: HTMLElement, props:
CountryMapProps) {
// Track mouse position to distinguish clicks from drags
let mousedownPos: { x: number; y: number } | null = null;
+ const sourceValue = (code: string) =>
+ sourceValues && Object.prototype.hasOwnProperty.call(sourceValues, code)
+ ? sourceValues[code]
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Use Map to avoid object injection</b></div>
<div id="fix">
The linter flags the dynamic key access `sourceValues[code]` as an
object-injection sink. The current `Object.prototype.hasOwnProperty.call` guard
already mitigates prototype pollution, but if you want to fully silence the
linter and harden the code, consider replacing `Record<string, string>` with
`Map<string, string>` and using `.has()`/`.get()`. Note that this requires
updating all related code consistently: transformProps.ts (where `sourceValues`
is constructed as a Record) and the test in transformProps.test.ts (which
asserts a plain-object shape).
</div>
</div>
<small><i>Code Review Run #fd76c7</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]
