Conversation
📝 WalkthroughWalkthroughImplements a working inbox refresh endpoint that parses ChangesInbox Refresh Flow
Dependency
Sequence DiagramsequenceDiagram
participant Client as HTTP Client
participant Handler as Inbox Handler
participant Service as Inbox Service
participant Events as Events Service
Client->>Handler: POST /3rdparty/v1/inbox/refresh (InboxRefreshRequest)
Handler->>Handler: Parse & validate request
alt invalid
Handler-->>Client: 400 Bad Request
else valid
Handler->>Service: Refresh(userID, deviceID, since, until, types, triggerWebhooks)
Service->>Events: NewMessagesExportRequestedEvent(...) / Notify(event)
alt event/service error
Handler-->>Client: 500 Internal Server Error
else success
Handler-->>Client: 202 Accepted
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 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. |
capcom6
force-pushed
the
inbox/refresh-options
branch
from
7b40c8e to
ce152da
Compare
![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/inbox/3rdparty.go (1)
66-79:⚠️ Potential issue | 🟡 MinorSync the refresh docs with the implemented behavior.
The handler now honors
triggerWebhooksand returns202fromSendStatus, but the docs still say webhooks are never triggered, advertise an object response, and include a501that this handler no longer returns.Suggested annotation cleanup
-// `@Description` Refreshes inbox messages. Webhooks will not be triggered. +// `@Description` Refreshes inbox messages. Webhooks are triggered when triggerWebhooks is true. ... -// `@Success` 202 {object} object "Inbox refresh request accepted" +// `@Success` 202 "Inbox refresh request accepted" ... -// `@Failure` 501 {object} smsgateway.ErrorResponse "Not implemented"Also applies to: 89-100
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/sms-gateway/handlers/inbox/3rdparty.go` around lines 66 - 79, Update the Swagger annotations for the inbox refresh handler to match current behavior: remove the note "Webhooks will not be triggered" and instead document that the request honors the triggerWebhooks flag; change the `@Success` 202 response to reflect the actual returned type (no object if SendStatus returns an empty body) and remove the obsolete `@Failure` 501 entry; apply the same annotation edits to the other duplicate block (lines ~89-100) so both Swagger comment blocks for this handler match the implemented SendStatus and triggerWebhooks behavior.
🤖 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/sms-gateway/handlers/messages/3rdparty.go`:
- Around line 245-247: The postInboxExport handler is documented as deprecated
but lacks a Swagger `@Router` annotation so the OpenAPI spec omits
/3rdparty/v1/messages/inbox/export; update the comment block for the
postInboxExport function to include a Swagger `@Router` line for the endpoint
(e.g. `@Router` /3rdparty/v1/messages/inbox/export [post]) and mark it deprecated
(add `@Deprecated` or include the deprecation note in the doc block consistent
with other handlers) so the generated spec includes the path; alternatively, if
you prefer removal, unregister the route where postInboxExport is registered
instead of adding the annotation.
In `@internal/sms-gateway/modules/events/events.go`:
- Around line 28-40: The current manual string-building of messageTypes using
strings.Builder with values from types can produce invalid JSON if any value
contains quotes or backslashes; update the code that constructs
data["messageTypes"] (the block using strings.Builder and the variable types) to
safely quote values—for example, replace the manual quoting with strconv.Quote
on each t or marshal the []string to JSON (using encoding/json) and assign the
resulting safe string to data["messageTypes"]; ensure you import strconv or
encoding/json and handle any marshal/quote errors appropriately.
---
Outside diff comments:
In `@internal/sms-gateway/handlers/inbox/3rdparty.go`:
- Around line 66-79: Update the Swagger annotations for the inbox refresh
handler to match current behavior: remove the note "Webhooks will not be
triggered" and instead document that the request honors the triggerWebhooks
flag; change the `@Success` 202 response to reflect the actual returned type (no
object if SendStatus returns an empty body) and remove the obsolete `@Failure` 501
entry; apply the same annotation edits to the other duplicate block (lines
~89-100) so both Swagger comment blocks for this handler match the implemented
SendStatus and triggerWebhooks behavior.
🪄 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: e5381ade-ebc5-4ae0-9e2d-606c961a6662
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (6)
go.modinternal/sms-gateway/handlers/inbox/3rdparty.gointernal/sms-gateway/handlers/messages/3rdparty.gointernal/sms-gateway/inbox/service.gointernal/sms-gateway/modules/events/events.gointernal/sms-gateway/openapi/docs.go
capcom6
force-pushed
the
inbox/refresh-options
branch
from
c221995 to
8c2c92c
Compare
capcom6
force-pushed
the
inbox/refresh-options
branch
from
8c2c92c to
7c684c3
Compare
capcom6
force-pushed
the
inbox/refresh-options
branch
from
7c684c3 to
ac8cf49
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
internal/sms-gateway/handlers/messages/3rdparty.go (1)
246-249:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDeprecated endpoint is still live but undocumented in Swagger/OpenAPI.
postInboxExportremains reachable (see Line 317 route registration), but this block no longer contributes a documented route, so generated clients lose visibility while the server still accepts calls.#!/bin/bash # Verify route is registered but no swagger router annotation exists for it. rg -n 'router\.Post\("inbox/export"' internal/sms-gateway/handlers/messages/3rdparty.go rg -n '@Router\s+/3rdparty/v1/messages/inbox/export' internal/sms-gateway/handlers/messages/3rdparty.go internal/sms-gateway/openapi/docs.goExpected result: first command returns a match; second returns none. If confirmed, either re-document as deprecated or remove the route in the same release.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/sms-gateway/handlers/messages/3rdparty.go` around lines 246 - 249, postInboxExport is a deprecated but still-registered endpoint that isn't present in the OpenAPI docs; either document it as deprecated or remove its registration. Fix by locating the route registration that wires postInboxExport (the router.Post("inbox/export", ...) call) and either (A) add proper Swagger/OpenAPI annotations for the handler function postInboxExport (include `@Summary`, `@Description` noting deprecated, `@Tags`, and `@Router` + method/path and mark as deprecated) so generated clients see it, or (B) remove the router.Post("inbox/export", ...) registration and any references to postInboxExport so the endpoint is no longer reachable; ensure corresponding tests and usages are updated accordingly.
🤖 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/sms-gateway/handlers/inbox/3rdparty.go`:
- Around line 88-95: The handler forwards req.MessageTypes directly to
h.inboxSvc.Refresh which means missing/empty messageTypes from clients causes no
filtering; change the handler to default messageTypes to SMS at the boundary:
before calling h.inboxSvc.Refresh, create a local variable (e.g., messageTypes)
= req.MessageTypes and if nil/len==0 set it to the SMS default, then pass that
variable into h.inboxSvc.Refresh instead of req.MessageTypes so downstream
services and event constructors always receive the API-defaulted value.
---
Duplicate comments:
In `@internal/sms-gateway/handlers/messages/3rdparty.go`:
- Around line 246-249: postInboxExport is a deprecated but still-registered
endpoint that isn't present in the OpenAPI docs; either document it as
deprecated or remove its registration. Fix by locating the route registration
that wires postInboxExport (the router.Post("inbox/export", ...) call) and
either (A) add proper Swagger/OpenAPI annotations for the handler function
postInboxExport (include `@Summary`, `@Description` noting deprecated, `@Tags`, and
`@Router` + method/path and mark as deprecated) so generated clients see it, or
(B) remove the router.Post("inbox/export", ...) registration and any references
to postInboxExport so the endpoint is no longer reachable; ensure corresponding
tests and usages are updated accordingly.
🪄 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: 05590014-bc06-48a4-9d31-4487eac7e04b
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (6)
go.modinternal/sms-gateway/handlers/inbox/3rdparty.gointernal/sms-gateway/handlers/messages/3rdparty.gointernal/sms-gateway/inbox/service.gointernal/sms-gateway/modules/events/events.gointernal/sms-gateway/openapi/docs.go
🚧 Files skipped from review as they are similar to previous changes (2)
- go.mod
- internal/sms-gateway/modules/events/events.go
capcom6
force-pushed
the
inbox/refresh-options
branch
2 times, most recently
from
5bacef9 to
b9f9cd4
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@internal/sms-gateway/openapi/docs.go`:
- Around line 1609-1633: The field descriptions for the auto-generated docs
still use "export" wording from MessagesExportRequest; update the source Go
struct comment/tags for smsgateway.InboxRefreshRequest (the struct and its
fields DeviceID, MessageTypes, Since, TriggerWebhooks, Until) to replace
"export"/"exported" with "refresh"/"refreshed" and adjust phrases (e.g., "to
refresh messages for", "message types to refresh", "time range to refresh", "for
the refreshed messages"), then re-run swaggo generation to regenerate
internal/sms-gateway/openapi/docs.go so the docs reflect the corrected "refresh"
wording.
- Around line 1622-1635: The generated OpenAPI is missing "format":"date-time"
for the time-range fields; update the swag annotations on the
InboxRefreshRequest struct fields InboxRefreshRequest.Since and
InboxRefreshRequest.Until to include the date-time format tag (e.g. add
format:"date-time" to their swagger/swag struct tags or the equivalent swag
annotation) so the generator emits "format":"date-time" for both fields.
🪄 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: 0e6e9bc5-037a-466c-ba9c-38b3f380de75
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (6)
go.modinternal/sms-gateway/handlers/inbox/3rdparty.gointernal/sms-gateway/handlers/messages/3rdparty.gointernal/sms-gateway/inbox/service.gointernal/sms-gateway/modules/events/events.gointernal/sms-gateway/openapi/docs.go
✅ Files skipped from review due to trivial changes (1)
- go.mod
🚧 Files skipped from review as they are similar to previous changes (4)
- internal/sms-gateway/modules/events/events.go
- internal/sms-gateway/handlers/messages/3rdparty.go
- internal/sms-gateway/handlers/inbox/3rdparty.go
- internal/sms-gateway/inbox/service.go
capcom6
force-pushed
the
inbox/refresh-options
branch
from
767dfa3 to
88251d6
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
go.mod (1)
7-7: ⚡ Quick winConsider using the stable v1.12.6 release instead of a pseudo-version.
The dependency uses a pseudo-version (
v1.12.7-0.20260508050135-17a184bc31e9) rather than an official tagged release. The latest stable release available isv1.12.6(published May 6, 2026), which is only 2 days older than this pseudo-version. If the specific commit isn't required for a critical fix, using the tagged release would improve stability and align with standard Go dependency practices.🤖 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 `@go.mod` at line 7, Replace the pseudo-version dependency entry for github.com/android-sms-gateway/client-go (currently "v1.12.7-0.20260508050135-17a184bc31e9") in go.mod with the official tagged release "v1.12.6"; update the module line to use github.com/android-sms-gateway/client-go v1.12.6 and run "go mod tidy" (or "go get github.com/android-sms-gateway/client-go@v1.12.6") to ensure the lockfile and go.sum are updated.
🤖 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.
Nitpick comments:
In `@go.mod`:
- Line 7: Replace the pseudo-version dependency entry for
github.com/android-sms-gateway/client-go (currently
"v1.12.7-0.20260508050135-17a184bc31e9") in go.mod with the official tagged
release "v1.12.6"; update the module line to use
github.com/android-sms-gateway/client-go v1.12.6 and run "go mod tidy" (or "go
get github.com/android-sms-gateway/client-go@v1.12.6") to ensure the lockfile
and go.sum are updated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 075ba42a-bb05-4294-a6c8-fdd656e03216
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (6)
go.modinternal/sms-gateway/handlers/inbox/3rdparty.gointernal/sms-gateway/handlers/messages/3rdparty.gointernal/sms-gateway/inbox/service.gointernal/sms-gateway/modules/events/events.gointernal/sms-gateway/openapi/docs.go
🚧 Files skipped from review as they are similar to previous changes (5)
- internal/sms-gateway/modules/events/events.go
- internal/sms-gateway/inbox/service.go
- internal/sms-gateway/handlers/inbox/3rdparty.go
- internal/sms-gateway/handlers/messages/3rdparty.go
- internal/sms-gateway/openapi/docs.go
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
Deprecations
Documentation
Chores