Skip to content

feat(seer): Allow bulk-editing Code Review triggers #113116

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

Open
ryan953 wants to merge 4 commits into
base: master
Choose a base branch
from
+250 −67
Open

feat(seer): Allow bulk-editing Code Review triggers #113116

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6eae0f8
feat(seer): Allow bulk-editing Code Review triggers
ryan953 Apr 15, 2026
1e009de
fix selectedIds === all case
ryan953 Apr 15, 2026
4354dfe
fix missing optional chaining operator
ryan953 Apr 15, 2026
85e0a18
falsy length is a better check instead of === 0
ryan953 Apr 15, 2026
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
3 changes: 2 additions & 1 deletion static/app/components/repositories/useBulkUpdateRepositorySettings.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {getRepositoryWithSettingsQueryKey} from 'sentry/components/repositories/useRepositoryWithSettings';
import type {RepositoryWithSettings} from 'sentry/types/integrations';
import type {CodeReviewTrigger} from 'sentry/types/seer';
import {
fetchMutation,
useMutation,
Expand All @@ -20,7 +21,7 @@ type RepositorySettings =
enabledCodeReview?: never;
}
| {
codeReviewTriggers: string[];
Copy link
Copy Markdown
Contributor

@cursor cursor bot Apr 15, 2026

Choose a reason for hiding this comment

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

Second union variant missing CodeReviewTrigger type update

Low Severity

The codeReviewTriggers field in the second union variant of RepositorySettings (line 19) still uses string[], while the third variant (line 24) was updated to CodeReviewTrigger[]. The handleBulkTriggers calls that pass only codeReviewTriggers without enabledCodeReview match the second variant, so they bypass the stricter CodeReviewTrigger type checking that the commit intended to introduce.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 85e0a18. Configure here.

codeReviewTriggers: CodeReviewTrigger[];
enabledCodeReview: boolean;
repositoryIds: string[];
};
Expand Down
54 changes: 28 additions & 26 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 @@ -132,33 +132,34 @@ export function SeerRepoTable() {
isFetchingNextPage,
} = result;

const {mutate: mutateRepositorySettings} = useBulkUpdateRepositorySettings({
onSuccess: mutations => {
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 => {
queryClient.invalidateQueries({
queryKey: getSeerOnboardingCheckQueryOptions({organization}).queryKey,
});
(mutations ?? []).forEach(mutation => {
const {mutate: mutateRepositorySettings, mutateAsync: mutateRepositorySettingsAsync} =
useBulkUpdateRepositorySettings({
onSuccess: mutations => {
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 => {
queryClient.invalidateQueries({
queryKey: getRepositoryWithSettingsQueryKey(organization, mutation.id),
queryKey: getSeerOnboardingCheckQueryOptions({organization}).queryKey,
});
});
},
});
(mutations ?? []).forEach(mutation => {
queryClient.invalidateQueries({
queryKey: getRepositoryWithSettingsQueryKey(organization, mutation.id),
});
});
},
});

const knownIds = useMemo(
() => repositories?.map(repository => repository.id) ?? [],
Expand Down Expand Up @@ -207,8 +208,9 @@ export function SeerRepoTable() {
gridColumns={GRID_COLUMNS}
isFetchingNextPage={isFetchingNextPage}
isPending={isPending}
mutateRepositorySettings={mutateRepositorySettings}
mutateRepositorySettings={mutateRepositorySettingsAsync}
onSortClick={setSort}
repositories={repositories ?? []}
sort={sort}
/>
{isPending ? (
Expand Down
260 changes: 220 additions & 40 deletions static/gsApp/views/seerAutomation/components/repoTable/seerRepoTableHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,23 @@
import {Fragment} from 'react';
import {Fragment, useMemo, useState} from 'react';
import styled from '@emotion/styled';

import {Alert} from '@sentry/scraps/alert';
import {Checkbox} from '@sentry/scraps/checkbox';
import {CompactSelect} from '@sentry/scraps/compactSelect';
import {Flex} from '@sentry/scraps/layout';
import {OverlayTrigger} from '@sentry/scraps/overlayTrigger';

import {addErrorMessage, addSuccessMessage} from 'sentry/actionCreators/indicator';
import {DropdownMenu} from 'sentry/components/dropdownMenu';
import {
addErrorMessage,
addLoadingMessage,
addSuccessMessage,
} from 'sentry/actionCreators/indicator';
import {QuestionTooltip} from 'sentry/components/questionTooltip';
import type {useBulkUpdateRepositorySettings} from 'sentry/components/repositories/useBulkUpdateRepositorySettings';
import {SimpleTable} from 'sentry/components/tables/simpleTable';
import {t, tct, tn} from 'sentry/locale';
import type {RepositoryWithSettings} from 'sentry/types/integrations';
import type {CodeReviewTrigger} from 'sentry/types/seer';
import {parseQueryKey} from 'sentry/utils/api/apiQueryKey';
import type {Sort} from 'sentry/utils/discover/fields';
import {useListItemCheckboxContext} from 'sentry/utils/list/useListItemCheckboxState';
Expand All @@ -21,8 +28,11 @@ interface Props {
gridColumns: string;
isFetchingNextPage: boolean;
isPending: boolean;
mutateRepositorySettings: ReturnType<typeof useBulkUpdateRepositorySettings>['mutate'];
mutateRepositorySettings: ReturnType<
typeof useBulkUpdateRepositorySettings
>['mutateAsync'];
onSortClick: (key: Sort) => void;
repositories: RepositoryWithSettings[];
sort: Sort;
}

Expand Down Expand Up @@ -53,6 +63,7 @@ export function SeerRepoTableHeader({
isPending,
mutateRepositorySettings,
onSortClick,
repositories,
sort,
}: Props) {
const canWrite = useCanWriteSettings();
Expand All @@ -71,34 +82,163 @@ export function SeerRepoTableHeader({
: undefined;
const queryString = queryOptions?.query?.query;

const handleBulkCodeReview = (enabledCodeReview: boolean) => {
const selectedRepos = useMemo(() => {
if (selectedIds === 'all') {
return repositories;
}
return repositories.filter(repo => selectedIds.includes(repo.id));
}, [repositories, selectedIds]);
Comment thread
sentry[bot] marked this conversation as resolved.
Comment thread
cursor[bot] marked this conversation as resolved.

const currentCodeReviewValue = useMemo(() => {
const someEnabled = selectedRepos.some(repo => repo?.settings?.enabledCodeReview);
const someDisabled = selectedRepos.some(
repo => repo?.settings?.enabledCodeReview === false
);
if (someEnabled && someDisabled) {
return undefined;
}
if (someEnabled) {
return 'enabled_code_review:enabled';
}
if (someDisabled) {
return 'enabled_code_review:disabled';
}
return undefined;
}, [selectedRepos]);

const currentTriggersValue = useMemo((): CodeReviewTrigger[] => {
const someOnReadyForReview = selectedRepos.every(repo =>
repo?.settings?.codeReviewTriggers?.includes('on_ready_for_review')
);
const someOnNewCommit = selectedRepos.every(repo =>
repo?.settings?.codeReviewTriggers?.includes('on_new_commit')
);
Copy link
Copy Markdown
Contributor

@cursor cursor bot Apr 15, 2026

Choose a reason for hiding this comment

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

Variables named some... actually use every() semantics

Low Severity

The variables someOnReadyForReview and someOnNewCommit are named with the prefix some but actually use .every(), which checks if ALL selected repos have the trigger. The naming directly contradicts the semantics — .some() returns true if any element matches, while .every() returns true only if all match. The every logic is correct for this feature (only show a trigger as selected when all repos have it), but the misleading names could cause a future developer to misunderstand the behavior.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 85e0a18. Configure here.

return [
Comment on lines +110 to +116
Copy link
Copy Markdown
Contributor

@sentry sentry bot Apr 15, 2026

Choose a reason for hiding this comment

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

Bug: The currentTriggersValue calculation using .every() on an empty selectedRepos array results in a "vacuous truth", incorrectly populating the bulk edit trigger state during transient renders.
Severity: LOW

Suggested Fix

Add a guard at the beginning of the useMemo hook for currentTriggersValue. If selectedRepos.length === 0, return an empty array [] to prevent the .every() logic from executing on an empty array and causing a vacuous truth.

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/seerRepoTableHeader.tsx#L110-L116

Potential issue: The `currentTriggersValue` is calculated using
`Array.prototype.every()` on the `selectedRepos` array. When `selectedRepos` is empty,
`.every()` returns `true` due to vacuous truth. This can occur in a transient state
during a query change where selections exist (`isAnySelected` is true) but the filtered
repository list (`selectedRepos`) is temporarily empty. This results in the UI
incorrectly showing all bulk edit triggers as selected. Interacting with this incorrect
state can cause incorrect bulk update requests to be sent to the server.

...(someOnReadyForReview ? ['on_ready_for_review' as const] : []),
...(someOnNewCommit ? ['on_new_commit' as const] : []),
];
}, [selectedRepos]);

const [isBulkUpdating, setIsBulkUpdating] = useState(false);

const handleBulkCodeReview = async (enabledCodeReview: boolean) => {
const repositoryIds = selectedIds === 'all' ? knownIds : selectedIds;
mutateRepositorySettings(
{
enabledCodeReview,
repositoryIds,
},
{
onError: () => {
addErrorMessage(
tn(
'Failed to update code review for %s repository',
'Failed to update code review for %s repositories',
repositoryIds.length
)
);
},
onSuccess: () => {
addSuccessMessage(
tn(
'Code review updated for %s repository',
'Code review updated for %s repositories',
repositoryIds.length
)
);
},
}
setIsBulkUpdating(true);
addLoadingMessage(
tn(
'Updating code review for %s repository…',
'Updating code review for %s repositories…',
repositoryIds.length
)
);
try {
await mutateRepositorySettings({enabledCodeReview, repositoryIds});
addSuccessMessage(
tn(
'Code review updated for %s repository',
'Code review updated for %s repositories',
repositoryIds.length
)
);
} catch {
addErrorMessage(
tn(
'Failed to update code review for %s repository',
'Failed to update code review for %s repositories',
repositoryIds.length
)
);
} finally {
setIsBulkUpdating(false);
}
};

const handleBulkTriggers = async ({
added,
removed,
}: {
added: CodeReviewTrigger | undefined;
removed: CodeReviewTrigger | undefined;
}) => {
const promises: Array<Promise<unknown>> = [];

if (added) {
const repoIdsWithZeroTriggers: string[] = [];
const repoIdsWithOneTrigger: string[] = [];
for (const repo of selectedRepos) {
if (!repo.settings?.codeReviewTriggers?.length) {
repoIdsWithZeroTriggers.push(repo.id);
} else if (!repo.settings?.codeReviewTriggers?.includes(added)) {
repoIdsWithOneTrigger.push(repo.id);
}
Comment thread
cursor[bot] marked this conversation as resolved.
}
// Some items start with 0 triggers, they'll be saved with 1 new trigger
if (repoIdsWithZeroTriggers.length > 0) {
promises.push(
mutateRepositorySettings({
codeReviewTriggers: [added],
repositoryIds: repoIdsWithZeroTriggers,
})
);
}
// Some items start with 1 trigger, they'll be saved with 1 new trigger for a total of 2
if (repoIdsWithOneTrigger.length > 0) {
promises.push(
mutateRepositorySettings({
codeReviewTriggers: ['on_new_commit', 'on_ready_for_review'],
repositoryIds: repoIdsWithOneTrigger,
})
);
}
}
if (removed) {
const repoIdsWithOneTrigger: string[] = [];
const repoIdsWithTwoTriggers: string[] = [];
for (const repo of selectedRepos) {
if (repo.settings?.codeReviewTriggers?.length === 2) {
repoIdsWithTwoTriggers.push(repo.id);
} else if (repo.settings?.codeReviewTriggers?.includes(removed)) {
repoIdsWithOneTrigger.push(repo.id);
}
}
// Some items start with 2 triggers, we'll remove one
const remainingTrigger =
removed === 'on_new_commit' ? 'on_ready_for_review' : 'on_new_commit';
if (repoIdsWithTwoTriggers.length > 0) {
promises.push(
mutateRepositorySettings({
codeReviewTriggers: [remainingTrigger],
repositoryIds: repoIdsWithTwoTriggers,
})
);
}
// Some items start with 1 trigger, we'll remove it
if (repoIdsWithOneTrigger.length > 0) {
promises.push(
mutateRepositorySettings({
codeReviewTriggers: [],
repositoryIds: repoIdsWithOneTrigger,
})
);
}
}

if (promises.length === 0) {
return;
}

setIsBulkUpdating(true);
addLoadingMessage(t('Updating triggers…'));

const results = await Promise.allSettled(promises);
const hasError = results.some(r => r.status === 'rejected');
setIsBulkUpdating(false);

if (hasError) {
addErrorMessage(t('Failed to update triggers'));
} else {
addSuccessMessage(t('Triggers updated'));
}
};

return (
Expand Down Expand Up @@ -147,22 +287,62 @@ export function SeerRepoTableHeader({
/>
</TableCellFirst>
<TableCellsRemainingContent align="center" gap="md">
<DropdownMenu
isDisabled={!canWrite}
<CompactSelect
disabled={!canWrite}
size="xs"
trigger={props => (
<OverlayTrigger.Button {...props}>
{t('Code Review')}
</OverlayTrigger.Button>
)}
options={[
{
value: 'enabled_code_review:enabled',
label: t('Enable'),
disabled: isBulkUpdating,
},
{
value: 'enabled_code_review:disabled',
label: t('Disable'),
disabled: isBulkUpdating,
},
]}
value={currentCodeReviewValue}
onChange={option => {
if (option.value === 'enabled_code_review:enabled') {
handleBulkCodeReview(true);
} else {
handleBulkCodeReview(false);
}
}}
/>

<CompactSelect<CodeReviewTrigger>
disabled={!canWrite}
multiple
size="xs"
items={[
trigger={props => (
<OverlayTrigger.Button {...props}>{t('Triggers')}</OverlayTrigger.Button>
)}
options={[
{
key: 'on',
label: t('On'),
onAction: () => handleBulkCodeReview(true),
value: 'on_ready_for_review',
label: t('On Ready for Review'),
disabled: isBulkUpdating,
},
{
key: 'off',
label: t('Off'),
onAction: () => handleBulkCodeReview(false),
value: 'on_new_commit',
label: t('On New Commit'),
disabled: isBulkUpdating,
},
]}
triggerLabel={t('Code Review')}
value={currentTriggersValue}
onChange={option => {
const value = option.map(v => v.value);
const added = value.findLast(v => !currentTriggersValue.includes(v));
const removed = currentTriggersValue.findLast(v => !value.includes(v));
handleBulkTriggers({added, removed});
}}
/>
</TableCellsRemainingContent>
</TableHeader>
Expand Down
Loading