Skip to content

fix(checkboxState): Update useListItemCheckboxContext to not rely on ApiQueryKey directly, only QueryKeyEndpointOptions #114997

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ryan953 merged 10 commits into from
May 14, 2026
Merged

fix(checkboxState): Update useListItemCheckboxContext to not rely on ApiQueryKey directly, only QueryKeyEndpointOptions #114997

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
be936ec
fix(checkboxState): Update useListItemCheckboxContext to not rely on …
ryan953 May 6, 2026
6ee5bb4
Use the provider in feedbackList and simplify away the useListItemChe…
ryan953 May 6, 2026
f96346a
knip
ryan953 May 6, 2026
71faf30
fix tests
ryan953 May 6, 2026
b296ada
fix: Update test to use renamed queryOptionsRef prop in ReplayBulkVie…
ryan953 May 6, 2026
cc365ed
rename all the things
ryan953 May 7, 2026
f1251a2
remove the `as unknown as ApiQueryKey` fake queryKey. kind of the who…
ryan953 May 7, 2026
bb5883e
fix list checkbox context test expectation
cursoragent May 12, 2026
782e427
fix replay table header checkbox state mock
cursoragent May 12, 2026
fc53962
Merge branch 'master' into ryan953/fix-checkbox-state-QueryKeyEndpoin…
ryan953 May 14, 2026
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
24 changes: 10 additions & 14 deletions static/app/components/feedback/list/feedbackList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {Fragment, useMemo} from 'react';
import {useMemo} from 'react';
import styled from '@emotion/styled';
import {useInfiniteQuery} from '@tanstack/react-query';
import uniqBy from 'lodash/uniqBy';
Expand All @@ -17,8 +17,9 @@ import {InfiniteListState} from 'sentry/components/infiniteList/infiniteListStat
import {LoadingIndicator} from 'sentry/components/loadingIndicator';
import {t} from 'sentry/locale';
import type {ApiResponse} from 'sentry/utils/api/apiFetch';
import {safeParseQueryKey} from 'sentry/utils/api/apiQueryKey';
import type {FeedbackIssueListItem} from 'sentry/utils/feedback/types';
import {useListItemCheckboxContext} from 'sentry/utils/list/useListItemCheckboxState';
import {ListItemCheckboxProvider} from 'sentry/utils/list/useListItemCheckboxState';

function NoFeedback() {
return (
Expand All @@ -43,15 +44,14 @@ export function FeedbackList({onItemSelect}: Props) {
() => uniqBy(queryResult.data?.pages.flatMap(page => page.json) ?? [], 'id'),
[queryResult.data?.pages]
);
const checkboxState = useListItemCheckboxContext({
hits: Number(queryResult.data?.pages[0]?.headers['X-Hits'] ?? issues.length),
knownIds: issues.map(issue => issue.id),
queryKey: listApiOptions.queryKey,
});

return (
<Fragment>
<FeedbackListHeader {...checkboxState} />
<ListItemCheckboxProvider
hits={Number(queryResult.data?.pages[0]?.headers['X-Hits'] ?? issues.length)}
knownIds={issues.map(issue => issue.id)}
endpointOptions={safeParseQueryKey(listApiOptions.queryKey)?.options}
>
<FeedbackListHeader />
<Stack flexGrow={1} paddingBottom="xs">
<InfiniteListState
queryResult={queryResult}
Expand All @@ -73,10 +73,6 @@ export function FeedbackList({onItemSelect}: Props) {
<ErrorBoundary mini>
<FeedbackListItem
feedbackItem={item}
isSelected={checkboxState.isSelected(item.id)}
onSelect={() => {
checkboxState.toggleSelected(item.id);
}}
onItemSelect={() => onItemSelect(itemIndex)}
/>
</ErrorBoundary>
Expand All @@ -94,7 +90,7 @@ export function FeedbackList({onItemSelect}: Props) {
/>
</InfiniteListState>
</Stack>
</Fragment>
</ListItemCheckboxProvider>
);
}

Expand Down
29 changes: 10 additions & 19 deletions static/app/components/feedback/list/feedbackListHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,26 +12,17 @@ import {useFeedbackHasNewItems} from 'sentry/components/feedback/useFeedbackHasN
import {useMailbox} from 'sentry/components/feedback/useMailbox';
import {IconRefresh} from 'sentry/icons';
import {t} from 'sentry/locale';
import type {ListItemCheckboxState} from 'sentry/utils/list/useListItemCheckboxState';
import {useListItemCheckboxContext} from 'sentry/utils/list/useListItemCheckboxState';

interface Props extends Pick<
ListItemCheckboxState,
| 'countSelected'
| 'deselectAll'
| 'isAllSelected'
| 'isAnySelected'
| 'selectAll'
| 'selectedIds'
> {}

export function FeedbackListHeader({
countSelected,
deselectAll,
isAllSelected,
isAnySelected,
selectAll,
selectedIds,
}: Props) {
export function FeedbackListHeader() {
const {
countSelected,
deselectAll,
isAllSelected,
isAnySelected,
selectAll,
selectedIds,
} = useListItemCheckboxContext();
const [mailbox, setMailbox] = useMailbox();

const {listPrefetchApiOptions, resetListHeadTime} = useFeedbackApiOptions();
Expand Down
20 changes: 8 additions & 12 deletions static/app/components/feedback/list/feedbackListItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import type {Group} from 'sentry/types/group';
import {trackAnalytics} from 'sentry/utils/analytics';
import {feedbackHasLinkedError} from 'sentry/utils/feedback/hasLinkedError';
import {type FeedbackIssueListItem} from 'sentry/utils/feedback/types';
import {useListItemCheckboxContext} from 'sentry/utils/list/useListItemCheckboxState';
import {decodeScalar} from 'sentry/utils/queryString';
import {useReplayCountForFeedbacks} from 'sentry/utils/replayCount/useReplayCountForFeedbacks';
import {useLocationQuery} from 'sentry/utils/url/useLocationQuery';
Expand All @@ -26,9 +27,7 @@ import {makeFeedbackPathname} from 'sentry/views/feedback/pathnames';

interface Props {
feedbackItem: FeedbackIssueListItem;
isSelected: 'all-selected' | boolean;
onItemSelect: () => void;
onSelect: (isSelected: boolean) => void;
}

function useIsSelectedFeedback({feedbackItem}: {feedbackItem: FeedbackIssueListItem}) {
Expand All @@ -39,12 +38,9 @@ function useIsSelectedFeedback({feedbackItem}: {feedbackItem: FeedbackIssueListI
return feedbackId === feedbackItem.id;
}

export function FeedbackListItem({
feedbackItem,
isSelected,
onSelect,
onItemSelect,
}: Props) {
export function FeedbackListItem({feedbackItem, onItemSelect}: Props) {
const {isSelected, toggleSelected} = useListItemCheckboxContext();

const organization = useOrganization();
const isOpen = useIsSelectedFeedback({feedbackItem});
const {feedbackHasReplay} = useReplayCountForFeedbacks();
Expand Down Expand Up @@ -84,10 +80,10 @@ export function FeedbackListItem({
}}
>
<Checkbox
disabled={isSelected === 'all-selected'}
checked={isSelected !== false}
onChange={e => {
onSelect(e.target.checked);
disabled={isSelected(feedbackItem.id) === 'all-selected'}
checked={isSelected(feedbackItem.id) !== false}
onChange={() => {
toggleSelected(feedbackItem.id);
}}
/>
</Row>
Expand Down
19 changes: 3 additions & 16 deletions static/app/components/replays/table/replayBulkViewedActions.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import {render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrar
import {ReplayBulkViewedActions} from 'sentry/components/replays/table/replayBulkViewedActions';
import {ProjectsStore} from 'sentry/stores/projectsStore';
import {trackAnalytics} from 'sentry/utils/analytics';
import type {ApiQueryKey} from 'sentry/utils/api/apiQueryKey';
import {getApiUrl} from 'sentry/utils/api/getApiUrl';
import type {ListCheckboxQueryKeyRef} from 'sentry/utils/list/useListItemCheckboxState';
import type {ReplayListRecord} from 'sentry/views/explore/replays/types';

Expand Down Expand Up @@ -51,30 +49,19 @@ describe('ReplayBulkViewedActions', () => {
const replay = createReplay();

const deselectAll = jest.fn();
const apiUrl = getApiUrl(
'/projects/$organizationIdOrSlug/$projectIdOrSlug/replays/$replayId/viewed-by/',
{
path: {
organizationIdOrSlug: organization.id,
projectIdOrSlug: 2,
replayId: replay.id,
},
}
);
const queryKey: ApiQueryKey = [apiUrl, {}, {infinite: false}];
const queryKeyRef = {current: queryKey};
const endpointOptionsRef: ListCheckboxQueryKeyRef = {current: {query: {}}};

const renderWithOrganization = (
overrides: {
queryKeyRef?: ListCheckboxQueryKeyRef;
endpointOptionsRef?: ListCheckboxQueryKeyRef;
replays?: ReplayListRecord[];
selectedIds?: string[];
} = {}
) =>
render(
<ReplayBulkViewedActions
deselectAll={deselectAll}
queryKeyRef={overrides.queryKeyRef ?? queryKeyRef}
endpointOptionsRef={overrides.endpointOptionsRef ?? endpointOptionsRef}
replays={overrides.replays ?? [replay]}
selectedIds={overrides.selectedIds ?? [replay.id]}
/>,
Expand Down
23 changes: 12 additions & 11 deletions static/app/components/replays/table/replayBulkViewedActions.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,15 @@ import {LoadingIndicator} from 'sentry/components/loadingIndicator';
import {IconCheckmark} from 'sentry/icons';
import {t, tn} from 'sentry/locale';
import {trackAnalytics} from 'sentry/utils/analytics';
import type {ApiResponse} from 'sentry/utils/api/apiFetch';
import type {ListCheckboxQueryKeyRef} from 'sentry/utils/list/useListItemCheckboxState';
import {fetchMutation} from 'sentry/utils/queryClient';
import {replayListApiOptions} from 'sentry/utils/replays/replayListApiOptions';
import {useOrganization} from 'sentry/utils/useOrganization';
import type {
HydratedReplayRecord,
ReplayListRecord,
} from 'sentry/views/explore/replays/types';
import type {ReplayListRecord} from 'sentry/views/explore/replays/types';

interface Props {
deselectAll: () => void;
queryKeyRef: ListCheckboxQueryKeyRef;
endpointOptionsRef: ListCheckboxQueryKeyRef;
replays: ReplayListRecord[];
selectedIds: string[];
}
Expand All @@ -28,7 +25,7 @@ export function ReplayBulkViewedActions({
deselectAll,
replays,
selectedIds,
queryKeyRef,
endpointOptionsRef,
}: Props) {
const organization = useOrganization();
const queryClient = useQueryClient();
Expand All @@ -55,10 +52,14 @@ export function ReplayBulkViewedActions({
);

if (succeededIds.size) {
if (queryKeyRef.current) {
// eslint-disable-next-line @sentry/no-query-data-type-parameters
queryClient.setQueryData<ApiResponse<{data: HydratedReplayRecord[]}>>(
queryKeyRef.current,
if (endpointOptionsRef.current) {
const replayListOptions = replayListApiOptions({
options: endpointOptionsRef.current,
organization,
queryReferrer: 'replayList',
});
queryClient.setQueryData(
replayListOptions.queryKey,
Comment thread
cursor[bot] marked this conversation as resolved.
Copy link
Copy Markdown
Contributor

@cursor cursor Bot May 14, 2026

Choose a reason for hiding this comment

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

Hardcoded queryReferrer causes fragile query key reconstruction

Medium Severity

The optimistic cache update reconstructs the full query key via replayListApiOptions with a hardcoded queryReferrer: 'replayList'. The endpointOptionsRef.current already contains the processed query params (including the original queryReferrer), but getQueryForReplaysList overwrites it with the hardcoded value. If ReplayTableHeader (which renders this component) is ever used in a different context (e.g. issueReplays), the reconstructed key won't match the cache entry and the setQueryData call will silently fail. The previous code avoided this entirely by storing and using the actual query key directly.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit fc53962. Configure here.

old =>
old && {
...old,
Expand Down
2 changes: 1 addition & 1 deletion static/app/components/replays/table/replayTableHeader.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ function baseListCheckboxState(overrides: Partial<ListItemCheckboxState>) {
isAnySelected: false,
isSelected: () => false,
knownIds: ['a', 'b'],
queryKeyRef: {current: undefined},
endpointOptionsRef: {current: undefined},
selectAll: jest.fn(),
selectedIds: [],
toggleSelected: jest.fn(),
Expand Down
13 changes: 5 additions & 8 deletions static/app/components/replays/table/replayTableHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
} from 'sentry/components/replays/table/replayTableColumns';
import {SimpleTable} from 'sentry/components/tables/simpleTable';
import {t, tct, tn} from 'sentry/locale';
import {parseQueryKey} from 'sentry/utils/api/apiQueryKey';
import type {Sort} from 'sentry/utils/discover/fields';
import {useListItemCheckboxContext} from 'sentry/utils/list/useListItemCheckboxState';
import type {ReplayListRecord} from 'sentry/views/explore/replays/types';
Expand All @@ -38,14 +37,12 @@ export function ReplayTableHeader({
deselectAll,
isAllSelected,
isAnySelected,
queryKeyRef,
endpointOptionsRef,
selectAll,
selectedIds,
} = listItemCheckboxState;
const queryOptions = queryKeyRef.current
? parseQueryKey(queryKeyRef.current).options
: undefined;
const rawQuery = queryOptions?.query?.query;
const endpointOptions = endpointOptionsRef.current;
const rawQuery = endpointOptions?.query?.query;
const queryString = typeof rawQuery === 'string' ? rawQuery : undefined;

const headerStyle: React.CSSProperties = stickyHeader
Expand Down Expand Up @@ -90,13 +87,13 @@ export function ReplayTableHeader({
{selectedIds !== 'all' && (
<ReplayBulkViewedActions
deselectAll={deselectAll}
queryKeyRef={queryKeyRef}
endpointOptionsRef={endpointOptionsRef}
replays={replays}
selectedIds={selectedIds}
/>
)}
<DeleteReplays
queryOptions={queryOptions}
queryOptions={endpointOptionsRef.current}
replays={replays}
selectedIds={selectedIds}
/>
Expand Down
Loading
Loading