diff --git a/src/components/ClaimableBalanceCard.test.tsx b/src/components/ClaimableBalanceCard.test.tsx index ff70657..a2d8507 100644 --- a/src/components/ClaimableBalanceCard.test.tsx +++ b/src/components/ClaimableBalanceCard.test.tsx @@ -2,7 +2,7 @@ import { fireEvent, render, screen, waitFor } from "@testing-library/react"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { useSorokit } from "@/context/useSorokit"; -import { getClient } from "@/lib/client"; +import type { ClaimableBalance } from "@/lib/client"; import { ClaimableBalanceCard } from "./ClaimableBalanceCard"; @@ -10,28 +10,34 @@ vi.mock("@/context/useSorokit", () => ({ useSorokit: vi.fn(), })); -vi.mock("@/lib/client", () => ({ - getClient: vi.fn(), -})); +type AccountStub = { + getClaimableBalances: ReturnType; + claimBalance: ReturnType; +}; describe("ClaimableBalanceCard", () => { - // `ClaimableBalanceCard` reads `client` from `useSorokit()` directly, not - // from `getClient()` — the `@/lib/client` mock only supplies the type, so - // `vi.mocked(getClient).mockReturnValue(...)` alone (the pattern every - // test below already used) never actually reached the component: `client` - // was always undefined, every fetch silently no-op'd, and the card stayed - // on its empty state regardless of what the test configured. `client` is - // a getter here (not a value captured at mockConnected()'s call time) so - // it still picks up whichever `getClient` mock a test configures - // afterward, regardless of call order. - function mockConnected(address = "GABC123") { + /** + * The component reads its client from the SorokitContext (see `useSorokit`), + * so the hook mock has to carry the account stub the rows call into. + */ + function mockConnected(account: AccountStub, address = "GABC123") { vi.mocked(useSorokit).mockReturnValue({ address, isConnected: true, - get client() { - return getClient(); - }, + client: { account }, } as unknown as ReturnType); + return account; + } + + function balance(overrides: Partial = {}): ClaimableBalance { + return { + id: "cb1", + asset: "native", + amount: "10.0", + sponsor: "GDEF", + claimants: [{ destination: "GDEF", predicate: { unconditional: true } }], + ...overrides, + } as ClaimableBalance; } beforeEach(() => { @@ -44,6 +50,7 @@ describe("ClaimableBalanceCard", () => { vi.mocked(useSorokit).mockReturnValue({ address: null, isConnected: false, + client: null, } as unknown as ReturnType); const { container } = render(); expect(container).toBeEmptyDOMElement(); @@ -52,30 +59,36 @@ describe("ClaimableBalanceCard", () => { describe("fetch states", () => { it("shows fetch error when getClaimableBalances returns an error", async () => { - mockConnected(); - vi.mocked(getClient).mockReturnValue({ - account: { - getClaimableBalances: vi.fn().mockResolvedValue({ data: null, error: "Failed to fetch balances" }), - claimBalance: vi.fn(), - }, - } as unknown as ReturnType); + mockConnected({ + getClaimableBalances: vi + .fn() + .mockResolvedValue({ data: null, error: "Failed to fetch balances" }), + claimBalance: vi.fn(), + }); render(); expect(await screen.findByText("Failed to fetch balances")).toBeInTheDocument(); }); + it("shows an error instead of hanging when getClaimableBalances rejects", async () => { + mockConnected({ + getClaimableBalances: vi.fn().mockRejectedValue(new Error("Horizon down")), + claimBalance: vi.fn(), + }); + render(); + expect(await screen.findByText("Horizon down")).toBeInTheDocument(); + }); + it("shows empty state when no claimable balances exist", async () => { - mockConnected(); - vi.mocked(getClient).mockReturnValue({ - account: { - getClaimableBalances: vi.fn().mockResolvedValue({ data: [], error: null }), - claimBalance: vi.fn(), - }, - } as unknown as ReturnType); + mockConnected({ + getClaimableBalances: vi.fn().mockResolvedValue({ data: [], error: null }), + claimBalance: vi.fn(), + }); render(); expect(await screen.findByText(/no claimable balances/i)).toBeInTheDocument(); }); }); + // Issue #441 acceptance criteria. describe("claim flow", () => { it("shows error and re-enables button on claim failure, removes the row on success", async () => { mockConnected(); @@ -97,16 +110,77 @@ describe("ClaimableBalanceCard", () => { render(); expect(await screen.findByText("10.00")).toBeInTheDocument(); + expect(screen.getByText("2 pending")).toBeInTheDocument(); + + fireEvent.click(screen.getAllByRole("button", { name: "Claim" })[0]); + + await waitFor(() => expect(screen.queryByText("10.00")).not.toBeInTheDocument()); + expect(account.claimBalance).toHaveBeenCalledWith("cb1"); + expect(screen.getByText("20.00")).toBeInTheDocument(); + expect(screen.getByText("1 pending")).toBeInTheDocument(); + }); + + it("keeps a claimed balance out of the list when the re-fetch still returns it", async () => { + const account = mockConnected({ + getClaimableBalances: vi.fn().mockResolvedValue({ + data: [balance(), balance({ id: "cb2", amount: "20.0" })], + error: null, + }), + claimBalance: vi.fn().mockResolvedValue({ data: { hash: "tx123" }, error: null }), + }); + + render(); + expect(await screen.findByText("10.00")).toBeInTheDocument(); + + fireEvent.click(screen.getAllByRole("button", { name: "Claim" })[0]); + + // A re-fetch is triggered after the claim; Horizon may still report the + // claimed balance, but it must not reappear in the list. + await waitFor(() => expect(account.getClaimableBalances).toHaveBeenCalledTimes(2)); + await waitFor(() => expect(screen.getByText("20.00")).toBeInTheDocument()); + expect(screen.queryByText("10.00")).not.toBeInTheDocument(); + }); + + it("shows an inline error and re-enables the button when the claim fails", async () => { + mockConnected({ + getClaimableBalances: vi.fn().mockResolvedValue({ data: [balance()], error: null }), + claimBalance: vi + .fn() + .mockResolvedValueOnce({ data: null, error: "Network error" }) + .mockResolvedValueOnce({ data: { hash: "tx123" }, error: null }), + }); + + render(); + expect(await screen.findByText("10.00")).toBeInTheDocument(); + + fireEvent.click(screen.getByRole("button", { name: "Claim" })); + const inlineError = await screen.findByTestId("claim-error-cb1"); + expect(inlineError).toHaveTextContent("Network error"); + expect(inlineError).toHaveAttribute("role", "alert"); + + // The row survives a failure and the button is usable again for a retry. const claimButton = screen.getByRole("button", { name: "Claim" }); - fireEvent.click(claimButton); - expect(await screen.findByText("Network error")).toBeInTheDocument(); expect(claimButton).not.toBeDisabled(); fireEvent.click(claimButton); - expect(await screen.findByText(/no claimable balances/i)).toBeInTheDocument(); - expect(screen.queryByText("10.00")).not.toBeInTheDocument(); - expect(screen.queryByRole("button", { name: "Claim" })).not.toBeInTheDocument(); + await waitFor(() => expect(screen.queryByText("10.00")).not.toBeInTheDocument()); + }); + + it("surfaces a rejected claimBalance call as an inline error instead of an unhandled rejection", async () => { + mockConnected({ + getClaimableBalances: vi.fn().mockResolvedValue({ data: [balance()], error: null }), + claimBalance: vi.fn().mockRejectedValue(new Error("Wallet rejected")), + }); + + render(); + expect(await screen.findByText("10.00")).toBeInTheDocument(); + + fireEvent.click(screen.getByRole("button", { name: "Claim" })); + + expect(await screen.findByTestId("claim-error-cb1")).toHaveTextContent("Wallet rejected"); + expect(screen.getByRole("button", { name: "Claim" })).not.toBeDisabled(); + expect(screen.getByText("10.00")).toBeInTheDocument(); }); it("shows an error and does not remove the row when the API resolves with no data and no error", async () => { @@ -176,22 +250,23 @@ describe("ClaimableBalanceCard", () => { describe("balance ID copy", () => { function setupBalanceIdTest() { - mockConnected(); - vi.mocked(getClient).mockReturnValue({ - account: { - getClaimableBalances: vi.fn().mockResolvedValue({ - data: [{ - id: "BALANCE_ID_1234567890", - asset: "native", - amount: "5.0", - sponsor: "GDEF", - claimants: [{ destination: "GDEF", predicate: { unconditional: true } }], - }], - error: null, - }), - claimBalance: vi.fn(), - }, - } as unknown as ReturnType); + return mockConnected({ + getClaimableBalances: vi.fn().mockResolvedValue({ + data: [balance({ id: "BALANCE_ID_1234567890", amount: "5.0" })], + error: null, + }), + claimBalance: vi.fn(), + }); + } + + function stubClipboard() { + const writeText = vi.fn().mockResolvedValue(undefined); + Object.defineProperty(navigator, "clipboard", { + value: { writeText }, + configurable: true, + writable: true, + }); + return writeText; } it("shows balance ID", async () => { @@ -201,24 +276,26 @@ describe("ClaimableBalanceCard", () => { }); it("copies balance ID to clipboard on click", async () => { - const writeText = vi.fn().mockResolvedValue(undefined); - Object.defineProperty(navigator, "clipboard", { - value: { writeText }, - configurable: true, - writable: true, - }); + const writeText = stubClipboard(); setupBalanceIdTest(); render(); await screen.findByText("5.00"); - const copyBtn = screen.getByRole("button", { name: /copy balance id/i }); - fireEvent.click(copyBtn); + fireEvent.click(screen.getByRole("button", { name: /copy balance id/i })); expect(writeText).toHaveBeenCalledWith("BALANCE_ID_1234567890"); }); it("shows copied state briefly after copying balance ID", async () => { - const writeText = vi.fn().mockResolvedValue(undefined); + stubClipboard(); + setupBalanceIdTest(); + render(); + expect(await screen.findByText("5.00")).toBeInTheDocument(); + fireEvent.click(screen.getByRole("button", { name: /copy balance id/i })); + expect(screen.getByRole("button", { name: /balance id copied/i })).toBeInTheDocument(); + }); + + it("does not blow up when the clipboard write is rejected", async () => { Object.defineProperty(navigator, "clipboard", { - value: { writeText }, + value: { writeText: vi.fn().mockRejectedValue(new Error("blocked")) }, configurable: true, writable: true, }); @@ -232,68 +309,57 @@ describe("ClaimableBalanceCard", () => { describe("predicate expiry disabled state", () => { it("disables Claim button and shows Expired badge when predicate has expired timebound", async () => { - mockConnected(); const expiredTime = Date.now() - 10000; - vi.mocked(getClient).mockReturnValue({ - account: { - getClaimableBalances: vi.fn().mockResolvedValue({ - data: [{ + mockConnected({ + getClaimableBalances: vi.fn().mockResolvedValue({ + data: [ + balance({ id: "cb-expired", - asset: "native", amount: "100.0", - sponsor: "GDEF", - claimants: [{ destination: "GDEF", predicate: { timeBound: { start: 0, end: expiredTime } } }], - }], - error: null, - }), - claimBalance: vi.fn(), - }, - } as unknown as ReturnType); + claimants: [ + { destination: "GDEF", predicate: { timeBound: { start: 0, end: expiredTime } } }, + ], + } as Partial), + ], + error: null, + }), + claimBalance: vi.fn(), + }); render(); expect(await screen.findByText("Expired")).toBeInTheDocument(); expect(screen.getByRole("button", { name: "Claim" })).toBeDisabled(); }); it("keeps Claim button enabled when predicate is unconditional", async () => { - mockConnected(); - vi.mocked(getClient).mockReturnValue({ - account: { - getClaimableBalances: vi.fn().mockResolvedValue({ - data: [{ - id: "cb-active", - asset: "native", - amount: "50.0", - sponsor: "GDEF", - claimants: [{ destination: "GDEF", predicate: { unconditional: true } }], - }], - error: null, - }), - claimBalance: vi.fn(), - }, - } as unknown as ReturnType); + mockConnected({ + getClaimableBalances: vi.fn().mockResolvedValue({ + data: [balance({ id: "cb-active", amount: "50.0" })], + error: null, + }), + claimBalance: vi.fn(), + }); render(); expect(await screen.findByText("50.00")).toBeInTheDocument(); expect(screen.getByRole("button", { name: "Claim" })).not.toBeDisabled(); }); it("keeps Claim button enabled when predicate has a future timebound", async () => { - mockConnected(); const futureTime = Date.now() + 100000; - vi.mocked(getClient).mockReturnValue({ - account: { - getClaimableBalances: vi.fn().mockResolvedValue({ - data: [{ + mockConnected({ + getClaimableBalances: vi.fn().mockResolvedValue({ + data: [ + balance({ id: "cb-future", - asset: "native", amount: "25.0", - sponsor: "GDEF", - claimants: [{ destination: "GDEF", predicate: { timeBound: { start: 0, end: futureTime } } }], - }], - error: null, - }), - claimBalance: vi.fn(), - }, - } as unknown as ReturnType); + claimants: [ + { destination: "GDEF", predicate: { timeBound: { start: 0, end: futureTime } } }, + ], + } as Partial), + ], + error: null, + }), + claimBalance: vi.fn(), + }); render(); expect(await screen.findByText("25.00")).toBeInTheDocument(); expect(screen.getByRole("button", { name: "Claim" })).not.toBeDisabled(); @@ -301,27 +367,18 @@ describe("ClaimableBalanceCard", () => { }); describe("confirmThreshold dialog", () => { - function setupThresholdTest(amount: string, _threshold: string) { - mockConnected(); - vi.mocked(getClient).mockReturnValue({ - account: { - getClaimableBalances: vi.fn().mockResolvedValue({ - data: [{ - id: "cb-threshold", - asset: "native", - amount, - sponsor: "GDEF", - claimants: [{ destination: "GDEF", predicate: { unconditional: true } }], - }], - error: null, - }), - claimBalance: vi.fn().mockResolvedValue({ data: { hash: "tx123" }, error: null }), - }, - } as unknown as ReturnType); + function setupThresholdTest(amount: string) { + return mockConnected({ + getClaimableBalances: vi.fn().mockResolvedValue({ + data: [balance({ id: "cb-threshold", amount })], + error: null, + }), + claimBalance: vi.fn().mockResolvedValue({ data: { hash: "tx123" }, error: null }), + }); } it("shows confirmation dialog when amount exceeds threshold", async () => { - setupThresholdTest("5000.0", "1000"); + setupThresholdTest("5000.0"); render(); expect(await screen.findByText("5,000.00")).toBeInTheDocument(); fireEvent.click(screen.getByRole("button", { name: "Claim" })); @@ -329,60 +386,63 @@ describe("ClaimableBalanceCard", () => { }); it("does not show confirmation dialog when amount is below threshold", async () => { - setupThresholdTest("5.0", "1000"); + setupThresholdTest("5.0"); render(); expect(await screen.findByText("5.00")).toBeInTheDocument(); fireEvent.click(screen.getByRole("button", { name: "Claim" })); - await screen.findByText(/no claimable balances/i); + await waitFor(() => expect(screen.queryByText("5.00")).not.toBeInTheDocument()); expect(screen.queryByText("Confirm Claim")).not.toBeInTheDocument(); }); it("cancels the dialog and does not claim", async () => { - setupThresholdTest("5000.0", "1000"); + const account = setupThresholdTest("5000.0"); render(); expect(await screen.findByText("5,000.00")).toBeInTheDocument(); fireEvent.click(screen.getByRole("button", { name: "Claim" })); expect(await screen.findByText("Confirm Claim")).toBeInTheDocument(); fireEvent.click(screen.getByRole("button", { name: /cancel/i })); expect(screen.queryByText("Confirm Claim")).not.toBeInTheDocument(); + expect(account.claimBalance).not.toHaveBeenCalled(); + expect(screen.getByText("5,000.00")).toBeInTheDocument(); }); - it("proceeds with claim after confirming the dialog", async () => { - setupThresholdTest("5000.0", "1000"); + it("proceeds with claim after confirming the dialog and removes the row", async () => { + setupThresholdTest("5000.0"); render(); expect(await screen.findByText("5,000.00")).toBeInTheDocument(); fireEvent.click(screen.getByRole("button", { name: "Claim" })); expect(await screen.findByText("Confirm Claim")).toBeInTheDocument(); fireEvent.click(screen.getByRole("button", { name: /confirm/i })); + await waitFor(() => expect(screen.queryByText("5,000.00")).not.toBeInTheDocument()); expect(await screen.findByText(/no claimable balances/i)).toBeInTheDocument(); }); }); describe("combined: balance ID + expiry + confirmThreshold", () => { it("renders all three b17 features together for a single balance", async () => { - const writeText = vi.fn().mockResolvedValue(undefined); Object.defineProperty(navigator, "clipboard", { - value: { writeText }, + value: { writeText: vi.fn().mockResolvedValue(undefined) }, configurable: true, writable: true, }); - mockConnected(); const futureTime = Date.now() + 100000; - vi.mocked(getClient).mockReturnValue({ - account: { - getClaimableBalances: vi.fn().mockResolvedValue({ - data: [{ + mockConnected({ + getClaimableBalances: vi.fn().mockResolvedValue({ + data: [ + balance({ id: "FEATURE_COMBO_ID_123", asset: "USDC:GBPL", amount: "2500.0", sponsor: "GDEF456", - claimants: [{ destination: "GDEF456", predicate: { timeBound: { start: 0, end: futureTime } } }], - }], - error: null, - }), - claimBalance: vi.fn(), - }, - } as unknown as ReturnType); + claimants: [ + { destination: "GDEF456", predicate: { timeBound: { start: 0, end: futureTime } } }, + ], + } as Partial), + ], + error: null, + }), + claimBalance: vi.fn(), + }); render(); expect(await screen.findByText("2,500.00")).toBeInTheDocument(); diff --git a/src/components/ClaimableBalanceCard.tsx b/src/components/ClaimableBalanceCard.tsx index 5f4d430..8da6b35 100644 --- a/src/components/ClaimableBalanceCard.tsx +++ b/src/components/ClaimableBalanceCard.tsx @@ -1,4 +1,4 @@ -import { useEffect, useRef, useState } from "react"; +import { useCallback, useEffect, useRef, useState } from "react"; import { Badge } from "@/components/ui/Badge"; import { Button } from "@/components/ui/Button"; @@ -27,11 +27,20 @@ interface BalanceRowProps { cb: ClaimableBalance; confirmThreshold?: string; currentTime?: number; - /** Called with the balance's id once it has been successfully claimed. */ + /** + * Issue #441: called with the balance id once a claim resolves successfully so + * the parent can drop the row from its list straight away (optimistic update) + * instead of leaving a stale row behind a "Claimed" badge. + */ onClaimed?: (id: string) => void; } -function BalanceRow({ cb, confirmThreshold, currentTime = Date.now(), onClaimed }: BalanceRowProps) { +function BalanceRow({ + cb, + confirmThreshold, + currentTime = Date.now(), + onClaimed, +}: BalanceRowProps) { const { client } = useSorokit(); const [claiming, setClaiming] = useState(false); const [claimed, setClaimed] = useState(false); @@ -55,29 +64,42 @@ function BalanceRow({ cb, confirmThreshold, currentTime = Date.now(), onClaimed } async function doClaim() { - if (!client) return; + if (!client) { + // Issue #441: never leave the row stuck in a loading state when there is + // no client to claim with - surface it instead. + setClaimError("Wallet client unavailable"); + return; + } setClaiming(true); setClaimError(null); setShowConfirm(false); try { - const { data, error } = await client.account.claimBalance(cb.id); + const { error } = await client.account.claimBalance(cb.id); if (error) { + // Issue #441: a failed claim shows an inline error and returns the + // button to its normal state so the user can retry. setClaimError(error); - return; - } - if (!data) { - setClaimError("Claim did not complete — please try again"); + setClaiming(false); return; } setClaimed(true); + setClaiming(false); + // Issue #441: hand the id up so the parent removes this row immediately; + // the local `claimed` flag only covers the standalone/no-handler case. onClaimed?.(cb.id); - } finally { + } catch (err) { + // Issue #441: claimBalance may reject (network/wallet throw). Catching it + // here keeps the rejection from escaping as an unhandled promise and + // still re-enables the button. + setClaimError(err instanceof Error ? err.message : "Claim failed"); setClaiming(false); } } function handleCopyId() { - navigator.clipboard.writeText(cb.id); + // Issue #441: writeText returns a promise that rejects when the clipboard + // is blocked - swallow it so it is never an unhandled rejection. + void Promise.resolve(navigator.clipboard?.writeText(cb.id)).catch(() => {}); setCopiedId(true); setTimeout(() => setCopiedId(false), 1600); } @@ -120,13 +142,19 @@ function BalanceRow({ cb, confirmThreshold, currentTime = Date.now(), onClaimed size="sm" loading={claiming} disabled={expired} - onClick={handleClaim} + onClick={() => void handleClaim()} className={cn("shrink-0", expired && "opacity-40 cursor-not-allowed")} > Claim {claimError && ( - + /* Issue #441: inline, wrapped (not truncated) and announced so a + failed claim is actually visible on the affected row. */ + {claimError} )} @@ -148,7 +176,7 @@ function BalanceRow({ cb, confirmThreshold, currentTime = Date.now(), onClaimed - @@ -168,6 +196,29 @@ export function ClaimableBalanceCard({ confirmThreshold }: ClaimableBalanceCardP const [balances, setBalances] = useState([]); const [loading, setLoading] = useState(false); const [error, setError] = useState(null); + // Issue #441: bumped after every successful claim to re-fetch the list so it + // stays server-consistent. + const [refreshKey, setRefreshKey] = useState(0); + // Issue #441: ids claimed in this session. Horizon can still return a just + // claimed balance for a short while, so the re-fetch is filtered against this + // set - otherwise the row the user just claimed would pop back in. + const claimedIdsRef = useRef>(new Set()); + // Issue #441: marks the next fetch as a post-claim refresh, which must not + // swap the already-rendered rows out for the loading skeleton. + const backgroundRefreshRef = useRef(false); + + useEffect(() => { + claimedIdsRef.current = new Set(); + }, [address, client]); + + const handleClaimed = useCallback((id: string) => { + // Issue #441: optimistic removal - the row disappears as soon as the claim + // resolves, then we ask the server for a fresh list. + claimedIdsRef.current.add(id); + setBalances((prev) => prev.filter((b) => b.id !== id)); + backgroundRefreshRef.current = true; + setRefreshKey((k) => k + 1); + }, []); function handleClaimSuccess(balanceId: string) { setBalances((prev) => prev.filter((b) => b.id !== balanceId)); @@ -178,9 +229,13 @@ export function ClaimableBalanceCard({ confirmThreshold }: ClaimableBalanceCardP return; } + const background = backgroundRefreshRef.current; + backgroundRefreshRef.current = false; + let active = true; const timerId = window.setTimeout(() => { - setLoading(true); + if (!background) setLoading(true); + setError(null); client .account.getClaimableBalances(address) .then(({ data, error: err }) => { @@ -189,10 +244,20 @@ export function ClaimableBalanceCard({ confirmThreshold }: ClaimableBalanceCardP setError(err); return; } - setBalances(data ?? []); + setBalances( + (data ?? []).filter((b) => !claimedIdsRef.current.has(b.id)), + ); + }) + .catch((err: unknown) => { + // Issue #441: a rejected fetch used to escape as an unhandled + // rejection and pin the card in its loading state. + if (!active) return; + setError( + err instanceof Error ? err.message : "Failed to load claimable balances", + ); }) .finally(() => { - if (active) setLoading(false); + if (active && !background) setLoading(false); }); }, 0); @@ -200,7 +265,7 @@ export function ClaimableBalanceCard({ confirmThreshold }: ClaimableBalanceCardP active = false; window.clearTimeout(timerId); }; - }, [address, client]); + }, [address, client, refreshKey]); function handleClaimed(id: string) { setBalances((prev) => prev.filter((b) => b.id !== id));