[api] use ListMessagesOptions from client-go#253
Open
capcom6 wants to merge 1 commit into
Open
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 (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe client-go dependency is updated, message listing parameters now use shared pointer-based options with direct filter mapping and bounded pagination defaults, and the OpenAPI settings schema gains work-hours configuration fields. ChangesParams Refactor and Dependency Update
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/sms-gateway/handlers/messages/params.go (1)
13-35: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRestore
from/toorder validation inthirdPartyGetQueryParams.QueryParserValidatoronly applies struct-tag rules, andToFilter()forwards the dates directly toSelectStates;from > tonow returns an empty result set instead of a 400. Add an explicit range check here or onsmsgateway.ListMessagesOptions.🤖 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/sms-gateway/handlers/messages/params.go` around lines 13 - 35, `thirdPartyGetQueryParams.ToFilter` currently forwards `From` and `To` without validating their ordering, so an invalid range can slip through and produce an empty response instead of a bad-request error. Add an explicit `from <= to` check in `ToFilter` or move the range validation into `smsgateway.ListMessagesOptions` so the `QueryParserValidator` path still rejects inverted date ranges before building the `messages.SelectFilter`.
🤖 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 `@api/requests.http`:
- Line 121: The sample request in api/requests.http has an inverted date range
because the Messages query uses a `from` value later than `to`. Update the
example under the `/3rdparty/v1/messages` request so the `to` timestamp matches
the same 2026 range as `from`, keeping the date window consistent with the other
sample requests.
In `@internal/sms-gateway/handlers/messages/params.go`:
- Around line 48-52: The Limit handling in the params parser only applies an
upper clamp, so zero or negative values can still flow into the SMS query
options. Update the limit assignment in the params logic (the branch that sets
options.Limit from p.Limit) to enforce a lower bound as well, matching the
Offset pattern with max(); clamp Limit to the valid range [1, maxLimit] and keep
the default when p.Limit is nil.
---
Outside diff comments:
In `@internal/sms-gateway/handlers/messages/params.go`:
- Around line 13-35: `thirdPartyGetQueryParams.ToFilter` currently forwards
`From` and `To` without validating their ordering, so an invalid range can slip
through and produce an empty response instead of a bad-request error. Add an
explicit `from <= to` check in `ToFilter` or move the range validation into
`smsgateway.ListMessagesOptions` so the `QueryParserValidator` path still
rejects inverted date ranges before building the `messages.SelectFilter`.
🪄 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: 201fd379-a6e7-4053-af23-7a64d3b15fac
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
api/requests.httpgo.modinternal/sms-gateway/handlers/messages/params.gointernal/sms-gateway/openapi/docs.go
🤖 Pull request artifacts
|
capcom6
force-pushed
the
api/use-client-go-list-messages-options
branch
from
ae8c2e9 to
957e74f
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
Bug Fixes