feat(doordash): add DoorDash Drive integration with 13 API operations#3838
feat(doordash): add DoorDash Drive integration with 13 API operations#3838waleedlatif1 wants to merge 1 commit intostagingfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryMedium Risk Overview Introduces an internal Next.js API proxy at Written by Cursor Bugbot for commit 10bfbb6. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| }), | ||
| ...(body.pickupTime && { pickup_time: body.pickupTime }), | ||
| ...(body.dropoffTime && { dropoff_time: body.dropoffTime }), | ||
| } |
There was a problem hiding this comment.
Duplicated request body construction across two operations
Low Severity
The create_quote and create_delivery cases build an identical ~30-line request body object with the same fields, same conditionals, and same type conversions. A bug fix or field addition in one would need to be manually replicated in the other. Extracting a shared helper (e.g. buildDeliveryRequestBody) would eliminate this duplication.
Additional Locations (1)
Greptile SummaryThis PR adds a complete DoorDash Drive integration with 13 API operations (delivery quotes, deliveries, businesses, and stores). Authentication is handled via HS256 JWT generation using the Confidence Score: 4/5Safe to merge after addressing the mutually exclusive pickupTime/dropoffTime fields, which would currently produce a DoorDash API error at runtime when both are set. The integration is structurally complete and follows project conventions. All 13 tools are registered, credentials use correct visibility, JWT generation is sound, and the executor correctly merges partial params. A new P1 finding adds a real user-facing failure path: both pickupTime and dropoffTime are shown simultaneously in the UI with no guard, but the DoorDash API rejects requests where both are set. apps/sim/blocks/blocks/doordash.ts (pickupTime/dropoffTime mutual exclusion, dropoffContactSendNotifications default) and apps/sim/app/api/tools/doordash/route.ts (open issues from prior review: NaN coercion, non-JSON error body). Important Files Changed
Sequence DiagramsequenceDiagram
participant UI as Block UI
participant Executor as Executor (generic-handler)
participant Tool as Tool Config
participant Route as /api/tools/doordash
participant DD as DoorDash API
UI->>Executor: Block inputs (operation, creds, fields)
Executor->>Executor: params() coerce orderValue/tip, clear empty dropdowns
Executor->>Tool: finalInputs = {...inputs, ...transformedParams}
Tool->>Route: POST /api/tools/doordash (operation + all params)
Route->>Route: Zod validate body
Route->>Route: generateJwt(developerId, keyId, signingSecret)
Route->>DD: API call (Bearer JWT)
DD-->>Route: JSON response
Route->>Route: extractDeliveryOutput / extractBusinessOutput / extractStoreOutput
Route-->>Tool: {success, output}
Tool->>Tool: transformResponse()
Tool-->>Executor: {success, output}
Executor-->>UI: Block output
Reviews (2): Last reviewed commit: "feat(doordash): add DoorDash Drive integ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const response = await fetch(url, fetchOptions) | ||
| const data = await response.json() | ||
|
|
||
| if (!response.ok) { | ||
| const errorMessage = | ||
| (data as Record<string, unknown>).message ?? | ||
| (data as Record<string, unknown>).error ?? | ||
| `DoorDash API error (${response.status})` | ||
| logger.error('DoorDash API error', { status: response.status, error: errorMessage }) | ||
| return NextResponse.json( | ||
| { success: false, error: String(errorMessage) }, | ||
| { status: response.status } | ||
| ) | ||
| } |
There was a problem hiding this comment.
response.json() called before response.ok check
response.json() is called unconditionally before the !response.ok guard. If DoorDash returns a non-JSON body (e.g., HTML on a 502/503 gateway error), response.json() will throw and control jumps to the outer catch, which returns a generic "Failed to communicate with DoorDash API" 500 — discarding the original HTTP status code and any meaningful error detail.
Consider reading the body as text first, then conditionally parsing as JSON:
const text = await response.text()
if (!response.ok) {
let errorMessage: string
try {
const errData = JSON.parse(text) as Record<string, unknown>
errorMessage = String(
errData.message ?? errData.error ?? `DoorDash API error (${response.status})`
)
} catch {
errorMessage = text || `DoorDash API error (${response.status})`
}
logger.error('DoorDash API error', { status: response.status, error: errorMessage })
return NextResponse.json(
{ success: false, error: errorMessage },
{ status: response.status }
)
}
const data = JSON.parse(text) as Record<string, unknown>| dropoff_address: body.dropoffAddress, | ||
| dropoff_phone_number: body.dropoffPhoneNumber, | ||
| dropoff_business_name: body.dropoffBusinessName, | ||
| order_value: Number(body.orderValue), |
There was a problem hiding this comment.
Number() cast without numeric validation — may produce NaN
Number(body.orderValue) is called without first validating that body.orderValue is a numeric string. If a user (or an LLM) passes a non-numeric value like "free", Number(...) returns NaN, which is serialised as null in JSON — causing a silent data issue in the DoorDash request body.
The same applies to Number(body.tip) on lines 185, 214, 234, and 269.
Consider adding a numeric refinement to the Zod schema:
orderValue: z.string().regex(/^\d+$/, 'Must be a whole number in cents').optional(),
tip: z.string().regex(/^\d+$/, 'Must be a whole number in cents').optional(),| id: 'dropoffContactSendNotifications', | ||
| title: 'Send SMS Notifications', | ||
| type: 'dropdown', | ||
| options: [ | ||
| { label: 'Yes', id: 'true' }, | ||
| { label: 'No', id: 'false' }, | ||
| ], | ||
| value: () => 'true', | ||
| mode: 'advanced', | ||
| condition: { field: 'operation', value: ['create_quote', 'create_delivery'] }, | ||
| }, |
There was a problem hiding this comment.
dropoffContactSendNotifications defaults to 'true' — may send unexpected SMS
The dropdown defaults to value: () => 'true', meaning SMS notifications to the recipient are enabled unless the user explicitly disables them. Unlike the other boolean dropdowns (contactlessDropoff, dropoffRequiresSignature) which default to '' (off), this one opts users in by default. Because it lives under mode: 'advanced', users who never open the advanced section will unknowingly trigger SMS notifications on every delivery.
If "opt-out" is preferred, change the default:
| id: 'dropoffContactSendNotifications', | |
| title: 'Send SMS Notifications', | |
| type: 'dropdown', | |
| options: [ | |
| { label: 'Yes', id: 'true' }, | |
| { label: 'No', id: 'false' }, | |
| ], | |
| value: () => 'true', | |
| mode: 'advanced', | |
| condition: { field: 'operation', value: ['create_quote', 'create_delivery'] }, | |
| }, | |
| value: () => '', |
…and add a { label: 'Default', id: '' } option so users can revert to the API default explicitly.
1 participant
Summary
Type of Change
Testing
Tested manually
Checklist