mirror of
https://github.com/apache/superset.git
synced 2026-08-25 09:31:16 +00:00
Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
c722f405a4 | ||
|
|
b930612a47 | ||
|
|
7ad5726717 |
+30
-1
@@ -779,6 +779,35 @@ function EditorsSelector({
|
||||
const ResultTable =
|
||||
extensionsRegistry.get('sqleditor.extension.resultTable') ?? FilterableTable;
|
||||
|
||||
// D3's '%' type is a valid spec that multiplies by 100, so it never trips
|
||||
// the "Invalid format" fallback even when applied to a raw count.
|
||||
const isPercentD3Format = (d3format?: string): boolean =>
|
||||
!!d3format && d3format.trim().endsWith('%');
|
||||
|
||||
const isCountExpression = (expression?: string): boolean =>
|
||||
!!expression && /^\s*count\s*\(/i.test(expression);
|
||||
|
||||
function renderMetricFormatWarning(item: Record<string, any>): ReactNode {
|
||||
if (
|
||||
!isCountExpression(item.expression) ||
|
||||
!isPercentD3Format(item.d3format)
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
return (
|
||||
<Alert
|
||||
css={themeParam => ({ marginBottom: themeParam.sizeUnit * 4 })}
|
||||
type="warning"
|
||||
showIcon
|
||||
message={t(
|
||||
'This metric is a count, but its D3 format is a percentage. ' +
|
||||
'Percent formats multiply the value by 100, which will make a ' +
|
||||
'raw count render as a misleadingly large number.',
|
||||
)}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
// Redux connector types
|
||||
interface QueryPayload {
|
||||
client_id?: string;
|
||||
@@ -2140,7 +2169,7 @@ function DatasourceEditor({
|
||||
}}
|
||||
expandFieldset={
|
||||
<FormContainer>
|
||||
<Fieldset compact>
|
||||
<Fieldset compact renderWarning={renderMetricFormatWarning}>
|
||||
<Field
|
||||
fieldKey="expression"
|
||||
label={t('SQL expression')}
|
||||
|
||||
+91
@@ -0,0 +1,91 @@
|
||||
/**
|
||||
* Licensed to the Apache Software Foundation (ASF) under one
|
||||
* or more contributor license agreements. See the NOTICE file
|
||||
* distributed with this work for additional information
|
||||
* regarding copyright ownership. The ASF licenses this file
|
||||
* to you under the Apache License, Version 2.0 (the
|
||||
* "License"); you may not use this file except in compliance
|
||||
* with the License. You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing,
|
||||
* software distributed under the License is distributed on an
|
||||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
* KIND, either express or implied. See the License for the
|
||||
* specific language governing permissions and limitations
|
||||
* under the License.
|
||||
*/
|
||||
import fetchMock from 'fetch-mock';
|
||||
import { screen, userEvent, waitFor } from 'spec/helpers/testing-library';
|
||||
import {
|
||||
createProps,
|
||||
DATASOURCE_ENDPOINT,
|
||||
setupDatasourceEditorMocks,
|
||||
cleanupAsyncOperations,
|
||||
fastRender,
|
||||
dismissDatasourceWarning,
|
||||
} from './DatasourceEditor.test.utils';
|
||||
|
||||
beforeEach(() => {
|
||||
fetchMock.get(DATASOURCE_ENDPOINT, [], { name: DATASOURCE_ENDPOINT });
|
||||
setupDatasourceEditorMocks();
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await cleanupAsyncOperations();
|
||||
fetchMock.clearHistory().removeRoutes();
|
||||
});
|
||||
|
||||
const WARNING_TEXT = /D3 format is a percentage/i;
|
||||
|
||||
// A '%' format is valid syntax, so it never hits the "Invalid format" fallback.
|
||||
test('warns when a percent D3 format is set on a COUNT metric', async () => {
|
||||
const testProps = createProps();
|
||||
fastRender(testProps);
|
||||
await dismissDatasourceWarning();
|
||||
|
||||
await userEvent.click(await screen.findByTestId('collection-tab-Metrics'));
|
||||
const expandToggles = await screen.findAllByLabelText(/expand row/i);
|
||||
// Rows sort by metric id descending, so `COUNT(*)` (id 7) is first.
|
||||
await userEvent.click(expandToggles[0]);
|
||||
|
||||
expect(screen.queryByText(WARNING_TEXT)).not.toBeInTheDocument();
|
||||
|
||||
await userEvent.type(await screen.findByPlaceholderText('%y/%m/%d'), '.0%');
|
||||
|
||||
expect(await screen.findByText(WARNING_TEXT)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
test('does not warn for a non-percent format on a COUNT metric', async () => {
|
||||
const testProps = createProps();
|
||||
fastRender(testProps);
|
||||
await dismissDatasourceWarning();
|
||||
|
||||
await userEvent.click(await screen.findByTestId('collection-tab-Metrics'));
|
||||
const expandToggles = await screen.findAllByLabelText(/expand row/i);
|
||||
await userEvent.click(expandToggles[0]);
|
||||
|
||||
await userEvent.type(await screen.findByPlaceholderText('%y/%m/%d'), ',.0f');
|
||||
|
||||
await waitFor(() =>
|
||||
expect(screen.queryByText(WARNING_TEXT)).not.toBeInTheDocument(),
|
||||
);
|
||||
});
|
||||
|
||||
test('does not warn for a percent format on a non-COUNT metric', async () => {
|
||||
const testProps = createProps();
|
||||
fastRender(testProps);
|
||||
await dismissDatasourceWarning();
|
||||
|
||||
await userEvent.click(await screen.findByTestId('collection-tab-Metrics'));
|
||||
const expandToggles = await screen.findAllByLabelText(/expand row/i);
|
||||
// Rows sort by metric id descending, so id 1 (`SUM(...)`) sorts last.
|
||||
await userEvent.click(expandToggles[6]);
|
||||
|
||||
await userEvent.type(await screen.findByPlaceholderText('%y/%m/%d'), '.0%');
|
||||
|
||||
await waitFor(() =>
|
||||
expect(screen.queryByText(WARNING_TEXT)).not.toBeInTheDocument(),
|
||||
);
|
||||
});
|
||||
@@ -28,6 +28,7 @@ export interface FieldsetProps {
|
||||
item?: Record<string, any>;
|
||||
title?: ReactNode;
|
||||
compact?: boolean;
|
||||
renderWarning?: (item: Record<string, any>) => ReactNode;
|
||||
}
|
||||
|
||||
type fieldKeyType = string | number;
|
||||
@@ -38,6 +39,7 @@ export default function Fieldset({
|
||||
item = {},
|
||||
title = null,
|
||||
compact = false,
|
||||
renderWarning,
|
||||
}: FieldsetProps) {
|
||||
// Controls report their edits asynchronously - TextControl debounces by
|
||||
// FAST_DEBOUNCE - so the callback that eventually fires was built during an
|
||||
@@ -78,6 +80,7 @@ export default function Fieldset({
|
||||
</Typography.Title>
|
||||
)}
|
||||
|
||||
{renderWarning?.(item)}
|
||||
{recurseReactClone(children, Field, propExtender)}
|
||||
</Form>
|
||||
);
|
||||
|
||||
+6
-1
@@ -88,7 +88,12 @@ export const ResultsPaneOnDashboard = ({
|
||||
|
||||
return (
|
||||
<Wrapper>
|
||||
<Tabs activeKey={activeTabKey} onChange={setActiveTabKey} items={items} />
|
||||
<Tabs
|
||||
fullHeight
|
||||
activeKey={activeTabKey}
|
||||
onChange={setActiveTabKey}
|
||||
items={items}
|
||||
/>
|
||||
</Wrapper>
|
||||
);
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user