Conversation
Add send, emails, scheduled, batch commands and fill gaps in keys/webhooks: - email send: single / scheduled (--scheduled-at) / batch (--batch-file), authenticated via --api-key or $ZSEND_API_KEY - email emails list/get: query sending records with --status/--job-type/--job-id filters - email scheduled list/get/cancel: manage scheduled emails (REST) - email batch list/get: manage batch email jobs (REST) - email keys get: retrieve API key details (GraphQL) - email webhooks get: retrieve webhook details (GraphQL) New packages: - pkg/api/zsend_rest.go: REST HTTP client for Z-Send API endpoints - pkg/model/zsend.go: REST request/reply types with Table display support Made-with: Cursor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (5)
WalkthroughAdds Z-Send email features: CLI commands for send (immediate/scheduled/batch), emails list/get, scheduled email management, batch job management, API key/webhook retrieval, plus REST client implementations and new request/response models. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as CLI (Cobra)
participant API as ApiClient
participant ZServer as Z‑Send REST
User->>CLI: invoke command (send/list/get/...)
CLI->>CLI: validate flags / prompt (interactive)
CLI->>API: call method (e.g., SendZSendEmail / ListZSendEmails)
API->>ZServer: HTTP request to /api/v1/zsend/...
ZServer-->>API: HTTP response (200 or error)
API-->>CLI: parsed model or error
CLI->>User: print JSON or formatted table / error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 5
🧹 Nitpick comments (3)
internal/cmd/email/batch/get/get.go (1)
32-41: Extract API-key resolution/validation into a shared helper.Lines 33-41 duplicate logic that now exists in multiple new email commands. Centralizing this in
cmdutilwould reduce drift and keep validation/error messaging consistent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cmd/email/batch/get/get.go` around lines 32 - 41, Extract the API-key resolution and validation logic from runGet into a shared helper in cmdutil (e.g., func ResolveAPIKey(provided string) (string, error) or GetValidatedAPIKey) that: 1) uses the provided value or falls back to os.Getenv("ZSEND_API_KEY"), 2) returns the same error text when empty ("Z-Send API key is required (--api-key or ZSEND_API_KEY)"), and 3) enforces the "zs_" prefix with the same error ("invalid API key format: must start with zs_"); then replace the duplicated block in runGet to call this new helper (passing apiKey) and handle the returned key/error.internal/cmd/email/send/send.go (1)
168-183: Consider validating individual batch entries locally.The batch validation checks for empty arrays and the 100-item limit, but doesn't validate that each email entry has required fields (
from,to,subject,html/text). While the API will reject invalid entries, local validation would provide faster feedback and clearer error messages.🔧 Optional: Add local validation for batch entries
if len(emails) > 100 { return fmt.Errorf("batch file contains %d emails; maximum is 100", len(emails)) } + for i, e := range emails { + if e.From == "" || len(e.To) == 0 || e.Subject == "" || (e.HTML == "" && e.Text == "") { + return fmt.Errorf("email at index %d is missing required fields (from, to, subject, html/text)", i) + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cmd/email/send/send.go` around lines 168 - 183, The runBatch function reads and unmarshals a batch file into []model.ZSendSendEmailRequest but only checks array length; add per-item validation to fail fast with clear errors: iterate the emails slice after unmarshalling (in runBatch, using the existing emails variable) and for each model.ZSendSendEmailRequest validate presence of required fields (from, to, subject, and at least one of html or text), collecting or returning the first descriptive error that includes the item index and missing fields; ensure the error messages reference opts.batchFile and keep the existing size checks intact.pkg/api/zsend_rest.go (1)
39-43: Consider reusing the HTTP client instead of creating a new one per request.Creating a new
http.Clientfor each request prevents connection reuse and increases latency. The*clientreceiver likely already has an HTTP client that could be reused, or a package-level client could be shared.♻️ Suggested refactor: Use a shared HTTP client
+var zsendHTTPClient = &http.Client{Timeout: 30 * time.Second} + func zsendDo(ctx context.Context, apiKey, method, path string, body any) ([]byte, int, error) { // ... body construction ... - c := &http.Client{Timeout: 30 * time.Second} - resp, err := c.Do(req) + resp, err := zsendHTTPClient.Do(req) if err != nil { return nil, 0, fmt.Errorf("do request: %w", err) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/api/zsend_rest.go` around lines 39 - 43, The code creates a new http.Client with `c := &http.Client{Timeout: 30 * time.Second}` and calls `c.Do(req)`, preventing connection reuse; instead reuse an existing shared client on the receiver (the `*client` type) or a package-level client: remove the per-request `&http.Client{...}` allocation and call `Do(req)` on the receiver's HTTP client field (or a singleton `httpClient`) that was initialized once with the desired Timeout to enable connection reuse and keep the same `Do(req)` call site.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/cmd/email/batch/list/list.go`:
- Around line 37-38: The runList function should validate pagination inputs
before calling the API: check opts.page is >= 1 and opts.perPage is between 1
and 100 (inclusive), and return a user-facing error (or exit) if values are out
of range; update the validation in the same function that handles listing
(runList) and reference opts.page and opts.perPage so invalid values like 0 or
1000 are rejected before the API call (do not rely on server-side validation).
In `@internal/cmd/email/emails/get/get.go`:
- Around line 15-20: Replace the strict argument check and add an interactive
fallback: change the cobra.ExactArgs(1) usage to allow 0 or 1 args (e.g.,
cobra.MaximumNArgs(1) or cobra.RangeArgs(0,1)) and modify the RunE handler (the
anonymous func and/or runGet) to detect when args length == 0, prompt the user
to select or enter an ID interactively, then call runGet(f, id) with the chosen
value; ensure any provided flag values still skip the prompt so non-interactive
usage is unchanged.
In `@internal/cmd/email/scheduled/cancel/cancel.go`:
- Around line 18-24: The cancel command currently calls runCancel directly and
performs a destructive action without user confirmation; update the Cobra
command in cancel.go so interactive runs prompt the user before proceeding and
only the non-interactive path skips the prompt. Concretely: change RunE to
detect interactive vs non-interactive (terminal/TTY or a --yes/--force flag)
and, for interactive runs, prompt for confirmation and only call the
cancellation when confirmed; for non-interactive runs call a new
runCancelNonInteractive (or similarly named) that performs the operation without
prompting. Keep the existing runCancel logic but refactor so there is a clear
interactive confirmation flow that references runCancel (or delegates to
runCancelNonInteractive) to execute the cancellation.
In `@pkg/api/zsend_rest.go`:
- Around line 127-137: The GetZSendScheduledEmail function (and other similar
functions constructing paths like "/emails/scheduled/"+id) currently
concatenates the id into the URL path unescaped; update these call sites (e.g.,
GetZSendScheduledEmail, and the other functions around lines 139-142 and 171-181
that build paths with id) to call url.PathEscape(id) before concatenation so
special characters are percent-encoded, ensuring correct routing and preventing
path injection issues.
In `@pkg/model/zsend.go`:
- Around line 304-453: The new REST DTOs in pkg/model (ZSendAttachment,
ZSendSendEmailRequest, ZSendSendEmailReply, ZSendScheduleEmailRequest,
ZSendScheduleEmailReply, ZSendBatchEmailRequest, ZSendBatchEmailReply,
ZSendScheduledEmail, ZSendListScheduledEmailsReply, ZSendScheduledEmails,
ZSendBatchJob, ZSendListBatchJobsReply, ZSendBatchJobs) violate the pkg/model
guideline; move these REST-specific structs out of pkg/model into a
REST-specific package/file (e.g., pkg/zsend or pkg/api/rest) or into an existing
REST DTO package, update any imports/usages to point to the new package, and
ensure no graphql struct tags are added in pkg/model; if you instead must keep
representations in pkg/model then convert them to use graphql:"fieldName" tags
and only include fields that exist in the backend GraphQL schema (matching
names/types) before merging.
---
Nitpick comments:
In `@internal/cmd/email/batch/get/get.go`:
- Around line 32-41: Extract the API-key resolution and validation logic from
runGet into a shared helper in cmdutil (e.g., func ResolveAPIKey(provided
string) (string, error) or GetValidatedAPIKey) that: 1) uses the provided value
or falls back to os.Getenv("ZSEND_API_KEY"), 2) returns the same error text when
empty ("Z-Send API key is required (--api-key or ZSEND_API_KEY)"), and 3)
enforces the "zs_" prefix with the same error ("invalid API key format: must
start with zs_"); then replace the duplicated block in runGet to call this new
helper (passing apiKey) and handle the returned key/error.
In `@internal/cmd/email/send/send.go`:
- Around line 168-183: The runBatch function reads and unmarshals a batch file
into []model.ZSendSendEmailRequest but only checks array length; add per-item
validation to fail fast with clear errors: iterate the emails slice after
unmarshalling (in runBatch, using the existing emails variable) and for each
model.ZSendSendEmailRequest validate presence of required fields (from, to,
subject, and at least one of html or text), collecting or returning the first
descriptive error that includes the item index and missing fields; ensure the
error messages reference opts.batchFile and keep the existing size checks
intact.
In `@pkg/api/zsend_rest.go`:
- Around line 39-43: The code creates a new http.Client with `c :=
&http.Client{Timeout: 30 * time.Second}` and calls `c.Do(req)`, preventing
connection reuse; instead reuse an existing shared client on the receiver (the
`*client` type) or a package-level client: remove the per-request
`&http.Client{...}` allocation and call `Do(req)` on the receiver's HTTP client
field (or a singleton `httpClient`) that was initialized once with the desired
Timeout to enable connection reuse and keep the same `Do(req)` call site.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b7d4aa9b-0555-4ee8-b19f-8cc7b1ce1e7d
📒 Files selected for processing (19)
internal/cmd/email/batch/batch.gointernal/cmd/email/batch/get/get.gointernal/cmd/email/batch/list/list.gointernal/cmd/email/email.gointernal/cmd/email/emails/emails.gointernal/cmd/email/emails/get/get.gointernal/cmd/email/emails/list/list.gointernal/cmd/email/keys/get/get.gointernal/cmd/email/keys/keys.gointernal/cmd/email/scheduled/cancel/cancel.gointernal/cmd/email/scheduled/get/get.gointernal/cmd/email/scheduled/list/list.gointernal/cmd/email/scheduled/scheduled.gointernal/cmd/email/send/send.gointernal/cmd/email/webhooks/get/get.gointernal/cmd/email/webhooks/webhooks.gopkg/api/interface.gopkg/api/zsend_rest.gopkg/model/zsend.go
- Switch all get/cancel commands from ExactArgs(1) to --id flag with interactive/non-interactive split (matching existing domain/key patterns) - Add confirmation prompt to scheduled cancel in interactive mode - Add pagination bounds validation to emails/scheduled/batch list commands - Apply url.PathEscape to all REST ID path segments Made-with: Cursor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
internal/cmd/email/batch/list/list.go (1)
36-36: Consider validating the--statusflag value client-side.The flag description lists valid values (
pending/processing/completed/failed), but invalid values will pass through to the API. Client-side validation would provide faster, clearer feedback.♻️ Proposed validation
func runList(f *cmdutil.Factory, opts Options) error { if opts.page < 1 { return fmt.Errorf("page must be >= 1") } if opts.perPage < 1 || opts.perPage > 100 { return fmt.Errorf("per-page must be between 1 and 100") } + if opts.status != "" { + validStatuses := map[string]bool{"pending": true, "processing": true, "completed": true, "failed": true} + if !validStatuses[opts.status] { + return fmt.Errorf("invalid status %q: must be one of pending, processing, completed, failed", opts.status) + } + } if opts.apiKey == "" {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cmd/email/batch/list/list.go` at line 36, The CLI accepts any value for the --status flag (set via cmd.Flags().StringVar(&opts.status, ...)) but the API only supports pending/processing/completed/failed; add client-side validation in the command's execution path (e.g., in the command's RunE or PreRunE handler) to check opts.status against the allowed set {"pending","processing","completed","failed"} and return a user-facing error if it's not one of those values so invalid inputs are rejected before calling the API.internal/cmd/email/emails/list/list.go (1)
78-86: Consider adding feedback for empty results.When there are no emails, the table output will display only headers with no rows. Consider printing a message like "No email records found" to provide clearer feedback to the user.
💡 Proposed enhancement
if f.JSON { if len(emails) == 0 { return f.Printer.JSON([]any{}) } return f.Printer.JSON(emails) } + if len(emails) == 0 { + fmt.Fprintln(f.Out, "No email records found") + return nil + } + f.Printer.Table(emails.Header(), emails.Rows()) return nil🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cmd/email/emails/list/list.go` around lines 78 - 86, When f.JSON is false and the emails slice is empty, the current code calls f.Printer.Table(emails.Header(), emails.Rows()) which shows only headers; modify the non-JSON branch to detect len(emails) == 0 and emit a clear user-facing message (e.g. "No email records found") instead of or in addition to the empty table. Update the logic around f.JSON, f.Printer.Table, emails.Header(), and emails.Rows() to either print the message via the printer (e.g. an info/notice method) or render a single-row table with that message so users get explicit feedback when no emails are returned.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/cmd/email/emails/get/get.go`:
- Around line 20-26: The cobra Command for the "get" subcommand ignores
positional args (RunE uses runGet(f, opts) and drops args), so add an argument
validator by setting the Command's Args field to cobra.NoArgs on the
cobra.Command literal (the same struct that defines Use, Short, RunE) to cause
the command to fail fast on unexpected positional arguments; keep RunE calling
runGet(f, opts) unchanged.
In `@internal/cmd/email/webhooks/get/get.go`:
- Around line 49-57: paramCheck is currently allowing whitespace-only webhook
IDs; update validation to trim the webhook ID before checking for emptiness so
strings like " " are rejected. Specifically, inside paramCheck (the function
used by runGetNonInteractive and the interactive path that calls getWebhook),
call strings.TrimSpace on the Options.WebhookID (or the field name used for the
webhook identifier) and validate the trimmed result, returning an error when
empty; keep callers (runGetNonInteractive, getWebhook caller) unchanged.
---
Nitpick comments:
In `@internal/cmd/email/batch/list/list.go`:
- Line 36: The CLI accepts any value for the --status flag (set via
cmd.Flags().StringVar(&opts.status, ...)) but the API only supports
pending/processing/completed/failed; add client-side validation in the command's
execution path (e.g., in the command's RunE or PreRunE handler) to check
opts.status against the allowed set
{"pending","processing","completed","failed"} and return a user-facing error if
it's not one of those values so invalid inputs are rejected before calling the
API.
In `@internal/cmd/email/emails/list/list.go`:
- Around line 78-86: When f.JSON is false and the emails slice is empty, the
current code calls f.Printer.Table(emails.Header(), emails.Rows()) which shows
only headers; modify the non-JSON branch to detect len(emails) == 0 and emit a
clear user-facing message (e.g. "No email records found") instead of or in
addition to the empty table. Update the logic around f.JSON, f.Printer.Table,
emails.Header(), and emails.Rows() to either print the message via the printer
(e.g. an info/notice method) or render a single-row table with that message so
users get explicit feedback when no emails are returned.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bb4aed53-280c-4379-8b24-6dc50a173810
📒 Files selected for processing (10)
internal/cmd/email/batch/get/get.gointernal/cmd/email/batch/list/list.gointernal/cmd/email/emails/get/get.gointernal/cmd/email/emails/list/list.gointernal/cmd/email/keys/get/get.gointernal/cmd/email/scheduled/cancel/cancel.gointernal/cmd/email/scheduled/get/get.gointernal/cmd/email/scheduled/list/list.gointernal/cmd/email/webhooks/get/get.gopkg/api/zsend_rest.go
✅ Files skipped from review due to trivial changes (1)
- pkg/api/zsend_rest.go
🚧 Files skipped from review as they are similar to previous changes (4)
- internal/cmd/email/batch/get/get.go
- internal/cmd/email/keys/get/get.go
- internal/cmd/email/scheduled/cancel/cancel.go
- internal/cmd/email/scheduled/list/list.go
- Add cobra.NoArgs to all new get commands to reject unexpected positional args - Apply strings.TrimSpace in all paramCheck functions to reject whitespace-only IDs - Ignored REST DTO in pkg/model finding: existing models (service.go etc.) already use json tags extensively, no pure-graphql constraint exists Made-with: Cursor
yuaanlin
deleted the
bruce/pla-933-cli-complete-zeabur-email-z-send-commands
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
email sendcommand supporting single / scheduled (--scheduled-at) / batch (--batch-file) sending, authenticated via--api-keyor$ZSEND_API_KEYemail emails list/getfor querying sending records with--status/--job-type/--job-idfiltersemail scheduled list/get/cancelfor managing scheduled emailsemail batch list/getfor managing batch email jobsemail keys getandemail webhooks getto fill existing gapsImplementation notes
zs_prefixed Z-Send API key; a newpkg/api/zsend_rest.goimplements a lightweight HTTP client for theseemail scheduled listandemail batch listare intentional convenience aliases foremail emails list --job-type scheduled/batch, but expose dedicated REST endpoints with type-specific fields (e.g.attempts,last_errorfor scheduled; job-level stats for batch)Test plan
go build ./...passes--helpoutputs verified locallyCloses PLA-933
Made with Cursor
Summary by CodeRabbit