Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ export function isNavItemVisible(
return typeof item.show === 'function' ? item.show(context) : item.show;
}
import {useNotificationPermission} from 'sentry/serviceWorker/client/useNotificationPermission';
import {getTransactionsDeprecation} from 'sentry/views/discover/utils';

import {CMDKAction} from './cmdk';
import {CommandPaletteSlot} from './commandPaletteSlot';
Expand Down Expand Up @@ -386,12 +387,24 @@ export function GlobalCommandPaletteActions() {
to={`${prefix}/explore/metrics/`}
/>
)}
{organization.features.includes('explore-errors') && (
<CMDKAction display={{label: t('Errors')}} to={`${prefix}/explore/errors/`} />
)}
{organization.features.includes('explore-errors') &&
!getTransactionsDeprecation(organization) && (
<CMDKAction
display={{label: t('Errors')}}
to={`${prefix}/explore/errors-v2/`}
/>
)}
<CMDKAction
display={{label: t('Discover')}}
to={`${prefix}/explore/discover/homepage/`}
display={{
label: getTransactionsDeprecation(organization)
? t('Errors')
: t('Discover'),
}}
to={
getTransactionsDeprecation(organization)
? `${prefix}/explore/errors/homepage/`
: `${prefix}/explore/discover/homepage/`
}
/>
{organization.features.includes('profiling') && (
<CMDKAction
Expand Down
19 changes: 12 additions & 7 deletions static/app/router/routes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1674,7 +1674,7 @@ function buildRoutes(): RouteObject[] {
],
};

const discoverChildren: SentryRouteObject[] = [
const discoverErrorsChildren: SentryRouteObject[] = [
{
index: true,
redirectTo: 'queries/',
Expand All @@ -1697,11 +1697,11 @@ function buildRoutes(): RouteObject[] {
component: make(() => import('sentry/views/discover/eventDetails')),
},
];
const discoverRoutes: SentryRouteObject = {
const discoverErrorsRoutes: SentryRouteObject = {
path: '/discover/',
component: make(() => import('sentry/views/discover')),
withOrgPath: true,
children: discoverChildren,
children: discoverErrorsChildren,
Comment thread
cursor[bot] marked this conversation as resolved.
};

const errorsChildren: SentryRouteObject[] = [
Expand All @@ -1711,7 +1711,7 @@ function buildRoutes(): RouteObject[] {
},
];
const errorsRoutes: SentryRouteObject = {
path: '/errors/',
path: '/errors-v2/',
component: make(() => import('sentry/views/explore/errors')),
withOrgPath: true,
children: errorsChildren,
Expand Down Expand Up @@ -2260,10 +2260,15 @@ function buildRoutes(): RouteObject[] {
component: make(() => import('sentry/views/explore/replays/index')),
children: replayChildren,
},
{
path: 'errors/',
component: make(() => import('sentry/views/discover')),
children: discoverErrorsChildren,
},
{
path: 'discover/',
component: make(() => import('sentry/views/discover')),
children: discoverChildren,
children: discoverErrorsChildren,
},
{
path: 'releases/',
Expand Down Expand Up @@ -2299,7 +2304,7 @@ function buildRoutes(): RouteObject[] {
],
},
{
path: 'errors/',
path: 'errors-v2/',
component: make(() => import('sentry/views/explore/errors')),
children: errorsChildren,
},
Expand Down Expand Up @@ -2772,7 +2777,7 @@ function buildRoutes(): RouteObject[] {
releasesRoutes,
snapshotsRedirect,
statsRoutes,
discoverRoutes,
discoverErrorsRoutes,
errorsRoutes,
performanceRoutes,
domainViewRoutes,
Expand Down
34 changes: 33 additions & 1 deletion static/app/views/discover/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,52 @@ import {AnalyticsArea} from 'sentry/components/analyticsArea';
import {NoProjectMessage} from 'sentry/components/noProjectMessage';
import {Redirect} from 'sentry/components/redirect';
import {t} from 'sentry/locale';
import {SavedQueryDatasets} from 'sentry/utils/discover/types';
import {normalizeUrl} from 'sentry/utils/url/normalizeUrl';
import {useLocation} from 'sentry/utils/useLocation';
import {useOrganization} from 'sentry/utils/useOrganization';
import {Dataset} from 'sentry/views/alerts/rules/metric/types';
import {makeDiscoverPathname} from 'sentry/views/discover/pathnames';
import {getTransactionsDeprecation} from 'sentry/views/discover/utils';
import {useRedirectNavigationV2Routes} from 'sentry/views/navigation/useRedirectNavigationV2Routes';

function DiscoverContainer() {
const organization = useOrganization();
const location = useLocation();
const discoverTransactionsDeprecation = getTransactionsDeprecation(organization);
const redirectPath = useRedirectNavigationV2Routes({
oldPathPrefix: '/discover/',
newPathPrefix: '/explore/discover/',
newPathPrefix: discoverTransactionsDeprecation
? '/explore/errors/'
: '/explore/discover/',
});

if (redirectPath) {
return <Redirect to={redirectPath} />;
}

// Tranasctions deprecation redirects
if (
discoverTransactionsDeprecation &&
location.pathname.includes('/explore/discover/')
) {
// errors dataset (or no dataset specified) redirects to errors url and keeps the same query params
if (
location.query.queryDataset !== SavedQueryDatasets.TRANSACTIONS &&
location.query.dataset !== Dataset.TRANSACTIONS
) {
const match = location.pathname.match(/\/explore\/discover\/([^/]+)\//);
const discoverPath = match?.[1] ?? 'homepage';
const targetPath = makeDiscoverPathname({
path: `/${discoverPath}/`,
organization,
});
return <Redirect to={targetPath + location.search} />;
Comment thread
cursor[bot] marked this conversation as resolved.
Comment thread
cursor[bot] marked this conversation as resolved.
}
// transactions dataset redirects to traces url as we don't support transactions anymore
return <Redirect to={normalizeUrl('/explore/traces/')} />;
Comment thread
cursor[bot] marked this conversation as resolved.
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Transaction redirect skips errors path

Medium Severity

With discover-saved-queries-deprecation, discover lives under explore/errors/, but transaction dataset redirects to traces only run when the pathname contains explore/discover/. Opening a transaction saved query via makeDiscoverPathname lands on explore/errors/ with transaction query params and never redirects to traces.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit dde9d50. Configure here.


function renderNoAccess() {
return (
<Stack flex={1} padding="2xl 3xl">
Expand Down
6 changes: 5 additions & 1 deletion static/app/views/discover/pathnames.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import type {Organization} from 'sentry/types/organization';
import {normalizeUrl} from 'sentry/utils/url/normalizeUrl';
import {getTransactionsDeprecation} from 'sentry/views/discover/utils';

const DISCOVER_BASE_PATHNAME = 'explore/discover';
const ERRORS_BASE_PATHNAME = 'explore/errors';

export function makeDiscoverPathname({
path,
Expand All @@ -11,6 +13,8 @@ export function makeDiscoverPathname({
path: '/' | `/${string}/`;
}) {
return normalizeUrl(
`/organizations/${organization.slug}/${DISCOVER_BASE_PATHNAME}${path}`
getTransactionsDeprecation(organization)
? `/organizations/${organization.slug}/${ERRORS_BASE_PATHNAME}${path}`
: `/organizations/${organization.slug}/${DISCOVER_BASE_PATHNAME}${path}`
);
}
39 changes: 31 additions & 8 deletions static/app/views/discover/results/resultsHeader.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import {Fragment, useCallback, useEffect, useState} from 'react';
import type {Location} from 'history';

import type {ContainerProps} from '@sentry/scraps/layout';

import {fetchHomepageQuery} from 'sentry/actionCreators/discoverHomepageQueries';
import {fetchSavedQuery} from 'sentry/actionCreators/discoverSavedQueries';
import type {Client} from 'sentry/api';
Expand All @@ -16,6 +18,7 @@ import {DiscoverBreadcrumb} from 'sentry/views/discover/breadcrumb';
import SavedQueryButtonGroup from 'sentry/views/discover/savedQuery';
import {DatasetSelectorTabs} from 'sentry/views/discover/savedQuery/datasetSelectorTabs';
import {getSavedQueryWithDataset} from 'sentry/views/discover/savedQuery/utils';
import {getTransactionsDeprecation} from 'sentry/views/discover/utils';
import {TopBar} from 'sentry/views/navigation/topBar';

type Props = {
Expand Down Expand Up @@ -103,7 +106,7 @@ function ResultsHeaderBase({

const title = (
<Fragment>
{t('Discover')}
{getTransactionsDeprecation(organization) ? t('Errors') : t('Discover')}
<PageHeadingQuestionTooltip
docsUrl="https://docs.sentry.io/product/discover-queries/"
title={t('Create queries to get insights into the health of your system.')}
Expand All @@ -121,8 +124,26 @@ function ResultsHeaderBase({
/>
);

// there's some styling that gets messed up when choosing to not render the
// dataset selector tabs so i'm injecting some styles fix it. This should be removed
// when the dataset selector tabs are removed.
const deprecationHeaderStyles: ContainerProps<'header'> = {
padding: {
'screen:sm': '0',
'screen:md': '0',
},
borderBottom: {
'2xs': 'none',
xs: 'none',
sm: 'none',
md: 'none',
},
};

return (
<Layout.Header>
<Layout.Header
{...(getTransactionsDeprecation(organization) ? deprecationHeaderStyles : {})}
>
<TopBar.Slot name="title">
{isHomepage ? (
<GuideAnchor target="discover_landing_header">{title}</GuideAnchor>
Expand All @@ -133,12 +154,14 @@ function ResultsHeaderBase({
)}
</TopBar.Slot>
<TopBar.Slot name="actions">{savedQueryButton}</TopBar.Slot>
<DatasetSelectorTabs
eventView={eventView}
isHomepage={isHomepage}
savedQuery={savedQuery}
splitDecision={splitDecision}
/>
{!getTransactionsDeprecation(organization) && (
<DatasetSelectorTabs
eventView={eventView}
isHomepage={isHomepage}
savedQuery={savedQuery}
splitDecision={splitDecision}
/>
)}
</Layout.Header>
);
}
Expand Down
4 changes: 4 additions & 0 deletions static/app/views/discover/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -903,3 +903,7 @@ export const SAVED_QUERY_DATASET_TO_WIDGET_TYPE = {
[SavedQueryDatasets.ERRORS]: WidgetType.ERRORS,
[SavedQueryDatasets.TRANSACTIONS]: WidgetType.TRANSACTIONS,
};

export function getTransactionsDeprecation(organization: Organization) {
return organization.features.includes('discover-saved-queries-deprecation');
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {FeatureBadge} from '@sentry/scraps/badge';
import Feature from 'sentry/components/acl/feature';
import {t} from 'sentry/locale';
import {useOrganization} from 'sentry/utils/useOrganization';
import {getTransactionsDeprecation} from 'sentry/views/discover/utils';
import {CONVERSATIONS_LANDING_SUB_PATH} from 'sentry/views/explore/conversations/settings';
import {
MAX_STARRED_SAVED_QUERIES_IN_NAV,
Expand All @@ -23,6 +24,8 @@ export function ExploreSecondaryNavigation() {
perPage: MAX_STARRED_SAVED_QUERIES_IN_NAV,
});

const discoverTransactionsDeprecation = getTransactionsDeprecation(organization);

return (
<Fragment>
<SecondaryNavigation.Header>{t('Explore')}</SecondaryNavigation.Header>
Expand Down Expand Up @@ -65,8 +68,8 @@ export function ExploreSecondaryNavigation() {
<Feature features="organizations:explore-errors">
<SecondaryNavigation.ListItem>
<SecondaryNavigation.Link
to={`${baseUrl}/errors/`}
activeTo={`${baseUrl}/errors/`}
to={`${baseUrl}/errors-v2/`}
activeTo={`${baseUrl}/errors-v2/`}
analyticsItemName="explore_errors"
trailingItems={<FeatureBadge type="alpha" />}
>
Expand All @@ -80,11 +83,19 @@ export function ExploreSecondaryNavigation() {
>
<SecondaryNavigation.ListItem>
<SecondaryNavigation.Link
to={`${baseUrl}/discover/homepage/`}
activeTo={`${baseUrl}/discover/`}
to={
discoverTransactionsDeprecation
? `${baseUrl}/errors/homepage/`
: `${baseUrl}/discover/homepage/`
}
activeTo={
discoverTransactionsDeprecation
? `${baseUrl}/errors/`
: `${baseUrl}/discover/homepage/`
}
analyticsItemName="explore_discover"
>
{t('Discover')}
{discoverTransactionsDeprecation ? t('Errors') : t('Discover')}
</SecondaryNavigation.Link>
</SecondaryNavigation.ListItem>
</Feature>
Expand Down
4 changes: 2 additions & 2 deletions tests/acceptance/test_organization_events_v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,8 @@ def setUp(self) -> None:
self.create_member(user=self.user, organization=self.org, role="owner", teams=[self.team])

self.login_as(self.user)
self.landing_path = f"/organizations/{self.org.slug}/discover/queries/"
self.result_path = f"/organizations/{self.org.slug}/discover/results/"
self.landing_path = f"/organizations/{self.org.slug}/explore/errors/queries/"
self.result_path = f"/organizations/{self.org.slug}/explore/errors/results/"

def wait_until_loaded(self) -> None:
self.browser.wait_until_not('[data-test-id="loading-indicator"]')
Expand Down
Loading