Skip to content
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.

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

fix(issue-stream): Wait until stats request has finished before saving groups into cache #83073

Merged
merged 2 commits into from
Jan 8, 2025

Conversation

malwilley
Copy link
Member

Fixes a bug in the refactored component where navigating to an issue and going back to the stream would never load the stats. This is because I started caching when the request completed instead of when the component unmounted, but never waited for the stats request to complete.

  • Makes request async and use requestPromise instead of request
  • Wait for request to finish before caching
  • Use api.clear() instead of lastRequestRef.current.cancel()

@malwilley malwilley requested a review from a team as a code owner January 8, 2025 00:12
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jan 8, 2025

await screen.findByRole('grid', {name: 'Create a search query'});
await userEvent.click(getSearchInput());
await userEvent.keyboard('foo{enter}');

expect(
await screen.findByText(/We couldn't find any issues that matched your filters/i)
Copy link
Member Author

Choose a reason for hiding this comment

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

This test was actually just wrong before, these changes made it start failing because it should show "No issues match your search" instead

const {id} = data[0];
redirect = `/organizations/${organization.slug}/issues/${id}/`;
}
api.request(`/organizations/${organization.slug}/issues/`, {
Copy link
Member Author

Choose a reason for hiding this comment

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

Best to view this diff with "hide whitespace" because there are only a few actual changes here

@@ -704,14 +672,6 @@ function IssueListOverviewFc({
// reload data.
if (!isEqual(previousRequestParams, requestParams)) {
fetchData(selectionChanged);
} else if (
Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think this is necessary anymore, it should react to the URL changing now

@malwilley malwilley merged commit 623e48a into master Jan 8, 2025
42 checks passed
@malwilley malwilley deleted the malwilley/fix/issue-stream-overview-cache-on-back branch January 8, 2025 17:16
Copy link

sentry-io bot commented Jan 8, 2025

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ **AbortError: The operation was aborted. ** /issues View Issue

Did you find this useful? React with a 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants