Skip to content

Commit

Permalink
feat(insights): remove ai overview page (#84480)
Browse files Browse the repository at this point in the history
closes #81492
remove ai overview page, ai transaction summary routes (as they were
only accessible via the ai overview page), and add some tests
  • Loading branch information
DominikB2014 authored and andrewshie-sentry committed Feb 5, 2025
1 parent 7a07608 commit c017d92
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 230 deletions.
3 changes: 2 additions & 1 deletion static/app/components/sidebar/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import useMedia from 'sentry/utils/useMedia';
import useOrganization from 'sentry/utils/useOrganization';
import usePageFilters from 'sentry/utils/usePageFilters';
import useProjects from 'sentry/utils/useProjects';
import {MODULE_BASE_URLS} from 'sentry/views/insights/common/utils/useModuleURL';
import {
AI_LANDING_SUB_PATH,
AI_SIDEBAR_LABEL,
Expand Down Expand Up @@ -461,7 +462,7 @@ function Sidebar() {
<SidebarItem
{...sidebarItemProps}
label={AI_SIDEBAR_LABEL}
to={`/organizations/${organization.slug}/${DOMAIN_VIEW_BASE_URL}/${AI_LANDING_SUB_PATH}/`}
to={`/organizations/${organization.slug}/${DOMAIN_VIEW_BASE_URL}/${AI_LANDING_SUB_PATH}/${MODULE_BASE_URLS[AI_LANDING_SUB_PATH]}/`}
id="performance-domains-ai"
icon={<SubitemDot collapsed />}
/>
Expand Down
4 changes: 0 additions & 4 deletions static/app/routes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1863,10 +1863,6 @@ function buildRoutes() {
{moduleRoutes}
</Route>
<Route path={`${AI_LANDING_SUB_PATH}/`}>
<IndexRoute
component={make(() => import('sentry/views/insights/pages/ai/aiOverviewPage'))}
/>
{transactionSummaryRoutes}
{traceViewRoute}
<Route
path="trends/"
Expand Down
220 changes: 0 additions & 220 deletions static/app/views/insights/pages/ai/aiOverviewPage.tsx

This file was deleted.

1 change: 1 addition & 0 deletions static/app/views/insights/pages/ai/aiPageHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export function AiHeader({

return (
<DomainViewHeader
hasOverviewPage={false}
domainBaseUrl={aiBaseUrl}
headerTitle={headerTitle}
domainTitle={AI_LANDING_TITLE}
Expand Down
73 changes: 73 additions & 0 deletions static/app/views/insights/pages/domainViewHeader.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import type {Location} from 'history';
import {OrganizationFixture} from 'sentry-fixture/organization';

import {render, screen} from 'sentry-test/reactTestingLibrary';

import {useLocation} from 'sentry/utils/useLocation';
import useOrganization from 'sentry/utils/useOrganization';
import {DomainViewHeader} from 'sentry/views/insights/pages/domainViewHeader';
import {ModuleName} from 'sentry/views/insights/types';

jest.mock('sentry/utils/useLocation');
jest.mock('sentry/utils/useOrganization');

describe('DomainViewHeader', function () {
const organization = OrganizationFixture({
features: ['insights-entry-points'],
});

beforeEach(() => {
jest.resetAllMocks();
jest.mocked(useLocation).mockReturnValue({
pathname: '/organizations/org-slug/insights/frontend/',
} as Location);
jest.mocked(useOrganization).mockReturnValue(organization);
});

it('renders', () => {
render(
<DomainViewHeader
domainBaseUrl="domainBaseUrl"
domainTitle="domainTitle"
modules={[ModuleName.HTTP]}
selectedModule={undefined}
/>
);

expect(screen.getByText('domainTitle')).toBeInTheDocument();
expect(screen.getByRole('tab', {name: 'Overview'})).toBeInTheDocument();
expect(screen.getByRole('tab', {name: 'Network Requests'})).toBeInTheDocument();
});

it('does not show network requests without features', () => {
jest.mocked(useOrganization).mockReturnValue(OrganizationFixture());
render(
<DomainViewHeader
domainBaseUrl="domainBaseUrl"
domainTitle="domainTitle"
modules={[ModuleName.HTTP]}
selectedModule={undefined}
/>
);

expect(screen.getByText('domainTitle')).toBeInTheDocument();
expect(screen.getByRole('tab', {name: 'Overview'})).toBeInTheDocument();
expect(screen.queryByRole('tab', {name: 'Network Requests'})).not.toBeInTheDocument();
});

it('does not show overview tab with hasOverviewPage=false', () => {
render(
<DomainViewHeader
domainBaseUrl="domainBaseUrl"
domainTitle="domainTitle"
modules={[ModuleName.HTTP]}
selectedModule={undefined}
hasOverviewPage={false}
/>
);

expect(screen.getByText('domainTitle')).toBeInTheDocument();
expect(screen.queryByRole('tab', {name: 'Overview'})).not.toBeInTheDocument();
expect(screen.getByRole('tab', {name: 'Network Requests'})).toBeInTheDocument();
});
});
17 changes: 12 additions & 5 deletions static/app/views/insights/pages/domainViewHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,16 @@ export type Props = {
selectedModule: ModuleName | undefined;
additionalBreadCrumbs?: Crumb[];
additonalHeaderActions?: React.ReactNode;
// TODO - hasOverviewPage could be improved, the overview page could just be a "module", but that has a lot of other implications that have to be considered
hasOverviewPage?: boolean;
headerTitle?: React.ReactNode;
hideDefaultTabs?: boolean;
tabs?: {onTabChange: (key: string) => void; tabList: React.ReactNode; value: string};
};

export function DomainViewHeader({
modules,
hasOverviewPage = true,
headerTitle,
domainTitle,
selectedModule,
Expand All @@ -58,11 +61,15 @@ export function DomainViewHeader({
hideDefaultTabs && tabs?.value ? tabs.value : selectedModule ?? OVERVIEW_PAGE_TITLE;

const tabList: TabListItemProps[] = [
{
key: OVERVIEW_PAGE_TITLE,
children: OVERVIEW_PAGE_TITLE,
to: domainBaseUrl,
},
...(hasOverviewPage
? [
{
key: OVERVIEW_PAGE_TITLE,
children: OVERVIEW_PAGE_TITLE,
to: domainBaseUrl,
},
]
: []),
...modules
.filter(moduleName => isModuleVisible(moduleName, organization))
.map(moduleName => ({
Expand Down

0 comments on commit c017d92

Please sign in to comment.