feat(seer): Implement bulk editing for repo code-review settings#104908
feat(seer): Implement bulk editing for repo code-review settings#104908
Conversation
github-actions
bot
added
the
Scope: Frontend
| onSuccess: mutations => { | ||
| setMutations(prev => { | ||
| return mutations.reduce( | ||
| (map, mutation) => ({...map, [mutation.id]: mutation}), |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| return updated; | ||
| }); | ||
| }, | ||
| onSettled: mutations => { | ||
| (mutations ?? []).forEach(mutation => { | ||
| queryClient.invalidateQueries({ | ||
| queryKey: getRepositoryWithSettingsQueryKey(organization, mutation.id), | ||
| }); | ||
| }); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Bug: The mutationData state is not cleared after a mutation, causing mutated repository rows to permanently and unnecessarily refetch data from the API on re-renders or window focus.
Severity: MEDIUM | Confidence: High
🔍 Detailed Analysis
After a repository's settings are mutated, an entry is added to the mutationData state object but is never removed. The onSuccess callback adds the entry, which correctly triggers a refetch by enabling the useRepositoryWithSettings hook. However, the onSettled callback, which runs after the mutation, only invalidates the query cache and fails to clear the corresponding entry from mutationData. Because the hook's enabled condition (mutationData[initialRepository.id] !== undefined) remains permanently true for that repository, it will continue to refetch from the API on every component re-render or window refocus for the remainder of the component's lifecycle, creating unnecessary network requests.
💡 Suggested Fix
Update the onSettled callback in the mutation to remove the entry from the mutationData state after the query has been invalidated. For example: setMutations(prev => {const {[mutation.id]: _, ...rest} = prev; return rest;}).
🤖 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/gsApp/views/seerAutomation/components/repoTable/seerRepoTable.tsx#L49-L70
Potential issue: After a repository's settings are mutated, an entry is added to the
`mutationData` state object but is never removed. The `onSuccess` callback adds the
entry, which correctly triggers a refetch by enabling the `useRepositoryWithSettings`
hook. However, the `onSettled` callback, which runs after the mutation, only invalidates
the query cache and fails to clear the corresponding entry from `mutationData`. Because
the hook's `enabled` condition (`mutationData[initialRepository.id] !== undefined`)
remains permanently true for that repository, it will continue to refetch from the API
on every component re-render or window refocus for the remainder of the component's
lifecycle, creating unnecessary network requests.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 7474880
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Bug: Accessing `.length` on union type without type guard
The condition selectedIds.length === repositories.length accesses .length on selectedIds which has type 'all' | string[]. When selectedIds === 'all', this returns 3 (string length) rather than the count of selected items. Currently this is protected by short-circuit evaluation with isAllSelected === true being checked first, but the code is fragile—if conditions are reordered or the relationship between selectedIds and isAllSelected changes, incorrect behavior would occur. Following the pattern used elsewhere in the codebase (e.g., useBulkEditFeedbacks.tsx), the code could explicitly check Array.isArray(selectedIds) before accessing .length.
static/gsApp/views/seerAutomation/components/repoTable/seerRepoTableHeader.tsx#L195-L196
| // TODO: we should not be overriding the existing code review triggers for the repositories | ||
| codeReviewTriggers: DEFAULT_CODE_REVIEW_TRIGGERS, |
…ings (#104912) Followup to #104908 Depends on #104918 Currently the backend expects that all fields will be passed to the bulk-edit endpoint, but that's only true in the wizard context, when we're creating everything for the first time: https://github.com/getsentry/sentry/blob/master/src/sentry/integrations/api/endpoints/organization_repository_settings.py#L16-L41 On the settings page people can bulk-update fields, one field at a time.
2 participants
This is a followup to #104886 which adds some complexity, but hoists some state up into the main Table component to share it better with each row.
Now we can bulk-edit items in the list in addition to setting things specifically.
Doing a bulk-edit will result in re-fetches for each item in the list. So if someone clicks to select all items it could be crazy as we re-fetch everything :(
For bulk editing a few items this works great. I don't expect people will be hammering this page with changes, so a little jank is going to be ok to start with.
Screen.Recording.2025-12-12.at.3.24.21.PM.mov