[inbox] introduce module and API placeholders#220
Conversation
📝 WalkthroughWalkthroughAdds a new third-party inbox controller with stubbed endpoints, permission scopes, DI wiring, OpenAPI docs and test HTTP requests; updates Makefile to run code generation and bumps a client-go dependency. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Router
participant Auth as Auth Middleware
participant Perm as Permission Middleware
participant Inbox as Inbox Controller
Client->>Router: GET /3rdparty/v1/inbox
Router->>Auth: with user context
Auth->>Perm: require scope inbox:list
Perm->>Inbox: invoke list()
Inbox-->>Client: 501 Not Implemented
Client->>Router: POST /3rdparty/v1/inbox/refresh
Router->>Auth: with user context
Auth->>Perm: require scope inbox:refresh
Perm->>Inbox: invoke refresh()
Inbox-->>Client: 501 Not Implemented
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 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. 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 45-50: The OpenAPI swagger comments advertise successful responses
(e.g., "@Success 200" and "@Success 202" returning smsgateway.IncomingMessage)
but the handlers actually return 501 "Not implemented"; update the swagger
annotations in this file so the contract matches runtime behavior by replacing
the success annotations ("@Success 200" / "@Success 202") with the 501 failure
annotation (e.g., "@Failure 501 {object} smsgateway.ErrorResponse \"Not
implemented\"") for the affected endpoints, ensuring the annotations referencing
smsgateway.IncomingMessage and smsgateway.ErrorResponse are consistent with the
handlers' current placeholder returns.
🪄 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: a397dca6-f4b3-41e9-8dd8-b2a89f799568
📒 Files selected for processing (4)
internal/sms-gateway/handlers/3rdparty.gointernal/sms-gateway/handlers/inbox/3rdparty.gointernal/sms-gateway/handlers/inbox/permissions.gointernal/sms-gateway/handlers/module.go
89990e1 to
8178609
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
internal/sms-gateway/handlers/messages/3rdparty.go (1)
164-173: Consider centralizing message-state response mapping to avoid drift.Lines 164-173 duplicate the state-to-DTO field mapping pattern already present in
internal/sms-gateway/handlers/converters/messages.go. A shared converter forGetMessageResponsewould reduce future mismatch risk.🤖 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 164 - 173, The block building a GetMessageResponse-style DTO (setting ID, DeviceID, State as smsgateway.ProcessingState, IsHashed, IsEncrypted, Recipients, States, TextMessage/DataMessage/HashedMessage) duplicates logic already in the messages converter; extract that mapping into a single shared converter function (e.g., ToGetMessageResponse or GetMessageResponseFromState) inside the existing messages converter module and replace this duplicated construction with a call to that converter. Ensure the new converter accepts the same source type used here (state) and returns the response DTO, and update any imports/usages so both this handler (3rdparty.go) and the original callers use that single shared function to avoid drift.
🤖 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/openapi/docs.go`:
- Around line 355-540: The OpenAPI docs advertise successful responses for the
endpoints "/3rdparty/v1/inbox" and "/3rdparty/v1/inbox/refresh" but the actual
handlers in inbox 3rdparty (handlers/inbox/3rdparty.go) always return 501;
update the docs to reflect the real behavior by removing or downgrading the 200
(for GET /3rdparty/v1/inbox) and 202 (for POST /3rdparty/v1/inbox/refresh)
success responses and instead only include the 501 Not Implemented response (and
adjust the endpoint descriptions/summary to state they are not implemented),
ensuring the schema and response descriptions reference smsgateway.ErrorResponse
for 501 and that no functional success schema is advertised.
- Around line 1662-1666: IncomingMessage.createdAt is missing the JSON Schema
format; update the OpenAPI schema for IncomingMessage (the "createdAt" property)
to include "format": "date-time" so it matches other timestamp fields in the
spec and tools will treat it as an RFC3339/ISO8601 datetime.
In `@Makefile`:
- Around line 36-37: The Makefile's gen target can be shadowed by a file named
"gen"; add "gen" to the .PHONY declaration so the gen target always runs; locate
the .PHONY line (or create one if missing) and include the target name "gen"
alongside other phony targets to prevent accidental skipping.
---
Nitpick comments:
In `@internal/sms-gateway/handlers/messages/3rdparty.go`:
- Around line 164-173: The block building a GetMessageResponse-style DTO
(setting ID, DeviceID, State as smsgateway.ProcessingState, IsHashed,
IsEncrypted, Recipients, States, TextMessage/DataMessage/HashedMessage)
duplicates logic already in the messages converter; extract that mapping into a
single shared converter function (e.g., ToGetMessageResponse or
GetMessageResponseFromState) inside the existing messages converter module and
replace this duplicated construction with a call to that converter. Ensure the
new converter accepts the same source type used here (state) and returns the
response DTO, and update any imports/usages so both this handler (3rdparty.go)
and the original callers use that single shared function to avoid drift.
🪄 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: 9f66d027-5b86-4b31-9e3b-eee435c1c5df
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (9)
Makefilego.modinternal/sms-gateway/handlers/3rdparty.gointernal/sms-gateway/handlers/converters/messages.gointernal/sms-gateway/handlers/inbox/3rdparty.gointernal/sms-gateway/handlers/inbox/permissions.gointernal/sms-gateway/handlers/messages/3rdparty.gointernal/sms-gateway/handlers/module.gointernal/sms-gateway/openapi/docs.go
✅ Files skipped from review due to trivial changes (2)
- internal/sms-gateway/handlers/inbox/permissions.go
- go.mod
🚧 Files skipped from review as they are similar to previous changes (3)
- internal/sms-gateway/handlers/module.go
- internal/sms-gateway/handlers/3rdparty.go
- internal/sms-gateway/handlers/inbox/3rdparty.go
🤖 Pull request artifacts
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
internal/sms-gateway/openapi/docs.go (1)
365-373:⚠️ Potential issue | 🟠 MajorAlign inbox OpenAPI responses with current 501-only behavior.
Line 426 and Line 508 still advertise success responses (
200/202), but the current handlers (internal/sms-gateway/handlers/inbox/3rdparty.go, Lines 38-83) always return501 Not Implemented. This overstates API readiness for integrators and SDK generation.Suggested fix (apply in source annotations, then regenerate docs)
- "description": "Retrieves incoming messages with filtering and pagination.", + "description": "Inbox API is not implemented yet.", ... - "200": { - "description": "A list of incoming messages", - "schema": { - "type": "array", - "items": { - "$ref": "#/definitions/smsgateway.IncomingMessage" - } - }, - "headers": { - "X-Total-Count": { - "type": "integer", - "description": "Total number of items available\"\tFormat(int64)" - } - } - }, ... - "description": "Refreshes inbox messages. Webhooks will not be triggered.", + "description": "Inbox API is not implemented yet.", ... - "202": { - "description": "Inbox refresh request accepted", - "schema": { - "type": "object" - } - },Also applies to: 426-440, 495-513
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/sms-gateway/openapi/docs.go` around lines 365 - 373, The OpenAPI docs in docs.go advertise 200/202 success responses for the inbox endpoints but the actual handlers (internal/sms-gateway/handlers/inbox/3rdparty.go) always return 501; update the source annotations that generate these docs so the inbox endpoints (e.g., the "Get incoming messages" operation and any other inbox operations around the documented blocks) declare 501 Not Implemented instead of 200/202, then regenerate the OpenAPI docs; locate the corresponding operation definitions in docs.go and the originating annotation comments tied to the inbox handlers and replace success response entries with a 501 response description to match the handler 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/openapi/docs.go`:
- Around line 435-437: The published OpenAPI header description for
"X-Total-Count" in internal/sms-gateway/openapi/docs.go contains escaped
quote/tab artifacts (`\"` and `\t`) — locate the "X-Total-Count" header entry in
docs.go and clean the description string by removing the escaped characters so
it reads plainly (e.g., "Total number of items available"); if a format note is
intended, separate it into a proper "format": "int64" field or append a clear
parenthetical like "(format: int64)" instead of embedding escaped characters.
- Around line 1509-1516: The OpenAPI docs declare fields (e.g., "dataMessage",
"binaryMessage", "callbackUrl") as returned only when includeContent=true but
the API has no includeContent parameter and the handlers/converters
(internal/sms-gateway/handlers/converters/messages.go and
internal/sms-gateway/handlers/messages/3rdparty.go) currently set these fields
to nil; either add a real includeContent query parameter and wire it through the
3rdparty message handlers to populate
smsgateway.DataMessage/smsgateway.BinaryMessage, or remove/mark those schema
properties as "reserved / not returned" (or remove the includeContent wording)
in the OpenAPI definitions (the blocks referencing smsgateway.DataMessage and
related refs) so the contract matches current behavior. Ensure the change
updates all repeated blocks (lines referencing
dataMessage/binaryMessage/callbackUrl across the file) and keep schema $ref
names unchanged so consumers can still rely on types.
---
Duplicate comments:
In `@internal/sms-gateway/openapi/docs.go`:
- Around line 365-373: The OpenAPI docs in docs.go advertise 200/202 success
responses for the inbox endpoints but the actual handlers
(internal/sms-gateway/handlers/inbox/3rdparty.go) always return 501; update the
source annotations that generate these docs so the inbox endpoints (e.g., the
"Get incoming messages" operation and any other inbox operations around the
documented blocks) declare 501 Not Implemented instead of 200/202, then
regenerate the OpenAPI docs; locate the corresponding operation definitions in
docs.go and the originating annotation comments tied to the inbox handlers and
replace success response entries with a 501 response description to match the
handler 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: 14309b9f-6d10-4dfc-a3dd-084dd6bb7f38
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
go.modinternal/sms-gateway/handlers/inbox/3rdparty.gointernal/sms-gateway/handlers/messages/3rdparty.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 (2)
- internal/sms-gateway/handlers/messages/3rdparty.go
- internal/sms-gateway/handlers/inbox/3rdparty.go
8ba6b7e to
7f36186
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (3)
internal/sms-gateway/openapi/docs.go (3)
435-437:⚠️ Potential issue | 🟡 MinorClean up the published
X-Total-Countheader description.Both header blocks still serialize
\"\tFormat(int64)into the description text, so the generated contract is malformed for consumers. Please fix the source annotations and regenerate.Also applies to: 690-693
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/sms-gateway/openapi/docs.go` around lines 435 - 437, The "X-Total-Count" header annotation contains stray escaped characters and an inlined format marker (\"\tFormat(int64)) that landed in the description; update both "X-Total-Count" header annotation blocks to set a clean description string (e.g. "Total number of items available") and move the type/format into the proper OpenAPI fields (type: integer and format: int64) so the generated contract is valid, then regenerate the docs; look for the two "X-Total-Count" header blocks in the OpenAPI annotations to apply the fix.
1509-1530:⚠️ Potential issue | 🟠 MajorThese message-content fields are still documented as returnable when the API always omits them.
/3rdparty/v1/messagesstill has noincludeContentparameter, and the current mapper paths forcetextMessage,dataMessage, andhashedMessagetonil(internal/sms-gateway/handlers/messages/3rdparty.go, Lines 162-174;internal/sms-gateway/handlers/converters/messages.go, Lines 46-59). Either implement population for these fields or mark them as reserved/not currently returned before regenerating the schema.Also applies to: 1571-1578, 1938-1959, 2000-2007
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/sms-gateway/openapi/docs.go` around lines 1509 - 1530, The OpenAPI docs incorrectly list textMessage, dataMessage, and hashedMessage as returnable for /3rdparty/v1/messages even though the 3rdparty messages handler and the messages converter currently always omit them; either (A) implement content population by adding an includeContent boolean param to the /3rdparty/v1/messages handler and update the messages converter to set textMessage, dataMessage, and hashedMessage when includeContent=true (update the 3rdparty messages handler logic and the messages converter mapping functions), or (B) change the OpenAPI generation for these fields to mark them as reserved/not returned (remove them from the response schema or add a clear note/nullable flag) before regenerating docs so the schema matches the actual behavior.
355-471:⚠️ Potential issue | 🟠 MajorDon’t publish success contracts for inbox placeholders yet.
This generated spec still advertises
200/202success paths, butinternal/sms-gateway/handlers/inbox/3rdparty.go, Lines 33-78, currently returns501 Not Implementedfor both routes. Please fix the source annotations and regenerate the spec so clients do not treat these endpoints as live.Also applies to: 474-545
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/sms-gateway/openapi/docs.go` around lines 355 - 471, The OpenAPI spec advertises 200/202 success for the "/3rdparty/v1/inbox" (and the other inbox 3rd-party route) but the handlers currently return 501; update the source Swagger/OpenAPI annotations on the inbox 3rd-party handler(s) to remove success response contracts (200/202) and instead document only 501 (Not Implemented) for these routes (the handler functions in the inbox 3rdparty handlers that currently return 501), then regenerate the OpenAPI docs so the generated docs match the actual behavior.
🧹 Nitpick comments (1)
internal/sms-gateway/handlers/messages/3rdparty.go (1)
164-173: Reuse a shared response mapper here.This literal now mirrors the message-state mapping already living in
internal/sms-gateway/handlers/converters/messages.go, Lines 46-59. Pullingpost,get, andlistthrough one shared DTO builder would make future schema changes much less likely to drift across endpoints.🤖 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 164 - 173, Replace the manual struct literal (the block building ID: state.ID, DeviceID: state.DeviceID, State: smsgateway.ProcessingState(state.State), IsHashed, IsEncrypted, Recipients, States, TextMessage/DataMessage/HashedMessage nil) with a call to the shared DTO mapper defined in internal/sms-gateway/handlers/converters/messages.go (the message-state mapping at lines 46-59); invoke that mapper with the current `state` to produce the response DTO and use its result in the post/get/list handlers so all endpoints share the same builder.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@internal/sms-gateway/openapi/docs.go`:
- Around line 435-437: The "X-Total-Count" header annotation contains stray
escaped characters and an inlined format marker (\"\tFormat(int64)) that landed
in the description; update both "X-Total-Count" header annotation blocks to set
a clean description string (e.g. "Total number of items available") and move the
type/format into the proper OpenAPI fields (type: integer and format: int64) so
the generated contract is valid, then regenerate the docs; look for the two
"X-Total-Count" header blocks in the OpenAPI annotations to apply the fix.
- Around line 1509-1530: The OpenAPI docs incorrectly list textMessage,
dataMessage, and hashedMessage as returnable for /3rdparty/v1/messages even
though the 3rdparty messages handler and the messages converter currently always
omit them; either (A) implement content population by adding an includeContent
boolean param to the /3rdparty/v1/messages handler and update the messages
converter to set textMessage, dataMessage, and hashedMessage when
includeContent=true (update the 3rdparty messages handler logic and the messages
converter mapping functions), or (B) change the OpenAPI generation for these
fields to mark them as reserved/not returned (remove them from the response
schema or add a clear note/nullable flag) before regenerating docs so the schema
matches the actual behavior.
- Around line 355-471: The OpenAPI spec advertises 200/202 success for the
"/3rdparty/v1/inbox" (and the other inbox 3rd-party route) but the handlers
currently return 501; update the source Swagger/OpenAPI annotations on the inbox
3rd-party handler(s) to remove success response contracts (200/202) and instead
document only 501 (Not Implemented) for these routes (the handler functions in
the inbox 3rdparty handlers that currently return 501), then regenerate the
OpenAPI docs so the generated docs match the actual behavior.
---
Nitpick comments:
In `@internal/sms-gateway/handlers/messages/3rdparty.go`:
- Around line 164-173: Replace the manual struct literal (the block building ID:
state.ID, DeviceID: state.DeviceID, State:
smsgateway.ProcessingState(state.State), IsHashed, IsEncrypted, Recipients,
States, TextMessage/DataMessage/HashedMessage nil) with a call to the shared DTO
mapper defined in internal/sms-gateway/handlers/converters/messages.go (the
message-state mapping at lines 46-59); invoke that mapper with the current
`state` to produce the response DTO and use its result in the post/get/list
handlers so all endpoints share the same builder.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 28d91d3a-5465-45d0-84be-435c203a0cc8
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (9)
Makefilego.modinternal/sms-gateway/handlers/3rdparty.gointernal/sms-gateway/handlers/converters/messages.gointernal/sms-gateway/handlers/inbox/3rdparty.gointernal/sms-gateway/handlers/inbox/permissions.gointernal/sms-gateway/handlers/messages/3rdparty.gointernal/sms-gateway/handlers/module.gointernal/sms-gateway/openapi/docs.go
✅ Files skipped from review due to trivial changes (3)
- internal/sms-gateway/handlers/inbox/permissions.go
- go.mod
- internal/sms-gateway/handlers/inbox/3rdparty.go
🚧 Files skipped from review as they are similar to previous changes (3)
- internal/sms-gateway/handlers/module.go
- internal/sms-gateway/handlers/converters/messages.go
- internal/sms-gateway/handlers/3rdparty.go
7f36186 to
3f118d7
Compare
3f118d7 to
3f37964
Compare
3f37964 to
2696e00
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
api/local.http (1)
205-205: Nit: inconsistent timestamp precision betweenfromandto.
fromincludes milliseconds (...00.000Z) whiletodoes not (...59Z). Both are valid ISO 8601 but harmonizing them makes the sample clearer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/local.http` at line 205, The sample GET request's query params have inconsistent ISO timestamps: the `from` param uses millisecond precision (`...00.000Z`) while the `to` param lacks milliseconds (`...59Z`); update the `to` parameter in the GET request (the line containing the GET .../inbox?type=SMS&limit=1&offset=...&from=...&to=...) to include millisecond precision so both `from` and `to` use the same format (e.g., append `.000` before the `Z`) for clarity and consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/local.http`:
- Around line 213-216: The sample request body is missing the required deviceId
field defined on smsgateway.MessagesExportRequest; update the JSON body in the
sample to include a valid deviceId property (e.g., a string/UUID) alongside
since and until so the example conforms to the schema and will pass validation
once the handler uses smsgateway.MessagesExportRequest.
---
Nitpick comments:
In `@api/local.http`:
- Line 205: The sample GET request's query params have inconsistent ISO
timestamps: the `from` param uses millisecond precision (`...00.000Z`) while the
`to` param lacks milliseconds (`...59Z`); update the `to` parameter in the GET
request (the line containing the GET
.../inbox?type=SMS&limit=1&offset=...&from=...&to=...) to include millisecond
precision so both `from` and `to` use the same format (e.g., append `.000`
before the `Z`) for clarity and consistency.
🪄 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: cb89b597-e6d2-4519-aa58-4f90be9d2b6f
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (9)
Makefileapi/local.httpgo.modinternal/sms-gateway/handlers/3rdparty.gointernal/sms-gateway/handlers/inbox/3rdparty.gointernal/sms-gateway/handlers/inbox/permissions.gointernal/sms-gateway/handlers/messages/3rdparty.gointernal/sms-gateway/handlers/module.gointernal/sms-gateway/openapi/docs.go
✅ Files skipped from review due to trivial changes (4)
- internal/sms-gateway/handlers/module.go
- internal/sms-gateway/handlers/messages/3rdparty.go
- internal/sms-gateway/handlers/inbox/permissions.go
- go.mod
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/sms-gateway/handlers/3rdparty.go
- internal/sms-gateway/handlers/inbox/3rdparty.go
2696e00 to
beaa14c
Compare
Motivation
501 Not Implementedresponse from server-side placeholders to avoid accidental exposure of incomplete logic./messages/inbox/exportbehavior unchanged as requested.Description
handlers/inboxthird-party controller with placeholder handlers forGET /3rdparty/v1/inboxandPOST /3rdparty/v1/inbox/refreshthat return501 Not Implementedand the message"Inbox API is not implemented yet".inbox:listandinbox:refreshand apply them to the new routes./inboxroute under/3rdparty/v1so the endpoints are mounted and protected by existing auth middleware.POST /3rdparty/v1/messages/inbox/exportroute or its implementation.Testing
go test ./internal/sms-gateway/handlers/...and the package tests completed successfully.Codex Task
Summary by CodeRabbit
New Features
Documentation
Chores