Handle affiliate approval notification failures#204
Conversation
Greptile SummaryThis PR hardens the affiliate application approval/rejection endpoint by treating notification delivery as best-effort and rejecting malformed or non-object request bodies before any database work occurs.
Confidence Score: 5/5Safe to merge — the change is narrowly scoped to input validation and notification error handling, both of which improve resilience without altering the successful status-update path. All three new failure modes (malformed JSON, non-object body, failed notification insert) are guarded correctly and covered by tests. The status-update path is unchanged. No auth or data-integrity regressions were introduced. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant C as Client
participant R as PATCH Route
participant DB as Supabase (DB)
participant N as Notifications Table
C->>R: PATCH /api/affiliates/offers/[id]/applications
R->>R: Parse JSON body (try/catch → 400 on failure)
R->>R: Validate body is non-null object (→ 400)
R->>R: "Validate application_id (string) and action (approve|reject) (→ 400)"
R->>DB: Verify seller ownership (affiliate_offers)
DB-->>R: offer or null
alt not owner
R-->>C: 404 Not found or not authorized
end
R->>DB: Update affiliate_applications (status, updated_at, approved_at)
DB-->>R: updated application or error
alt update error
R-->>C: 400 error.message
end
Note over R,N: Status already committed — notification is best-effort
R->>N: insert notification (try/catch)
alt notification error / throw
R->>R: console.warn (swallow error)
end
R-->>C: "200 { application }"
Reviews (3): Last reviewed commit: "Validate affiliate application status bo..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| it("still returns the updated application when notification insert fails", async () => { | ||
| mockGetAuthContext.mockResolvedValue({ | ||
| user: { id: "seller-1", authMethod: "session" }, | ||
| }); | ||
|
|
||
| const notificationInsert = vi.fn().mockResolvedValue({ | ||
| error: { message: "notification table unavailable" }, | ||
| }); | ||
| const consoleWarn = vi.spyOn(console, "warn").mockImplementation(() => {}); | ||
|
|
||
| mockFrom.mockImplementation((table: string) => { | ||
| if (table === "affiliate_offers") { | ||
| return chainable({ id: "offer-1", seller_id: "seller-1" }); | ||
| } | ||
|
|
||
| if (table === "affiliate_applications") { | ||
| return chainable({ | ||
| id: "app-1", | ||
| affiliate_id: "affiliate-1", | ||
| offer_id: "offer-1", | ||
| status: "approved", | ||
| profiles: { username: "alice" }, | ||
| }); | ||
| } | ||
|
|
||
| if (table === "notifications") { | ||
| return { insert: notificationInsert }; | ||
| } | ||
|
|
||
| return chainable(null); | ||
| }); | ||
|
|
||
| try { | ||
| const res = await PATCH( | ||
| makePatchRequest("offer-1", { | ||
| application_id: "app-1", | ||
| action: "approve", | ||
| }), | ||
| makeParams("offer-1") | ||
| ); | ||
|
|
||
| expect(res.status).toBe(200); | ||
| const body = await res.json(); | ||
| expect(body.application).toMatchObject({ | ||
| id: "app-1", | ||
| status: "approved", | ||
| }); | ||
| expect(notificationInsert).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| user_id: "affiliate-1", | ||
| type: "affiliate_approved", | ||
| data: { offer_id: "offer-1", application_id: "app-1" }, | ||
| }) | ||
| ); | ||
| expect(consoleWarn).toHaveBeenCalledWith( | ||
| "Failed to create affiliate application notification", | ||
| { message: "notification table unavailable" } | ||
| ); | ||
| } finally { | ||
| consoleWarn.mockRestore(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
The new notification wrapping has two distinct failure modes: notificationError returned in the result (tested here) and an exception thrown by .insert() itself (the catch (error) branch at line 140 of route.ts). Only the first path is covered. If .insert were to throw synchronously or reject, the catch branch would fire and the route would still return 200, but that invariant is never verified. A small additional test case — where notificationInsert rejects instead of resolving with an error — would complete the coverage for this new behavior.
2 participants
Fixes #203.
Fixes #163.
Summary
uGig task
Submitted for the active uGig repo testing task: https://ugig.net/gigs/4741218f-a723-46bb-82cb-6516120331ae
No payout details included; those can be provided after acceptance.
Verification