From f1a53609362180ed9801ea2de22e43bcf71aaa2b Mon Sep 17 00:00:00 2001 From: D-Lord-15 Date: Sun, 18 Oct 2026 02:52:00 +0000 Subject: [PATCH 1/4] test(contracts): cover missing-state views and InvalidRate rollback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add the two sibling-view tests missing from #793's coverage and prove a rejected create_stream rolls back fully. #1541: assert get_stream returns None for a never-created id, get_fee_config returns None before initialize, and is_stream_completed returns false for a never-created id. #1542: with a 1% fee configured, a create_stream whose net rate rounds to zero returns InvalidRate while leaving the sender, treasury, and contract token balances untouched, and the stream-id counter is not consumed — the next successful stream is still id 1. --- contracts/stream_contract/src/test.rs | 78 +++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/contracts/stream_contract/src/test.rs b/contracts/stream_contract/src/test.rs index 3585d7f5..d2cf04d2 100644 --- a/contracts/stream_contract/src/test.rs +++ b/contracts/stream_contract/src/test.rs @@ -1561,6 +1561,48 @@ fn test_create_stream_zero_rate() { assert_stream_error!(result, StreamError::InvalidRate); } +// Issue #1542: a rejected `create_stream` must roll back *everything* it did — +// the gross transfer from the sender, any fee it computed for the treasury, and +// the stream-id counter bump — not merely return `InvalidRate`. +#[test] +fn test_create_stream_invalid_rate_rolls_back_transfer_fee_and_counter() { + let env = Env::default(); + env.mock_all_auths(); + let (token, _) = create_token(&env); + let sender = Address::generate(&env); + let recipient = Address::generate(&env); + let admin = Address::generate(&env); + let treasury = Address::generate(&env); + mint(&env, &token, &sender, 100); + + let client = create_contract(&env); + // A 1% protocol fee makes the gross (100) and net (99) diverge: 99 / 1_000 + // rounds to a 0/s rate, which `create_stream` rejects as `InvalidRate`. + client.initialize(&admin, &treasury, &100); + + let token_client = token::Client::new(&env, &token); + let sender_before = token_client.balance(&sender); + let treasury_before = token_client.balance(&treasury); + let contract_before = token_client.balance(&client.address); + + let result = client.try_create_stream(&sender, &recipient, &token, &100, &1_000); + assert_stream_error!(result, StreamError::InvalidRate); + + // The sender's gross transfer to the contract is unwound and no fee was + // retained by the contract or paid to the treasury. + assert_eq!(token_client.balance(&sender), sender_before); + assert_eq!(token_client.balance(&treasury), treasury_before); + assert_eq!(token_client.balance(&client.address), contract_before); + + // `next_stream_id` bumps before the rate guard, so the failed attempt must + // not consume an ID: the next successful stream is still stream #1. + let stream_id = client.create_stream(&sender, &recipient, &token, &100, &10); + assert_eq!(stream_id, 1); + assert!(client.get_stream(&stream_id).is_some()); + // The successful create did charge the fee, proving the fee machinery ran. + assert_eq!(token_client.balance(&treasury), treasury_before + 1); +} + #[test] fn test_create_stream_rate_exactly_one_succeeds() { // amount == duration → rate = 1, which is the smallest valid rate. @@ -2019,6 +2061,42 @@ fn test_is_stream_completed_helper() { assert!(client.is_stream_completed(&id)); } +// ─── Missing-state read-only queries (#1541) ───────────────────────────────── + +#[test] +fn test_get_stream_missing_id_returns_none() { + let env = Env::default(); + let client = create_contract(&env); + + // A never-created ID must read as `None` rather than a `StreamNotFound` + // error, exercising `try_load_stream`'s absent-key branch. + assert!(client.get_stream(&0).is_none()); + assert!(client.get_stream(&1).is_none()); + assert!(client.get_stream(&u64::MAX).is_none()); +} + +#[test] +fn test_get_fee_config_before_initialize_returns_none() { + let env = Env::default(); + let client = create_contract(&env); + + // `initialize` has never been called, so `try_load_config` has no record to + // decode and `get_fee_config` must report the absence as `None`. + assert!(client.get_fee_config().is_none()); +} + +#[test] +fn test_is_stream_completed_missing_id_returns_false() { + let env = Env::default(); + let client = create_contract(&env); + + // A never-created stream is not "completed"; the missing branch of + // `try_load_stream` must map to `false` rather than panic. + assert!(!client.is_stream_completed(&0)); + assert!(!client.is_stream_completed(&1)); + assert!(!client.is_stream_completed(&u64::MAX)); +} + #[test] fn test_completed_event_emitted_on_final_withdrawal() { let env = Env::default(); From 1bfcc54796246e8ebed890677eaca1ca94f0df2f Mon Sep 17 00:00:00 2001 From: D-Lord-15 Date: Sun, 18 Oct 2026 02:52:04 +0000 Subject: [PATCH 2/4] test(frontend): add Button, Card, and Stepper unit tests These primitives are imported across most pages and modals but, unlike their sibling Skeleton, had no test files. - Button: variant/size class application, disabled when disabled or loading, and the aria-busy + visually-hidden Loading label while loading. - Card: glow and hover prop toggles, base styling, children, and custom className merging. - Stepper: aria-current=step on the active step only, and the correct completed/active/upcoming class and screen-reader suffix per index. Closes #1539 --- frontend/src/components/ui/Button.test.tsx | 66 +++++++++++++++++++++ frontend/src/components/ui/Card.test.tsx | 57 ++++++++++++++++++ frontend/src/components/ui/Stepper.test.tsx | 58 ++++++++++++++++++ 3 files changed, 181 insertions(+) create mode 100644 frontend/src/components/ui/Button.test.tsx create mode 100644 frontend/src/components/ui/Card.test.tsx create mode 100644 frontend/src/components/ui/Stepper.test.tsx diff --git a/frontend/src/components/ui/Button.test.tsx b/frontend/src/components/ui/Button.test.tsx new file mode 100644 index 00000000..72c49e7c --- /dev/null +++ b/frontend/src/components/ui/Button.test.tsx @@ -0,0 +1,66 @@ +import { describe, it, expect } from "vitest"; +import { render, screen } from "@testing-library/react"; +import { Button } from "./Button"; + +describe("Button", () => { + it("renders its children", () => { + render(); + expect( + screen.getByRole("button", { name: "Click me" }) + ).toBeInTheDocument(); + }); + + it("applies the primary/md styling by default", () => { + render(); + const button = screen.getByRole("button"); + expect(button.className).toContain("bg-accent"); + expect(button.className).toContain("px-6"); + expect(button.className).toContain("text-sm"); + }); + + it.each([ + ["secondary", "bg-accent-secondary"], + ["outline", "bg-glass"], + ["ghost", "text-slate-400"], + ] as const)("applies the %s variant classes", (variant, expectedClass) => { + render(); + expect(screen.getByRole("button").className).toContain(expectedClass); + }); + + it.each([ + ["sm", "px-4"], + ["md", "px-6"], + ["lg", "px-8"], + ] as const)("applies the %s size classes", (size, expectedClass) => { + render(); + expect(screen.getByRole("button").className).toContain(expectedClass); + }); + + it("applies the glow shadow classes when glow is set", () => { + render(); + expect(screen.getByRole("button").className).toContain( + "shadow-[0_0_20px_rgba(16,185,129,0.3)]" + ); + }); + + it("is disabled when disabled is true", () => { + render(); + expect(screen.getByRole("button")).toBeDisabled(); + }); + + it("is disabled, aria-busy, and announces Loading while loading", () => { + render(); + const button = screen.getByRole("button"); + expect(button).toBeDisabled(); + expect(button).toHaveAttribute("aria-busy", "true"); + expect(screen.getByText("Loading")).toBeInTheDocument(); + }); + + it("is idle and not busy when not loading", () => { + render(); + const button = screen.getByRole("button"); + expect(button).not.toBeDisabled(); + expect(button).toHaveAttribute("aria-busy", "false"); + expect(screen.queryByText("Loading")).not.toBeInTheDocument(); + }); +}); diff --git a/frontend/src/components/ui/Card.test.tsx b/frontend/src/components/ui/Card.test.tsx new file mode 100644 index 00000000..59f4dfb5 --- /dev/null +++ b/frontend/src/components/ui/Card.test.tsx @@ -0,0 +1,57 @@ +import { describe, it, expect } from "vitest"; +import { render, screen } from "@testing-library/react"; +import { Card } from "./Card"; + +describe("Card", () => { + it("renders its children", () => { + render( + + Inner content + + ); + expect(screen.getByText("Inner content")).toBeInTheDocument(); + }); + + it("applies the base card styling", () => { + const { container } = render(Base); + const el = container.firstElementChild as HTMLElement; + expect(el.className).toContain("glass-card"); + expect(el.className).toContain("rounded-2xl"); + expect(el.className).toContain("p-6"); + }); + + it("adds the glow class when glow is set", () => { + const { container } = render(Glowing); + expect((container.firstElementChild as HTMLElement).className).toContain( + "glow-card" + ); + }); + + it("omits the glow class by default", () => { + const { container } = render(Plain); + expect((container.firstElementChild as HTMLElement).className).not.toContain( + "glow-card" + ); + }); + + it("applies hover styling by default", () => { + const { container } = render(Hover); + expect((container.firstElementChild as HTMLElement).className).toContain( + "hover:-translate-y-1" + ); + }); + + it("omits hover styling when hover is false", () => { + const { container } = render(Static); + expect((container.firstElementChild as HTMLElement).className).not.toContain( + "hover:-translate-y-1" + ); + }); + + it("merges a custom className", () => { + const { container } = render(Custom); + const el = container.firstElementChild as HTMLElement; + expect(el.className).toContain("glass-card"); + expect(el.className).toContain("custom-class"); + }); +}); diff --git a/frontend/src/components/ui/Stepper.test.tsx b/frontend/src/components/ui/Stepper.test.tsx new file mode 100644 index 00000000..21533fef --- /dev/null +++ b/frontend/src/components/ui/Stepper.test.tsx @@ -0,0 +1,58 @@ +import { describe, it, expect } from "vitest"; +import { render, screen } from "@testing-library/react"; +import { Stepper } from "./Stepper"; + +const steps = ["Connect", "Configure", "Confirm"]; + +describe("Stepper", () => { + it("renders every step inside an accessible Progress landmark", () => { + render(); + expect( + screen.getByRole("navigation", { name: "Progress" }) + ).toBeInTheDocument(); + expect(screen.getAllByRole("listitem")).toHaveLength(3); + expect(screen.getByText("Connect")).toBeInTheDocument(); + expect(screen.getByText("Configure")).toBeInTheDocument(); + expect(screen.getByText("Confirm")).toBeInTheDocument(); + }); + + it("marks only the active step with aria-current=step", () => { + render(); + const items = screen.getAllByRole("listitem"); + expect(items[0]).not.toHaveAttribute("aria-current"); + expect(items[1]).toHaveAttribute("aria-current", "step"); + expect(items[2]).not.toHaveAttribute("aria-current"); + }); + + it("applies completed/active/upcoming classes per step index", () => { + const { container } = render(); + const circles = container.querySelectorAll(".stepper__circle"); + const labels = container.querySelectorAll(".stepper__label"); + + expect(circles[0].className).toContain("stepper__circle--completed"); + expect(circles[1].className).toContain("stepper__circle--active"); + expect(circles[2].className).toContain("stepper__circle--upcoming"); + + expect(labels[0].className).toContain("stepper__label--completed"); + expect(labels[1].className).toContain("stepper__label--active"); + expect(labels[2].className).toContain("stepper__label--upcoming"); + }); + + it("adds the matching screen-reader suffix to each step", () => { + render(); + expect(screen.getByText("(completed)")).toBeInTheDocument(); + expect(screen.getByText("(current step)")).toBeInTheDocument(); + expect(screen.getByText("(upcoming)")).toBeInTheDocument(); + }); + + it("shows a check icon for completed steps and a number otherwise", () => { + const { container } = render(); + const circles = container.querySelectorAll(".stepper__circle"); + + expect(circles[0].querySelector("svg")).not.toBeNull(); + expect(circles[1].querySelector("svg")).toBeNull(); + expect(circles[2].querySelector("svg")).toBeNull(); + expect(circles[1].querySelector(".stepper__number")?.textContent).toBe("2"); + expect(circles[2].querySelector(".stepper__number")?.textContent).toBe("3"); + }); +}); From 29106d6a129b6fd291bd07c80165d119a4b34ca1 Mon Sep 17 00:00:00 2001 From: D-Lord-15 Date: Sun, 18 Oct 2026 02:52:12 +0000 Subject: [PATCH 3/4] docs(ci): document and enforce the OpenAPI api-types drift check The CI job that regenerates api-types.generated.ts and diffs it against the checked-in snapshot already existed, but nothing told contributors it was there and the api-types.ts header still claimed codegen was manual-only. - Correct the api-types.ts header to describe the committed spec, the codegen commands, and the CI enforcement. - Add an 'API Types Drift Check' section to CONTRIBUTING.md plus local run steps and a PR checklist. - Add the drift check to the PR template checklist. Closes #1540 --- .github/pull_request_template.md | 1 + CONTRIBUTING.md | 40 ++++++++++++++++++++++++++++++-- frontend/src/lib/api-types.ts | 21 +++++++++++------ 3 files changed, 53 insertions(+), 9 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index e35c06cc..f7896d59 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -56,6 +56,7 @@ Closes # - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] Updated Postman/Hoppscotch API collections if routes changed +- [ ] If routes or response shapes changed, ran the OpenAPI api-types drift check (`npm run codegen:openapi` in `backend/`, then `npm run codegen:api-types` in `frontend/`) and committed the regenerated `flowfi.openapi.json` / `api-types.generated.ts` - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index cef8039e..6d074a40 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -287,11 +287,31 @@ This repository uses GitHub Actions for continuous integration. Workflows are lo - **CI** (`.github/workflows/ci.yml`) - Runs on: push to `main`/`develop` and pull requests - Performs: - - Frontend: lint and build - - Backend: prisma generation, build, and tests + - Frontend: lint, tests, and build + - Backend: prisma generation, build, OpenAPI drift check, and tests - Soroban Contracts: build (wasm) and tests - View workflow: [CI](.github/workflows/ci.yml) +### API Types Drift Check + +The frontend's hand-written contract types in +`frontend/src/lib/api-types.ts` are mirrored by a machine-generated snapshot, +`frontend/src/lib/api-types.generated.ts`. CI keeps the two in sync so a +frontend build can never silently disagree with the backend's real response +shape: + +1. The backend exports the committed spec: `cd backend && npm run codegen:openapi` + (writes `backend/swagger/flowfi.openapi.json`). +2. The frontend regenerates the types from that spec: + `cd frontend && npm run codegen:api-types`. +3. CI runs `git diff --exit-code` over both artifacts. Any uncommitted change + means the spec or the generated types drifted and the job fails. + +When you change a backend response shape or route annotation, run both commands +above and commit the updated `flowfi.openapi.json` and `api-types.generated.ts` +alongside your change. If a change would alter the shape consumed by the +frontend, update `api-types.ts` and its call sites by hand too. + ### Running CI Checks Locally Before pushing, ensure your changes pass all the same checks that run in GitHub Actions. @@ -312,6 +332,14 @@ npm run test # Runs backend vitest suite ``` *Note: Backend tests require a running PostgreSQL instance and `DATABASE_URL` environment variable.* +Response-shape changes must also pass the OpenAPI drift check: + +```bash +cd backend && npm run codegen:openapi +cd ../frontend && npm run codegen:api-types +cd .. && git diff --exit-code -- backend/swagger/flowfi.openapi.json frontend/src/lib/api-types.generated.ts +``` + #### 3. Smart Contract Checks ```bash cd contracts @@ -446,6 +474,14 @@ Your PR must: - Be properly described - Stay focused (avoid large unrelated changes) +**PR checklist** + +- [ ] Frontend lint, tests, and build pass (`cd frontend && npm run lint && npm run test && npm run build`) +- [ ] Backend build and tests pass (`cd backend && npm run build && npm run test`) +- [ ] Contract build, `cargo fmt`, `cargo clippy`, and tests pass (`cd contracts`) +- [ ] If you touched a backend response shape or route annotations, the OpenAPI drift check is clean (`npm run codegen:openapi` in `backend/`, then `npm run codegen:api-types` in `frontend/`, then `git diff --exit-code`) and the regenerated `flowfi.openapi.json` / `api-types.generated.ts` are committed +- [ ] Related issues are linked with `Closes #NNN` + --- ## Code Review diff --git a/frontend/src/lib/api-types.ts b/frontend/src/lib/api-types.ts index 0ff354c2..c717add4 100644 --- a/frontend/src/lib/api-types.ts +++ b/frontend/src/lib/api-types.ts @@ -4,13 +4,20 @@ * backend/src/config/swagger.ts (JSDoc annotations on the route files feed * swagger-jsdoc, which builds the spec served at /api-docs.json). * - * An opt-in codegen script exists to help cross-check this file against the - * live spec without replacing it automatically: - * `npm run codegen:api-types` (in frontend/) runs openapi-typescript - * against a locally running backend's /api-docs.json and writes the result - * to src/lib/api-types.generated.ts. It is NOT wired into CI and does NOT - * replace these hand-written types/call sites — run it manually, diff the - * output against this file, and update this file by hand when they drift. + * The checked-in `api-types.generated.ts` is produced from the committed spec + * by `npm run codegen:api-types` (in frontend/), which runs openapi-typescript + * against backend/swagger/flowfi.openapi.json. That generated file is the + * machine-readable mirror of this hand-written contract and is enforced in CI: + * the backend job re-exports the OpenAPI spec and regenerates the types, then + * `git diff --exit-code` fails the build if either drifts. When this file + * changes, update the backend spec/annotations and regenerate with: + * + * cd backend && npm run codegen:openapi + * cd ../frontend && npm run codegen:api-types + * + * See the "API Types Drift Check" section of CONTRIBUTING.md. This file is + * still maintained by hand because the generated output does not replace these + * narrow, call-site-oriented interfaces. */ export interface BackendUser { id: string; From 3305b0075edf3be45ca42777dfd38a36c4554255 Mon Sep 17 00:00:00 2001 From: D-Lord-15 Date: Sun, 18 Oct 2026 03:10:10 +0000 Subject: [PATCH 4/4] fix(frontend): satisfy noUncheckedIndexedAccess in Stepper test next build's TypeScript pass failed with TS2532 on the indexed querySelectorAll accesses because tsconfig sets noUncheckedIndexedAccess. Use helpers that assert element presence and compare class lists with asymmetric matchers instead of raw indexing. --- frontend/src/components/ui/Stepper.test.tsx | 52 +++++++++++++++------ 1 file changed, 37 insertions(+), 15 deletions(-) diff --git a/frontend/src/components/ui/Stepper.test.tsx b/frontend/src/components/ui/Stepper.test.tsx index 21533fef..47a8b06e 100644 --- a/frontend/src/components/ui/Stepper.test.tsx +++ b/frontend/src/components/ui/Stepper.test.tsx @@ -4,6 +4,22 @@ import { Stepper } from "./Stepper"; const steps = ["Connect", "Configure", "Confirm"]; +/** Class names of every element matching `selector`, in document order. */ +function allClasses(container: HTMLElement, selector: string): string[] { + return Array.from(container.querySelectorAll(selector)).map( + (el) => el.className + ); +} + +/** The `index`-th match for `selector`, failing loudly if it is missing. */ +function nth(container: HTMLElement, selector: string, index: number): HTMLElement { + const el = container.querySelectorAll(selector)[index]; + if (!el) { + throw new Error(`expected at least ${index + 1} match(es) for "${selector}"`); + } + return el; +} + describe("Stepper", () => { it("renders every step inside an accessible Progress landmark", () => { render(); @@ -26,16 +42,17 @@ describe("Stepper", () => { it("applies completed/active/upcoming classes per step index", () => { const { container } = render(); - const circles = container.querySelectorAll(".stepper__circle"); - const labels = container.querySelectorAll(".stepper__label"); - - expect(circles[0].className).toContain("stepper__circle--completed"); - expect(circles[1].className).toContain("stepper__circle--active"); - expect(circles[2].className).toContain("stepper__circle--upcoming"); - expect(labels[0].className).toContain("stepper__label--completed"); - expect(labels[1].className).toContain("stepper__label--active"); - expect(labels[2].className).toContain("stepper__label--upcoming"); + expect(allClasses(container, ".stepper__circle")).toEqual([ + expect.stringContaining("stepper__circle--completed"), + expect.stringContaining("stepper__circle--active"), + expect.stringContaining("stepper__circle--upcoming"), + ]); + expect(allClasses(container, ".stepper__label")).toEqual([ + expect.stringContaining("stepper__label--completed"), + expect.stringContaining("stepper__label--active"), + expect.stringContaining("stepper__label--upcoming"), + ]); }); it("adds the matching screen-reader suffix to each step", () => { @@ -47,12 +64,17 @@ describe("Stepper", () => { it("shows a check icon for completed steps and a number otherwise", () => { const { container } = render(); - const circles = container.querySelectorAll(".stepper__circle"); - expect(circles[0].querySelector("svg")).not.toBeNull(); - expect(circles[1].querySelector("svg")).toBeNull(); - expect(circles[2].querySelector("svg")).toBeNull(); - expect(circles[1].querySelector(".stepper__number")?.textContent).toBe("2"); - expect(circles[2].querySelector(".stepper__number")?.textContent).toBe("3"); + expect(nth(container, ".stepper__circle", 0).querySelector("svg")).not.toBeNull(); + expect(nth(container, ".stepper__circle", 1).querySelector("svg")).toBeNull(); + expect(nth(container, ".stepper__circle", 2).querySelector("svg")).toBeNull(); + expect( + nth(container, ".stepper__circle", 1).querySelector(".stepper__number") + ?.textContent + ).toBe("2"); + expect( + nth(container, ".stepper__circle", 2).querySelector(".stepper__number") + ?.textContent + ).toBe("3"); }); });