Browse Source

fix(issue-stream): Fix pagination caption when realtime is enabled (#48718)

Closes https://github.com/getsentry/sentry/issues/48717

- Ensure that `numPreviousIssues` doesn't go negative so we don't get
`-1-10 of 10`
- Update `queryCount` when poll updates come in so that we don't get
`1-11 of 10`
Malachi Willey 1 year ago
parent
commit
d9ae351b9b

+ 5 - 2
static/app/utils/cursorPoller.tsx

@@ -1,9 +1,10 @@
 import {Client, Request} from 'sentry/api';
+import {defined} from 'sentry/utils';
 import parseLinkHeader from 'sentry/utils/parseLinkHeader';
 
 type Options = {
   linkPreviousHref: string;
-  success: (data: any, link?: string | null) => void;
+  success: (data: any, headers: {queryCount: number}) => void;
 };
 
 const BASE_DELAY = 3000;
@@ -89,10 +90,12 @@ class CursorPoller {
         }
 
         const linksHeader = resp?.getResponseHeader('Link') ?? null;
+        const hitsHeader = resp?.getResponseHeader('X-Hits') ?? null;
+        const queryCount = defined(hitsHeader) ? parseInt(hitsHeader, 10) || 0 : 0;
         const links = parseLinkHeader(linksHeader);
         this.setEndpoint(links.previous.href);
 
-        this.options.success(data, linksHeader);
+        this.options.success(data, {queryCount});
       },
       error: resp => {
         if (!resp) {

+ 35 - 1
static/app/views/issueList/overview.polling.spec.jsx

@@ -1,12 +1,15 @@
 import {initializeOrg} from 'sentry-test/initializeOrg';
 import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
+import {textWithMarkupMatcher} from 'sentry-test/utils';
 
 import StreamGroup from 'sentry/components/stream/group';
 import TagStore from 'sentry/stores/tagStore';
 import IssueList from 'sentry/views/issueList/overview';
 
 jest.mock('sentry/views/issueList/filters', () => jest.fn(() => null));
-jest.mock('sentry/components/stream/group', () => jest.fn(() => null));
+jest.mock('sentry/components/stream/group', () =>
+  jest.fn(({id}) => <div data-test-id={id} />)
+);
 
 jest.mock('js-cookie', () => ({
   get: jest.fn(),
@@ -43,6 +46,7 @@ describe('IssueList -> Polling', function () {
   });
 
   const group = TestStubs.Group({project});
+  const group2 = TestStubs.Group({project, id: 2});
 
   const defaultProps = {
     location: {query: {query: 'is:unresolved'}, search: 'query=is:unresolved'},
@@ -130,6 +134,7 @@ describe('IssueList -> Polling', function () {
       body: [group],
       headers: {
         Link: DEFAULT_LINKS_HEADER,
+        'X-Hits': 1,
       },
     });
     MockApiClient.addMockResponse({
@@ -141,6 +146,7 @@ describe('IssueList -> Polling', function () {
       body: [],
       headers: {
         Link: DEFAULT_LINKS_HEADER,
+        'X-Hits': 1,
       },
     });
 
@@ -184,6 +190,34 @@ describe('IssueList -> Polling', function () {
     expect(pollRequest).toHaveBeenCalledTimes(2);
   });
 
+  it('displays new group and pagination caption correctly', async function () {
+    pollRequest = MockApiClient.addMockResponse({
+      url: `/api/0/organizations/org-slug/issues/?cursor=${PREVIOUS_PAGE_CURSOR}:0:1`,
+      body: [group2],
+      headers: {
+        Link: DEFAULT_LINKS_HEADER,
+        'X-Hits': 2,
+      },
+    });
+
+    await renderComponent();
+    expect(screen.getByText(textWithMarkupMatcher('1-1 of 1'))).toBeInTheDocument();
+
+    // Enable realtime updates
+    await userEvent.click(
+      screen.getByRole('button', {name: 'Enable real-time updates'}),
+      {delay: null}
+    );
+
+    jest.advanceTimersByTime(3001);
+    expect(pollRequest).toHaveBeenCalledTimes(1);
+
+    // We mock out the stream group component and only render the ID as a testid
+    await screen.findByTestId('2');
+
+    expect(screen.getByText(textWithMarkupMatcher('1-2 of 2'))).toBeInTheDocument();
+  });
+
   it('stops polling for new issues when endpoint returns a 401', async function () {
     pollRequest = MockApiClient.addMockResponse({
       url: `/api/0/organizations/org-slug/issues/?cursor=${PREVIOUS_PAGE_CURSOR}:0:1`,

+ 3 - 2
static/app/views/issueList/overview.tsx

@@ -727,10 +727,11 @@ class IssueListOverview extends Component<Props, State> {
     }
   };
 
-  onRealtimePoll = (data: any, _links: any) => {
+  onRealtimePoll = (data: any, {queryCount}: {queryCount: number}) => {
     // Note: We do not update state with cursors from polling,
     // `CursorPoller` updates itself with new cursors
     GroupStore.addToFront(data);
+    this.setState({queryCount});
   };
 
   listener = GroupStore.listen(() => this.onGroupChange(), undefined);
@@ -1151,7 +1152,7 @@ class IssueListOverview extends Component<Props, State> {
     // validate that it's correct at the first and last page
     if (!links?.next?.results || this.allResultsVisible()) {
       // On last available page
-      numPreviousIssues = queryCount - groupIds.length;
+      numPreviousIssues = Math.max(queryCount - groupIds.length, 0);
     } else if (!links?.previous?.results) {
       // On first available page
       numPreviousIssues = 0;