bito-code-review[bot] commented on code in PR #32958:
URL: https://github.com/apache/superset/pull/32958#discussion_r3485978505
##########
superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.tsx:
##########
@@ -208,7 +208,10 @@ const Chart = (props: ChartProps) => {
);
const isExpanded = useSelector(
(state: RootState) =>
- !!(state.dashboardState as JsonObject).expandedSlices?.[props.id],
+ !!(
+ (state.dashboardState as JsonObject).expandedSlices?.[props.id] ??
+ (state.dashboardState as JsonObject).expandAllSlices
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing type declaration</b></div>
<div id="fix">
The `expandAllSlices` property is set in `hydrate.ts` (line 378) and used in
this selector (line 213), but it's missing from the `DashboardState` type
definition in `types.ts` and the reducer interface in `dashboardState.ts`. This
creates a type inconsistency where the property exists at runtime but isn't
declared in the type system.
</div>
</div>
<small><i>Code Review Run #7bdc42</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/src/dashboard/components/gridComponents/Chart/Chart.test.tsx:
##########
@@ -146,17 +146,80 @@ test('should render a ChartContainer', () => {
expect(getByTestId('chart-container')).toBeInTheDocument();
});
-test('should render a description if it has one and isExpanded=true', () => {
- const { container } = setup(
- {},
- {
- dashboardState: {
- ...defaultState.dashboardState,
- expandedSlices: { [props.id]: true },
- },
- },
- );
- expect(container.querySelector('.slice_description')).toBeInTheDocument();
+describe('no description set', () => {
+ for (const sliceExpanded of [undefined, false, true]) {
+ for (const allExpanded of [undefined, false, true]) {
+ test(`should not render a description, expandedSlices=${sliceExpanded}
and expandAllSlices=${allExpanded}`, () => {
+ const { container } = setup(
+ {},
+ {
+ dashboardState: {
+ ...defaultState.dashboardState,
+ expandedSlices: { [props.id]: sliceExpanded },
+ expandAllSlices: allExpanded,
+ },
+ sliceEntities: {
+ ...sliceEntities,
+ slices: {
+ [queryId]: {
+ ...sliceEntities.slices[queryId],
+ description_markeddown: undefined,
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Typo in test property name</b></div>
<div id="fix">
Typo in property name: the test sets `description_markeddown` but the
component at Chart.tsx:742 checks `slice.description_markdown`. This mismatch
means the test doesn't test what it claims — it always passes regardless of the
actual description value since the source slice retains its original
`description_markdown` from the fixture.
</div>
</div>
<small><i>Code Review Run #7bdc42</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]
