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/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(); 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..47a8b06e --- /dev/null +++ b/frontend/src/components/ui/Stepper.test.tsx @@ -0,0 +1,80 @@ +import { describe, it, expect } from "vitest"; +import { render, screen } from "@testing-library/react"; +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(); + 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(); + + 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", () => { + 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(); + + 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"); + }); +}); 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;