Skip to content

Handle malformed affiliate conversion bodies - #162

Closed
morganschp wants to merge 2 commits into
profullstack:masterfrom
morganschp:fix-affiliate-conversion-invalid-json
Closed

Handle malformed affiliate conversion bodies#162
morganschp wants to merge 2 commits into
profullstack:masterfrom
morganschp:fix-affiliate-conversion-invalid-json

Conversation

@morganschp

Copy link
Copy Markdown
Contributor

Summary

  • parse manual affiliate conversion bodies with safeParseBody
  • return 400 Invalid request body for malformed JSON instead of falling through to the generic 500 handler
  • cover malformed JSON with a regression test that verifies recordConversion is not called

Fixes #159

Verification

  • corepack pnpm vitest run 'src/app/api/affiliates/offers/[id]/conversions/route.test.ts'\n- corepack pnpm type-check\n- git diff --check -- 'src/app/api/affiliates/offers/[id]/conversions/route.ts' 'src/app/api/affiliates/offers/[id]/conversions/route.test.ts'

@greptile-apps

greptile-apps Bot commented May 23, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes malformed JSON handling across all three mutation handlers (POST, PUT, DELETE) in the affiliate conversions route by replacing raw request.json() calls with safeParseBody, which catches parse errors and returns a structured null instead of throwing. A regression test for each handler verifies the 400 response and confirms downstream operations are not reached.

  • POST: safeParseBody replaces request.json(), with an early 400 return on null; note handling is also refactored to use noteText.
  • PUT / DELETE: Both handlers receive the same safeParseBody treatment, addressing the gap raised in a prior review of this route.
  • Tests: Three new describe blocks cover malformed JSON for POST, PUT, and DELETE, each asserting status 400, correct error text, and absence of DB side-effects.

Confidence Score: 5/5

Safe to merge — the change is a targeted defensive improvement with no new logic paths that could cause regressions.

All three mutation handlers now consistently guard against malformed JSON before touching the database, and each is covered by a dedicated regression test that confirms the 400 path and the absence of downstream DB calls. The safeParseBody utility is already used elsewhere in the codebase and its behavior is well-understood. No existing validated paths are altered.

No files require special attention.

Important Files Changed

Filename Overview
src/app/api/affiliates/offers/[id]/conversions/route.ts All three mutation handlers migrated from raw request.json() to safeParseBody with null-guard 400 responses; note handling in POST refactored to noteText variable.
src/app/api/affiliates/offers/[id]/conversions/route.test.ts Added makeRawPostRequest/makeRawRequest helpers and three malformed-JSON regression tests covering POST, PUT, and DELETE handlers.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Incoming Request] --> B[getAuthContext]
    B -->|No auth| C[401 Unauthorized]
    B -->|Authenticated| D[Query affiliate_offers]
    D -->|Not found or wrong owner| E[403/404]
    D -->|Offer owned by user| F[safeParseBody]
    F -->|Returns null| G[400 Invalid request body]
    F -->|Returns parsed object| H[Field Validation]
    H -->|Invalid fields| I[400 field error]
    H -->|Fields valid| J[DB Mutation / recordConversion]
    J --> K[200/201 Response]
Loading

Reviews (2): Last reviewed commit: 2a4ee9e | Re-trigger Greptile

Comment on lines +189 to 192
const noteText = typeof note === "string" ? note.trim() : null;
if (noteText) {
updateData.note = noteText;
}

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 When note is an empty string, noteText evaluates to "" (falsy), so the database update is correctly skipped — but the response still returns note: "" rather than note: null. Before this change, the response used note?.trim() || null, which coerced empty-string back to null. The new approach creates a subtle inconsistency between what the response claims and what is actually stored in the database.

Suggested change
const noteText = typeof note === "string" ? note.trim() : null;
if (noteText) {
updateData.note = noteText;
}
const noteText = typeof note === "string" && note.trim() ? note.trim() : null;
if (noteText) {
updateData.note = noteText;
}

@ralyodio ralyodio closed this May 23, 2026
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: manual affiliate conversion API returns 500 on malformed JSON

2 participants