ref(grouplist): Refactor GroupList to use useApiQuery #105688
Conversation
malwilley
commented
Jan 5, 2026
malwilley
commented
- Fixes a bug I was encountering due to GroupStore
- Gets us a tiny bit closer to removing GroupStore
- GroupList responses are now cached
- Testing components with GroupList is simpler now since you don't need to specify the URL params in the mock
github-actions
bot
added
the
Scope: Frontend
| it('renders custom error when query has boolean logic', async () => { | ||
| const renderErrorMessage = jest.fn(() => <div>custom error</div>); | ||
| const issuesRequest = MockApiClient.addMockResponse({ | ||
| url: `${issuesUrl}?${qs.stringify({...defaultQueryParams, query: 'issue.id:1'})}`, |
There was a problem hiding this comment.
Most of the file changes here are simplifying the mocks.
/organizations/org-slug/issues/?query=foo&cursor=bar -> /organizations/org-slug/issues/
| * If you have access to the group data, it is preferred to pass it in as a prop here. | ||
| * Otherwise, the group data will come from the deprecated GroupStore. | ||
| */ | ||
| group?: Group; |
There was a problem hiding this comment.
Needed to modify StreamGroup since it was looking directly at the store. This prop will help us get off of GroupStore if we add it to all the existing usages
There was a problem hiding this comment.
This is a really good call, I wish there was a way in TS to mark it as 'will-be-required-in-the-future', or 'deprecated-to-omit'
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| query: 'error.unhandled:true is:unresolved', | ||
| }), | ||
| }) | ||
| ); |
There was a problem hiding this comment.
Test assertion verifies wrong query after clicking "New Issues"
The test clicks on "New Issues" segment but then asserts that the API was called with query: 'error.unhandled:true is:unresolved', which is the IssuesQuery.UNHANDLED value. After clicking "New Issues", the expected query should be 'is:unresolved is:for_review' (IssuesQuery.NEW). Since toHaveBeenCalledWith checks if any call matched, this assertion passes due to the initial render (which uses UNHANDLED by default), not because clicking "New Issues" actually works. The test doesn't verify the intended behavior.
|
note: we talked about how assignee selector would work in person. Worth double checking that assignee isn't putting that into only group store. |
| * If you have access to the group data, it is preferred to pass it in as a prop here. | ||
| * Otherwise, the group data will come from the deprecated GroupStore. | ||
| */ | ||
| group?: Group; |
There was a problem hiding this comment.
This is a really good call, I wish there was a way in TS to mark it as 'will-be-required-in-the-future', or 'deprecated-to-omit'
static/app/views/issueDetails/groupRelatedIssues/index.spec.tsx
Outdated
Show resolved
Hide resolved
| }); | ||
|
|
||
| beforeEach(() => { | ||
| // NOTE: This mock is jank. `GroupList` concatenates the query string with the URL. This means we have to mock the full URL including the parameters. There are a few other tests that have to do the same. |
