[smsgate] add support for scheduled messages#31
Merged
Conversation
|
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 ignored due to path filters (1)
📒 Files selected for processing (10)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds a --schedule-at RFC3339 timestamp flag (future-time validation), centralizes CLI category labels into constants, updates go.mod dependency versions, and propagates ScheduleAt into single and batch message send payloads. ChangesMessage Scheduling and CLI Organization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🤖 Pull request artifacts
|
Member
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
Member
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
Member
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
Member
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
Member
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
Member
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![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
🧹 Nitpick comments (2)
cmd/smsgate/smsgate.go (1)
76-78: ⚡ Quick winComplete the category constant extraction.
The "Output" category on line 78 remains hardcoded while this refactor extracts other categories into constants. For consistency, consider adding a
categoryOutputconstant.♻️ Proposed refactor
const ( categoryConfiguration = "Configuration" + categoryOutput = "Output" )Then update the format flag:
&cli.StringFlag{ Name: "format", - Category: "Output", + Category: categoryOutput, Usage: "Output format. Supported: text, json, raw",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/smsgate/smsgate.go` around lines 76 - 78, The "Output" string in the cli.StringFlag for the "format" flag is still hardcoded; add a new constant named categoryOutput (e.g., const categoryOutput = "Output") alongside the other category constants and change the Category on the flag (the cli.StringFlag with Name "format") to use categoryOutput so all categories are consistently extracted as constants.internal/commands/flags/send.go (1)
145-153: ⚡ Quick winDon’t add a correctness cross-field check for
schedule-atvsvalid-until/ttl: the SMS Gateway for Android API rejects scheduled requests wherescheduleAtis after the expiration time (scheduleAt <= validUntilwhenvalidUntilis set; same rule applies withttl), so it won’t “silently never send.” Optional: a client-side check could still improve error UX.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/commands/flags/send.go` around lines 145 - 153, Remove any cross-field correctness check that rejects a request when scheduleAt is after validUntil/ttl; keep only the existing future-only validation for scheduleAt (the c.Timestamp("schedule-at") check that returns error if not After(time.Now())) and assign to fl.ScheduleAt, and if you want UX help add a non-fatal warning/log (not a returned error) when scheduleAt > validUntil or conflicts with ttl; do not return an error or block setting fl.ScheduleAt based on validUntil or ttl.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@go.mod`:
- Around line 27-29: Update the vulnerable module entries in go.mod by bumping
golang.org/x/crypto and golang.org/x/net to the patched releases that address
the cited advisories (replace the current v0.52.0 and v0.55.0 entries with the
fixed versions), optionally update golang.org/x/text to a recent release, then
run go get golang.org/x/crypto@<fixed-version> and go get
golang.org/x/net@<fixed-version> (or go get <module>`@latest` if you intend the
latest), followed by go mod tidy and running your test suite to verify no
breakages; ensure the go.mod lines for modules golang.org/x/crypto and
golang.org/x/net reflect the new versions.
---
Nitpick comments:
In `@cmd/smsgate/smsgate.go`:
- Around line 76-78: The "Output" string in the cli.StringFlag for the "format"
flag is still hardcoded; add a new constant named categoryOutput (e.g., const
categoryOutput = "Output") alongside the other category constants and change the
Category on the flag (the cli.StringFlag with Name "format") to use
categoryOutput so all categories are consistently extracted as constants.
In `@internal/commands/flags/send.go`:
- Around line 145-153: Remove any cross-field correctness check that rejects a
request when scheduleAt is after validUntil/ttl; keep only the existing
future-only validation for scheduleAt (the c.Timestamp("schedule-at") check that
returns error if not After(time.Now())) and assign to fl.ScheduleAt, and if you
want UX help add a non-fatal warning/log (not a returned error) when scheduleAt
> validUntil or conflicts with ttl; do not return an error or block setting
fl.ScheduleAt based on validUntil or ttl.
🪄 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: 0a3963ba-b8ce-4ba9-ac59-cea3de9a4a4e
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (10)
cmd/smsgate/smsgate.gogo.modinternal/commands/flags/send.gointernal/commands/messages/batch/send.gointernal/commands/messages/send.gointernal/commands/webhooks/consts.gointernal/commands/webhooks/delete.gointernal/commands/webhooks/list.gointernal/commands/webhooks/register.gointernal/commands/webhooks/webhooks.go
capcom6
force-pushed
the
smsgate/schedule-at-support
branch
from
06fd19d to
ac3502c
Compare
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
1 participant
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 by CodeRabbit
New Features
Chores