diff --git a/static/app/utils/discover/fieldRenderers.spec.tsx b/static/app/utils/discover/fieldRenderers.spec.tsx index 2994fe4f1a489e..97408287129520 100644 --- a/static/app/utils/discover/fieldRenderers.spec.tsx +++ b/static/app/utils/discover/fieldRenderers.spec.tsx @@ -1,5 +1,7 @@ +import {OrganizationFixture} from 'sentry-fixture/organization'; import {ThemeFixture} from 'sentry-fixture/theme'; import {UserFixture} from 'sentry-fixture/user'; +import {WidgetFixture} from 'sentry-fixture/widget'; import {initializeOrg} from 'sentry-test/initializeOrg'; import {act, render, screen, waitFor} from 'sentry-test/reactTestingLibrary'; @@ -8,6 +10,7 @@ import ProjectsStore from 'sentry/stores/projectsStore'; import EventView from 'sentry/utils/discover/eventView'; import {getFieldRenderer} from 'sentry/utils/discover/fieldRenderers'; import {SPAN_OP_RELATIVE_BREAKDOWN_FIELD} from 'sentry/utils/discover/fields'; +import {WidgetType, type DashboardFilters} from 'sentry/views/dashboards/types'; const theme = ThemeFixture(); @@ -15,7 +18,9 @@ describe('getFieldRenderer', () => { let location: any, context: any, project: any, organization: any, data: any, user: any; beforeEach(() => { - context = initializeOrg(); + context = initializeOrg({ + organization: OrganizationFixture({features: ['dashboards-drilldown-flow']}), + }); organization = context.organization; project = context.project; act(() => ProjectsStore.loadInitialData([project])); @@ -113,6 +118,44 @@ describe('getFieldRenderer', () => { expect(screen.getByText(data.numeric)).toBeInTheDocument(); }); + it('can render dashboard links', () => { + const widget = WidgetFixture({ + widgetType: WidgetType.SPANS, + queries: [ + { + linkedDashboards: [{dashboardId: '123', field: 'transaction'}], + aggregates: [], + columns: [], + conditions: '', + name: '', + orderby: '', + }, + ], + }); + const dashboardFilters: DashboardFilters = {}; + + const renderer = getFieldRenderer( + 'transaction', + {transaction: 'string'}, + undefined, + widget, + dashboardFilters + ); + + render( + renderer(data, { + location, + organization, + theme, + }) as React.ReactElement + ); + + expect(screen.getByRole('link')).toHaveAttribute( + 'href', + '/organizations/org-slug/dashboard/123/?globalFilter=%7B%22dataset%22%3A%22spans%22%2C%22tag%22%3A%7B%22key%22%3A%22transaction%22%2C%22name%22%3A%22transaction%22%2C%22kind%22%3A%22tag%22%7D%2C%22value%22%3A%22transaction%3A%5Bapi.do_things%5D%22%2C%22isTemporary%22%3Atrue%7D' + ); + }); + describe('rate', () => { it('can render null rate', () => { const renderer = getFieldRenderer( diff --git a/static/app/utils/discover/fieldRenderers.tsx b/static/app/utils/discover/fieldRenderers.tsx index 5c696d3ca78872..5cb640999985de 100644 --- a/static/app/utils/discover/fieldRenderers.tsx +++ b/static/app/utils/discover/fieldRenderers.tsx @@ -1430,7 +1430,11 @@ function getDashboardUrl( if (dashboardLink && dashboardLink.dashboardId !== '-1') { const newTemporaryFilters: GlobalFilter[] = [ ...(dashboardFilters[DashboardFilterKeys.GLOBAL_FILTER] ?? []), - ].filter(filter => Boolean(filter.value)); + ].filter( + filter => + Boolean(filter.value) && + !(filter.tag.key === field && filter.dataset === widget.widgetType) + ); // Format the value as a proper filter condition string const mutableSearch = new MutableSearch(''); @@ -1440,6 +1444,7 @@ function getDashboardUrl( dataset: widget.widgetType, tag: {key: field, name: field, kind: FieldKind.TAG}, value: formattedValue, + isTemporary: true, }); // Preserve project, environment, and time range query params @@ -1455,7 +1460,7 @@ function getDashboardUrl( const url = `/organizations/${organization.slug}/dashboard/${dashboardLink.dashboardId}/?${qs.stringify( { ...filterParams, - [DashboardFilterKeys.TEMPORARY_FILTERS]: newTemporaryFilters.map(filter => + [DashboardFilterKeys.GLOBAL_FILTER]: newTemporaryFilters.map(filter => JSON.stringify(filter) ), } diff --git a/static/app/views/dashboards/detail.tsx b/static/app/views/dashboards/detail.tsx index a57b11b5e82406..def3a806e43390 100644 --- a/static/app/views/dashboards/detail.tsx +++ b/static/app/views/dashboards/detail.tsx @@ -960,7 +960,6 @@ class DashboardDetail extends Component { filters={{}} // Default Dashboards don't have filters set location={location} hasUnsavedChanges={false} - hasTemporaryFilters={false} isEditingDashboard={false} isPreview={false} onDashboardFilterChange={this.handleChangeFilter} @@ -1043,10 +1042,6 @@ class DashboardDetail extends Component { dashboardState !== DashboardState.CREATE && hasUnsavedFilterChanges(dashboard, location); - const hasTemporaryFilters = defined( - location.query?.[DashboardFilterKeys.TEMPORARY_FILTERS] - ); - const eventView = generatePerformanceEventView(location, projects, {}, organization); const isDashboardUsingTransaction = dashboard.widgets.some( @@ -1179,7 +1174,6 @@ class DashboardDetail extends Component { dashboardCreator={dashboard.createdBy} location={location} hasUnsavedChanges={!this.isEmbedded && hasUnsavedFilters} - hasTemporaryFilters={hasTemporaryFilters} isEditingDashboard={ dashboardState !== DashboardState.CREATE && this.isEditingDashboard @@ -1187,7 +1181,7 @@ class DashboardDetail extends Component { isPreview={this.isPreview} onDashboardFilterChange={this.handleChangeFilter} shouldBusySaveButton={this.state.isSavingDashboardFilters} - isPrebuiltDashboard={defined(dashboard.prebuiltId)} + prebuiltDashboardId={dashboard.prebuiltId} onCancel={() => { resetPageFilters(dashboard, location); trackAnalytics('dashboards2.filter.cancel', { diff --git a/static/app/views/dashboards/filtersBar.spec.tsx b/static/app/views/dashboards/filtersBar.spec.tsx new file mode 100644 index 00000000000000..87fe071c139180 --- /dev/null +++ b/static/app/views/dashboards/filtersBar.spec.tsx @@ -0,0 +1,172 @@ +// create a basic test for filters bar + +import {LocationFixture} from 'sentry-fixture/locationFixture'; +import {OrganizationFixture} from 'sentry-fixture/organization'; +import {ReleaseFixture} from 'sentry-fixture/release'; +import {TagsFixture} from 'sentry-fixture/tags'; + +import {render, screen, waitForElementToBeRemoved} from 'sentry-test/reactTestingLibrary'; + +import type {Organization} from 'sentry/types/organization'; +import {FieldKind} from 'sentry/utils/fields'; +import FiltersBar, {type FiltersBarProps} from 'sentry/views/dashboards/filtersBar'; +import { + DashboardFilterKeys, + WidgetType, + type GlobalFilter, +} from 'sentry/views/dashboards/types'; +import {PrebuiltDashboardId} from 'sentry/views/dashboards/utils/prebuiltConfigs'; + +describe('FiltersBar', () => { + let organization: Organization; + + beforeEach(() => { + mockNetworkRequests(); + + organization = OrganizationFixture({ + features: ['dashboards-basic', 'dashboards-edit', 'dashboards-global-filters'], + }); + }); + + afterEach(() => { + MockApiClient.clearMockResponses(); + jest.clearAllMocks(); + }); + + const renderFilterBar = (overrides: Partial = {}) => { + const props: FiltersBarProps = { + filters: {}, + hasUnsavedChanges: false, + isEditingDashboard: false, + isPreview: false, + location: LocationFixture(), + onDashboardFilterChange: () => {}, + ...overrides, + }; + + return render(, {organization}); + }; + + it('should render basic global filter', async () => { + const newLocation = LocationFixture({ + query: { + [DashboardFilterKeys.GLOBAL_FILTER]: JSON.stringify({ + dataset: WidgetType.SPANS, + tag: {key: 'browser.name', name: 'Browser Name', kind: FieldKind.FIELD}, + value: `browser.name:[Chrome]`, + } satisfies GlobalFilter), + }, + }); + renderFilterBar({location: newLocation}); + await waitForElementToBeRemoved(() => screen.queryByTestId('loading-indicator')); + expect( + screen.getByRole('button', {name: /browser\.name.*Chrome/i}) + ).toBeInTheDocument(); + }); + + it('should render save button with unsaved changes', async () => { + const newLocation = LocationFixture({ + query: { + [DashboardFilterKeys.GLOBAL_FILTER]: JSON.stringify({ + dataset: WidgetType.SPANS, + tag: {key: 'browser.name', name: 'Browser Name', kind: FieldKind.FIELD}, + value: `browser.name:[Chrome]`, + } satisfies GlobalFilter), + }, + }); + renderFilterBar({location: newLocation, hasUnsavedChanges: true}); + await waitForElementToBeRemoved(() => screen.queryByTestId('loading-indicator')); + expect(screen.getByRole('button', {name: 'Save'})).toBeInTheDocument(); + expect(screen.getByRole('button', {name: 'Cancel'})).toBeInTheDocument(); + }); + + it('should not render save button with temporary filter', async () => { + const newLocation = LocationFixture({ + query: { + [DashboardFilterKeys.GLOBAL_FILTER]: JSON.stringify({ + dataset: WidgetType.SPANS, + tag: {key: 'browser.name', name: 'Browser Name', kind: FieldKind.FIELD}, + value: `browser.name:[Chrome]`, + isTemporary: true, + } satisfies GlobalFilter), + }, + }); + + renderFilterBar({location: newLocation}); + await waitForElementToBeRemoved(() => screen.queryByTestId('loading-indicator')); + expect( + screen.getByRole('button', {name: /browser\.name.*Chrome/i}) + ).toBeInTheDocument(); + expect(screen.queryByRole('button', {name: 'Save'})).not.toBeInTheDocument(); + expect(screen.queryByRole('button', {name: 'Cancel'})).not.toBeInTheDocument(); + }); + + it('should not render save button on prebuilt dashboard', async () => { + const newLocation = LocationFixture({ + query: { + [DashboardFilterKeys.GLOBAL_FILTER]: JSON.stringify({ + dataset: WidgetType.SPANS, + tag: {key: 'browser.name', name: 'Browser Name', kind: FieldKind.FIELD}, + value: `browser.name:[Chrome]`, + } satisfies GlobalFilter), + }, + }); + renderFilterBar({ + location: newLocation, + prebuiltDashboardId: PrebuiltDashboardId.FRONTEND_SESSION_HEALTH, + }); + await waitForElementToBeRemoved(() => screen.queryByTestId('loading-indicator')); + expect( + screen.getByRole('button', {name: /browser\.name.*Chrome/i}) + ).toBeInTheDocument(); + expect(screen.queryByRole('button', {name: 'Save'})).not.toBeInTheDocument(); + expect(screen.queryByRole('button', {name: 'Cancel'})).not.toBeInTheDocument(); + }); +}); + +const mockNetworkRequests = () => { + MockApiClient.addMockResponse({ + url: '/organizations/org-slug/releases/', + body: [ReleaseFixture()], + }); + MockApiClient.addMockResponse({ + url: '/organizations/org-slug/tags/', + body: TagsFixture(), + }); + MockApiClient.addMockResponse({ + url: `/organizations/org-slug/measurements-meta/`, + body: { + 'measurements.custom.measurement': { + functions: ['p99'], + }, + 'measurements.another.custom.measurement': { + functions: ['p99'], + }, + }, + }); + + const mockSearchResponse = [ + { + key: 'browser.name', + value: 'Chrome', + name: 'Chrome', + first_seen: null, + last_seen: null, + times_seen: null, + }, + { + key: 'browser.name', + value: 'Firefox', + name: 'Firefox', + first_seen: null, + last_seen: null, + times_seen: null, + }, + ]; + + MockApiClient.addMockResponse({ + url: `/organizations/org-slug/trace-items/attributes/browser.name/values/`, + body: mockSearchResponse, + match: [MockApiClient.matchQuery({attributeType: 'string'})], + }); +}; diff --git a/static/app/views/dashboards/filtersBar.tsx b/static/app/views/dashboards/filtersBar.tsx index 1bba81779b887e..ed937bc0ec9441 100644 --- a/static/app/views/dashboards/filtersBar.tsx +++ b/static/app/views/dashboards/filtersBar.tsx @@ -11,6 +11,7 @@ import {ProjectPageFilter} from 'sentry/components/organizations/projectPageFilt import {t} from 'sentry/locale'; import {space} from 'sentry/styles/space'; import type {User} from 'sentry/types/user'; +import {defined} from 'sentry/utils'; import {trackAnalytics} from 'sentry/utils/analytics'; import {ToggleOnDemand} from 'sentry/utils/performance/contexts/onDemandControl'; import {ReleasesProvider} from 'sentry/utils/releases/releasesProvider'; @@ -22,21 +23,20 @@ import AddFilter from 'sentry/views/dashboards/globalFilter/addFilter'; import GenericFilterSelector from 'sentry/views/dashboards/globalFilter/genericFilterSelector'; import {globalFilterKeysAreEqual} from 'sentry/views/dashboards/globalFilter/utils'; import {useDatasetSearchBarData} from 'sentry/views/dashboards/hooks/useDatasetSearchBarData'; -import {useHasDrillDownFlows} from 'sentry/views/dashboards/hooks/useHasDrillDownFlows'; import {useInvalidateStarredDashboards} from 'sentry/views/dashboards/hooks/useInvalidateStarredDashboards'; +import {getDashboardFiltersFromURL} from 'sentry/views/dashboards/utils'; import { - getCombinedDashboardFilters, - getDashboardFiltersFromURL, -} from 'sentry/views/dashboards/utils'; + PREBUILT_DASHBOARDS, + type PrebuiltDashboardId, +} from 'sentry/views/dashboards/utils/prebuiltConfigs'; import {checkUserHasEditAccess} from './utils/checkUserHasEditAccess'; import ReleasesSelectControl from './releasesSelectControl'; import type {DashboardFilters, DashboardPermissions, GlobalFilter} from './types'; import {DashboardFilterKeys} from './types'; -type FiltersBarProps = { +export type FiltersBarProps = { filters: DashboardFilters; - hasTemporaryFilters: boolean; hasUnsavedChanges: boolean; isEditingDashboard: boolean; isPreview: boolean; @@ -44,15 +44,14 @@ type FiltersBarProps = { onDashboardFilterChange: (activeFilters: DashboardFilters) => void; dashboardCreator?: User; dashboardPermissions?: DashboardPermissions; - isPrebuiltDashboard?: boolean; onCancel?: () => void; onSave?: () => Promise; + prebuiltDashboardId?: PrebuiltDashboardId; shouldBusySaveButton?: boolean; }; export default function FiltersBar({ filters, - hasTemporaryFilters, dashboardPermissions, dashboardCreator, hasUnsavedChanges, @@ -63,14 +62,17 @@ export default function FiltersBar({ onDashboardFilterChange, onSave, shouldBusySaveButton, - isPrebuiltDashboard, + prebuiltDashboardId, }: FiltersBarProps) { const {selection} = usePageFilters(); const organization = useOrganization(); const currentUser = useUser(); const {teams: userTeams} = useUserTeams(); const getSearchBarData = useDatasetSearchBarData(); - const hasDrillDownFlowsFeature = useHasDrillDownFlows(); + const isPrebuiltDashboard = defined(prebuiltDashboardId); + const prebuiltDashboardFilters: GlobalFilter[] = prebuiltDashboardId + ? (PREBUILT_DASHBOARDS[prebuiltDashboardId].filters.globalFilter ?? []) + : []; const hasEditAccess = checkUserHasEditAccess( currentUser, @@ -89,19 +91,11 @@ export default function FiltersBar({ []; const [activeGlobalFilters, setActiveGlobalFilters] = useState(() => { - const globalFilters = + return ( dashboardFiltersFromURL?.[DashboardFilterKeys.GLOBAL_FILTER] ?? filters?.[DashboardFilterKeys.GLOBAL_FILTER] ?? - []; - - if (hasDrillDownFlowsFeature && dashboardFiltersFromURL) { - return getCombinedDashboardFilters( - globalFilters, - dashboardFiltersFromURL?.[DashboardFilterKeys.TEMPORARY_FILTERS] - ); - } - - return globalFilters; + [] + ); }); const updateGlobalFilters = (newGlobalFilters: GlobalFilter[]) => { @@ -112,6 +106,8 @@ export default function FiltersBar({ }); }; + const hasTemporaryFilters = activeGlobalFilters.some(filter => filter.isTemporary); + return ( @@ -163,6 +159,14 @@ export default function FiltersBar({ {activeGlobalFilters.map(filter => ( + prebuiltFilter.tag.key === filter.tag.key && + prebuiltFilter.dataset === filter.dataset + ) + } key={filter.tag.key + filter.value} globalFilter={filter} searchBarData={getSearchBarData(filter.dataset)} diff --git a/static/app/views/dashboards/globalFilter/filterSelector.tsx b/static/app/views/dashboards/globalFilter/filterSelector.tsx index 32ccaa086cec39..e3726738eaade5 100644 --- a/static/app/views/dashboards/globalFilter/filterSelector.tsx +++ b/static/app/views/dashboards/globalFilter/filterSelector.tsx @@ -50,6 +50,7 @@ type FilterSelectorProps = { onRemoveFilter: (filter: GlobalFilter) => void; onUpdateFilter: (filter: GlobalFilter) => void; searchBarData: SearchBarData; + disableRemoveFilter?: boolean; }; function FilterSelector({ @@ -57,6 +58,7 @@ function FilterSelector({ searchBarData, onRemoveFilter, onUpdateFilter, + disableRemoveFilter, }: FilterSelectorProps) { const {selection} = usePageFilters(); @@ -275,13 +277,15 @@ function FilterSelector({ {t('Clear')} )} - onRemoveFilter(globalFilter)} - > - {t('Remove Filter')} - + {!disableRemoveFilter && ( + onRemoveFilter(globalFilter)} + > + {t('Remove Filter')} + + )} ); diff --git a/static/app/views/dashboards/globalFilter/genericFilterSelector.tsx b/static/app/views/dashboards/globalFilter/genericFilterSelector.tsx index f13822ee42072f..ece28824a9d028 100644 --- a/static/app/views/dashboards/globalFilter/genericFilterSelector.tsx +++ b/static/app/views/dashboards/globalFilter/genericFilterSelector.tsx @@ -10,6 +10,7 @@ export type GenericFilterSelectorProps = { onRemoveFilter: (filter: GlobalFilter) => void; onUpdateFilter: (filter: GlobalFilter) => void; searchBarData: SearchBarData; + disableRemoveFilter?: boolean; }; function getFilterSelector( diff --git a/static/app/views/dashboards/globalFilter/numericFilterSelector.tsx b/static/app/views/dashboards/globalFilter/numericFilterSelector.tsx index fb87b42dc09672..f95ec3dd077a9e 100644 --- a/static/app/views/dashboards/globalFilter/numericFilterSelector.tsx +++ b/static/app/views/dashboards/globalFilter/numericFilterSelector.tsx @@ -204,6 +204,7 @@ function NumericFilterSelector({ globalFilter, onRemoveFilter, onUpdateFilter, + disableRemoveFilter, }: GenericFilterSelectorProps) { const globalFilterQueries = useMemo( () => globalFilter.value.split(FILTER_QUERY_SEPARATOR), @@ -291,15 +292,19 @@ function NumericFilterSelector({ {t('%s Filter', getDatasetLabel(globalFilter.dataset))} } - menuHeaderTrailingItems={() => ( - onRemoveFilter(globalFilter)} - > - {t('Remove Filter')} - - )} + menuHeaderTrailingItems={ + disableRemoveFilter + ? undefined + : () => ( + onRemoveFilter(globalFilter)} + > + {t('Remove Filter')} + + ) + } menuBody={ diff --git a/static/app/views/dashboards/types.tsx b/static/app/views/dashboards/types.tsx index d8693151401262..a9fe12c688f183 100644 --- a/static/app/views/dashboards/types.tsx +++ b/static/app/views/dashboards/types.tsx @@ -175,14 +175,11 @@ export type DashboardListItem = { export enum DashboardFilterKeys { RELEASE = 'release', GLOBAL_FILTER = 'globalFilter', - // temporary filters are filters that are not saved to the dashboard, they occur when you link from one dashboard to another - TEMPORARY_FILTERS = 'temporaryFilters', } export type DashboardFilters = { [DashboardFilterKeys.RELEASE]?: string[]; [DashboardFilterKeys.GLOBAL_FILTER]?: GlobalFilter[]; - [DashboardFilterKeys.TEMPORARY_FILTERS]?: GlobalFilter[]; }; export type GlobalFilter = { @@ -192,6 +189,7 @@ export type GlobalFilter = { tag: Tag; // The raw filter condition string (e.g. 'tagKey:[values,...]') value: string; + isTemporary?: boolean; }; /** diff --git a/static/app/views/dashboards/utils.tsx b/static/app/views/dashboards/utils.tsx index bbb1d03fd65341..63096e47807296 100644 --- a/static/app/views/dashboards/utils.tsx +++ b/static/app/views/dashboards/utils.tsx @@ -46,7 +46,6 @@ import {decodeList, decodeScalar} from 'sentry/utils/queryString'; import type { DashboardDetails, DashboardFilters, - GlobalFilter, Widget, WidgetQuery, } from 'sentry/views/dashboards/types'; @@ -570,16 +569,6 @@ export function getDashboardFiltersFromURL(location: Location): DashboardFilters } }) .filter(filter => filter !== null); - } else if (key === DashboardFilterKeys.TEMPORARY_FILTERS) { - dashboardFilters[key] = queryFilters - .map(filter => { - try { - return JSON.parse(filter); - } catch (error) { - return null; - } - }) - .filter(filter => filter !== null); } else { dashboardFilters[key] = queryFilters; } @@ -594,10 +583,7 @@ export function dashboardFiltersToString( ): string { let dashboardFilterConditions = ''; - const pinnedFilters = omit(dashboardFilters, [ - DashboardFilterKeys.GLOBAL_FILTER, - DashboardFilterKeys.TEMPORARY_FILTERS, - ]); + const pinnedFilters = omit(dashboardFilters, DashboardFilterKeys.GLOBAL_FILTER); if (pinnedFilters) { for (const [key, activeFilters] of Object.entries(pinnedFilters)) { if (activeFilters.length === 1) { @@ -610,14 +596,12 @@ export function dashboardFiltersToString( } } - const combinedFilters = getCombinedDashboardFilters( - dashboardFilters?.[DashboardFilterKeys.GLOBAL_FILTER], - dashboardFilters?.[DashboardFilterKeys.TEMPORARY_FILTERS] - ); + const globalFilters = dashboardFilters?.[DashboardFilterKeys.GLOBAL_FILTER]; + // If widgetType is provided, concatenate global filters that apply - if (widgetType && combinedFilters) { + if (widgetType && globalFilters) { dashboardFilterConditions += - combinedFilters + globalFilters .filter(globalFilter => globalFilter.dataset === widgetType) .map(globalFilter => globalFilter.value) .join(' ') ?? ''; @@ -626,26 +610,6 @@ export function dashboardFiltersToString( return dashboardFilterConditions; } -// Combines global and temporary filters into a single array, deduplicating by dataset and key prioritizing the temporary filter. -export function getCombinedDashboardFilters( - globalFilters?: GlobalFilter[], - temporaryFilters?: GlobalFilter[] -): GlobalFilter[] { - const finalFilters = [...(globalFilters ?? [])]; - const temporaryFiltersCopy = [...(temporaryFilters ?? [])]; - finalFilters.forEach((filter, idx) => { - // if a temporary filter exists for the same dataset and key, override it and delete it from the temporary filters to avoid duplicates - const temporaryFilter = temporaryFiltersCopy.find( - tf => tf.dataset === filter.dataset && tf.tag.key === filter.tag.key - ); - if (temporaryFilter) { - finalFilters[idx] = {...filter, value: temporaryFilter.value}; - temporaryFiltersCopy.splice(temporaryFiltersCopy.indexOf(temporaryFilter), 1); - } - }); - return [...finalFilters, ...temporaryFiltersCopy]; -} - export function connectDashboardCharts(groupName: string) { connect?.(groupName); }