Skip to content

Commit

Permalink
fix(browser-starfish): cursor persisting when navigating between page…
Browse files Browse the repository at this point in the history
…s/filters (#60378)

Fixes two issues in resource module
1. The cursor was not being reset when adding filters such as type,
blocking status in the main resources page.
2. When you navigate to another page in the resources table, then click
on a resource, that same cursor would be used to fetch the "found in
pages" table. To solve this we use the `spansCursor` query param in the
resources page, and `pagesCursor` query param in the resourceSummary.
  • Loading branch information
DominikB2014 authored Nov 21, 2023
1 parent a3fa4b3 commit e5d4b98
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
import {useResourcePagesQuery} from 'sentry/views/performance/browser/resources/utils/useResourcePagesQuery';
import {useResourceSort} from 'sentry/views/performance/browser/resources/utils/useResourceSort';
import {ModuleName} from 'sentry/views/starfish/types';
import {QueryParameterNames} from 'sentry/views/starfish/views/queryParameters';
import {SpanTimeCharts} from 'sentry/views/starfish/views/spans/spanTimeCharts';
import {ModuleFilters} from 'sentry/views/starfish/views/spans/useModuleFilters';

Expand Down Expand Up @@ -88,6 +89,7 @@ function ResourceTypeSelector({value}: {value?: string}) {
query: {
...location.query,
[RESOURCE_TYPE]: newValue?.value,
[QueryParameterNames.SPANS_CURSOR]: undefined,
},
});
}}
Expand Down Expand Up @@ -159,6 +161,7 @@ export function TransactionSelector({
query: {
...location.query,
[TRANSACTION]: newValue?.value,
[QueryParameterNames.SPANS_CURSOR]: undefined,
},
});
}}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {Fragment} from 'react';
import {browserHistory} from 'react-router';
import styled from '@emotion/styled';
import {PlatformIcon} from 'platformicons';

Expand All @@ -7,9 +8,10 @@ import GridEditable, {
GridColumnHeader,
GridColumnOrder,
} from 'sentry/components/gridEditable';
import Pagination from 'sentry/components/pagination';
import Pagination, {CursorHandler} from 'sentry/components/pagination';
import {t} from 'sentry/locale';
import {space} from 'sentry/styles/space';
import {decodeScalar} from 'sentry/utils/queryString';
import {useLocation} from 'sentry/utils/useLocation';
import {RESOURCE_THROUGHPUT_UNIT} from 'sentry/views/performance/browser/resources';
import ResourceSize from 'sentry/views/performance/browser/resources/shared/resourceSize';
Expand All @@ -21,6 +23,7 @@ import {SpanDescriptionCell} from 'sentry/views/starfish/components/tableCells/s
import {ThroughputCell} from 'sentry/views/starfish/components/tableCells/throughputCell';
import {TimeSpentCell} from 'sentry/views/starfish/components/tableCells/timeSpentCell';
import {ModuleName, SpanFunction, SpanMetricsField} from 'sentry/views/starfish/types';
import {QueryParameterNames} from 'sentry/views/starfish/views/queryParameters';
import {DataTitles, getThroughputTitle} from 'sentry/views/starfish/views/spans/types';

const {
Expand Down Expand Up @@ -61,9 +64,12 @@ type Props = {

function ResourceTable({sort, defaultResourceTypes}: Props) {
const location = useLocation();
const cursor = decodeScalar(location.query?.[QueryParameterNames.SPANS_CURSOR]);

const {data, isLoading, pageLinks} = useResourcesQuery({
sort,
defaultResourceTypes,
cursor,
});

const columnOrder: GridColumnOrder<keyof Row>[] = [
Expand Down Expand Up @@ -159,6 +165,13 @@ function ResourceTable({sort, defaultResourceTypes}: Props) {
return <span>{row[key]}</span>;
};

const handleCursor: CursorHandler = (newCursor, pathname, query) => {
browserHistory.push({
pathname,
query: {...query, [QueryParameterNames.SPANS_CURSOR]: newCursor},
});
};

return (
<Fragment>
<GridEditable
Expand All @@ -182,7 +195,7 @@ function ResourceTable({sort, defaultResourceTypes}: Props) {
}}
location={location}
/>
<Pagination pageLinks={pageLinks} />
<Pagination pageLinks={pageLinks} onCursor={handleCursor} />
</Fragment>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ function ResourceSummaryTable() {
const {groupId} = useParams();
const sort = useResourceSummarySort();
const filters = useResourceModuleFilters();
const cursor = decodeScalar(location.query?.[QueryParameterNames.SPANS_CURSOR]);
const cursor = decodeScalar(location.query?.[QueryParameterNames.PAGES_CURSOR]);
const {data, isLoading, pageLinks} = useResourcePagesQuery(groupId, {
sort,
cursor,
Expand Down Expand Up @@ -121,7 +121,7 @@ function ResourceSummaryTable() {
const handleCursor: CursorHandler = (newCursor, pathname, query) => {
browserHistory.push({
pathname,
query: {...query, [QueryParameterNames.SPANS_CURSOR]: newCursor},
query: {...query, [QueryParameterNames.PAGES_CURSOR]: newCursor},
});
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export function DomainSelector({
query: {
...location.query,
[SPAN_DOMAIN]: newValue?.value,
cursor: undefined,
},
});
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import SelectControlWithProps, {
Option,
} from 'sentry/views/performance/browser/resources/shared/selectControlWithProps';
import {SpanMetricsField} from 'sentry/views/starfish/types';
import {QueryParameterNames} from 'sentry/views/starfish/views/queryParameters';

const {RESOURCE_RENDER_BLOCKING_STATUS} = SpanMetricsField;

Expand All @@ -29,6 +30,7 @@ function RenderBlockingSelector({value}: {value?: string}) {
query: {
...location.query,
[RESOURCE_RENDER_BLOCKING_STATUS]: newValue?.value,
[QueryParameterNames.SPANS_CURSOR]: undefined,
},
});
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const {TIME_SPENT_PERCENTAGE} = SpanFunction;

type Props = {
sort: ValidSort;
cursor?: string;
defaultResourceTypes?: string[];
limit?: number;
query?: string;
Expand All @@ -54,7 +55,13 @@ export const getResourcesEventViewQuery = (
];
};

export const useResourcesQuery = ({sort, defaultResourceTypes, query, limit}: Props) => {
export const useResourcesQuery = ({
sort,
defaultResourceTypes,
query,
limit,
cursor,
}: Props) => {
const pageFilters = usePageFilters();
const location = useLocation();
const resourceFilters = useResourceModuleFilters();
Expand Down Expand Up @@ -102,6 +109,7 @@ export const useResourcesQuery = ({sort, defaultResourceTypes, query, limit}: Pr
options: {
refetchOnWindowFocus: false,
},
cursor,
});

const data = result?.data?.data.map(row => ({
Expand Down
1 change: 1 addition & 0 deletions static/app/views/starfish/views/queryParameters.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ export enum QueryParameterNames {
SPANS_SORT = 'spansSort',
ENDPOINTS_CURSOR = 'endpointsCursor',
ENDPOINTS_SORT = 'endpointsSort',
PAGES_CURSOR = 'pagesCursor',
}

0 comments on commit e5d4b98

Please sign in to comment.