mirror of
https://github.com/hyperdxio/hyperdx
synced 2026-04-21 13:37:15 +00:00
fix: Fix minor bugs in chart editor (#2050)
## Summary This PR makes a few minor improvements to dashboard tiles and the chart editor ### Hide the "Add Alert" button on dashboard tiles based on raw SQL These tiles don't yet support alerts <img width="766" height="424" alt="Screenshot 2026-04-03 at 8 37 36 AM" src="https://github.com/user-attachments/assets/4405c5bb-419d-4ae8-a121-7ddcd2623d87" /> <img width="767" height="447" alt="Screenshot 2026-04-03 at 8 37 42 AM" src="https://github.com/user-attachments/assets/c9b27e7a-9a2b-4f23-863b-d1679d3ea770" /> Closes HDX-3910 ### Hide the "Group By" button on the Attribute explorer for Number Charts Number charts don't support Group By <img width="1224" height="475" alt="Screenshot 2026-04-03 at 8 41 10 AM" src="https://github.com/user-attachments/assets/e854ff39-09b6-4452-b008-cd7bc1e26d09" /> <img width="1219" height="501" alt="Screenshot 2026-04-03 at 8 41 00 AM" src="https://github.com/user-attachments/assets/4180a784-2b1c-4353-a84f-b25f367ff36c" /> <img width="1224" height="476" alt="Screenshot 2026-04-03 at 8 41 02 AM" src="https://github.com/user-attachments/assets/89780bea-d53a-4287-8056-e73c8ce6927f" /> <img width="1227" height="485" alt="Screenshot 2026-04-03 at 8 40 56 AM" src="https://github.com/user-attachments/assets/197cbcdd-4264-45d5-a0f0-10e4c67ab67d" /> Closes HDX-3871 ### Disable the "Custom" Aggregation for Metric queries These were already broken because there was no input available for the user to provide the custom aggregation. Custom aggregations don't make much sense for metric sources, since the queries we build for metrics would be very difficult for users to build custom aggregations on. We also now have SQL-based charts if users want to do custom aggregations on metric sources. <img width="459" height="581" alt="Screenshot 2026-04-03 at 9 03 40 AM" src="https://github.com/user-attachments/assets/5230627c-5f51-4640-9b16-4719f9a1ca91" /> Closes HDX-3799 ### How to test locally or on Vercel These can be tested in the preview environment (except for the alert button, that must be tested locally)
This commit is contained in:
parent
58e2e8c660
commit
b4e1498eb3
7 changed files with 126 additions and 38 deletions
5
.changeset/short-wombats-develop.md
Normal file
5
.changeset/short-wombats-develop.md
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
"@hyperdx/app": patch
|
||||
---
|
||||
|
||||
fix: Fix minor bugs in chart editor
|
||||
|
|
@ -367,27 +367,28 @@ const Tile = forwardRef(
|
|||
>
|
||||
{(chart.config.displayType === DisplayType.Line ||
|
||||
chart.config.displayType === DisplayType.StackedBar ||
|
||||
chart.config.displayType === DisplayType.Number) && (
|
||||
<Indicator
|
||||
size={alert?.state === AlertState.OK ? 6 : 8}
|
||||
zIndex={1}
|
||||
color={alertIndicatorColor}
|
||||
processing={alert?.state === AlertState.ALERT}
|
||||
label={!alert && <span className="fs-8">+</span>}
|
||||
mr={4}
|
||||
>
|
||||
<Tooltip label={alertTooltip} withArrow>
|
||||
<ActionIcon
|
||||
data-testid={`tile-alerts-button-${chart.id}`}
|
||||
variant="subtle"
|
||||
size="sm"
|
||||
onClick={onEditClick}
|
||||
>
|
||||
<IconBell size={16} />
|
||||
</ActionIcon>
|
||||
</Tooltip>
|
||||
</Indicator>
|
||||
)}
|
||||
chart.config.displayType === DisplayType.Number) &&
|
||||
!isRawSqlSavedChartConfig(chart.config) && (
|
||||
<Indicator
|
||||
size={alert?.state === AlertState.OK ? 6 : 8}
|
||||
zIndex={1}
|
||||
color={alertIndicatorColor}
|
||||
processing={alert?.state === AlertState.ALERT}
|
||||
label={!alert && <span className="fs-8">+</span>}
|
||||
mr={4}
|
||||
>
|
||||
<Tooltip label={alertTooltip} withArrow>
|
||||
<ActionIcon
|
||||
data-testid={`tile-alerts-button-${chart.id}`}
|
||||
variant="subtle"
|
||||
size="sm"
|
||||
onClick={onEditClick}
|
||||
>
|
||||
<IconBell size={16} />
|
||||
</ActionIcon>
|
||||
</Tooltip>
|
||||
</Indicator>
|
||||
)}
|
||||
|
||||
<ActionIcon
|
||||
data-testid={`tile-duplicate-button-${chart.id}`}
|
||||
|
|
@ -469,7 +470,7 @@ const Tile = forwardRef(
|
|||
alertIndicatorColor,
|
||||
alertTooltip,
|
||||
availableSections,
|
||||
chart.config.displayType,
|
||||
chart.config,
|
||||
chart.id,
|
||||
chart.containerId,
|
||||
hovered,
|
||||
|
|
|
|||
|
|
@ -13,10 +13,12 @@ function AggFnSelect({
|
|||
value,
|
||||
defaultValue,
|
||||
onChange,
|
||||
hideCustom,
|
||||
}: {
|
||||
value: string;
|
||||
defaultValue: string;
|
||||
onChange: (value: OnChangeValue) => void;
|
||||
hideCustom?: boolean;
|
||||
}) {
|
||||
const _onChange = useCallback(
|
||||
(value: string | null) => {
|
||||
|
|
@ -42,7 +44,7 @@ function AggFnSelect({
|
|||
value={value}
|
||||
defaultValue={defaultValue}
|
||||
onChange={_onChange}
|
||||
data={AGG_FNS}
|
||||
data={hideCustom ? AGG_FNS.filter(fn => fn.value !== 'none') : AGG_FNS}
|
||||
data-testid="agg-fn-select"
|
||||
/>
|
||||
);
|
||||
|
|
@ -52,11 +54,13 @@ export function AggFnSelectControlled({
|
|||
aggFnName,
|
||||
quantileLevelName,
|
||||
defaultValue,
|
||||
hideCustom,
|
||||
...props
|
||||
}: {
|
||||
defaultValue: string;
|
||||
aggFnName: string;
|
||||
quantileLevelName: string;
|
||||
hideCustom?: boolean;
|
||||
} & Omit<UseControllerProps<any>, 'name'>) {
|
||||
const {
|
||||
field: { onChange: onAggFnChange, value: aggFnValue },
|
||||
|
|
@ -96,6 +100,7 @@ export function AggFnSelectControlled({
|
|||
value={value}
|
||||
defaultValue={defaultValue}
|
||||
onChange={onChange}
|
||||
hideCustom={hideCustom}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -220,11 +220,17 @@ function ChartSeriesEditorComponent({
|
|||
const metricType = useWatch({ control, name: `${namePrefix}metricType` });
|
||||
|
||||
// Initialize metricType to 'gauge' when switching to a metric source
|
||||
// and reset 'custom' aggFn to 'count' since custom is not supported for metrics
|
||||
useEffect(() => {
|
||||
if (tableSource?.kind === SourceKind.Metric && !metricType) {
|
||||
setValue(`${namePrefix}metricType`, MetricsDataType.Gauge);
|
||||
if (tableSource?.kind === SourceKind.Metric) {
|
||||
if (!metricType) {
|
||||
setValue(`${namePrefix}metricType`, MetricsDataType.Gauge);
|
||||
}
|
||||
if (aggFn === 'none') {
|
||||
setValue(`${namePrefix}aggFn`, 'count');
|
||||
}
|
||||
}
|
||||
}, [tableSource?.kind, metricType, namePrefix, setValue]);
|
||||
}, [tableSource?.kind, metricType, aggFn, namePrefix, setValue]);
|
||||
|
||||
const tableName =
|
||||
tableSource?.kind === SourceKind.Metric
|
||||
|
|
@ -363,6 +369,7 @@ function ChartSeriesEditorComponent({
|
|||
quantileLevelName={`${namePrefix}level`}
|
||||
defaultValue={AGG_FNS[0]?.value ?? 'avg'}
|
||||
control={control}
|
||||
hideCustom={tableSource?.kind === SourceKind.Metric}
|
||||
/>
|
||||
</div>
|
||||
{tableSource?.kind === SourceKind.Metric && metricType && (
|
||||
|
|
@ -496,7 +503,7 @@ function ChartSeriesEditorComponent({
|
|||
language={aggConditionLanguage === 'sql' ? 'sql' : 'lucene'}
|
||||
metricMetadata={metricMetadata}
|
||||
onAddToWhere={handleAddToWhere}
|
||||
onAddToGroupBy={handleAddToGroupBy}
|
||||
onAddToGroupBy={showGroupBy ? handleAddToGroupBy : undefined}
|
||||
/>
|
||||
)}
|
||||
</>
|
||||
|
|
|
|||
|
|
@ -42,7 +42,7 @@ interface MetricAttributeHelperPanelProps {
|
|||
language: 'sql' | 'lucene';
|
||||
metricMetadata?: MetricMetadata | null;
|
||||
onAddToWhere: (clause: string) => void;
|
||||
onAddToGroupBy: (clause: string) => void;
|
||||
onAddToGroupBy?: (clause: string) => void;
|
||||
}
|
||||
|
||||
const CATEGORY_LABELS: Record<AttributeCategory, string> = {
|
||||
|
|
@ -170,7 +170,7 @@ interface AttributeValueListProps {
|
|||
language: 'sql' | 'lucene';
|
||||
onAddToWhere: (clause: string) => void;
|
||||
onBack: () => void;
|
||||
onAddToGroupBy: (clause: string) => void;
|
||||
onAddToGroupBy?: (clause: string) => void;
|
||||
}
|
||||
|
||||
function AttributeValueList({
|
||||
|
|
@ -217,7 +217,7 @@ function AttributeValueList({
|
|||
attribute.name,
|
||||
'sql',
|
||||
);
|
||||
onAddToGroupBy(clause);
|
||||
onAddToGroupBy?.(clause);
|
||||
}, [attribute, onAddToGroupBy]);
|
||||
|
||||
return (
|
||||
|
|
@ -234,14 +234,16 @@ function AttributeValueList({
|
|||
</Badge>
|
||||
</Group>
|
||||
</UnstyledButton>
|
||||
<Button
|
||||
variant="secondary"
|
||||
size="xs"
|
||||
leftSection={<IconPlus size={14} />}
|
||||
onClick={handleAddToGroupBy}
|
||||
>
|
||||
Group By
|
||||
</Button>
|
||||
{onAddToGroupBy && (
|
||||
<Button
|
||||
variant="secondary"
|
||||
size="xs"
|
||||
leftSection={<IconPlus size={14} />}
|
||||
onClick={handleAddToGroupBy}
|
||||
>
|
||||
Group By
|
||||
</Button>
|
||||
)}
|
||||
</Group>
|
||||
|
||||
<TextInput
|
||||
|
|
|
|||
|
|
@ -18,6 +18,7 @@ export class ChartEditorComponent {
|
|||
private readonly chartTypeInput: Locator;
|
||||
private readonly sourceSelector: Locator;
|
||||
private readonly metricSelector: Locator;
|
||||
private readonly aggFnSelect: Locator;
|
||||
private readonly addOrRemoveAlertButton: Locator;
|
||||
private readonly webhookSelector: Locator;
|
||||
private readonly runQueryButton: Locator;
|
||||
|
|
@ -29,6 +30,7 @@ export class ChartEditorComponent {
|
|||
this.chartTypeInput = page.getByTestId('chart-type-input');
|
||||
this.sourceSelector = page.getByTestId('source-selector');
|
||||
this.metricSelector = page.getByTestId('metric-name-selector');
|
||||
this.aggFnSelect = page.getByTestId('agg-fn-select');
|
||||
this.addOrRemoveAlertButton = page.getByTestId('alert-button');
|
||||
this.webhookSelector = page.getByTestId('select-webhook');
|
||||
this.addNewWebhookButton = page.getByTestId('add-new-webhook-button');
|
||||
|
|
@ -99,6 +101,33 @@ export class ChartEditorComponent {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Select an aggregation function from the dropdown
|
||||
*/
|
||||
async selectAggFn(label: string) {
|
||||
await this.aggFnSelect.click();
|
||||
await this.page.getByRole('option', { name: label }).click();
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the currently selected aggregation function value
|
||||
*/
|
||||
async getSelectedAggFn(): Promise<string | null> {
|
||||
return this.aggFnSelect.inputValue();
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if an aggregation function option is available in the dropdown
|
||||
*/
|
||||
async isAggFnOptionAvailable(label: string): Promise<boolean> {
|
||||
await this.aggFnSelect.click();
|
||||
const option = this.page.getByRole('option', { name: label });
|
||||
const visible = await option.isVisible().catch(() => false);
|
||||
// Close the dropdown
|
||||
await this.page.keyboard.press('Escape');
|
||||
return visible;
|
||||
}
|
||||
|
||||
async clickAddAlert() {
|
||||
await this.addOrRemoveAlertButton.click();
|
||||
this.addNewWebhookButton.waitFor({
|
||||
|
|
@ -250,6 +279,10 @@ export class ChartEditorComponent {
|
|||
return this.metricSelector;
|
||||
}
|
||||
|
||||
get aggFn() {
|
||||
return this.aggFnSelect;
|
||||
}
|
||||
|
||||
get alertButton() {
|
||||
return this.addOrRemoveAlertButton;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -786,6 +786,41 @@ test.describe('Dashboard', { tag: ['@dashboard'] }, () => {
|
|||
});
|
||||
});
|
||||
|
||||
test(
|
||||
'should deselect and hide the Custom aggregation function when switching to a metric source',
|
||||
{ tag: '@full-stack' },
|
||||
async () => {
|
||||
await test.step('Navigate to dashboard and open new tile editor', async () => {
|
||||
await dashboardPage.openNewTileEditor();
|
||||
});
|
||||
|
||||
await test.step('Select the "Custom" aggregation function', async () => {
|
||||
await dashboardPage.chartEditor.selectAggFn('Custom');
|
||||
const selectedAggFn =
|
||||
await dashboardPage.chartEditor.getSelectedAggFn();
|
||||
expect(selectedAggFn).toBe('Custom');
|
||||
});
|
||||
|
||||
await test.step('Switch the source to a metric source', async () => {
|
||||
await dashboardPage.chartEditor.selectSource(
|
||||
DEFAULT_METRICS_SOURCE_NAME,
|
||||
);
|
||||
});
|
||||
|
||||
await test.step('Verify the aggregation function was automatically changed away from "Custom"', async () => {
|
||||
const selectedAggFn =
|
||||
await dashboardPage.chartEditor.getSelectedAggFn();
|
||||
expect(selectedAggFn).toBe('Count of Events');
|
||||
});
|
||||
|
||||
await test.step('Verify the "Custom" option is NOT available in the aggregation dropdown', async () => {
|
||||
const isCustomAvailable =
|
||||
await dashboardPage.chartEditor.isAggFnOptionAvailable('Custom');
|
||||
expect(isCustomAvailable).toBe(false);
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
test(
|
||||
'should clear saved query when WHERE input is cleared and saved',
|
||||
{},
|
||||
|
|
|
|||
Loading…
Reference in a new issue