Skip to content

ref(feedback): Refactor feedback query keys #112552

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

Open
ryan953 wants to merge 5 commits into
base: master
Choose a base branch
from
Open

ref(feedback): Refactor feedback query keys #112552

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
32f3553
ref(feedback): Refactor feedback query keys so there is less stuff in…
ryan953 Apr 8, 2026
01d20c8
fix(feedback): Fix invalidateCachedListPage predicate for new page shape
ryan953 Apr 8, 2026
a139c1c
rm extra changes
ryan953 Apr 8, 2026
44e0719
missed a spot
ryan953 Apr 8, 2026
2f4619f
more memoization, override statsPeriod only if set
ryan953 Apr 9, 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
43 changes: 26 additions & 17 deletions static/app/components/feedback/list/feedbackList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,20 @@ import waitingForEventImg from 'sentry-images/spot/waiting-for-event.svg';
import {Stack} from '@sentry/scraps/layout';
import {Tooltip} from '@sentry/scraps/tooltip';

import type {ApiResult} from 'sentry/api';
import {ErrorBoundary} from 'sentry/components/errorBoundary';
import {FeedbackListHeader} from 'sentry/components/feedback/list/feedbackListHeader';
import {FeedbackListItem} from 'sentry/components/feedback/list/feedbackListItem';
import {useInfiniteFeedbackListQueryOptions} from 'sentry/components/feedback/useFeedbackListQueryOptions';
import {useFeedbackQueryKeys} from 'sentry/components/feedback/useFeedbackQueryKeys';
import {InfiniteListItems} from 'sentry/components/infiniteList/infiniteListItems';
import {InfiniteListState} from 'sentry/components/infiniteList/infiniteListState';
import {LoadingIndicator} from 'sentry/components/loadingIndicator';
import {t} from 'sentry/locale';
import type {ApiResponse} from 'sentry/utils/api/apiFetch';
import type {FeedbackIssueListItem} from 'sentry/utils/feedback/types';
import {useListItemCheckboxContext} from 'sentry/utils/list/useListItemCheckboxState';
import {useInfiniteApiQuery} from 'sentry/utils/queryClient';
import type {InfiniteApiQueryKey} from 'sentry/utils/queryClient';
import {useInfiniteQuery} from 'sentry/utils/queryClient';
import {useOrganization} from 'sentry/utils/useOrganization';

function NoFeedback() {
return (
Expand All @@ -36,25 +37,33 @@ interface Props {
}

export function FeedbackList({onItemSelect}: Props) {
const {listQueryKey} = useFeedbackQueryKeys();
const queryResult = useInfiniteApiQuery<FeedbackIssueListItem[]>({
queryKey:
listQueryKey ??
([{infinite: true, version: 'v1'}, ''] as unknown as InfiniteApiQueryKey),
enabled: Boolean(listQueryKey),
const {listHeadTime} = useFeedbackQueryKeys();
const organization = useOrganization();
const listQueryOptions = useInfiniteFeedbackListQueryOptions({
listHeadTime,
organization,
});
const queryResult = useInfiniteQuery({
...listQueryOptions,
enabled: Boolean(listQueryOptions.queryKey),
});

// Deduplicated issues. In case one page overlaps with another.
// Can't use `select()` in useInfiniteQuery() because `<InfiniteListItems>`
// has it's own stuff going on, and that's a larger refactor for another time.
const issues = useMemo(
() => uniqBy(queryResult.data?.pages.flatMap(result => result[0]) ?? [], 'id'),
() =>
uniqBy(
queryResult.data?.pages.flatMap(result => result.json ?? []),
'id'
),
[queryResult.data?.pages]
);
const hits = queryResult.data?.pages[0]?.headers['X-Hits'] ?? issues.length;

const checkboxState = useListItemCheckboxContext({
hits: Number(
queryResult.data?.pages[0]?.[2]?.getResponseHeader('X-Hits') ?? issues.length
),
hits,
knownIds: issues.map(issue => issue.id),
queryKey: listQueryKey,
queryKey: listQueryOptions.queryKey,
});

return (
Expand All @@ -66,10 +75,10 @@ export function FeedbackList({onItemSelect}: Props) {
backgroundUpdatingMessage={() => null}
loadingMessage={() => <LoadingIndicator />}
>
<InfiniteListItems<FeedbackIssueListItem, ApiResult<FeedbackIssueListItem[]>>
<InfiniteListItems<FeedbackIssueListItem, ApiResponse<FeedbackIssueListItem[]>>
deduplicateItems={pages =>
uniqBy(
pages.flatMap(page => page[0]),
pages.flatMap(page => page.json ?? []),
'id'
)
}
Expand Down
4 changes: 2 additions & 2 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 @@ -34,8 +34,8 @@ export function FeedbackListHeader({
}: Props) {
const [mailbox, setMailbox] = useMailbox();

const {listPrefetchQueryKey, resetListHeadTime} = useFeedbackQueryKeys();
const hasNewItems = useFeedbackHasNewItems({listPrefetchQueryKey});
const {listHeadTime, resetListHeadTime} = useFeedbackQueryKeys();
const hasNewItems = useFeedbackHasNewItems({listHeadTime});
const {invalidateListCache} = useFeedbackCache();

return (
Expand Down
61 changes: 29 additions & 32 deletions static/app/components/feedback/list/useMailboxCounts.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@ import type {Organization} from 'sentry/types/organization';
import {getApiUrl} from 'sentry/utils/api/getApiUrl';
import {coaleseIssueStatsPeriodQuery} from 'sentry/utils/feedback/coaleseIssueStatsPeriodQuery';
import {useApiQuery, type UseApiQueryResult} from 'sentry/utils/queryClient';
import {decodeList, decodeScalar} from 'sentry/utils/queryString';
import type {RequestError} from 'sentry/utils/requestError/requestError';
import {useLocationQuery} from 'sentry/utils/url/useLocationQuery';
import {useLocation} from 'sentry/utils/useLocation';
import {
useListQueryState,
useSearchQueryState,
} from 'sentry/utils/url/useSentryQueryState';

interface Props {
organization: Organization;
Expand All @@ -27,43 +28,39 @@ type HookReturnType = {
export function useMailboxCounts({
organization,
}: Props): UseApiQueryResult<HookReturnType, RequestError> {
const location = useLocation();
const locationQuery = decodeScalar(location.query.query, '');
const {listHeadTime} = useFeedbackQueryKeys();

// We should fetch the counts while taking the query into account
const MAILBOX: Record<keyof HookReturnType, keyof ApiReturnType> = {
unresolved: 'issue.category:feedback is:unassigned is:unresolved ' + locationQuery,
resolved: 'issue.category:feedback is:unassigned is:resolved ' + locationQuery,
ignored: 'issue.category:feedback is:unassigned is:ignored ' + locationQuery,
};
const listQueryState = useListQueryState();
const searchQueryState = useSearchQueryState();

const mailboxQuery = Object.values(MAILBOX);
const MAILBOX = useMemo(
() => ({
unresolved:
'issue.category:feedback is:unassigned is:unresolved ' + searchQueryState.query,
resolved:
'issue.category:feedback is:unassigned is:resolved ' + searchQueryState.query,
ignored:
'issue.category:feedback is:unassigned is:ignored ' + searchQueryState.query,
}),
[searchQueryState.query]
);

const queryView = useLocationQuery({
fields: {
end: decodeScalar,
environment: decodeList,
field: decodeList,
project: decodeList,
query: mailboxQuery,
queryReferrer: 'feedback_mailbox_count',
start: decodeScalar,
statsPeriod: decodeScalar,
utc: decodeScalar,
},
});
const queryViewWithStatsPeriod = useMemo(() => {
// We should fetch the counts while taking the query into account
const mailboxQuery = Object.values(MAILBOX);

const queryViewWithStatsPeriod = useMemo(
() =>
coaleseIssueStatsPeriodQuery({
defaultStatsPeriod: '0d',
return {
...listQueryState,
...searchQueryState,
queryReferrer: 'feedback_mailbox_count',
query: mailboxQuery,
...coaleseIssueStatsPeriodQuery({
listHeadTime,
prefetch: false,
queryView,
statsPeriod: listQueryState.statsPeriod,
}),
[listHeadTime, queryView]
);
};
}, [listHeadTime, listQueryState, searchQueryState, MAILBOX]);

const result = useApiQuery<ApiReturnType>(
[
Expand Down
13 changes: 10 additions & 3 deletions static/app/components/feedback/list/useRefetchFeedbackList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,26 @@
import {useCallback} from 'react';

import {useFeedbackCache} from 'sentry/components/feedback/useFeedbackCache';
import {useInfiniteFeedbackListQueryOptions} from 'sentry/components/feedback/useFeedbackListQueryOptions';
import {useFeedbackQueryKeys} from 'sentry/components/feedback/useFeedbackQueryKeys';
import {useQueryClient} from 'sentry/utils/queryClient';
import {useOrganization} from 'sentry/utils/useOrganization';

export function useRefetchFeedbackList() {
const queryClient = useQueryClient();
const {listQueryKey, resetListHeadTime} = useFeedbackQueryKeys();
const organization = useOrganization();
const {listHeadTime, resetListHeadTime} = useFeedbackQueryKeys();
const listQueryOptions = useInfiniteFeedbackListQueryOptions({
listHeadTime,
organization,
});
const {invalidateListCache} = useFeedbackCache();

const refetchFeedbackList = useCallback(() => {
queryClient.invalidateQueries({queryKey: listQueryKey});
queryClient.invalidateQueries({queryKey: listQueryOptions.queryKey});
resetListHeadTime();
invalidateListCache();
}, [queryClient, listQueryKey, resetListHeadTime, invalidateListCache]);
}, [queryClient, listQueryOptions.queryKey, resetListHeadTime, invalidateListCache]);

return {refetchFeedbackList};
}
33 changes: 20 additions & 13 deletions static/app/components/feedback/useFeedbackCache.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
import {useCallback} from 'react';

import type {ApiResult} from 'sentry/api';
import {useInfiniteFeedbackListQueryOptions} from 'sentry/components/feedback/useFeedbackListQueryOptions';
import {useFeedbackQueryKeys} from 'sentry/components/feedback/useFeedbackQueryKeys';
import {defined} from 'sentry/utils';
import type {ApiResponse} from 'sentry/utils/api/apiFetch';
import type {FeedbackIssue, FeedbackIssueListItem} from 'sentry/utils/feedback/types';
import type {ApiQueryKey, InfiniteData, QueryState} from 'sentry/utils/queryClient';
import type {ApiQueryKey, InfiniteData} from 'sentry/utils/queryClient';
import {setApiQueryData, useQueryClient} from 'sentry/utils/queryClient';
import {useOrganization} from 'sentry/utils/useOrganization';

type TFeedbackIds = 'all' | string[];

type ListCache = {
pageParams: unknown[];
pages: Array<ApiResult<FeedbackIssueListItem[]>>;
pages: Array<ApiResponse<FeedbackIssueListItem[]>>;
};

const issueApiEndpointRegexp = /^\/organizations\/\w+\/issues\/\d+\/$/;
Expand All @@ -22,7 +24,13 @@ function isIssueEndpointUrl(query: any) {

export function useFeedbackCache() {
const queryClient = useQueryClient();
const {getItemQueryKeys, listQueryKey} = useFeedbackQueryKeys();
const {getItemQueryKeys, listHeadTime} = useFeedbackQueryKeys();
const organization = useOrganization();
const listQueryOptions = useInfiniteFeedbackListQueryOptions({
listHeadTime,
organization,
});
const listQueryKey = listQueryOptions.queryKey;

const updateCachedQueryKey = useCallback(
(queryKey: ApiQueryKey, payload: Partial<FeedbackIssue>) => {
Expand Down Expand Up @@ -58,13 +66,12 @@ export function useFeedbackCache() {
}
const listData = queryClient.getQueryData<ListCache>(listQueryKey);
if (listData) {
const pages = listData.pages.map(([data, statusText, resp]) => [
data.map(item =>
const pages = listData.pages.map(({json, headers}) => ({
json: json.map(item =>
ids === 'all' || ids.includes(item.id) ? {...item, ...payload} : item
),
statusText,
resp,
]);
headers,
}));
queryClient.setQueryData(listQueryKey, {...listData, pages});
}
},
Expand Down Expand Up @@ -108,11 +115,11 @@ export function useFeedbackCache() {
queryClient.refetchQueries({
queryKey: listQueryKey,
predicate: query => {
// Check if any of the pages contain the items we want to invalidate
const data = query.state.data as
| InfiniteData<ApiResponse<FeedbackIssueListItem[]>>
| undefined;
return Boolean(
(
query.state.data as QueryState<InfiniteData<FeedbackIssueListItem[]>>
).data?.pages.some(items => items.some(item => ids.includes(item.id)))
data?.pages.some(page => page.json.some(item => ids.includes(item.id)))
);
},
});
Expand Down
34 changes: 21 additions & 13 deletions static/app/components/feedback/useFeedbackHasNewItems.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,33 @@
import {useEffect, useState} from 'react';
import {useQuery} from '@tanstack/react-query';

import type {ApiQueryKey} from 'sentry/utils/queryClient';
import {useApiQuery} from 'sentry/utils/queryClient';
import {parseQueryKey} from 'sentry/utils/api/apiQueryKey';
import {useOrganization} from 'sentry/utils/useOrganization';

import {usePrefetchFeedbackListQueryOptions} from './useFeedbackListQueryOptions';

interface Props {
listPrefetchQueryKey: ApiQueryKey | undefined;
listHeadTime: number;
}

const POLLING_INTERVAL_MS = 10_000;

export function useFeedbackHasNewItems({listPrefetchQueryKey}: Props) {
export function useFeedbackHasNewItems({listHeadTime}: Props) {
const organization = useOrganization();
const listPrefetchQueryOptions = usePrefetchFeedbackListQueryOptions({
listHeadTime,
organization,
});

const [foundData, setFoundData] = useState(false);

const {data} = useApiQuery<unknown[]>(
listPrefetchQueryKey ?? ([''] as unknown as ApiQueryKey),
{
refetchInterval: POLLING_INTERVAL_MS,
staleTime: 0,
enabled: Boolean(listPrefetchQueryKey) && !foundData,
}
);
const {statsPeriod} =
parseQueryKey(listPrefetchQueryOptions.queryKey).options?.query ?? {};
const {data} = useQuery({
...listPrefetchQueryOptions,
refetchInterval: POLLING_INTERVAL_MS,
enabled: statsPeriod && !foundData,
Comment on lines +24 to +29
Copy link
Copy Markdown
Contributor

@sentry sentry bot Apr 9, 2026

Choose a reason for hiding this comment

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

Bug: The enabled condition for the useFeedbackHasNewItems hook always evaluates to false because it checks for statsPeriod in the query key, which is stripped out during key construction.
Severity: HIGH

Suggested Fix

The logic for the enabled flag should be changed. Instead of checking for statsPeriod within the parsed query key, which will always be undefined, the logic should revert to a check similar to the previous implementation. For example, check if listPrefetchQueryKey is defined, as it will only be undefined for absolute date ranges where polling is not intended. This will correctly enable polling for relative date ranges.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: static/app/components/feedback/useFeedbackHasNewItems.tsx#L24-L29

Potential issue: A refactoring in `useFeedbackHasNewItems` introduced a logic error that
disables polling for new feedback items. The `enabled` condition now checks for the
`statsPeriod` property within the query key. However, the `stripUndefinedValues`
function removes this property before the query key is constructed. As a result, the
condition `statsPeriod && !foundData` always evaluates to `false`, preventing the
polling request from ever being made. This means users will never see the "Load new
feedback" banner, even when new items are available and a relative date range is
selected.

});
Comment on lines +26 to +30
Copy link
Copy Markdown
Member Author

@ryan953 ryan953 Apr 8, 2026

Choose a reason for hiding this comment

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

The idea here is that we have stored a timestamp listHeadTime into react context when we first load the page and start the infinite list going.
We can scroll down the list and load more older items incrementally. As we scroll we get things older and older.

That listHeadTime is used to fetch and see if there are any items that are newer, something appeared since we loaded the page. We're fetching with a timerange like between now(aka payload) and tomorrow. Then if we find something we can clear caches, and reset the top of the list to an updated timestamp.


useEffect(() => {
// Once we found something, no need to keep polling.
Expand All @@ -29,7 +37,7 @@ export function useFeedbackHasNewItems({listPrefetchQueryKey}: Props) {
useEffect(() => {
// New key, start polling again
setFoundData(false);
}, [listPrefetchQueryKey]);
}, [listHeadTime]);

return Boolean(data?.length);
}
Loading
Loading