Skip to content
Draft
Show file tree
Hide file tree
Changes from 3 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 @@ -107,6 +107,8 @@ export function isNavItemVisible(
}
return typeof item.show === 'function' ? item.show(context) : item.show;
}
import {getTransactionsDeprecation} from 'sentry/views/discover/utils';

import {CMDKAction} from './cmdk';
import {CommandPaletteSlot} from './commandPaletteSlot';
import {useCommandPaletteState} from './commandPaletteStateContext';
Expand Down Expand Up @@ -381,12 +383,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 @@ -2267,10 +2267,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 @@ -2306,7 +2311,7 @@ function buildRoutes(): RouteObject[] {
],
},
{
path: 'errors/',
path: 'errors-v2/',
component: make(() => import('sentry/views/explore/errors')),
children: errorsChildren,
},
Expand Down Expand Up @@ -2779,7 +2784,7 @@ function buildRoutes(): RouteObject[] {
releasesRoutes,
snapshotsRedirect,
statsRoutes,
discoverRoutes,
discoverErrorsRoutes,
errorsRoutes,
performanceRoutes,
domainViewRoutes,
Expand Down
35 changes: 34 additions & 1 deletion static/app/views/discover/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,53 @@ 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 discoverPath = location.pathname
.replace('/explore/discover/', '')
.replaceAll('/', '');
const targetPath = makeDiscoverPathname({
path: `/${discoverPath}/`,
organization,
});
return <Redirect to={targetPath + location.search} />;
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.

Discover redirect mangles pathname

High Severity

For orgs with discover-saved-queries-deprecation, bookmarks or links under /explore/discover/ are rewritten by stripping /explore/discover/ then calling replaceAll('/', '') on the full pathname. That removes the org segment and every slash, so makeDiscoverPathname gets garbage like /organizationsacmeresults/ instead of /results/.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 50ff58a. Configure here.

}
// transactions dataset redirects to traces url as we don't support transactions anymore
return <Redirect to={normalizeUrl('/explore/traces/')} />;

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 drops query

Medium Severity

The transactions-dataset deprecation redirect sends users to traces with only normalizeUrl('/explore/traces/') and does not append location.search, unlike the errors-dataset branch which keeps query parameters via targetPath + location.search.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 1c34048. 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/queries/"
self.result_path = f"/organizations/{self.org.slug}/explore/results/"
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated

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