sadpandajoe commented on code in PR #44452:
URL: https://github.com/apache/superset/pull/44452#discussion_r4234100241
##########
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:
`userEvent.type()` clicks its target before typing (unless `skipClick` is
set), so here the click on "Turn off server pagination" already calls
`onChange(false)` and removes the button before `{enter}` is sent. The
unchecked-state and `toHaveBeenCalledTimes(1)` assertions then pass without any
keyboard activation, so a regression that breaks Enter on the reset button
while leaving mouse clicks working would still go green, and this is the only
test that claims to cover the keyboard reset. Could this use `await
userEvent.keyboard('{Enter}')` after the existing focus assertion so the
keyboard path is actually exercised?
--
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]
