Skip to content

Commit 1763fa2

Browse files
scttcperclaude
andcommitted
fix(ownership): Handle ownership rule owners without an id
The backend can return ownership rule owners with just a name and type but no id — this happens with unresolved codeowner entries. The frontend was treating `id` as always present which broke avatar rendering and the "my teams" filter. Make `id` optional on `OwnershipRuleOwner`, skip avatar stack rendering for owners missing an id, and filter them out of the owner selector. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 7860532 commit 1763fa2

File tree

3 files changed

+56
-8
lines changed

3 files changed

+56
-8
lines changed

static/app/types/group.tsx

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -523,9 +523,18 @@ type SuggestedOwner = {
523523
type: SuggestedOwnerReason;
524524
};
525525

526+
/**
527+
* Mirrors OwnershipRuleOwnerResponse from the backend
528+
*/
529+
interface OwnershipRuleOwner {
530+
name: string;
531+
type: 'user' | 'team';
532+
id?: string;
533+
}
534+
526535
export interface ParsedOwnershipRule {
527536
matcher: {pattern: string; type: string};
528-
owners: Actor[];
537+
owners: OwnershipRuleOwner[];
529538
}
530539

531540
export type IssueOwnership = {

static/app/views/settings/project/projectOwnership/ownershipRulesTable.spec.tsx

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,32 @@ describe('OwnershipRulesTable', () => {
168168
expect(screen.getByRole('button', {name: 'Everyone'})).toBeInTheDocument();
169169
});
170170

171+
it('should render owners without an id', async () => {
172+
const rules: ParsedOwnershipRule[] = [
173+
{
174+
matcher: {pattern: 'src/app/*', type: 'path'},
175+
owners: [{type: 'user', name: 'unresolved-user@example.com'}],
176+
},
177+
{
178+
matcher: {pattern: 'src/utils/*', type: 'path'},
179+
owners: [
180+
{type: 'user', id: user1.id, name: user1.name},
181+
{type: 'team', name: 'backend'},
182+
],
183+
},
184+
];
185+
186+
render(<OwnershipRulesTable projectRules={rules} codeowners={[]} />);
187+
188+
// Clear the "My Teams" filter to see all rules
189+
await userEvent.click(screen.getByRole('button', {name: 'My Teams'}));
190+
await userEvent.click(screen.getByRole('button', {name: 'Clear'}));
191+
192+
expect(screen.getByText('src/app/*')).toBeInTheDocument();
193+
expect(screen.getByText('unresolved-user@example.com')).toBeInTheDocument();
194+
expect(screen.getByText('src/utils/*')).toBeInTheDocument();
195+
});
196+
171197
it('should paginate results', async () => {
172198
const owners: Actor[] = [{type: 'user', id: user1.id, name: user1.name}];
173199
const rules: ParsedOwnershipRule[] = new Array(100).fill(0).map((_, i) => ({

static/app/views/settings/project/projectOwnership/ownershipRulesTable.tsx

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,10 @@ import {IconChevron} from 'sentry/icons';
1616
import {t, tn} from 'sentry/locale';
1717
import {MemberListStore} from 'sentry/stores/memberListStore';
1818
import {TeamStore} from 'sentry/stores/teamStore';
19+
import type {Actor} from 'sentry/types/core';
1920
import type {ParsedOwnershipRule} from 'sentry/types/group';
2021
import type {CodeOwner} from 'sentry/types/integrations';
22+
import {defined} from 'sentry/utils';
2123
import {useTeams} from 'sentry/utils/useTeams';
2224
import {useUser} from 'sentry/utils/useUser';
2325
import {OwnershipOwnerFilter} from 'sentry/views/settings/project/projectOwnership/ownershipOwnerFilter';
@@ -84,6 +86,10 @@ export function OwnershipRulesTable({
8486
const myTeams = useMemo(() => {
8587
const memberTeamsIds = teams.filter(team => team.isMember).map(team => team.id);
8688
return allActors.filter(actor => {
89+
if (!defined(actor.id)) {
90+
return false;
91+
}
92+
8793
if (actor.type === 'user') {
8894
return actor.id === user.id;
8995
}
@@ -140,7 +146,7 @@ export function OwnershipRulesTable({
140146
<RulesTableWrapper data-test-id="ownership-rules-table">
141147
<Flex align="center" gap="xl">
142148
<OwnershipOwnerFilter
143-
actors={allActors}
149+
actors={allActors.filter((actor): actor is Actor => defined(actor.id))}
144150
selectedTeams={selectedActors ?? []}
145151
handleChangeFilter={handleChangeFilter}
146152
isMyTeams={
@@ -166,7 +172,11 @@ export function OwnershipRulesTable({
166172
emptyMessage={t('No ownership rules found')}
167173
>
168174
{chunkedRules[page]?.map((rule, index) => {
175+
const hasUnknownOwners = rule.owners.some(owner => !defined(owner.id));
169176
const ownerNames = rule.owners.map(owner => {
177+
if (!owner.id) {
178+
return owner.name;
179+
}
170180
if (owner.type === 'team') {
171181
const team = TeamStore.getById(owner.id);
172182
return team?.slug ? `#${team.slug}` : owner.name;
@@ -185,12 +195,15 @@ export function OwnershipRulesTable({
185195
<RowRule>{rule.matcher.pattern}</RowRule>
186196
<Flex align="center" gap="md">
187197
<AvatarContainer numAvatars={Math.min(rule.owners.length, 3)}>
188-
<SuggestedAvatarStack
189-
owners={rule.owners}
190-
suggested={false}
191-
reverse={false}
192-
tooltip={ownerNames.join(', ')}
193-
/>
198+
{/* Avoid attempting to render the avatar stack if there are broken owners */}
199+
{!hasUnknownOwners && (
200+
<SuggestedAvatarStack
201+
owners={rule.owners as Actor[]}
202+
suggested={false}
203+
reverse={false}
204+
tooltip={ownerNames.join(', ')}
205+
/>
206+
)}
194207
</AvatarContainer>
195208
{name}
196209
{rule.owners.length > 1 &&

0 commit comments

Comments
 (0)