fix(profiling): do not forward unwanted qs (#38740)

Some parts of the query string collide between the flamechart views (query) and some break the API (cursor). Make sure we strip these out when navigating between the pages. This is more brittle than I would have wanted it to be, but trying to preserve as many qs as we can makes for a nicer back/forth navigation as state is preserved. We should rethink how we handle these if it proves to be too brittle.
getsentry · Sep 13, 2022 · 6d898bf · 6d898bf
1 parent 61befe8
commit 6d898bf
Show file tree

Hide file tree

Showing 7 changed files with 33 additions and 40 deletions.
diff --git a/static/app/components/profiling/breadcrumb.spec.tsx b/static/app/components/profiling/breadcrumb.spec.tsx
@@ -1,27 +1,19 @@
-import {initializeOrg} from 'sentry-test/initializeOrg';
 import {render, screen} from 'sentry-test/reactTestingLibrary';
 
 import {Breadcrumb} from 'sentry/components/profiling/breadcrumb';
 
 describe('Breadcrumb', function () {
-  let location, organization;
-
-  beforeEach(function () {
-    location = TestStubs.location();
-    const context = initializeOrg();
-    organization = context.organization;
-  });
-
   it('renders the profiling link', function () {
+    const organization = TestStubs.Organization();
     render(
       <Breadcrumb
-        location={location}
         organization={organization}
         trails={[
-          {type: 'landing'},
+          {type: 'landing', payload: {query: {}}},
           {
             type: 'flamechart',
             payload: {
+              query: {},
               transaction: 'foo',
               profileId: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa',
               projectSlug: 'bar',

diff --git a/static/app/components/profiling/breadcrumb.tsx b/static/app/components/profiling/breadcrumb.tsx
@@ -1,6 +1,7 @@
 import {useMemo} from 'react';
 import styled from '@emotion/styled';
 import {Location} from 'history';
+import omit from 'lodash/omit';
 
 import Breadcrumbs, {Crumb} from 'sentry/components/breadcrumbs';
 import {t} from 'sentry/locale';
@@ -13,34 +14,33 @@ import {
 } from 'sentry/utils/profiling/routes';
 
 interface BreadcrumbProps {
-  location: Location;
   organization: Organization;
   trails: Trail[];
 }
 
-function Breadcrumb({location, organization, trails}: BreadcrumbProps) {
+function Breadcrumb({organization, trails}: BreadcrumbProps) {
   const crumbs = useMemo(
-    () => trails.map(trail => trailToCrumb(trail, {location, organization})),
-    [location, organization, trails]
+    () => trails.map(trail => trailToCrumb(trail, {organization})),
+    [organization, trails]
   );
   return <StyledBreadcrumbs crumbs={crumbs} />;
 }
 
 function trailToCrumb(
   trail: Trail,
   {
-    location,
     organization,
   }: {
-    location: Location;
     organization: Organization;
   }
 ): Crumb {
   switch (trail.type) {
     case 'landing': {
       return {
         to: generateProfilingRouteWithQuery({
-          location,
+          // cursor and query are not used in the landing page
+          // and break the API call as the qs gets forwarded to the API
+          query: omit(trail.payload.query, ['cursor', 'query']),
           orgSlug: organization.slug,
         }),
         label: t('Profiling'),
@@ -50,7 +50,9 @@ function trailToCrumb(
     case 'profile summary': {
       return {
         to: generateProfileSummaryRouteWithQuery({
-          location,
+          // cursor and query are not used in the summary page
+          // and break the API call as the qs gets forwarded to the API
+          query: omit(trail.payload.query, ['cursor', 'query']),
           orgSlug: organization.slug,
           projectSlug: trail.payload.projectSlug,
           transaction: trail.payload.transaction,
@@ -66,7 +68,7 @@ function trailToCrumb(
           : generateProfileDetailsRouteWithQuery;
       return {
         to: generateRouteWithQuery({
-          location,
+          query: trail.payload.query,
           orgSlug: organization.slug,
           projectSlug: trail.payload.projectSlug,
           profileId: trail.payload.profileId,
@@ -81,12 +83,16 @@ function trailToCrumb(
 }
 
 type ProfilingTrail = {
+  payload: {
+    query: Location['query'];
+  };
   type: 'landing';
 };
 
 type ProfileSummaryTrail = {
   payload: {
     projectSlug: Project['slug'];
+    query: Location['query'];
     transaction: string;
   };
   type: 'profile summary';
@@ -96,6 +102,7 @@ type FlamegraphTrail = {
   payload: {
     profileId: string;
     projectSlug: string;
+    query: Location['query'];
     tab: 'flamechart' | 'details';
     transaction: string;
   };

diff --git a/static/app/components/profiling/profileHeader.tsx b/static/app/components/profiling/profileHeader.tsx
@@ -26,15 +26,15 @@ function ProfileHeader() {
     <Layout.Header style={{gridTemplateColumns: 'minmax(0, 1fr)'}}>
       <Layout.HeaderContent style={{marginBottom: 0}}>
         <Breadcrumb
-          location={location}
           organization={organization}
           trails={[
-            {type: 'landing'},
+            {type: 'landing', payload: {query: location.query}},
             {
               type: 'profile summary',
               payload: {
                 projectSlug,
                 transaction,
+                query: location.query,
               },
             },
             {
@@ -43,6 +43,7 @@ function ProfileHeader() {
                 transaction,
                 profileId,
                 projectSlug,
+                query: location.query,
                 tab: location.pathname.endsWith('details/') ? 'details' : 'flamechart',
               },
             },
@@ -56,7 +57,7 @@ function ProfileHeader() {
               orgSlug: organization.slug,
               projectSlug,
               profileId,
-              location,
+              query: location.query,
             })}
           >
             {t('Flamechart')}
@@ -68,7 +69,7 @@ function ProfileHeader() {
               orgSlug: organization.slug,
               projectSlug,
               profileId,
-              location,
+              query: location.query,
             })}
           >
             {t('Details')}

diff --git a/static/app/components/profiling/profileTransactionsTable.tsx b/static/app/components/profiling/profileTransactionsTable.tsx
@@ -54,7 +54,7 @@ function ProfileTransactionsTable(props: ProfileTransactionsTableProps) {
         transaction: project ? (
           <Link
             to={generateProfileSummaryRouteWithQuery({
-              location,
+              query: location.query,
               orgSlug: organization.slug,
               projectSlug: project.slug,
               transaction: transaction.name,

diff --git a/static/app/components/profiling/profilesTable.tsx b/static/app/components/profiling/profilesTable.tsx
@@ -142,7 +142,7 @@ function ProfilesTableCell({column, dataRow}: ProfilesTableCellProps) {
       }
 
       const profileSummaryTarget = generateProfileSummaryRouteWithQuery({
-        location,
+        query: location.query,
         orgSlug: organization.slug,
         projectSlug: project.slug,
         transaction: dataRow.transaction_name,

diff --git a/static/app/utils/profiling/routes.tsx b/static/app/utils/profiling/routes.tsx
@@ -42,26 +42,22 @@ export function generateProfileDetailsRoute({
 }
 
 export function generateProfilingRouteWithQuery({
-  location,
   orgSlug,
   query,
 }: {
   orgSlug: Organization['slug'];
-  location?: Location;
   query?: Location['query'];
 }): LocationDescriptor {
   const pathname = generateProfilingRoute({orgSlug});
   return {
     pathname,
     query: {
-      ...location?.query,
       ...query,
     },
   };
 }
 
 export function generateProfileSummaryRouteWithQuery({
-  location,
   orgSlug,
   projectSlug,
   transaction,
@@ -70,22 +66,19 @@ export function generateProfileSummaryRouteWithQuery({
   orgSlug: Organization['slug'];
   projectSlug: Project['slug'];
   transaction: string;
-  location?: Location;
   query?: Location['query'];
 }): LocationDescriptor {
   const pathname = generateProfileSummaryRoute({orgSlug, projectSlug});
   return {
     pathname,
     query: {
-      ...location?.query,
       ...query,
       transaction,
     },
   };
 }
 
 export function generateProfileFlamechartRouteWithQuery({
-  location,
   orgSlug,
   projectSlug,
   profileId,
@@ -94,7 +87,6 @@ export function generateProfileFlamechartRouteWithQuery({
   orgSlug: Organization['slug'];
   profileId: Trace['id'];
   projectSlug: Project['slug'];
-  location?: Location;
   query?: Location['query'];
 }): LocationDescriptor {
   const pathname = generateProfileFlamechartRoute({
@@ -105,14 +97,12 @@ export function generateProfileFlamechartRouteWithQuery({
   return {
     pathname,
     query: {
-      ...location?.query,
       ...query,
     },
   };
 }
 
 export function generateProfileDetailsRouteWithQuery({
-  location,
   orgSlug,
   projectSlug,
   profileId,
@@ -121,14 +111,12 @@ export function generateProfileDetailsRouteWithQuery({
   orgSlug: Organization['slug'];
   profileId: Trace['id'];
   projectSlug: Project['slug'];
-  location?: Location;
   query?: Location['query'];
 }): LocationDescriptor {
   const pathname = generateProfileDetailsRoute({orgSlug, projectSlug, profileId});
   return {
     pathname,
     query: {
-      ...location?.query,
       ...query,
     },
   };

diff --git a/static/app/views/profiling/profileSummary/index.tsx b/static/app/views/profiling/profileSummary/index.tsx
@@ -115,14 +115,19 @@ function ProfileSummaryPage(props: ProfileSummaryPageProps) {
               <Layout.Header>
                 <Layout.HeaderContent>
                   <Breadcrumb
-                    location={props.location}
                     organization={organization}
                     trails={[
-                      {type: 'landing'},
+                      {
+                        type: 'landing',
+                        payload: {
+                          query: props.location.query,
+                        },
+                      },
                       {
                         type: 'profile summary',
                         payload: {
                           projectSlug: project.slug,
+                          query: props.location.query,
                           transaction,
                         },
                       },