Skip to content

ref(seer): Refactor Code Review table to better optimistically update #112979

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.

Sign up for GitHub

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

Merged
ryan953 merged 1 commit into from
Apr 15, 2026
+16 −36
Merged

ref(seer): Refactor Code Review table to better optimistically update #112979

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 14 additions & 16 deletions static/gsApp/views/seerAutomation/components/repoTable/seerRepoTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,18 +107,20 @@ export function SeerRepoTable() {
isFetchingNextPage,
} = result;

const [mutationData, setMutations] = useState<Record<string, RepositoryWithSettings>>(
{}
);

const {mutate: mutateRepositorySettings} = useBulkUpdateRepositorySettings({
onSuccess: mutations => {
setMutations(prev => {
const updated = {...prev};
mutations.forEach(mutation => {
updated[mutation.id] = mutation;
});
return updated;
const mutationMap = new Map(mutations.map(m => [m.id, m]));
queryClient.setQueryData(queryOptions.queryKey, prev => {
if (!prev) {
return prev;
}
return {
...prev,
pages: prev.pages.map(page => ({
...page,
json: page.json.map(repo => mutationMap.get(repo.id) ?? repo),
})),
};
});
},
onSettled: mutations => {
Expand Down Expand Up @@ -192,10 +194,10 @@ export function SeerRepoTable() {
<TablePanel>
<SeerRepoTableHeader
gridColumns={GRID_COLUMNS}
isFetchingNextPage={isFetchingNextPage}
isPending={isPending}
mutateRepositorySettings={mutateRepositorySettings}
onSortClick={setSort}
isPending={isPending}
isFetchingNextPage={isFetchingNextPage}
sort={sort}
/>
{isPending ? (
Expand All @@ -221,7 +223,6 @@ export function SeerRepoTable() {
hasNextPage={hasNextPage}
isFetchingNextPage={isFetchingNextPage}
mutateRepositorySettings={mutateRepositorySettings}
mutationData={mutationData}
repositories={repositories}
scrollBodyRef={scrollBodyRef}
/>
Expand All @@ -236,14 +237,12 @@ function VirtualizedRepoTable({
hasNextPage,
isFetchingNextPage,
mutateRepositorySettings,
mutationData,
repositories,
scrollBodyRef,
}: {
hasNextPage: boolean;
isFetchingNextPage: boolean;
mutateRepositorySettings: ReturnType<typeof useBulkUpdateRepositorySettings>['mutate'];
mutationData: Record<string, RepositoryWithSettings>;
repositories: RepositoryWithSettings[];
scrollBodyRef: React.RefObject<HTMLDivElement | null>;
}) {
Expand Down Expand Up @@ -296,7 +295,6 @@ function VirtualizedRepoTable({
gridColumns={GRID_COLUMNS}
style={{transform: `translateY(${virtualItem.start}px)`}}
mutateRepositorySettings={mutateRepositorySettings}
mutationData={mutationData}
repository={repository}
/>
);
Expand Down
22 changes: 2 additions & 20 deletions static/gsApp/views/seerAutomation/components/repoTable/seerRepoTableRow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,7 @@ import {
} from 'sentry/actionCreators/indicator';
import {getRepoStatusLabel} from 'sentry/components/repositories/getRepoStatusLabel';
import {useBulkUpdateRepositorySettings} from 'sentry/components/repositories/useBulkUpdateRepositorySettings';
import {
getRepositoryWithSettingsQueryKey,
useRepositoryWithSettings,
} from 'sentry/components/repositories/useRepositoryWithSettings';
import {getRepositoryWithSettingsQueryKey} from 'sentry/components/repositories/useRepositoryWithSettings';
import {SimpleTable} from 'sentry/components/tables/simpleTable';
import {IconOpen} from 'sentry/icons/iconOpen';
import {t} from 'sentry/locale';
Expand All @@ -35,36 +32,21 @@ import {useCanWriteSettings} from 'getsentry/views/seerAutomation/components/use
interface Props {
gridColumns: string;
mutateRepositorySettings: ReturnType<typeof useBulkUpdateRepositorySettings>['mutate'];
mutationData: Record<string, RepositoryWithSettings>;
repository: RepositoryWithSettings;
style?: React.CSSProperties;
}

export function SeerRepoTableRow({
gridColumns,
mutateRepositorySettings,
mutationData,
repository: initialRepository,
repository,
style,
}: Props) {
const queryClient = useQueryClient();
const organization = useOrganization();
const canWrite = useCanWriteSettings();
const {isSelected, toggleSelected} = useListItemCheckboxContext();

// We're defaulting to read from the list data, which is passed in as `initialRepository`
// But if an update has been made to one record, we're going to enable reading
// from `useRepositoryWithSettings` which will have the latest data, letting us
// do optimistic updates, without re-rendering the entire table.
// `initialRepository` will become stale at that point, but we'll have fresh data
// in the cache to override it.
const {data, isError, isPending} = useRepositoryWithSettings({
repositoryId: initialRepository.id,
initialData: [initialRepository, undefined, undefined],
enabled: mutationData[initialRepository.id] !== undefined,
});
const repository = isError || isPending ? initialRepository : data;

return (
<Grid
columns={gridColumns}
Comment on lines +42 to 52
Copy link
Copy Markdown
Contributor

@sentry sentry bot Apr 14, 2026

Choose a reason for hiding this comment

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

Bug: When a repository setting toggle fails, the UI doesn't revert. The onError handler updates an unused query cache instead of the infinite query cache driving the UI.
Severity: MEDIUM

Suggested Fix

The onError handler should be updated to roll back the state of the correct query cache. Instead of updating the individual repository cache, it should find the specific repository within the infinite query's cached data and revert its state there. This will ensure the UI, which reads from the infinite query, reflects the rollback immediately upon failure.

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/seerRepoTableRow.tsx#L32-L52

Potential issue: When a user toggles a repository setting and the backend mutation
fails, the `Switch` component does not visually revert to its previous state. The
`onError` handler attempts a rollback by calling `queryClient.setQueryData` on an
individual repository's query cache. However, the UI's state is derived from a parent
component's infinite query cache. Since the error handler only updates the individual
cache, the UI remains in the incorrect, optimistically toggled state until the infinite
query is invalidated and refetched. This presents a temporarily misleading state to the
user.

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Copy Markdown
Member Author

@ryan953 ryan953 Apr 15, 2026

Choose a reason for hiding this comment

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

fixing this will conflict with #112926

I'll revisit once 112926 is in

Expand Down
Loading