Update api documentation examples and layout #28
Conversation
WalkthroughThe PR updates SDK usage and docs to import Sendook as a default export from Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant SDK as Sendook SDK
participant API as Sendook HTTP API
Dev->>SDK: import Sendook from "@sendook/node"\nconst client = new Sendook(apiKey)
Dev->>SDK: client.inbox.create(payload)
SDK->>API: POST /v1/inboxes
API-->>SDK: 201 Created (inbox object)
SDK-->>Dev: returns inbox
Dev->>SDK: client.inbox.message.send(inboxId, message)
SDK->>API: POST /v1/inboxes/{id}/messages
API-->>SDK: 200 OK (message object)
SDK-->>Dev: returns message
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🔇 Additional comments (1)
Comment |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
marcrupt
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
landing/content/1.docs/2.api/7.webhooks.md (1)
78-100: Critical: Webhook field naming inconsistency.Line 94 uses
"_id"while the create webhook response (line 50), list webhooks response (line 69), delete webhook response (line 136), and webhook attempts (line 162) all use"id". This inconsistency will confuse API consumers and contradicts the broader PR changes renaming_idtoid.Apply this diff to fix the field name:
### Response `200 OK` ```json { - "_id": "wh_01J3ZP8K9M2N3O4P5Q6R7S8T9U", + "id": "wh_01J3ZP8K9M2N3O4P5Q6R7S8T9U", "url": "https://example.com/webhooks/sendook", "events": ["message.received", "message.delivered"], "createdAt": "2024-10-10T19:30:15Z", "updatedAt": "2024-10-10T19:30:15Z" }
🧹 Nitpick comments (3)
landing/app/pages/docs/[...slug].vue (1)
54-54: Replaceanytype with proper type annotation.Using
anytype annotation bypasses TypeScript's type checking. Consider defining or importing a proper type for TOC links to maintain type safety.Apply this diff to improve type safety:
- <UContentToc :links="page.body.toc.links?.map((link: any) => ({ ...link, children: [] }))" /> + <UContentToc :links="page.body.toc.links?.map((link) => ({ ...link, children: [] }))" />Alternatively, if a specific type is available from the content/UI library:
// Add proper type import at the top import type { TocLink } from '#ui/types' // or wherever the type is defined- <UContentToc :links="page.body.toc.links?.map((link: any) => ({ ...link, children: [] }))" /> + <UContentToc :links="page.body.toc.links?.map((link: TocLink) => ({ ...link, children: [] }))" />Note: The transformation forces all TOC links to have empty children arrays, effectively flattening nested table of contents. If this is intentional for the layout update, consider adding a comment explaining this behavior.
landing/content/1.docs/2.api/5.messages.md (1)
33-52: Clarify required vs. optional attachment fields in table format.Line 52 documents attachments with mixed formatting. The inline description clarifies that
contentis required whilenameandcontentTypeare optional, but a table format like other complex fields would improve consistency and scannability.For improved consistency with other field documentation, consider expanding line 52 to a mini-table or formatted list:
-| `attachments` | object[] | No | Array of file attachments. Each attachment must include `content` (base64-encoded), optional `name` (filename), and optional `contentType` (MIME type). | +| `attachments` | object[] | No | Array of file attachments (see attachment schema below). | + +**Attachment schema:** +| Field | Type | Required | Description | +| ----- | ---- | -------- | ----------- | +| `content` | string | Yes | Base64-encoded file content. | +| `name` | string | No | Filename for the attachment. | +| `contentType` | string | No | MIME type of the attachment. |node-sdk/openapi.yaml (1)
54-56: Consider adding maxItems constraints to array responses.Per static analysis (CKV_OPENAPI_21), array responses should have a maximum item count to prevent excessive payloads. Currently, array responses for webhooks (line 54), webhook attempts (lines 140-142), domains (lines 269-271), DNS records (lines 352-354), inboxes (lines 399-401), messages (lines 518-520), and threads (lines 563-565) lack
maxItemsconstraints.This is a security hardening measure to prevent abuse. Consider adding
maxItemsto each array response. For example:responses: "200": description: A collection of webhooks content: application/json: schema: type: array + maxItems: 1000 items: $ref: '#/components/schemas/Webhook'Determine the appropriate
maxItemslimit for each endpoint based on your API's design (typically 100-1000 for list endpoints).Also applies to: 126-148, 258-277, 338-362
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
landing/app/pages/docs/[...slug].vue(1 hunks)landing/content/1.docs/2.api/2.api-keys.md(2 hunks)landing/content/1.docs/2.api/3.domains.md(1 hunks)landing/content/1.docs/2.api/4.inboxes.md(2 hunks)landing/content/1.docs/2.api/5.messages.md(3 hunks)landing/content/1.docs/2.api/6.threads.md(1 hunks)landing/content/1.docs/2.api/7.webhooks.md(4 hunks)node-sdk/openapi.yaml(16 hunks)
🧰 Additional context used
🪛 Checkov (3.2.334)
node-sdk/openapi.yaml
[medium] 54-58: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
🔇 Additional comments (3)
landing/content/1.docs/2.api/7.webhooks.md (1)
144-197: Well-documented new webhook attempts endpoint.The new "List webhook attempts" section is comprehensive, with clear path parameters, response structure, and detailed field descriptions. The attempt schema is well-defined and aligns with the OpenAPI specification.
landing/content/1.docs/2.api/3.domains.md (1)
1-206: Comprehensive domain API documentation.The new domains file is well-structured with clear endpoint documentation, proper formatting, and consistent response patterns aligned with the PR's API surface updates. All endpoints (create, list, retrieve, delete, verify, get DNS records) are properly documented with appropriate HTTP methods, paths, request/response bodies, and field descriptions.
node-sdk/openapi.yaml (1)
636-642: Well-structured webhook and domain schema additions.The new
WebhookCreateRequest,Webhook,WebhookAttempt,Attachment, andDnsRecordschemas are properly defined with clear field descriptions. TheWebhookIdparameter follows the existing parameter naming convention. Domain schema enhancements (addingverified,records,updatedAtfields) align with the documentation changes.Also applies to: 846-951
| "200": | ||
| description: Webhook deleted | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: '#/components/schemas/Webhook' |
There was a problem hiding this comment.
Webhook field naming inconsistency propagated to OpenAPI spec.
While the OpenAPI schemas correctly use id in the Webhook definition, the documentation for the "Retrieve webhook details" response in landing/content/1.docs/2.api/7.webhooks.md line 94 uses _id. This creates a mismatch between the specification and documentation. The OpenAPI spec is correct; the documentation needs to be fixed (see separate critical issue flag in webhooks.md review).
Verify that the actual webhook responses from the backend always use id (not _id) before merging this PR.
Also applies to: 221-226, 305-310, 435-440
ahmedmawiri
deleted the
marc/rup-892-docs-polishing-remove-unwanted-stuff
branch
Summary by CodeRabbit
Documentation
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.