Added types for npm #15
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThe PR modifies inbox creation to accept email addresses instead of domain fields, adds a copy-to-clipboard feature for inbox IDs in the dashboard, updates SDK packaging configuration with entry points and exports, and introduces comprehensive TypeScript type definitions for the SendookAPI surface. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as POST /inboxes
participant Validator
participant DomainResolver
participant InboxService
Client->>API: Request with email
API->>Validator: Validate email format
alt Invalid Email
Validator-->>API: Validation error
API-->>Client: 400 Bad Request
else Valid Email
Validator-->>API: Extract domain part
API->>DomainResolver: Resolve domain by name + org
alt Domain Not Found
DomainResolver-->>API: Not found
API-->>Client: 404 Not Found
else Domain Found
DomainResolver-->>API: Return domainId
API->>InboxService: Create inbox (name, email, domainId)
InboxService-->>API: Inbox created
API-->>Client: 201 Created
end
end
sequenceDiagram
participant User
participant UI as Inbox Card
participant Clipboard
participant Component as State
User->>UI: Click Copy button
UI->>Component: copyInboxId(inbox)
Component->>Clipboard: navigator.clipboard.writeText(inboxId)
alt Copy Success
Clipboard-->>Component: Resolved
Component->>Component: Set copiedInboxId = inboxId
Component->>UI: Update button to "Copied"
Component->>Component: Schedule cleanup (2s)
alt Timeout fires
Component->>Component: Clear copiedInboxId
Component->>UI: Revert button to "Copy"
end
else Copy Failed
Clipboard-->>Component: Rejected
Component->>Component: Log error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
Comment |
![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)
app/app/pages/inboxes.vue (1)
402-418: Remove the invalidmax-widthdeclarationThe
frunit is a flexible fraction (), not a length type, so it cannot be used insidecalc()or combined with length/percentage values incalc()expressions. The linemax-width: calc(3 * 1fr + 3 * 1.5rem);is invalid and browsers will ignore it. Remove it as suggested, or replace it with a concrete value likemax-width: 960px;if width constraint is needed.
🧹 Nitpick comments (3)
api/routes/v1/inboxes/index.ts (1)
29-64: Email-to-domain inbox creation flow looks sound; consider tightening validationThe new flow of deriving
domainIdfromreq.body.emailand short‑circuiting with 400 (invalid email) and 404 (domain not found) is logically consistent and matches the updated payload{ name: string; email?: string }. PersistingdomainIdexists also makes sense.If you want to harden this further, two optional tweaks:
- Use
.isEmail()in the validator chain forbody("email")so obviously malformed addresses never reach the handler.- Normalize
domainName(e.g.,toLowerCase()) before callinggetVerifiedDomainByOrganizationIdAndNameto avoid case‑sensitive mismatches.node-sdk/package.json (1)
3-23: Dual ESM/CJS entry points look good; please verify the.d.ctsactually existsThe move to
"files": ["dist"]plusmodule/mainand conditionalexportsis aligned with standard Node dual‑package patterns.One thing to double‑check before publishing:
"require": { "types": "./dist/index.d.cts" }assumes your build emitsindex.d.cts. If the build only emitsindex.d.ts, thattypespath will be broken for consumers resolving types via theexportsmap.node-sdk/index.ts (1)
2-16: Public method typings align with the new interfaces; consider reusing the param types in signaturesTyping
apiKey,domain, andinboxasApiKeyMethods,DomainMethods, andInboxMethodscleanly exposes the SDK surface and matches the new declarations intypes/sendook-api.d.ts. The implementations also look structurally consistent with those interfaces.Since you already import the parameter types, you can tighten things up and avoid duplication by using them directly in the method signatures. For example:
- public apiKey: ApiKeyMethods = { - create: async ({ - name, - }: { - name: string; - }) => { + public apiKey: ApiKeyMethods = { + create: async ({ name }: CreateApiKeyParams) => { const response = await axios.post(`${this.apiUrl}/v1/api_keys`, { name }, { headers: { Authorization: `Bearer ${this.apiSecret}` }, }); return response.data; }, @@ - create: async ({ - name, - email, - }: { - name: string; - email?: string; - }) => { + create: async ({ name, email }: CreateInboxParams) => { @@ - send: async ({ - inboxId, - to, - cc, - bcc, - labels, - subject, - text, - html, - }: { - inboxId: string; - to: string[]; - cc?: string[]; - bcc?: string[]; - labels?: string[]; - subject: string; - text: string; - html: string; - }) => { + send: async ({ + inboxId, + to, + cc, + bcc, + labels, + subject, + text, + html, + }: SendMessageParams) => { @@ - reply: async ({ - inboxId, - messageId, - text, - html, - }: { - inboxId: string; - messageId: string; - text: string; - html: string; - }) => { + reply: async ({ + inboxId, + messageId, + text, + html, + }: ReplyMessageParams) => {Same pattern can be applied to the other API key and domain methods (
CreateDomainParams,GetDomainParams, etc.) if you’d like everything fully aligned.Also applies to: 27-265
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
api/routes/v1/inboxes/index.ts(2 hunks)app/app/pages/inboxes.vue(5 hunks)node-sdk/index.ts(4 hunks)node-sdk/package.json(1 hunks)node-sdk/types/sendook-api.d.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
node-sdk/index.ts (1)
node-sdk/types/sendook-api.d.ts (3)
ApiKeyMethods(56-61)DomainMethods(63-68)InboxMethods(82-89)
api/routes/v1/inboxes/index.ts (2)
api/middlewares/expressValidatorMiddleware.ts (1)
expressValidatorMiddleware(4-16)api/controllers/DomainController.ts (1)
getVerifiedDomainByOrganizationIdAndName(43-55)
🔇 Additional comments (1)
node-sdk/types/sendook-api.d.ts (1)
1-90: Type declaration surface is coherent and matches the SDK methodsThe new
sendook-api.d.tscleanly documents the SDK surface:
- Parameter interfaces (
Create*Params,SendMessageParams,ReplyMessageParams) line up with whatnode-sdk/index.tsexpects.- Method group interfaces (
ApiKeyMethods,DomainMethods,MessageMethods,ThreadMethods,InboxMethods) match the methods exposed onSendookAPI.Using
Promise<any>everywhere is fine for now; you can always tighten those return types later once the API response shapes are stable.
| <div class="key-secret"> | ||
| <span class="secret-label">ID</span> | ||
| <span class="secret-value">{{ inbox._id ?? '••••••••••••••••' }}</span> | ||
| <button type="button" class="button-copy" aria-label="Copy API key" @click="copyInboxId(inbox)"> | ||
| <span v-if="copiedInboxId === (inbox._id ?? inbox.email)">Copied</span> | ||
| <span v-else>Copy</span> | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
Copy‑ID UX is solid; fix “API key” wording and tighten copied state check
The clipboard implementation with guards and a 2s reset is clean. A couple of small inconsistencies:
- The button
aria-labeland the error message say “API key”, but the UI is clearly copying the inbox ID. copiedInboxIdis only ever set frominbox._id, so comparing against(inbox._id ?? inbox.email)is unnecessary and slightly confusing.
You can address both with a small tweak:
- <button type="button" class="button-copy" aria-label="Copy API key" @click="copyInboxId(inbox)">
- <span v-if="copiedInboxId === (inbox._id ?? inbox.email)">Copied</span>
+ <button type="button" class="button-copy" aria-label="Copy inbox ID" @click="copyInboxId(inbox)">
+ <span v-if="copiedInboxId === inbox._id">Copied</span>
<span v-else>Copy</span>
</button>- } catch (error) {
- console.error('Failed to copy API key', error);
+ } catch (error) {
+ console.error('Failed to copy inbox ID', error);
}Also applies to: 171-207
🤖 Prompt for AI Agents
In app/app/pages/inboxes.vue around lines 46 to 53 (and similarly at lines
171–207), the button aria-label and error text incorrectly reference "API key"
while the UI copies an inbox ID, and the copied state comparison mixes fallback
email causing unnecessary complexity; change the aria-label and any error text
to "Copy inbox ID" (or similar consistent wording), and simplify the copied
state check to compare only against inbox._id (remove the (inbox._id ??
inbox.email) fallback) and ensure copiedInboxId is set/reset consistently with
that value.
Summary by CodeRabbit
New Features
Improvements