Browse Source

feat(feedback): Poll for new feedbacks, and show a message so people can load them in (#63529)

This PR adds code that polls in the background, every 30second when the
feedback page is open, to see if there are newer feedbacks available.

Polling disables itself once it discovers that one new item is available
(we're not asking for a count of new things).
Polling won't start if the user picks an absolute date range.

The purple button takes up it's own row. So when it first appears the
list will get a little shorter, and the list items will not scroll
'behind' it, no overlapping.

Clicking the button will reset the `listHeadTime` timestamp, then evict
the list cache. Using the new `listHeadTime` a new 1st page will be
fetched. If you had previously scrolled down a bunch and loaded in all
that data.. it will be re-fetched as you scroll down again.

<img width="436" alt="SCR-20240119-mqvb"
src="https://github.com/getsentry/sentry/assets/187460/18dba9bd-2389-48f0-aaf5-a2ff4aa3d363">

Fixes https://github.com/getsentry/sentry/issues/61474
Ryan Albrecht 1 year ago
parent
commit
d3f5b71b32

+ 66 - 24
static/app/components/feedback/list/feedbackListHeader.tsx

@@ -1,11 +1,16 @@
 import styled from '@emotion/styled';
 
+import {Button} from 'sentry/components/button';
 import Checkbox from 'sentry/components/checkbox';
 import decodeMailbox from 'sentry/components/feedback/decodeMailbox';
 import FeedbackListBulkSelection from 'sentry/components/feedback/list/feedbackListBulkSelection';
 import MailboxPicker from 'sentry/components/feedback/list/mailboxPicker';
 import type useListItemCheckboxState from 'sentry/components/feedback/list/useListItemCheckboxState';
-import PanelItem from 'sentry/components/panels/panelItem';
+import useFeedbackCache from 'sentry/components/feedback/useFeedbackCache';
+import useFeedbackHasNewItems from 'sentry/components/feedback/useFeedbackHasNewItems';
+import useFeedbackQueryKeys from 'sentry/components/feedback/useFeedbackQueryKeys';
+import {IconRefresh} from 'sentry/icons';
+import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
 import useLocationQuery from 'sentry/utils/url/useLocationQuery';
 import useUrlParams from 'sentry/utils/useUrlParams';
@@ -36,35 +41,72 @@ export default function FeedbackListHeader({
   });
   const {setParamValue: setMailbox} = useUrlParams('mailbox');
 
+  const {listPrefetchQueryKey, resetListHeadTime} = useFeedbackQueryKeys();
+  const hasNewItems = useFeedbackHasNewItems({listPrefetchQueryKey}) || true;
+  const {invalidateListCache} = useFeedbackCache();
+
   return (
-    <HeaderPanelItem>
-      <Checkbox
-        checked={isAllSelected}
-        onChange={() => {
-          if (isAllSelected === true) {
-            deselectAll();
-          } else {
-            selectAll();
-          }
-        }}
-      />
-      {isAnySelected ? (
-        <FeedbackListBulkSelection
-          mailbox={mailbox}
-          countSelected={countSelected}
-          selectedIds={selectedIds}
-          deselectAll={deselectAll}
+    <HeaderPanel>
+      <HeaderPanelItem>
+        <Checkbox
+          checked={isAllSelected}
+          onChange={() => {
+            if (isAllSelected === true) {
+              deselectAll();
+            } else {
+              selectAll();
+            }
+          }}
         />
-      ) : (
-        <MailboxPicker value={mailbox} onChange={setMailbox} />
-      )}
-    </HeaderPanelItem>
+        {isAnySelected ? (
+          <FeedbackListBulkSelection
+            mailbox={mailbox}
+            countSelected={countSelected}
+            selectedIds={selectedIds}
+            deselectAll={deselectAll}
+          />
+        ) : (
+          <MailboxPicker value={mailbox} onChange={setMailbox} />
+        )}
+      </HeaderPanelItem>
+      {hasNewItems ? (
+        <RefreshContainer>
+          <Button
+            priority="primary"
+            size="xs"
+            icon={<IconRefresh />}
+            onClick={() => {
+              // Get a new date for polling:
+              resetListHeadTime();
+              // Clear the list cache and let people start over from the newest
+              // data in the list:
+              invalidateListCache();
+            }}
+          >
+            {t('Load new feedback')}
+          </Button>
+        </RefreshContainer>
+      ) : null}
+    </HeaderPanel>
   );
 }
 
-const HeaderPanelItem = styled(PanelItem)`
-  display: flex;
+const HeaderPanel = styled('div')`
+  flex-direction: column;
+`;
+
+const HeaderPanelItem = styled('div')`
   padding: ${space(1)} ${space(2)} ${space(1)} ${space(2)};
+  display: flex;
   gap: ${space(1)};
   align-items: center;
+  border-bottom: 1px solid ${p => p.theme.innerBorder};
+`;
+
+const RefreshContainer = styled('div')`
+  display: flex;
+  align-items: center;
+  justify-content: center;
+  flex-grow: 1;
+  padding: ${space(0.5)};
 `;

+ 1 - 2
static/app/components/feedback/list/useListItemCheckboxState.tsx

@@ -52,10 +52,9 @@ interface Return {
 }
 
 export default function useListItemCheckboxState({hits, knownIds}: Props): Return {
-  const {getListQueryKey} = useFeedbackQueryKeys();
+  const {listQueryKey} = useFeedbackQueryKeys();
   const [state, setState] = useState<State>({ids: new Set()});
 
-  const listQueryKey = getListQueryKey();
   useEffect(() => {
     // Reset the state when the list changes
     setState({ids: new Set()});

+ 11 - 8
static/app/components/feedback/useFeedbackCache.tsx

@@ -21,7 +21,7 @@ function isIssueEndpointUrl(query) {
 
 export default function useFeedbackCache() {
   const queryClient = useQueryClient();
-  const {getItemQueryKeys, getListQueryKey} = useFeedbackQueryKeys();
+  const {getItemQueryKeys, listQueryKey} = useFeedbackQueryKeys();
 
   const updateCachedQueryKey = useCallback(
     (queryKey: ApiQueryKey, payload: Partial<FeedbackIssue>) => {
@@ -52,8 +52,7 @@ export default function useFeedbackCache() {
 
   const updateCachedListPage = useCallback(
     (ids: TFeedbackIds, payload: Partial<FeedbackIssue>) => {
-      const queryKey = getListQueryKey();
-      const listData = queryClient.getQueryData<ListCache>(queryKey);
+      const listData = queryClient.getQueryData<ListCache>(listQueryKey);
       if (listData) {
         const pages = listData.pages.map(([data, statusText, resp]) => [
           data.map(item =>
@@ -62,10 +61,10 @@ export default function useFeedbackCache() {
           statusText,
           resp,
         ]);
-        queryClient.setQueryData(queryKey, {...listData, pages});
+        queryClient.setQueryData(listQueryKey, {...listData, pages});
       }
     },
-    [getListQueryKey, queryClient]
+    [listQueryKey, queryClient]
   );
 
   const updateCached = useCallback(
@@ -93,15 +92,14 @@ export default function useFeedbackCache() {
 
   const invalidateCachedListPage = useCallback(
     (ids: TFeedbackIds) => {
-      const queryKey = getListQueryKey();
       queryClient.invalidateQueries({
-        queryKey,
+        queryKey: listQueryKey,
         refetchPage: ([results]: ApiResult<FeedbackIssueList>) => {
           return ids === 'all' || results.some(item => ids.includes(item.id));
         },
       });
     },
-    [getListQueryKey, queryClient]
+    [listQueryKey, queryClient]
   );
 
   const invalidateCached = useCallback(
@@ -112,8 +110,13 @@ export default function useFeedbackCache() {
     [invalidateCachedIssue, invalidateCachedListPage]
   );
 
+  const invalidateListCache = useCallback(() => {
+    invalidateCachedListPage('all');
+  }, [invalidateCachedListPage]);
+
   return {
     updateCached,
     invalidateCached,
+    invalidateListCache,
   };
 }

+ 31 - 0
static/app/components/feedback/useFeedbackHasNewItems.tsx

@@ -0,0 +1,31 @@
+import {useEffect, useState} from 'react';
+
+import {ApiQueryKey, useApiQuery} from 'sentry/utils/queryClient';
+
+interface Props {
+  listPrefetchQueryKey: ApiQueryKey | undefined;
+}
+
+const POLLING_INTERVAL_MS = 10_000;
+
+export default function useFeedbackHasNewItems({listPrefetchQueryKey}: Props) {
+  const [foundData, setFoundData] = useState(false);
+
+  const {data} = useApiQuery<unknown[]>(listPrefetchQueryKey ?? [''], {
+    refetchInterval: POLLING_INTERVAL_MS,
+    staleTime: 0,
+    enabled: listPrefetchQueryKey && !foundData,
+  });
+
+  useEffect(() => {
+    // Once we found something, no need to keep polling.
+    setFoundData(Boolean(data?.length));
+  }, [data]);
+
+  useEffect(() => {
+    // New key, start polling again
+    setFoundData(false);
+  }, [listPrefetchQueryKey]);
+
+  return Boolean(data?.length);
+}

+ 47 - 20
static/app/components/feedback/useFeedbackListQueryKey.tsx

@@ -8,12 +8,19 @@ import {decodeList, decodeScalar} from 'sentry/utils/queryString';
 import useLocationQuery from 'sentry/utils/url/useLocationQuery';
 
 interface Props {
+  listHeadTime: number;
   organization: Organization;
+  prefetch: boolean;
 }
 
 const PER_PAGE = 25;
+const ONE_DAY_MS = intervalToMilliseconds('1d');
 
-export default function useFeedbackListQueryKey({organization}: Props): ApiQueryKey {
+export default function useFeedbackListQueryKey({
+  listHeadTime,
+  organization,
+  prefetch,
+}: Props): ApiQueryKey | undefined {
   const queryView = useLocationQuery({
     fields: {
       limit: PER_PAGE,
@@ -38,38 +45,58 @@ export default function useFeedbackListQueryKey({organization}: Props): ApiQuery
     // the user wants to see fresher content (like, after the page has been open
     // for a while) they can trigger that specifically.
 
+    // The issues endpoint cannot handle when statsPeroid has a value of "", so
+    // we remove that from the rest and do not use it to query.
     const {statsPeriod, ...rest} = queryView;
-    const now = Date.now();
-    const statsPeriodMs = intervalToMilliseconds(statsPeriod);
-    return statsPeriod
-      ? {
-          ...rest,
-          start: new Date(now - statsPeriodMs).toISOString(),
-          end: new Date(now).toISOString(),
-        }
-      : // The issues endpoint cannot handle when statsPeroid has a value of "", so
-        // we remove that from the rest and do not use it to query.
-        rest;
-  }, [queryView]);
+
+    // Usually we want to fetch starting from `now` and looking back in time.
+    // `prefetch` in this case changes the mode: instead of looking back, we want
+    // to look forward for new data, and fetch it before it's time to render.
+    // Note: The ApiQueryKey that we return isn't actually for a full page of
+    // prefetched data, it's just one row actually.
+    if (prefetch) {
+      if (!statsPeriod) {
+        // We shouldn't prefetch if the query uses an absolute date range
+        return undefined;
+      }
+      // Look 1 day into the future, from the time the page is loaded for new
+      // feedbacks to come in.
+      const intervalMS = ONE_DAY_MS;
+      const start = new Date(listHeadTime).toISOString();
+      const end = new Date(listHeadTime + intervalMS).toISOString();
+      return statsPeriod ? {...rest, limit: 1, start, end} : undefined;
+    }
+
+    const intervalMS = intervalToMilliseconds(statsPeriod);
+    const start = new Date(listHeadTime - intervalMS).toISOString();
+    const end = new Date(listHeadTime).toISOString();
+    return statsPeriod ? {...rest, start, end} : rest;
+  }, [listHeadTime, prefetch, queryView]);
 
   return useMemo(() => {
+    if (!queryViewWithStatsPeriod) {
+      return undefined;
+    }
     const {mailbox, ...fixedQueryView} = queryViewWithStatsPeriod;
+
     return [
       `/organizations/${organization.slug}/issues/`,
       {
         query: {
           ...fixedQueryView,
           collapse: ['inbox'],
-          expand: [
-            'owners', // Gives us assignment
-            'stats', // Gives us `firstSeen`
-            'pluginActions', // Gives us plugin actions available
-            'pluginIssues', // Gives us plugin issues available
-          ],
+          expand: prefetch
+            ? []
+            : [
+                'owners', // Gives us assignment
+                'stats', // Gives us `firstSeen`
+                'pluginActions', // Gives us plugin actions available
+                'pluginIssues', // Gives us plugin issues available
+              ],
           shortIdLookup: 0,
           query: `issue.category:feedback status:${mailbox} ${fixedQueryView.query}`,
         },
       },
     ];
-  }, [queryViewWithStatsPeriod, organization.slug]);
+  }, [organization.slug, prefetch, queryViewWithStatsPeriod]);
 }

+ 35 - 6
static/app/components/feedback/useFeedbackQueryKeys.tsx

@@ -1,4 +1,5 @@
-import {createContext, ReactNode, useCallback, useContext, useRef} from 'react';
+import {createContext, ReactNode, useCallback, useContext, useRef, useState} from 'react';
+import invariant from 'invariant';
 
 import getFeedbackItemQueryKey from 'sentry/components/feedback/getFeedbackItemQueryKey';
 import useFeedbackListQueryKey from 'sentry/components/feedback/useFeedbackListQueryKey';
@@ -14,19 +15,34 @@ type ItemQueryKeys = ReturnType<typeof getFeedbackItemQueryKey>;
 
 interface TContext {
   getItemQueryKeys: (id: string) => ItemQueryKeys;
-  getListQueryKey: () => ListQueryKey;
+  listHeadTime: number;
+  listPrefetchQueryKey: ListQueryKey;
+  listQueryKey: NonNullable<ListQueryKey>;
+  resetListHeadTime: () => void;
 }
 
 const EMPTY_ITEM_QUERY_KEYS = {issueQueryKey: undefined, eventQueryKey: undefined};
 
 const DEFAULT_CONTEXT: TContext = {
   getItemQueryKeys: () => EMPTY_ITEM_QUERY_KEYS,
-  getListQueryKey: () => [''],
+  listHeadTime: 0,
+  listPrefetchQueryKey: [''],
+  listQueryKey: [''],
+  resetListHeadTime: () => undefined,
 };
 
 const FeedbackQueryKeysProvider = createContext<TContext>(DEFAULT_CONTEXT);
 
 export function FeedbackQueryKeys({children, organization}: Props) {
+  // The "Head time" is the timestamp of the newest feedback that we can show in
+  // the list (the head element in the array); the same time as when we loaded
+  // the page. It can be updated without loading the page, when we want to see
+  // fresh list items.
+  const [listHeadTime, setHeadTime] = useState(() => Date.now());
+  const resetListHeadTime = useCallback(() => {
+    setHeadTime(Date.now());
+  }, []);
+
   const itemQueryKeyRef = useRef<Map<string, ItemQueryKeys>>(new Map());
   const getItemQueryKeys = useCallback(
     (feedbackId: string) => {
@@ -41,14 +57,27 @@ export function FeedbackQueryKeys({children, organization}: Props) {
     [organization]
   );
 
-  const listQueryKey = useFeedbackListQueryKey({organization});
-  const getListQueryKey = useCallback(() => listQueryKey, [listQueryKey]);
+  const listQueryKey = useFeedbackListQueryKey({
+    listHeadTime,
+    organization,
+    prefetch: false,
+  });
+  invariant(listQueryKey, 'listQueryKey cannot be nullable when prefetch=false');
+
+  const listPrefetchQueryKey = useFeedbackListQueryKey({
+    listHeadTime,
+    organization,
+    prefetch: true,
+  });
 
   return (
     <FeedbackQueryKeysProvider.Provider
       value={{
         getItemQueryKeys,
-        getListQueryKey,
+        listHeadTime,
+        listPrefetchQueryKey,
+        listQueryKey,
+        resetListHeadTime,
       }}
     >
       {children}

+ 3 - 3
static/app/components/feedback/useFetchFeedbackInfiniteListData.tsx

@@ -35,8 +35,8 @@ function uniqueIssues(issues: FeedbackIssueList) {
 }
 
 export default function useFetchFeedbackInfiniteListData() {
-  const {getListQueryKey} = useFeedbackQueryKeys();
-  const queryKey = getListQueryKey();
+  const {listQueryKey} = useFeedbackQueryKeys();
+
   const {
     data,
     error,
@@ -48,7 +48,7 @@ export default function useFetchFeedbackInfiniteListData() {
     isFetchingPreviousPage,
     isLoading, // If anything is loaded yet
   } = useInfiniteApiQuery<FeedbackIssueList>({
-    queryKey,
+    queryKey: listQueryKey,
   });
 
   const issues = useMemo(

+ 2 - 2
static/app/components/feedback/useMutateFeedback.tsx

@@ -22,7 +22,7 @@ export default function useMutateFeedback({feedbackIds, organization}: Props) {
   const api = useApi({
     persistInFlight: false,
   });
-  const {getListQueryKey} = useFeedbackQueryKeys();
+  const {listQueryKey} = useFeedbackQueryKeys();
   const {updateCached, invalidateCached} = useFeedbackCache();
 
   const mutation = useMutation<TData, TError, TVariables, TContext>({
@@ -41,7 +41,7 @@ export default function useMutateFeedback({feedbackIds, organization}: Props) {
       const options = isSingleId
         ? {}
         : ids === 'all'
-        ? getListQueryKey()[1]!
+        ? listQueryKey[1]!
         : {query: {id: ids}};
       return fetchMutation(api)(['PUT', url, options, payload]);
     },