Validate affiliate offer edit text fields - #148
Conversation
Greptile SummaryThis PR adds type validation for
Confidence Score: 4/5The validation logic is correct and the new tests confirm the 400 paths work as intended; the change is safe to merge. The core fix is straightforward and well-scoped. The main gap is that no success-path test was added alongside the error-path tests, so a regression in the happy path would go undetected. The removed ESLint suppression comment is a minor side change that does not affect runtime behaviour. route.test.ts would benefit from at least one test verifying a valid string update reaches the DB and returns 200. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[PATCH /api/affiliates/offers/:id] --> B{Auth check}
B -->|no auth| C[401 Unauthorized]
B -->|authenticated| D{Offer exists and seller owns it?}
D -->|no| E[404 Not found or not authorized]
D -->|yes| F[Parse request body]
F --> G{title present?}
G -->|yes| H{typeof title === string?}
H -->|no| I[400 title must be a string]
H -->|yes| J[Add trimmed title to updateData]
G -->|no| K{description present?}
J --> K
K -->|yes| L{typeof description === string?}
L -->|no| M[400 description must be a string]
L -->|yes| N[Add trimmed description to updateData]
K -->|no| O[Assign remaining fields]
N --> O
O --> P[DB update]
P -->|error| Q[400 DB error]
P -->|success| R[200 offer]
Reviews (1): Last reviewed commit: "Validate affiliate offer edit text field..." | 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" } }); | ||
| }); | ||
|
|
||
| it("rejects non-string title updates", async () => { | ||
| mockFrom.mockReturnValue(chainable({ id: "offer1", seller_id: "seller1" })); | ||
|
|
||
| const res = await PATCH( | ||
| makePatchRequest("offer1", { title: 123 }), | ||
| makeParams("offer1") | ||
| ); | ||
| const body = await res.json(); | ||
|
|
||
| expect(res.status).toBe(400); | ||
| expect(body.error).toBe("title must be a string"); | ||
| }); | ||
|
|
||
| it("rejects non-string description updates", async () => { | ||
| mockFrom.mockReturnValue(chainable({ id: "offer1", seller_id: "seller1" })); | ||
|
|
||
| const res = await PATCH( | ||
| makePatchRequest("offer1", { description: { text: "not a string" } }), | ||
| makeParams("offer1") | ||
| ); | ||
| const body = await res.json(); | ||
|
|
||
| expect(res.status).toBe(400); | ||
| expect(body.error).toBe("description must be a string"); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The two new tests only exercise rejection (400) scenarios. There is no test that verifies a valid string title or description actually flows through to a DB update and returns 200. If the update logic were accidentally broken (e.g., updateData construction or the .update() call), both existing tests would still pass, leaving the regression undetected.
| import { createServiceClient } from "@/lib/supabase/service"; | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| type AnySupabase = any; |
There was a problem hiding this comment.
any usage
The // eslint-disable-next-line @typescript-eslint/no-explicit-any comment was deleted, but type AnySupabase = any still uses an explicit any. If the project ever tightens the @typescript-eslint/no-explicit-any rule to "error", this line will start failing lint without any obvious history of why the suppression existed.
2 participants
Summary
titleis present but not a stringdescriptionis present but not a stringCloses #147.
This is for the active uGig affiliate/invite testing task: 4741218f-a723-46bb-82cb-6516120331ae.
Payment address for the uGig SOL bounty, if accepted:
27sdMYXofqoM9qR13bZhccRNYeEgYn5EoHXTSJn4QWKPValidation
pnpm test:run 'src/app/api/affiliates/offers/[id]/route.test.ts'\n-pnpm exec eslint 'src/app/api/affiliates/offers/[id]/route.ts' 'src/app/api/affiliates/offers/[id]/route.test.ts'\n-pnpm type-check\n-git diff --check -- 'src/app/api/affiliates/offers/[id]/route.ts' 'src/app/api/affiliates/offers/[id]/route.test.ts'Payment fallback: PayPal cultofrozen@gmail.com