mikebridge commented on code in PR #44452:
URL: https://github.com/apache/superset/pull/44452#discussion_r4235503329
##########
superset-frontend/plugins/plugin-chart-table/test/controlPanel.test.tsx:
##########
@@ -473,6 +484,394 @@ test('columnOptions defaults type_generic to String when
missing from datasource
);
});
+function getControl(name: string): ControlConfig {
+ for (const section of config.controlPanelSections) {
+ if (!section) continue;
+ for (const row of section.controlSetRows) {
+ for (const control of row) {
+ if (isCustomControlItem(control) && control.name === name) {
+ return control.config;
+ }
+ }
+ }
+ }
+ throw new Error(`Missing control: ${name}`);
+}
+
+const pagination = getControl('server_pagination');
+const pageLength = getControl('server_page_length');
+
+function panelState(
+ features?: string[],
+ type = 'semantic_view',
+ value = false,
+): ControlPanelState {
+ return {
+ datasource: {
+ id: 1,
+ uid: 'provider-view',
+ type,
+ semantic_view_features: features,
+ } as Dataset,
+ form_data: {
+ datasource: `1__${type}`,
+ viz_type: 'table',
+ server_pagination: value,
+ },
+ controls: { server_pagination: { type: 'CheckboxControl', value } },
+ slice: { slice_id: 1 },
+ common: {},
+ };
+}
+
+test('recomputes pagination controls when datasource metadata changes', () => {
+ const registry = getChartControlPanelRegistry();
+ registry.registerValue('table', {
+ controlPanelSections: [
+ {
+ label: 'Options',
+ expanded: true,
+ controlSetRows: [
+ [{ name: 'server_pagination', config: pagination }],
+ [{ name: 'server_page_length', config: pageLength }],
+ ],
+ },
+ ],
+ });
+ const state = panelState(['ROW_OFFSET']);
+ const props = {
+ actions: { setControlValue: jest.fn() },
+ chart: { chartStatus: 'success', queriesResponse: null },
+ controls: {
+ ...state.controls,
+ server_page_length: { type: 'SelectControl', value: 10 },
+ },
+ datasource_type: DatasourceType.SemanticView,
+ exploreState: state,
+ form_data: state.form_data,
+ isDatasourceMetaLoading: false,
+ errorMessage: null,
+ onQuery: jest.fn(),
+ onStop: jest.fn(),
+ canStopQuery: false,
+ chartIsStale: false,
+ } as unknown as ControlPanelsContainerProps;
+
+ try {
+ const { rerender } = render(<ControlPanelsContainer {...props} />, {
+ useRedux: true,
+ });
+ expect(screen.getByRole('checkbox')).toBeEnabled();
+ rerender(
+ <ControlPanelsContainer
+ {...props}
+ exploreState={{
+ ...props.exploreState,
+ datasource: panelState([]).datasource as Dataset,
+ }}
+ />,
+ );
+ expect(screen.getByRole('checkbox')).toBeDisabled();
+ } finally {
+ registry.remove('table');
+ }
+});
+
+test.each([
+ ['server_pagination', pagination],
+ ['server_page_length', pageLength],
+])('%s remaps when datasource capabilities change', (name, control) => {
+ const before = panelState(['ROW_OFFSET']);
+ const after = panelState([]);
+ const controlState = after.controls[name];
+
+ expect(control.shouldMapStateToProps?.(before, after, controlState)).toBe(
+ true,
+ );
+ expect(control.mapStateToProps?.(after, controlState)).toMatchObject({
+ disabled: true,
+ });
+});
+
+function renderPagination(state: ControlPanelState, onChange = jest.fn()) {
+ return render(
+ <CheckboxControl
+ name="server_pagination"
+ label="Server pagination"
+ {...pagination.mapStateToProps?.(state,
state.controls.server_pagination)}
+ value={Boolean(state.controls.server_pagination.value)}
+ onChange={onChange}
+ />,
+ );
+}
+
+test.each([undefined, [], ['UNKNOWN'], ['row_offset']])(
+ 'disables unsupported semantic pagination with explanation: %s',
+ async features => {
+ const onChange = jest.fn();
+ const state = panelState(features);
+ renderPagination(state, onChange);
+ const checkbox = screen.getByRole('checkbox');
+ expect(pagination.type).toBe('CheckboxControl');
+ expect(checkbox).toBeDisabled();
+ expect(checkbox).toHaveAccessibleDescription(
+ 'This semantic view does not support server pagination.',
+ );
+ await userEvent.click(screen.getByText('Server pagination'));
+ expect(onChange).not.toHaveBeenCalled();
+ },
+);
+
+test('fails closed while semantic datasource metadata is unavailable', () => {
+ const state = { ...panelState(), datasource: null };
+ renderPagination(state);
+ expect(screen.getByRole('checkbox')).toBeDisabled();
+});
+
+test.each([
+ ['1__semantic_view', '2__semantic_view'],
+ ['cube__orders', 'cube__customers'],
+])(
+ 'withholds pagination advice for stale metadata %s -> %s',
+ (uid, selected) => {
+ const state = panelState([], 'semantic_view', true);
+ state.datasource = {
+ ...state.datasource,
+ id: 1,
+ uid,
+ type: 'semantic_view',
+ semantic_view_features: [],
+ } as Dataset;
+ state.form_data.datasource = selected;
+ const panel = getControl('server_pagination');
+ expect(
+ panel.mapStateToProps?.(state, state.controls.server_pagination),
+ ).toMatchObject({
+ disabled: true,
+ resetLabel: undefined,
+ disabledReason: undefined,
+ });
+ state.datasource = { ...state.datasource, id: 2, uid: selected } as
Dataset;
+ expect(
+ panel.mapStateToProps?.(state, state.controls.server_pagination),
+ ).toMatchObject({
+ disabled: true,
+ resetLabel: 'Turn off server pagination',
+ disabledReason: 'This semantic view does not support server pagination.',
+ });
+ },
+);
+
+test.each([true, false])(
+ 'matches Explore id/type independently of provider uid: offset=%s',
+ supportsOffset => {
+ const state = panelState([], 'semantic_view', true);
+ state.datasource = {
+ ...state.datasource,
+ id: 42,
+ uid: 'provider-orders',
+ type: 'semantic_view',
+ semantic_view_features: supportsOffset ? ['ROW_OFFSET'] : [],
+ } as Dataset;
+ state.form_data.datasource = '42__semantic_view';
+ const panel = getControl('server_pagination');
+ expect(
+ panel.mapStateToProps?.(state, state.controls.server_pagination),
+ ).toMatchObject({
+ disabled: !supportsOffset,
+ resetLabel: 'Turn off server pagination',
+ disabledReason: 'This semantic view does not support server pagination.',
+ });
+ state.datasource = {
+ ...state.datasource,
+ id: 43,
+ uid: '42__semantic_view',
+ } as Dataset;
+ expect(
+ panel.mapStateToProps?.(state, state.controls.server_pagination),
+ ).toMatchObject({
+ disabled: true,
+ resetLabel: undefined,
+ disabledReason: undefined,
+ });
+ state.datasource = {
+ ...state.datasource,
+ id: 42,
+ type: 'table',
+ } as Dataset;
+ expect(
+ panel.mapStateToProps?.(state, state.controls.server_pagination),
+ ).toMatchObject({
+ disabled: true,
+ resetLabel: undefined,
+ disabledReason: undefined,
+ });
+ },
+);
+
+test.each([undefined, [], ['ROW_OFFSET']])(
+ 'matches opaque provider uid before parsing datasource type: %s',
+ features => {
+ const state = panelState(features);
+ state.datasource = { ...state.datasource, uid: 'cube__orders' } as Dataset;
+ state.form_data.datasource = 'cube__orders';
+ renderPagination(state);
+ const disabled = !features?.includes('ROW_OFFSET');
+ expect(screen.getByRole('checkbox').hasAttribute('disabled')).toBe(
+ disabled,
+ );
+ expect(
+ pageLength.mapStateToProps?.(state, state.controls.server_page_length),
+ ).toMatchObject({ disabled });
+ },
+);
+
+test.each([
+ ['table', 'semantic_view', true],
+ ['semantic_view', 'table', false],
+])(
+ 'trusts form datasource %s -> %s while metadata is stale',
+ (previous, next, disabled) => {
+ const state = panelState([], previous, true);
+ state.form_data.datasource = `2__${next}`;
+ renderPagination(state);
+ expect(screen.getByRole('checkbox').hasAttribute('disabled')).toBe(
+ disabled,
+ );
+ expect(
+ pageLength.mapStateToProps?.(state, state.controls.server_pagination),
+ ).toMatchObject({ disabled });
+ expect(state.controls.server_pagination.value).toBe(true);
+ },
+);
+
+test('ignores stale offset capability when switching opaque semantic UIDs', ()
=> {
+ const state = panelState(['ROW_OFFSET'], 'semantic_view', true);
+ state.datasource = {
+ ...state.datasource,
+ uid: 'cube__orders',
+ type: 'semantic_view',
+ semantic_view_features: ['ROW_OFFSET'],
+ } as Dataset;
+ state.form_data.datasource = 'cube__customers';
+
+ for (const name of ['server_pagination', 'server_page_length']) {
+ expect(
+ getControl(name).mapStateToProps?.(state, state.controls[name]),
+ ).toMatchObject({ disabled: true });
+ }
+});
+
+test.each(['server_pagination', 'server_page_length'])(
+ '%s ignores stale offset capability from another semantic view',
+ name => {
+ const state = panelState(['ROW_OFFSET'], 'semantic_view', true);
+ state.datasource = {
+ ...state.datasource,
+ uid: '1__semantic_view',
+ } as Dataset;
+ state.form_data.datasource = '2__semantic_view';
+
+ expect(
+ getControl(name).mapStateToProps?.(state, state.controls[name]),
+ ).toMatchObject({ disabled: true });
+ expect(state.controls.server_pagination.value).toBe(true);
+ },
+);
+
+test('does not offer a reset before datasource metadata arrives', () => {
+ const state = {
+ ...panelState(undefined, 'semantic_view', true),
+ datasource: null,
+ };
+ renderPagination(state);
+ expect(
+ getControl('server_pagination').mapStateToProps?.(
+ state,
+ state.controls.server_pagination,
+ ),
+ ).toMatchObject({ disabledReason: undefined, resetLabel: undefined });
+ expect(screen.getByRole('checkbox')).toBeDisabled();
+ expect(screen.getByRole('checkbox')).toBeChecked();
+ expect(
+ screen.queryByRole('button', { name: 'Turn off server pagination' }),
+ ).not.toBeInTheDocument();
+});
+
+test.each([
+ ['semantic_view', ['ROW_OFFSET']],
+ ['table', undefined],
+ ['table', []],
+])('keeps %s pagination editable with %s', async (type, features) => {
+ const onChange = jest.fn();
+ renderPagination(panelState(features, type), onChange);
+ expect(screen.getByRole('checkbox')).toBeEnabled();
+ await userEvent.click(screen.getByRole('checkbox'));
+ expect(onChange).toHaveBeenCalledWith(true);
+ expect(
+ screen.queryByText(/does not support server pagination/),
+ ).not.toBeInTheDocument();
+});
+
+test('disables dependent page length without hiding or resetting saved state',
() => {
+ const state = panelState([], 'semantic_view', true);
+ expect(
+ pageLength.mapStateToProps?.(state, state.controls.server_pagination),
+ ).toMatchObject({ disabled: true });
+ expect(
+ pageLength.visibility?.(
+ { ...state, actions: { setDatasource: jest.fn() }, exportState: {} },
+ {},
+ ),
+ ).toBe(true);
+ expect(state.controls.server_pagination.value).toBe(true);
+});
+
+test('saved true survives datasource changes until an explicit keyboard edit',
async () => {
+ const saved = Object.freeze({ server_pagination: true });
+ const onEdit = jest.fn();
+ function Editor({ features }: { features?: string[] }) {
+ const [value, setValue] = useState<boolean>(saved.server_pagination);
+ const state = panelState(features, 'semantic_view', value);
+ return (
+ <CheckboxControl
+ name="server_pagination"
+ label="Server pagination"
+ {...pagination.mapStateToProps?.(
+ state,
+ state.controls.server_pagination,
+ )}
+ value={value}
+ onChange={next => {
+ setValue(next);
+ onEdit(next);
+ }}
+ />
+ );
+ }
+ const { rerender } = render(<Editor features={['ROW_OFFSET']} />);
+ expect(screen.getByRole('checkbox')).toBeChecked();
+ rerender(<Editor features={[]} />);
+ expect(screen.getByRole('checkbox')).toBeChecked();
+ expect(screen.getByRole('checkbox')).toBeDisabled();
+ expect(onEdit).not.toHaveBeenCalled();
+ await userEvent.tab();
+ expect(
+ screen.getByRole('button', { name: 'Turn off server pagination' }),
+ ).toHaveFocus();
+ await userEvent.type(
Review Comment:
Good catch—fixed in dfe94778cfac1c91d28f167b910c7ffa4f3f7a79 by using
userEvent.keyboard("{Enter}") after the existing Tab/focus assertion. I
verified the gap with a temporary mutant that prevents Enter activation but
leaves mouse clicks working: the old test passed, the corrected test failed,
and restoring the component made both focused suites pass (54 tests). The
change is test-only.
--
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]
