Skip to content

Commit 71408f1

Browse files
JonasBaclaude
andcommitted
fix(cmdk): Filter out async resource actions with 0 results
Resource actions (those with a `resource` prop) are async group containers — they only make sense when they have children. When an async resource returns no results, the parent node should not appear in the list. Previously the browse-mode filter only ran on top-level nodes, so a resource action nested inside a group would still be pushed to the results list. The search-mode path had no guard at all, so a resource action whose label matched the query would show up as a (non-functional) clickable item. Fix both paths and add tests for top-level, nested, and search-mode variants of the 0-results case. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent cd2aa33 commit 71408f1

File tree

2 files changed

+95
-0
lines changed

2 files changed

+95
-0
lines changed

static/app/components/commandPalette/ui/commandPalette.spec.tsx

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ jest.mock('@tanstack/react-virtual', () => ({
2424

2525
import {closeModal} from 'sentry/actionCreators/modal';
2626
import * as modalActions from 'sentry/actionCreators/modal';
27+
import type {CommandPaletteAction} from 'sentry/components/commandPalette/types';
2728
import {CommandPaletteProvider} from 'sentry/components/commandPalette/ui/cmdk';
2829
import {CMDKAction} from 'sentry/components/commandPalette/ui/cmdk';
2930
import type {CMDKActionData} from 'sentry/components/commandPalette/ui/cmdk';
@@ -647,4 +648,84 @@ describe('CommandPalette', () => {
647648
expect(screen.getAllByRole('option')).toHaveLength(1);
648649
});
649650
});
651+
652+
describe('resource action with 0 results', () => {
653+
function emptyResource() {
654+
return {
655+
queryKey: ['test-empty-resource'] as const,
656+
queryFn: (): CommandPaletteAction[] => [],
657+
};
658+
}
659+
660+
it('is omitted from browse mode at the top level', async () => {
661+
render(
662+
<CommandPaletteProvider>
663+
<CMDKAction display={{label: 'Async Resource'}} resource={emptyResource}>
664+
{data =>
665+
data.map((_, i) => (
666+
<CMDKAction key={i} to="/x/" display={{label: 'Result'}} />
667+
))
668+
}
669+
</CMDKAction>
670+
<CMDKAction display={{label: 'Real Action'}} onAction={jest.fn()} />
671+
<CommandPalette onAction={jest.fn()} />
672+
</CommandPaletteProvider>
673+
);
674+
675+
await screen.findByRole('option', {name: 'Real Action'});
676+
expect(
677+
screen.queryByRole('option', {name: 'Async Resource'})
678+
).not.toBeInTheDocument();
679+
});
680+
681+
it('is omitted from browse mode when nested inside a group', async () => {
682+
render(
683+
<CommandPaletteProvider>
684+
<CMDKAction display={{label: 'Group'}}>
685+
<CMDKAction display={{label: 'Async Resource'}} resource={emptyResource}>
686+
{data =>
687+
data.map((_, i) => (
688+
<CMDKAction key={i} to="/x/" display={{label: 'Result'}} />
689+
))
690+
}
691+
</CMDKAction>
692+
<CMDKAction display={{label: 'Real Action'}} onAction={jest.fn()} />
693+
</CMDKAction>
694+
<CommandPalette onAction={jest.fn()} />
695+
</CommandPaletteProvider>
696+
);
697+
698+
await screen.findByRole('option', {name: 'Real Action'});
699+
expect(
700+
screen.queryByRole('option', {name: 'Async Resource'})
701+
).not.toBeInTheDocument();
702+
});
703+
704+
it('is omitted from search mode even when the label matches the query', async () => {
705+
render(
706+
<CommandPaletteProvider>
707+
<CMDKAction display={{label: 'Async Resource'}} resource={emptyResource}>
708+
{data =>
709+
data.map((_, i) => (
710+
<CMDKAction key={i} to="/x/" display={{label: 'Result'}} />
711+
))
712+
}
713+
</CMDKAction>
714+
<CMDKAction display={{label: 'Other'}} onAction={jest.fn()} />
715+
<CommandPalette onAction={jest.fn()} />
716+
</CommandPaletteProvider>
717+
);
718+
719+
const input = await screen.findByRole('textbox', {name: 'Search commands'});
720+
await userEvent.type(input, 'async');
721+
722+
// Wait for search to take effect — 'Other' should be filtered out since it doesn't match
723+
await waitFor(() => {
724+
expect(screen.queryByRole('option', {name: 'Other'})).not.toBeInTheDocument();
725+
});
726+
expect(
727+
screen.queryByRole('option', {name: 'Async Resource'})
728+
).not.toBeInTheDocument();
729+
});
730+
});
650731
});

static/app/components/commandPalette/ui/commandPalette.tsx

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -480,6 +480,15 @@ function flattenActions(
480480
results.push({...node, listItemType: isGroup ? 'section' : 'action'});
481481
if (isGroup) {
482482
for (const child of node.children) {
483+
const childIsGroup = child.children.length > 0;
484+
if (
485+
!childIsGroup &&
486+
'resource' in child &&
487+
!('to' in child) &&
488+
!('onAction' in child)
489+
) {
490+
continue;
491+
}
483492
results.push({...child, listItemType: 'action'});
484493
}
485494
}
@@ -530,6 +539,11 @@ function flattenActions(
530539
.map(c => ({...c, listItemType: 'action' as const})),
531540
];
532541
}
542+
// Skip resource nodes with no children — they are async group containers that
543+
// returned 0 results and have no executable action of their own.
544+
if ('resource' in item && !('to' in item) && !('onAction' in item)) {
545+
return [];
546+
}
533547
return scores.get(item.key)?.score.matched ? [{...item, listItemType: 'action'}] : [];
534548
});
535549

0 commit comments

Comments
 (0)