fix(frontend): sprint 3 post-review — real dedup test + Apply 0 guard + Link stopPropagation
- MitreTechniquesField test: rewrite dedup test to actually exercise picker selection path — types query, waits for option, fires pointerDown, asserts no PATCH sent (dedup guard in handleSelect now truly covered) - MitreMatrixModal: Apply button disabled only when totalSelected === 0 AND initialSelection.length === 0 (no-op case); when totalSelected === 0 but initialSelection was non-empty, shows "Clear all" and stays enabled so user can explicitly wipe the list - MitreMatrixModal tests: update disabled test to match "Clear all" label, add "Clear all" enabled + onApply([]) path test - SimulationList: stopPropagation on Name <Link> to prevent double-navigate with row onClick handler Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -333,9 +333,11 @@ export function MitreMatrixModal({
|
||||
type="button"
|
||||
className="btn-primary"
|
||||
onClick={handleApply}
|
||||
disabled={isLoading || isError}
|
||||
disabled={isLoading || isError || (totalSelected === 0 && initialSelection.length === 0)}
|
||||
>
|
||||
Apply {totalSelected > 0 ? `${totalSelected} technique${totalSelected !== 1 ? 's' : ''}` : ''}
|
||||
{totalSelected === 0
|
||||
? 'Clear all'
|
||||
: `Apply ${totalSelected} technique${totalSelected !== 1 ? 's' : ''}`}
|
||||
</button>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
@@ -90,6 +90,7 @@ export function SimulationList({ engagementId }: SimulationListProps): JSX.Eleme
|
||||
<Link
|
||||
to={`/engagements/${engagementId}/simulations/${sim.id}/edit`}
|
||||
className="text-ink font-medium hover:underline"
|
||||
onClick={(e) => e.stopPropagation()}
|
||||
>
|
||||
{sim.name}
|
||||
</Link>
|
||||
|
||||
@@ -189,6 +189,41 @@ describe('MitreMatrixModal', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('Apply button is disabled when no techniques selected and no initial selection', async () => {
|
||||
renderWithProviders(
|
||||
<MitreMatrixModal isOpen initialSelection={[]} onApply={vi.fn()} onCancel={vi.fn()} />,
|
||||
);
|
||||
|
||||
await waitFor(() => screen.getByText('T1078'));
|
||||
|
||||
// Label is "Clear all" when totalSelected === 0, but it's disabled when initialSelection is also empty
|
||||
const applyBtn = screen.getByRole('button', { name: /Clear all/i });
|
||||
expect(applyBtn).toBeDisabled();
|
||||
});
|
||||
|
||||
it('Apply button shows "Clear all" and stays enabled when initial selection is deselected', async () => {
|
||||
const onApply = vi.fn();
|
||||
const user = userEvent.setup();
|
||||
|
||||
renderWithProviders(
|
||||
<MitreMatrixModal isOpen initialSelection={SELECTION} onApply={onApply} onCancel={vi.fn()} />,
|
||||
);
|
||||
|
||||
await waitFor(() => screen.getByText('T1078'));
|
||||
|
||||
// Deselect T1078 (it was pre-selected)
|
||||
const t1078Btn = screen.getAllByRole('button').find(
|
||||
(btn) => btn.textContent?.includes('T1078') && !btn.getAttribute('aria-label'),
|
||||
);
|
||||
await user.click(t1078Btn!);
|
||||
|
||||
// Button should show "Clear all" and be enabled (user explicitly clearing the list)
|
||||
const applyBtn = screen.getByRole('button', { name: /Clear all/i });
|
||||
expect(applyBtn).not.toBeDisabled();
|
||||
await user.click(applyBtn);
|
||||
expect(onApply).toHaveBeenCalledWith([]);
|
||||
});
|
||||
|
||||
it('backdrop click calls onCancel', async () => {
|
||||
const onCancel = vi.fn();
|
||||
const user = userEvent.setup();
|
||||
|
||||
@@ -110,16 +110,29 @@ describe('MitreTechniquesField', () => {
|
||||
expect(screen.getByRole('combobox')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('dedup: adding an already-present technique does not PATCH', async () => {
|
||||
it('dedup: selecting an already-present technique does not PATCH', async () => {
|
||||
mock.onGet('/mitre/techniques').reply(200, [T1059]);
|
||||
const user = userEvent.setup();
|
||||
renderWithProviders(
|
||||
<MitreTechniquesField value={[T1059]} simulationId={7} engagementId={42} />,
|
||||
);
|
||||
// open picker
|
||||
|
||||
// Open the quick-search picker
|
||||
await user.click(screen.getByRole('button', { name: /Quick search/i }));
|
||||
// Picker shows; but we can't easily select the same item without triggering real debounce in this test.
|
||||
// Instead just verify no PATCH happened yet — dedup is the key invariant.
|
||||
const combobox = screen.getByRole('combobox');
|
||||
expect(combobox).toBeInTheDocument();
|
||||
|
||||
// Type to trigger the search (debounce is 200ms but fake timers not needed — mock responds immediately)
|
||||
await user.type(combobox, 'T1059');
|
||||
|
||||
// Wait for the option to appear in the listbox
|
||||
const option = await screen.findByRole('option', { name: /T1059/i });
|
||||
expect(option).toBeInTheDocument();
|
||||
|
||||
// Select it via pointerDown (mirrors the component's onPointerDown handler)
|
||||
await user.pointer({ target: option, keys: '[MouseLeft>]' });
|
||||
|
||||
// Dedup guard should have fired — no PATCH should have been sent
|
||||
expect(mock.history.patch.length).toBe(0);
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user