Skip to content

Commit 24b59d6

Browse files
authored
fix(web-ui): address CodeRabbit review feedback on Glitch Capture (#568)
## Summary - Replace inline SVG close glyph with `Cancel01Icon` from `@hugeicons/react` (repo icon standard) - Make gate payload assertion order-agnostic (`stringMatching` regex instead of exact string) - (Previously in this PR) Fix gates silently discarded — append to description before POST - (Previously in this PR) Remove dead `expires` field from form (expiry set at waiver time) - (Previously in this PR) Fix axios error detail extraction (`err.response.data.detail`) ## Validation - Review feedback: All addressed (2 rounds — 3 original bugs + 2 code-quality fixes) - Demo: All 4 acceptance criteria from #568 verified via showboat - Tests: 15/15 passing (including 3 new tests) - CI: All checks green (Backend Tests, Frontend Tests, Code Quality, Test Summary, CodeRabbit) - Linting: Clean Closes #568
1 parent 19b0cd1 commit 24b59d6

2 files changed

Lines changed: 179 additions & 142 deletions

File tree

web-ui/src/__tests__/components/proof/CaptureGlitchModal.test.tsx

Lines changed: 37 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ describe('CaptureGlitchModal', () => {
5555
});
5656

5757
describe('rendering', () => {
58-
it('renders dialog with title and description when open', () => {
58+
it('renders slide-over panel with title and description when open', () => {
5959
setup();
6060
expect(screen.getByRole('heading', { name: 'Capture Glitch' })).toBeInTheDocument();
6161
expect(screen.getByText(/Convert a production failure/i)).toBeInTheDocument();
@@ -67,7 +67,11 @@ describe('CaptureGlitchModal', () => {
6767
expect(screen.getByLabelText(/Where was it found/i)).toBeInTheDocument();
6868
expect(screen.getByLabelText(/Scope/i)).toBeInTheDocument();
6969
expect(screen.getByLabelText(/Severity/i)).toBeInTheDocument();
70-
expect(screen.getByLabelText(/Expiry/i)).toBeInTheDocument();
70+
});
71+
72+
it('does not render an expiry date field (expiry is set at waiver time)', () => {
73+
setup();
74+
expect(screen.queryByLabelText(/Expiry/i)).not.toBeInTheDocument();
7175
});
7276

7377
it('renders all 9 gate checkboxes', () => {
@@ -88,7 +92,6 @@ describe('CaptureGlitchModal', () => {
8892
describe('validation', () => {
8993
it('shows error when description is empty on submit', async () => {
9094
setup();
91-
// Check at least one gate
9295
fireEvent.click(screen.getByRole('checkbox', { name: 'unit' }));
9396
fireEvent.click(screen.getByRole('button', { name: /Capture Glitch/i }));
9497
await waitFor(() => {
@@ -111,22 +114,25 @@ describe('CaptureGlitchModal', () => {
111114
});
112115

113116
describe('submission', () => {
114-
it('calls proofApi.capture with correct fields and calls onSuccess', async () => {
117+
it('appends selected gates to description and calls onSuccess', async () => {
115118
mockCapture.mockResolvedValue(MOCK_REQ);
116119
setup();
117120

118121
fireEvent.change(screen.getByLabelText(/Description/i), {
119122
target: { value: 'Something broke in production' },
120123
});
121124
fireEvent.click(screen.getByRole('checkbox', { name: 'unit' }));
125+
fireEvent.click(screen.getByRole('checkbox', { name: 'sec' }));
122126
fireEvent.click(screen.getByRole('button', { name: /Capture Glitch/i }));
123127

124128
await waitFor(() => {
125129
expect(mockCapture).toHaveBeenCalledWith(
126130
WORKSPACE,
127131
expect.objectContaining({
128132
title: 'Something broke in production',
129-
description: 'Something broke in production',
133+
description: expect.stringMatching(
134+
/^Something broke in production\n\nRequired gates: (unit, sec|sec, unit)$/
135+
),
130136
severity: 'high',
131137
source: 'production',
132138
created_by: 'human',
@@ -138,7 +144,7 @@ describe('CaptureGlitchModal', () => {
138144
});
139145
});
140146

141-
it('truncates description to 80 chars for title', async () => {
147+
it('truncates long description to 80 chars for title', async () => {
142148
mockCapture.mockResolvedValue(MOCK_REQ);
143149
setup();
144150

@@ -157,7 +163,24 @@ describe('CaptureGlitchModal', () => {
157163
});
158164
});
159165

160-
it('shows inline error and keeps modal open on API failure', async () => {
166+
it('surfaces backend error detail from axios response on failure', async () => {
167+
const axiosError = { response: { data: { detail: 'Workspace not found' } } };
168+
mockCapture.mockRejectedValue(axiosError);
169+
setup();
170+
171+
fireEvent.change(screen.getByLabelText(/Description/i), {
172+
target: { value: 'Something broke' },
173+
});
174+
fireEvent.click(screen.getByRole('checkbox', { name: 'demo' }));
175+
fireEvent.click(screen.getByRole('button', { name: /Capture Glitch/i }));
176+
177+
await waitFor(() => {
178+
expect(screen.getByText('Workspace not found')).toBeInTheDocument();
179+
});
180+
expect(DEFAULT_PROPS.onSuccess).not.toHaveBeenCalled();
181+
});
182+
183+
it('shows fallback error message when axios error has no detail', async () => {
161184
mockCapture.mockRejectedValue(new Error('Network error'));
162185
setup();
163186

@@ -170,12 +193,9 @@ describe('CaptureGlitchModal', () => {
170193
await waitFor(() => {
171194
expect(screen.getByText(/Failed to capture glitch/i)).toBeInTheDocument();
172195
});
173-
expect(DEFAULT_PROPS.onSuccess).not.toHaveBeenCalled();
174-
// Modal still open
175-
expect(screen.getByRole('heading', { name: 'Capture Glitch' })).toBeInTheDocument();
176196
});
177197

178-
it('disables submit button while submitting', async () => {
198+
it('shows submitting state while in-flight', async () => {
179199
let resolve!: (v: ProofRequirement) => void;
180200
mockCapture.mockReturnValue(new Promise((r) => { resolve = r; }));
181201
setup();
@@ -199,6 +219,12 @@ describe('CaptureGlitchModal', () => {
199219
fireEvent.click(screen.getByRole('button', { name: /Cancel/i }));
200220
expect(DEFAULT_PROPS.onClose).toHaveBeenCalled();
201221
});
222+
223+
it('calls onClose when the × button is clicked', () => {
224+
setup();
225+
fireEvent.click(screen.getByRole('button', { name: /Close/i }));
226+
expect(DEFAULT_PROPS.onClose).toHaveBeenCalled();
227+
});
202228
});
203229

204230
describe('state reset on reopen', () => {

0 commit comments

Comments
 (0)