bito-code-review[bot] commented on code in PR #37112:
URL: https://github.com/apache/superset/pull/37112#discussion_r2700646548
##########
superset-frontend/src/dashboard/components/SliceHeader/SliceHeader.test.tsx:
##########
@@ -677,8 +671,187 @@ test('Should show RowCountLabel in embedded when
uiConfig.showRowLimitWarning is
initialState: rowCountState,
});
- expect(screen.getByTestId('warning')).toBeInTheDocument();
+ expect(screen.queryByTestId('warning')).not.toBeInTheDocument();
+ expect(screen.queryByTestId('row-count-label')).not.toBeInTheDocument();
mockIsEmbedded.mockRestore();
mockUseUiConfig.mockRestore();
});
+
+test('Should show row count badge for table chart without server pagination',
() => {
+ const mockUseUiConfig = useUiConfig as jest.MockedFunction<
+ typeof useUiConfig
+ >;
+ mockUseUiConfig.mockReturnValue({
+ hideTitle: false,
+ hideTab: false,
+ hideNav: false,
+ hideChartControls: false,
+ emitDataMasks: false,
+ showRowLimitWarning: true,
+ });
+
+ const props = createProps({
+ formData: {
+ ...createProps().formData,
+ viz_type: VizType.Table,
+ row_limit: 10,
+ },
+ slice: {
+ ...createProps().slice,
+ form_data: {
+ ...createProps().slice.form_data,
+ viz_type: VizType.Table,
+ row_limit: 10,
+ },
+ viz_type: VizType.Table,
+ },
+ });
+ const tableState = {
+ ...initialState,
+ charts: {
+ [props.slice.slice_id]: {
+ id: MOCKED_CHART_ID,
+ chartStatus: 'rendered',
+ queriesResponse: [
+ {
+ sql_rowcount: 50,
+ data: [],
+ },
+ ],
+ },
+ },
+ };
+
+ render(<SliceHeader {...props} />, {
+ useRedux: true,
+ useRouter: true,
+ initialState: tableState,
+ });
+
+ expect(screen.getByTestId('warning')).toBeInTheDocument();
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing testId on warning icon</b></div>
<div id="fix">
Add `data-testid="warning"` to the Icons.WarningOutlined element to satisfy
the test’s `getByTestId('warning')` query.
</div>
</div>
<small><i>Code Review Run #86d858</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/SliceHeader/SliceHeader.test.tsx:
##########
@@ -677,8 +671,187 @@ test('Should show RowCountLabel in embedded when
uiConfig.showRowLimitWarning is
initialState: rowCountState,
});
- expect(screen.getByTestId('warning')).toBeInTheDocument();
+ expect(screen.queryByTestId('warning')).not.toBeInTheDocument();
+ expect(screen.queryByTestId('row-count-label')).not.toBeInTheDocument();
mockIsEmbedded.mockRestore();
mockUseUiConfig.mockRestore();
});
+
+test('Should show row count badge for table chart without server pagination',
() => {
+ const mockUseUiConfig = useUiConfig as jest.MockedFunction<
+ typeof useUiConfig
+ >;
+ mockUseUiConfig.mockReturnValue({
+ hideTitle: false,
+ hideTab: false,
+ hideNav: false,
+ hideChartControls: false,
+ emitDataMasks: false,
+ showRowLimitWarning: true,
+ });
+
+ const props = createProps({
+ formData: {
+ ...createProps().formData,
+ viz_type: VizType.Table,
+ row_limit: 10,
+ },
+ slice: {
+ ...createProps().slice,
+ form_data: {
+ ...createProps().slice.form_data,
+ viz_type: VizType.Table,
+ row_limit: 10,
+ },
+ viz_type: VizType.Table,
+ },
+ });
+ const tableState = {
+ ...initialState,
+ charts: {
+ [props.slice.slice_id]: {
+ id: MOCKED_CHART_ID,
+ chartStatus: 'rendered',
+ queriesResponse: [
+ {
+ sql_rowcount: 50,
+ data: [],
+ },
+ ],
+ },
+ },
+ };
+
+ render(<SliceHeader {...props} />, {
+ useRedux: true,
+ useRouter: true,
+ initialState: tableState,
+ });
+
+ expect(screen.getByTestId('warning')).toBeInTheDocument();
+
+ mockUseUiConfig.mockRestore();
+});
+
+test('Should show row count warning for table chart with server pagination
when limit is reached', () => {
+ const props = createProps({
+ formData: {
+ ...createProps().formData,
+ viz_type: VizType.Table,
+ row_limit: 10,
+ server_pagination: true,
+ },
+ slice: {
+ ...createProps().slice,
+ form_data: {
+ ...createProps().slice.form_data,
+ viz_type: VizType.Table,
+ row_limit: 10,
+ server_pagination: true,
+ },
+ viz_type: VizType.Table,
+ },
+ });
+ const tableWithPaginationState = {
+ ...initialState,
+ charts: {
+ [props.slice.slice_id]: {
+ id: MOCKED_CHART_ID,
+ chartStatus: 'rendered',
+ queriesResponse: [
+ {
+ sql_rowcount: 10,
+ data: Array(10).fill({}),
+ },
+ {
+ data: [{ rowcount: 50 }],
+ },
+ ],
+ },
+ },
+ };
+
+ const mockUseUiConfig = useUiConfig as jest.MockedFunction<
+ typeof useUiConfig
+ >;
+ mockUseUiConfig.mockReturnValue({
+ hideTitle: false,
+ hideTab: false,
+ hideNav: false,
+ hideChartControls: false,
+ emitDataMasks: false,
+ showRowLimitWarning: true,
+ });
+
+ render(<SliceHeader {...props} />, {
+ useRedux: true,
+ useRouter: true,
+ initialState: tableWithPaginationState,
+ });
+
+ expect(screen.getByTestId('warning')).toBeInTheDocument();
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing testId on warning icon</b></div>
<div id="fix">
Add `data-testid="warning"` to the Icons.WarningOutlined element to satisfy
the test’s `getByTestId('warning')` query.
</div>
</div>
<small><i>Code Review Run #86d858</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]