Compare commits

...
4 changed files with 130 additions and 2 deletions
@@ -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')}
@@ -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>
);
@@ -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>
);
};