apps/cowswap-frontend/src/modules/captcha/containers/CaptchaWidget.container.test.tsx (1)

26-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add direct coverage for the new onError reset path.

The production callback now resets the widget on Turnstile errors, but the mock only exposes onSuccess, so this behavior can regress without a focused test.

🧪 Proposed test coverage

-let mockTurnstileProps: { style?: CSSProperties; onSuccess?: (token: string) => void } = {}
+let mockTurnstileProps: {
+  style?: CSSProperties
+  onSuccess?: (token: string) => void | Promise<void>
+  onError?: (errorCode: string) => void
+} = {}
 const mockReset = jest.fn()
@@
-      (props: { style?: CSSProperties; onSuccess?: (token: string) => void }, ref: unknown) => {
+      (props: typeof mockTurnstileProps, ref: unknown) => {
         mockTurnstileProps = props
         react.useImperativeHandle(ref, () => ({ reset: mockReset, execute: jest.fn() }))
         return <div data-testid="turnstile" style={props.style} />
       },
@@
   it('resets the widget so the user can re-confirm when the JWT exchange fails', async () => {
@@
     expect(store.get(captchaJwtAtom)).toBeNull()
   })
+
+  it('resets the widget when Turnstile reports an error', async () => {
+    const store = createStore()
+    store.set(featureFlagsAtom, { isCaptchaEnabled: true })
+
+    renderWithStore(store)
+
+    act(() => {
+      mockTurnstileProps.onError?.('mock-error')
+    })
+
+    expect(mockReset).toHaveBeenCalled()
+    expect(store.get(captchaJwtAtom)).toBeNull()
+  })

Also applies to: 146-162

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@apps/cowswap-frontend/src/modules/captcha/containers/CaptchaWidget.container.test.tsx`
around lines 26 - 42, The CaptchaWidget test mock only captures onSuccess, so
the new Turnstile error reset behavior is not directly covered. Update the
`@marsidev/react-turnstile` mock in CaptchaWidget.container.test.tsx to expose
onError alongside onSuccess via mockTurnstileProps, then add a focused test
against CaptchaWidget that invokes the captured onError callback and asserts
mockReset is called on the mocked Turnstile ref. Use the existing
mockTurnstileProps and mockReset setup to locate the relevant test block.

Finding: [BLOCKING] Pending JWT exchange still shows the solved captcha

• Location: apps/cowswap-frontend/src/modules/captcha/containers/CaptchaWidget.container.test.tsx:134
• This one is important: the linked issue asks for the captcha message to disappear when the box is checked, and the PR body still says the widget hides as soon as the challenge succeeds.
• Current head now asserts the opposite: after onSuccess, the widget remains display: block until exchangeTurnstileToken() resolves. If /auth/turnstile is slow or never settles, the “Success!” box can still linger on the form for the whole pending exchange.
• The failure reset path is better now, but it does not cover the slow/pending case.

Suggested fix

• Keep the immediate hide-on-success state for the pending exchange, and on exchange failure call captchaRef.current?.reset() before showing/re-enabling the widget for a fresh challenge.
• Update the pending-exchange test to assert the solved widget is hidden immediately, plus keep the failure test asserting reset() is called.

Verify this finding against current PR head. The linked issue #7768 expects the Turnstile success message to disappear when the checkbox is checked, but current tests assert the widget remains display:block while exchangeTurnstileToken is pending.

Fix only if still valid:
- Hide the solved Turnstile widget immediately after onSuccess while the JWT exchange is pending.
- If the exchange fails, reset the Turnstile ref so the stale Success state is cleared before showing/retrying.
- Preserve stale request guards.
- Add/update tests for pending exchange hidden immediately, success unmount, exchange failure reset/retry, and optionally onError reset.

Generated using the pr-review skill from the CoW Protocol skills repo.

apps/cowswap-frontend/src/modules/captcha/containers/CaptchaWidget.container.test.tsx (1)

164-176: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Exercise onError from the hidden solved state.

This currently starts from a visible widget, so it does not prove onError clears the new isSolved state. Trigger onSuccess first, assert display: none, then fire onError.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@apps/cowswap-frontend/src/modules/captcha/containers/CaptchaWidget.container.test.tsx`
around lines 164 - 176, The test for CaptchaWidget.container’s Turnstile error
handling should start from the hidden solved state instead of a visible widget.
In the CaptchaWidget.container.test.tsx case that calls
mockTurnstileProps.onError, first trigger mockTurnstileProps.onSuccess to put
the widget into the solved state and assert the element is hidden with display
none, then fire onError and verify mockReset is called and the widget becomes
visible again. This should exercise the isSolved state transition in the
CaptchaWidget container behavior.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Rechecked

• Author response: a7f0ed5fd claims the solved widget now hides immediately on onSuccess, then reveals + resets on exchange failure.
• Code path: apps/cowswap-frontend/src/modules/captcha/containers/CaptchaWidget.container.tsx:onSuccess.
• Test coverage: CaptchaWidget.container.test.tsx now asserts the widget is display: none while the JWT exchange is still pending, and display: block again after exchange failure/reset.

Result: Fixed for the issue I raised. The prior slow /auth/turnstile failure mode no longer applies: onSuccess sets isSolved before starting the JWT exchange, so the Turnstile stays mounted but hidden while the exchange is pending.

Follow-up context: Elena raised a newer review comment, and #7801 adds a separate UX requirement: failed JWT verification should show a failure state, not just reset/reveal the widget. I would keep my original finding marked addressed, but I would not treat the PR as fully clear if #7801 is intended to be solved here.

Prior finding:
- After onSuccess, the Turnstile success box stayed visible until exchangeTurnstileToken resolved.

Current verification:
- onSuccess sets isSolved(true) before calling handleChallengeSuccess.
- Turnstile style uses display: isSolved ? "none" : "block".
- Exchange failure sets isSolved(false) and calls captchaRef.current?.reset().
- Tests cover pending exchange hidden immediately and exchange failure reset/reveal.

Remaining separate UX question:
- #7801 expects a visible failed state when JWT verification fails. Current behavior resets/reveals for retry; it does not implement a failed-state UI.

Generated using the pr-review skill from the CoW Protocol skills repo.

Rechecked

• Author response: 873104d81 / #7770 (comment).
• Code path: apps/cowswap-frontend/src/modules/captcha/containers/CaptchaWidget.container.tsx:41.
• Test coverage: current tests cover hidden-pending, rejected-exchange failed notice, retry reset/reveal, and Turnstile error reset.

Result: Addressed for rejected JWT verification. The failed notice shown in Elena's screenshot matches #7801's expected state and looks fine to me.

Finding: [BLOCKING] Hidden pending exchange still has no timeout/fallback

• Location: apps/cowswap-frontend/src/modules/captcha/containers/CaptchaWidget.container.tsx:171 and apps/cowswap-frontend/src/modules/captcha/api/captchaApi.ts:9
• This one is important: onSuccess now hides Turnstile immediately with status = 'solved', but exchangeTurnstileToken() is a bare fetch() without an abort timeout.
• If /auth/turnstile never settles, the catch path never runs, so the user gets no failed notice, no Try again button, no JWT, and the captcha area stays hidden indefinitely.

Suggested fix

• Add a timeout/abort fallback to the JWT exchange request and route timeout/abort into the same status = 'failed' UI.
• Add a focused test for a non-settling exchange becoming retryable after the timeout.

Verify this follow-up against current PR head.

Context:
- CaptchaWidget sets status to "solved" in onSuccess and hides Turnstile while exchangeTurnstileToken is pending.
- exchangeTurnstileToken uses fetch(TURNSTILE_AUTH_URL) without an AbortController or request timeout.
- Rejected exchanges show the failed notice correctly, but a never-settling exchange never reaches that path.

Fix:
- Add a bounded timeout/abort fallback for the JWT exchange.
- Ensure timeout/abort shows the same failed verification notice with Try again.
- Add/update tests for non-settling exchange timeout, rejected exchange failed notice, and retry reset/reveal.

Generated using the pr-review skill from the CoW Protocol skills repo.