Skip to content

Validate affiliate application notes - #146

Merged
ralyodio merged 1 commit into
profullstack:masterfrom
absalonCRC:fix-affiliate-application-note-validation
May 23, 2026
Merged

Validate affiliate application notes#146
ralyodio merged 1 commit into
profullstack:masterfrom
absalonCRC:fix-affiliate-application-note-validation

Conversation

@absalonCRC

@absalonCRC absalonCRC commented May 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Why

The affiliate apply endpoint passed body.note || null straight into the insert payload. Malformed clients could send objects, arrays, or numbers and reach the database write path instead of getting a clear validation error.

Validation

  • pnpm test:run src/app/api/affiliates/offers/[id]/apply/route.test.ts
  • pnpm exec eslint src/app/api/affiliates/offers/[id]/apply/route.ts src/app/api/affiliates/offers/[id]/apply/route.test.ts
  • pnpm type-check
  • git diff --check

Payment for the active uGig affiliate testing bounty can go to SOL: 27sdMYXofqoM9qR13bZhccRNYeEgYn5EoHXTSJn4QWKP.

Payment fallback: PayPal cultofrozen@gmail.com

@greptile-apps

greptile-apps Bot commented May 23, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds input validation to the affiliate apply endpoint's note field: non-string values (objects, arrays, numbers) now return a 400 before any DB write, and string notes are trimmed with blank ones stored as null instead of an empty string.

  • route.ts: inserts a type guard and normalization step for body.note between the profile lookup and the affiliate_applications insert; also cleans up import order and removes the now-redundant eslint-disable comment above type AnySupabase = any.
  • route.test.ts: new test file with two scenarios — rejection of a non-string note and confirmation that a whitespace-only note is stored as null; the rejection test does not assert that the insert spy was never called, leaving a small gap between its stated goal and what is actually verified.

Confidence Score: 4/5

Safe to merge — the validation logic in route.ts is correct and the happy-path and normalization behaviour are well tested.

The route change is small and correct: the type guard fires before any DB write and the trim/null normalization is straightforward. The only gap is in the test file — the rejection test does not assert that the database insert was never called, so a future regression that accidentally reached the insert would not be caught by this test alone.

route.test.ts — the first test case would benefit from asserting the insert spy was not called.

Important Files Changed

Filename Overview
src/app/api/affiliates/offers/[id]/apply/route.ts Adds correct note validation (type guard + trim/null normalization) before the DB insert; import order cleaned up and eslint-disable comment removed from the AnySupabase type alias.
src/app/api/affiliates/offers/[id]/apply/route.test.ts New test file covering the two validation scenarios; the rejection test is missing an assertion that insert was not called, leaving a gap between the test name and what is actually verified.

Sequence Diagram

sequenceDiagram
    participant Client
    participant POST as POST /apply
    participant DB as Supabase

    Client->>POST: "POST { note: ... }"
    POST->>DB: fetch affiliate_offer
    DB-->>POST: offer data
    POST->>DB: check existing application
    DB-->>POST: null (not applied)
    POST->>DB: fetch profile (username)
    DB-->>POST: profile

    alt note is non-string (object / array / number)
        POST-->>Client: "400 { error: note must be a string }"
    else note is string
        Note over POST: trim → blank → null
        POST->>DB: insert affiliate_application (normalizedNote)
        DB-->>POST: application record
        POST->>DB: increment total_affiliates
        POST->>DB: insert notification
        POST-->>Client: "201 { application, tracking_code, tracking_url }"
    end
Loading

Reviews (1): Last reviewed commit: "Validate affiliate application notes" | Re-trigger Greptile

Comment on lines +87 to +88
insert: vi.fn(),
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Test title claims "before creating" but never verifies the insert was skipped

The test is named "rejects non-string notes before creating an application", yet the insert spy is created anonymously with vi.fn() and no reference is kept, making it impossible to assert insert was not called. Contrast this with the second test (line 113) where insert is captured and asserted. If the validation guard were accidentally removed, this test would still pass as long as the 400 status was somehow returned — the DB write side-effect would go undetected.

Suggested change
insert: vi.fn(),
};
if (table === "affiliate_applications") {
const insertSpy = vi.fn();
return {
select: () => ({
eq: () => ({
eq: () => ({
single: () => Promise.resolve({ data: null, error: null }),
}),
}),
}),
insert: insertSpy,
};
}

@ralyodio
ralyodio merged commit 4acd43a into profullstack:master May 23, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: affiliate applications accept non-string notes

2 participants