Validate affiliate edit product URLs - #138
Conversation
Greptile SummaryThis PR adds URL validation to the
Confidence Score: 4/5Safe to merge — the core fix is correct and well-scoped; only test coverage for the clearing paths is missing. The validation logic itself is solid: all five input cases (null, non-string, blank, invalid scheme, valid URL) are handled correctly in the handler. The gap is that the null and blank-string clearing behaviors documented in the PR description are not exercised by any test, so a future regression there would go undetected. route.test.ts — the new PATCH describe block is missing tests for the null and whitespace-only clearing branches. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[PATCH /api/affiliates/offers/:id] --> B{Authenticated?}
B -- No --> C[401 Unauthorized]
B -- Yes --> D{Offer exists & owned?}
D -- No --> E[404 Not found]
D -- Yes --> F{product_url in body?}
F -- No --> K[Skip field]
F -- Yes --> G{value === null?}
G -- Yes --> H[Set product_url = null]
G -- No --> I{typeof string?}
I -- No --> J[400 must be a string]
I -- Yes --> L{trimmed length === 0?}
L -- Yes --> H
L -- No --> M{isValidUrl?}
M -- No --> N[400 must use http/https]
M -- Yes --> O[Set product_url = trimmed value]
H & K & O --> P[Run DB update]
P --> Q[200 OK with offer]
Reviews (1): Last reviewed commit: "Validate affiliate edit product URLs" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| describe("PATCH /api/affiliates/offers/[id]", () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| mockGetAuthContext.mockResolvedValue({ | ||
| user: { id: "seller1", authMethod: "session" }, | ||
| }); | ||
| }); | ||
|
|
||
| it("rejects non-http product_url updates", async () => { | ||
| mockFrom.mockImplementation((table: string) => { | ||
| if (table === "affiliate_offers") { | ||
| return chainable({ id: "offer-1", seller_id: "seller1" }); | ||
| } | ||
| return chainable([]); | ||
| }); | ||
|
|
||
| const req = new NextRequest("http://localhost/api/affiliates/offers/offer-1", { | ||
| method: "PATCH", | ||
| body: JSON.stringify({ product_url: "javascript:alert(1)" }), | ||
| }); | ||
|
|
||
| const res = await PATCH(req, makeParams("offer-1")); | ||
| const body = await res.json(); | ||
|
|
||
| expect(res.status).toBe(400); | ||
| expect(body.error).toBe("product_url must use http:// or https:// scheme"); | ||
| }); | ||
|
|
||
| it("trims valid product_url updates before saving", async () => { | ||
| mockFrom.mockImplementation((table: string) => { | ||
| if (table !== "affiliate_offers") return chainable([]); | ||
|
|
||
| const update = vi.fn((data: Record<string, unknown>) => ({ | ||
| eq: vi.fn(() => ({ | ||
| select: vi.fn(() => ({ | ||
| single: vi.fn(() => Promise.resolve({ | ||
| data: { id: "offer-1", seller_id: "seller1", ...data }, | ||
| error: null, | ||
| })), | ||
| })), | ||
| })), | ||
| })); | ||
|
|
||
| return { | ||
| select: vi.fn(() => ({ | ||
| eq: vi.fn(() => ({ | ||
| single: vi.fn(() => Promise.resolve({ | ||
| data: { id: "offer-1", seller_id: "seller1" }, | ||
| error: null, | ||
| })), | ||
| })), | ||
| })), | ||
| update, | ||
| }; | ||
| }); | ||
|
|
||
| const req = new NextRequest("http://localhost/api/affiliates/offers/offer-1", { | ||
| method: "PATCH", | ||
| body: JSON.stringify({ product_url: " https://example.com/product " }), | ||
| }); | ||
|
|
||
| const res = await PATCH(req, makeParams("offer-1")); | ||
| const body = await res.json(); | ||
|
|
||
| expect(res.status).toBe(200); | ||
| expect(body.offer.product_url).toBe("https://example.com/product"); | ||
| }); |
There was a problem hiding this comment.
The PR description calls out "allow explicit clearing with null or blank strings" as a key behavior, but neither branch is exercised. Sending { product_url: null } should persist null, and sending { product_url: " " } should also clear it — both paths exist in the handler (lines 104–111) but have no regression test. If a future refactor accidentally drops the === null guard or the length === 0 branch, nothing will catch it.
2 participants
Summary
product_urlupdates inPATCH /api/affiliates/offers/[id]instead of writing the raw request valuenullor blank stringsjavascript:URLs and preserving trimmed valid URLsFixes #137
Validation
pnpm test:run 'src/app/api/affiliates/offers/[id]/route.test.ts' src/lib/affiliates/validation.test.tspnpm exec eslint 'src/app/api/affiliates/offers/[id]/route.ts' 'src/app/api/affiliates/offers/[id]/route.test.ts' src/lib/affiliates/validation.ts src/lib/affiliates/validation.test.tspnpm type-checkgit diff --checkBounty / payment
Submitted for the active uGig affiliate-program testing task. SOL receive address:
27sdMYXofqoM9qR13bZhccRNYeEgYn5EoHXTSJn4QWKP.Payment fallback: PayPal cultofrozen@gmail.com