Skip to content

ref(explore): Prepare for the aggregate editor #92177

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 23, 2025
+39 −204
Merged

ref(explore): Prepare for the aggregate editor #92177

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3bcd8dd
ref(explore): Prepare for the aggregate editor
Zylphrex May 23, 2025
f8d4d5e
remove unused equation builder
Zylphrex May 23, 2025
511029d
fix tests
Zylphrex May 23, 2025
4ccac9f
remove unused test
Zylphrex May 23, 2025
4f16da4
Merge branch 'master' into txiao/ref/prepare-explore-for-aggregate-ed…
Zylphrex May 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions static/app/views/explore/components/typeBadge.tsx
View file
Open in desktop
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make this component more similar to how discover renders it

Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,16 @@ interface TypeBadgeProps {
}

export function TypeBadge({func, kind}: TypeBadgeProps) {
if (defined(func)) {
return <Tag type="warning">{t('aggregation')}</Tag>;
if (defined(func) || kind === FieldKind.FUNCTION) {
return <Tag type="success">{t('f(x)')}</Tag>;
}

if (kind === FieldKind.MEASUREMENT) {
return <Tag type="success">{t('number')}</Tag>;
return <Tag type="highlight">{t('number')}</Tag>;
}

if (kind === FieldKind.TAG) {
return <Tag type="highlight">{t('string')}</Tag>;
return <Tag type="warning">{t('string')}</Tag>;
}

return null;
Expand Down
4 changes: 2 additions & 2 deletions static/app/views/explore/tables/columnEditorModal.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ describe('ColumnEditorModal', function () {

await userEvent.click(screen.getByRole('button', {name: 'Add a Column'}));

const columns2 = ['id', 'project', 'None'];
const columns2 = ['id', 'project', '\u2014'];
screen.getAllByTestId('editor-column').forEach((column, i) => {
expect(column).toHaveTextContent(columns2[i]!);
});
Expand All @@ -134,7 +134,7 @@ describe('ColumnEditorModal', function () {
['span.duration', 'number'],
['span.op', 'string'],
];
await userEvent.click(screen.getByRole('button', {name: 'Column None'}));
await userEvent.click(screen.getByRole('button', {name: 'Column \u2014'}));
const columnOptions = await screen.findAllByRole('option');
columnOptions.forEach((option, i) => {
expect(option).toHaveTextContent(options[i]![0]);
Expand Down
14 changes: 10 additions & 4 deletions static/app/views/explore/tables/columnEditorModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ function ColumnEditorRow({
);
}
}
return <TriggerLabel>{!column.column && t('None')}</TriggerLabel>;
return <TriggerLabel>{column.column || t('\u2014')}</TriggerLabel>;
}, [column.column, options]);

return (
Expand All @@ -255,7 +255,7 @@ function ColumnEditorRow({
}}
{...attributes}
>
<Button
<StyledButton
aria-label={t('Drag to reorder')}
borderless
size="sm"
Expand All @@ -276,13 +276,13 @@ function ColumnEditorRow({
},
}}
/>
<Button
<StyledButton
aria-label={t('Remove Column')}
borderless
disabled={!canDelete}
size="sm"
icon={<IconDelete size="sm" />}
onClick={() => onColumnDelete()}
onClick={onColumnDelete}
/>
</RowContainer>
);
Expand All @@ -292,12 +292,18 @@ const RowContainer = styled('div')`
display: flex;
flex-direction: row;
align-items: center;
gap: ${space(1)};

:not(:first-child) {
margin-top: ${space(1)};
}
`;

const StyledButton = styled(Button)`
padding-left: 0;
padding-right: 0;
`;

const StyledCompactSelect = styled(CompactSelect)`
flex: 1 1;
min-width: 0;
Expand Down
10 changes: 10 additions & 0 deletions static/app/views/explore/tables/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {IconTable} from 'sentry/icons/iconTable';
import {t} from 'sentry/locale';
import {space} from 'sentry/styles/space';
import type {Confidence} from 'sentry/types/organization';
import useOrganization from 'sentry/utils/useOrganization';
import {
useExploreFields,
useExploreMode,
Expand Down Expand Up @@ -39,6 +40,8 @@ interface ExploreTablesProps extends BaseExploreTablesProps {
}

export function ExploreTables(props: ExploreTablesProps) {
const organization = useOrganization();

const mode = useExploreMode();
const setMode = useSetExploreMode();

Expand All @@ -63,6 +66,8 @@ export function ExploreTables(props: ExploreTablesProps) {
);
}, [fields, setFields, stringTags, numberTags]);

const openAggregateColumnEditor = useCallback(() => {}, []);

// HACK: This is pretty gross but to not break anything in the
// short term, we avoid introducing/removing any fields on the
// query. So we continue using the existing `mode` value and
Expand Down Expand Up @@ -93,6 +98,11 @@ export function ExploreTables(props: ExploreTablesProps) {
<Button onClick={openColumnEditor} icon={<IconTable />} size="sm">
{t('Edit Table')}
</Button>
) : tab === Mode.AGGREGATE &&
organization.features.includes('visibility-explore-aggregate-editor') ? (
<Button onClick={openAggregateColumnEditor} icon={<IconTable />} size="sm">
{t('Edit Table')}
</Button>
) : (
<Tooltip
title={
Expand Down
74 changes: 0 additions & 74 deletions static/app/views/explore/toolbar/index.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,80 +220,6 @@ describe('ExploreToolbar', function () {
expect(within(section).queryByLabelText('Remove Overlay')).not.toBeInTheDocument();
});

it('allows changing visualizes equations', async function () {
let fields!: string[];
let visualizes: any;
function Component() {
fields = useExploreFields();
visualizes = useExploreVisualizes();
return <ExploreToolbar extras={['equations']} />;
}

render(
<PageParamsProvider>
<SpanTagsProvider dataset={DiscoverDatasets.SPANS_EAP} enabled>
<Component />
</SpanTagsProvider>
</PageParamsProvider>
);

const section = screen.getByTestId('section-visualizes');

// this is the default
expect(visualizes).toEqual([new Visualize(['count(span.duration)'], {label: 'A'})]);

expect(fields).toEqual([
'id',
'span.op',
'span.description',
'span.duration',
'transaction',
'timestamp',
]); // default

// try changing the field
const input = within(section).getByRole('combobox', {
name: 'Select an attribute',
});
await userEvent.click(input);
await userEvent.click(within(section).getByRole('option', {name: 'span.self_time'}));
await userEvent.keyboard('{Escape}');

expect(fields).toEqual([
'id',
'span.op',
'span.description',
'span.duration',
'transaction',
'timestamp',
'span.self_time',
]);

await userEvent.click(input);
await userEvent.keyboard('{Backspace}');

await userEvent.click(within(section).getByRole('option', {name: 'avg(\u2026)'}));
await userEvent.click(within(section).getByRole('option', {name: 'span.self_time'}));
await userEvent.keyboard('{Escape}');
await userEvent.click(within(section).getByText('Visualize'));

expect(visualizes).toEqual([new Visualize(['avg(span.self_time)'], {label: 'A'})]);

// try adding a new chart
await userEvent.click(within(section).getByRole('button', {name: 'Add Chart'}));
expect(visualizes).toEqual([
new Visualize(['avg(span.self_time)'], {label: 'A'}),
new Visualize(['count(span.duration)'], {label: 'B'}),
]);

// delete second chart
await userEvent.click(within(section).getAllByLabelText('Remove Overlay')[1]!);
expect(visualizes).toEqual([new Visualize(['avg(span.self_time)'], {label: 'A'})]);

// only one left so cant be deleted
expect(within(section).getByLabelText('Remove Overlay')).toBeDisabled();
});

it('allows changing group bys', async function () {
let groupBys: any;

Expand Down
4 changes: 2 additions & 2 deletions static/app/views/explore/toolbar/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ interface ExploreToolbarProps {
width?: number;
}

export function ExploreToolbar({extras, width}: ExploreToolbarProps) {
export function ExploreToolbar({width}: ExploreToolbarProps) {
const fields = useExploreFields();
const groupBys = useExploreGroupBys();
const visualizes = useExploreVisualizes();
Expand All @@ -29,7 +29,7 @@ export function ExploreToolbar({extras, width}: ExploreToolbarProps) {

return (
<Container width={width}>
<ToolbarVisualize equationSupport={extras?.includes('equations')} />
<ToolbarVisualize />
<ToolbarGroupBy autoSwitchToAggregates />
<ToolbarSortBy
fields={fields}
Expand Down
129 changes: 11 additions & 118 deletions static/app/views/explore/toolbar/toolbarVisualize.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
import {Fragment, useCallback, useMemo} from 'react';
import styled from '@emotion/styled';

import {ArithmeticBuilder} from 'sentry/components/arithmeticBuilder';
import type {Expression} from 'sentry/components/arithmeticBuilder/expression';
import {isTokenFunction} from 'sentry/components/arithmeticBuilder/token';
import {Button} from 'sentry/components/core/button';
import type {SelectKey, SelectOption} from 'sentry/components/core/compactSelect';
import {CompactSelect} from 'sentry/components/core/compactSelect';
Expand Down Expand Up @@ -36,11 +33,7 @@ import {
ToolbarSection,
} from './styles';

interface ToolbarVisualizeProps {
equationSupport?: boolean;
}

export function ToolbarVisualize({equationSupport}: ToolbarVisualizeProps) {
export function ToolbarVisualize() {
const visualizes = useExploreVisualizes();
const setVisualizes = useSetExploreVisualizes();

Expand Down Expand Up @@ -90,29 +83,16 @@ export function ToolbarVisualize({equationSupport}: ToolbarVisualizeProps) {
<Fragment key={group}>
{visualize.yAxes.map((yAxis, index) => (
<Fragment key={index}>
{equationSupport ? (
<VisualizeEquation
canDelete={canDelete}
deleteOverlay={deleteOverlay}
group={group}
index={index}
label={shouldRenderLabel ? visualize.label : undefined}
yAxis={visualizes[group]?.yAxes?.[index]}
visualizes={visualizes}
setVisualizes={setVisualizes}
/>
) : (
<VisualizeDropdown
canDelete={canDelete}
deleteOverlay={deleteOverlay}
group={group}
index={index}
label={shouldRenderLabel ? visualize.label : undefined}
yAxis={yAxis}
visualizes={visualizes}
setVisualizes={setVisualizes}
/>
)}
<VisualizeDropdown
canDelete={canDelete}
deleteOverlay={deleteOverlay}
group={group}
index={index}
label={shouldRenderLabel ? visualize.label : undefined}
yAxis={yAxis}
visualizes={visualizes}
setVisualizes={setVisualizes}
/>
</Fragment>
))}
</Fragment>
Expand Down Expand Up @@ -255,93 +235,6 @@ function VisualizeDropdown({
);
}

interface VisualizeEquationProps {
canDelete: boolean;
deleteOverlay: (group: number, index: number) => void;
group: number;
index: number;
setVisualizes: (visualizes: BaseVisualize[], fields?: string[]) => void;
visualizes: Visualize[];
label?: string;
yAxis?: string;
}

function VisualizeEquation({
canDelete,
deleteOverlay,
group,
index,
setVisualizes,
label,
yAxis,
visualizes,
}: VisualizeEquationProps) {
const setChartYAxis = useCallback(
(expression: Expression) => {
if (expression.isValid) {
const functions = expression.tokens.filter(isTokenFunction);
const newVisualizes = visualizes.map((visualize, i) => {
if (i === group) {
const yAxes = [...visualize.yAxes];
yAxes[index] = expression.text;
visualize = visualize.replace({yAxes});
}
return visualize.toJSON();
});
setVisualizes(
newVisualizes,
functions.flatMap(func => func.attributes.map(attr => attr.format()))
);
}
},
[group, index, setVisualizes, visualizes]
);

const aggregateFunctions = useMemo(() => {
return ALLOWED_EXPLORE_VISUALIZE_AGGREGATES.map(aggregate => {
return {
name: aggregate,
label: `${aggregate}(\u2026)`,
};
});
}, []);

const yAxes: string[] = useMemo(() => {
return visualizes.flatMap(visualize => visualize.yAxes);
}, [visualizes]);

const fieldOptions: Array<SelectOption<string>> = useVisualizeFields({yAxes, yAxis});

const functionArguments = useMemo(() => {
return fieldOptions.map(o => {
return {
name: o.value,
label: o.label,
};
});
}, [fieldOptions]);

return (
<ToolbarRow>
{label && <ChartLabel>{label}</ChartLabel>}
<ArithmeticBuilder
expression={yAxis || ''}
setExpression={setChartYAxis}
aggregateFunctions={aggregateFunctions}
functionArguments={functionArguments}
/>
<Button
borderless
icon={<IconDelete />}
size="zero"
disabled={!canDelete}
onClick={() => deleteOverlay(group, index)}
aria-label={t('Remove Overlay')}
/>
</ToolbarRow>
);
}

const ChartLabel = styled('div')`
background-color: ${p => p.theme.purple100};
border-radius: ${p => p.theme.borderRadius};
Expand Down
Loading