Skip to content

[inbox] introduce module and API placeholders#220

Open
capcom6 wants to merge 3 commits intomasterfrom
codex/implement-/inbox-routes-returning-not-implemented
Open

[inbox] introduce module and API placeholders#220
capcom6 wants to merge 3 commits intomasterfrom
codex/implement-/inbox-routes-returning-not-implemented

Conversation

@capcom6
Copy link
Copy Markdown
Member

@capcom6 capcom6 commented Apr 11, 2026

Motivation

  • Provide API surface for incoming-message inbox endpoints so clients see stable routes while the feature is unimplemented.
  • Return a clear 501 Not Implemented response from server-side placeholders to avoid accidental exposure of incomplete logic.
  • Keep the existing /messages/inbox/export behavior unchanged as requested.

Description

  • Add a new handlers/inbox third-party controller with placeholder handlers for GET /3rdparty/v1/inbox and POST /3rdparty/v1/inbox/refresh that return 501 Not Implemented and the message "Inbox API is not implemented yet".
  • Introduce inbox-specific permission scopes inbox:list and inbox:refresh and apply them to the new routes.
  • Wire the new inbox controller into the handlers DI and register the /inbox route under /3rdparty/v1 so the endpoints are mounted and protected by existing auth middleware.
  • Do not modify the existing POST /3rdparty/v1/messages/inbox/export route or its implementation.

Testing

  • Ran go test ./internal/sms-gateway/handlers/... and the package tests completed successfully.
  • Confirmed the new package builds as part of the handlers module and that no existing handler tests regressed.

Codex Task

Summary by CodeRabbit

  • New Features

    • Added third‑party inbox API: GET /3rdparty/v1/inbox (filtering, paging) and POST /3rdparty/v1/inbox/refresh; new permission scopes inbox:list and inbox:refresh.
  • Documentation

    • OpenAPI updated with inbox endpoints, new IncomingMessage schema and enum, and documented X-Total-Count response header for list endpoints.
  • Chores

    • Build workflow: added a gen step to the default make target.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 11, 2026

📝 Walkthrough

Walkthrough

Adds 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

Cohort / File(s) Summary
New Inbox Controller
internal/sms-gateway/handlers/inbox/3rdparty.go
Adds ThirdPartyController, NewThirdPartyController, and Register; implements GET "" and POST "/refresh" handlers that return 501 Not Implemented and are wired with auth and scope middleware; includes Swagger annotations.
Inbox Permissions
internal/sms-gateway/handlers/inbox/permissions.go
Adds exported permission scope constants ScopeList = "inbox:list" and ScopeRefresh = "inbox:refresh".
Handler Wiring
internal/sms-gateway/handlers/3rdparty.go
Injects inbox.ThirdPartyController into the third-party handler and registers it under /3rdparty/v1/inbox.
DI Module
internal/sms-gateway/handlers/module.go
Adds inbox.NewThirdPartyController to Fx provider list.
OpenAPI / Docs
internal/sms-gateway/openapi/docs.go
Adds IncomingMessage and IncomingMessageType schemas; documents GET /3rdparty/v1/inbox and POST /3rdparty/v1/inbox/refresh; adds X-Total-Count response header to messages list.
Local HTTP examples
api/local.http
Adds requests for listing inbox messages and triggering inbox refresh; adds a named request getInbox and variable bindings.
Build / Tooling
Makefile
Adds gen target running go generate ./... and includes it in the all target and .PHONY.
Dependencies
go.mod
Bumps github.com/android-sms-gateway/client-go from v1.12.3v1.12.4.
Docs-only tweak
internal/sms-gateway/handlers/messages/3rdparty.go
Adds Swagger response header annotation X-Total-Count to existing GET /3rdparty/v1/messages doc.

Sequence Diagram

sequenceDiagram
    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
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title '[inbox] introduce module and API placeholders' accurately reflects the main change: introducing a new inbox module with placeholder API endpoints that return 501 Not Implemented.

✏️ 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 0b5ebf5 and 89990e1.

📒 Files selected for processing (4)
  • internal/sms-gateway/handlers/3rdparty.go
  • internal/sms-gateway/handlers/inbox/3rdparty.go
  • internal/sms-gateway/handlers/inbox/permissions.go
  • internal/sms-gateway/handlers/module.go

Comment thread internal/sms-gateway/handlers/inbox/3rdparty.go Outdated
@capcom6 capcom6 force-pushed the codex/implement-/inbox-routes-returning-not-implemented branch from 89990e1 to 8178609 Compare April 11, 2026 07:09
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 for GetMessageResponse would 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

📥 Commits

Reviewing files that changed from the base of the PR and between 89990e1 and 8178609.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (9)
  • Makefile
  • go.mod
  • internal/sms-gateway/handlers/3rdparty.go
  • internal/sms-gateway/handlers/converters/messages.go
  • internal/sms-gateway/handlers/inbox/3rdparty.go
  • internal/sms-gateway/handlers/inbox/permissions.go
  • internal/sms-gateway/handlers/messages/3rdparty.go
  • internal/sms-gateway/handlers/module.go
  • internal/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

Comment thread internal/sms-gateway/openapi/docs.go
Comment thread internal/sms-gateway/openapi/docs.go
Comment thread Makefile
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 11, 2026

🤖 Pull request artifacts

Platform File
🐳 Docker GitHub Container Registry
🍎 Darwin arm64 server_Darwin_arm64.tar.gz
🍎 Darwin x86_64 server_Darwin_x86_64.tar.gz
🐧 Linux arm64 server_Linux_arm64.tar.gz
🐧 Linux i386 server_Linux_i386.tar.gz
🐧 Linux x86_64 server_Linux_x86_64.tar.gz
🪟 Windows arm64 server_Windows_arm64.zip
🪟 Windows i386 server_Windows_i386.zip
🪟 Windows x86_64 server_Windows_x86_64.zip

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
internal/sms-gateway/openapi/docs.go (1)

365-373: ⚠️ Potential issue | 🟠 Major

Align 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 return 501 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

📥 Commits

Reviewing files that changed from the base of the PR and between 8178609 and dfc6ef2.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (4)
  • go.mod
  • internal/sms-gateway/handlers/inbox/3rdparty.go
  • internal/sms-gateway/handlers/messages/3rdparty.go
  • internal/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

Comment thread internal/sms-gateway/openapi/docs.go Outdated
Comment thread internal/sms-gateway/openapi/docs.go
@capcom6 capcom6 force-pushed the codex/implement-/inbox-routes-returning-not-implemented branch 2 times, most recently from 8ba6b7e to 7f36186 Compare April 12, 2026 01:33
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (3)
internal/sms-gateway/openapi/docs.go (3)

435-437: ⚠️ Potential issue | 🟡 Minor

Clean up the published X-Total-Count header 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 | 🟠 Major

These message-content fields are still documented as returnable when the API always omits them.

/3rdparty/v1/messages still has no includeContent parameter, and the current mapper paths force textMessage, dataMessage, and hashedMessage to nil (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 | 🟠 Major

Don’t publish success contracts for inbox placeholders yet.

This generated spec still advertises 200/202 success paths, but internal/sms-gateway/handlers/inbox/3rdparty.go, Lines 33-78, currently returns 501 Not Implemented for 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. Pulling post, get, and list through 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

📥 Commits

Reviewing files that changed from the base of the PR and between dfc6ef2 and 7f36186.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (9)
  • Makefile
  • go.mod
  • internal/sms-gateway/handlers/3rdparty.go
  • internal/sms-gateway/handlers/converters/messages.go
  • internal/sms-gateway/handlers/inbox/3rdparty.go
  • internal/sms-gateway/handlers/inbox/permissions.go
  • internal/sms-gateway/handlers/messages/3rdparty.go
  • internal/sms-gateway/handlers/module.go
  • internal/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

@capcom6 capcom6 marked this pull request as ready for review April 12, 2026 07:14
@capcom6 capcom6 changed the title Add placeholder /inbox third-party routes returning 501 [inbox] add routes placeholders Apr 12, 2026
@capcom6 capcom6 added the ready label Apr 13, 2026
@capcom6 capcom6 force-pushed the codex/implement-/inbox-routes-returning-not-implemented branch from 7f36186 to 3f118d7 Compare April 15, 2026 02:17
@github-actions github-actions bot removed the ready label Apr 15, 2026
@capcom6 capcom6 force-pushed the codex/implement-/inbox-routes-returning-not-implemented branch from 3f118d7 to 3f37964 Compare April 16, 2026 01:59
@capcom6 capcom6 added the ready label Apr 16, 2026
@capcom6 capcom6 force-pushed the codex/implement-/inbox-routes-returning-not-implemented branch from 3f37964 to 2696e00 Compare April 17, 2026 00:45
@github-actions github-actions bot removed the ready label Apr 17, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
api/local.http (1)

205-205: Nit: inconsistent timestamp precision between from and to.

from includes milliseconds (...00.000Z) while to does 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

📥 Commits

Reviewing files that changed from the base of the PR and between 3f37964 and 2696e00.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (9)
  • Makefile
  • api/local.http
  • go.mod
  • internal/sms-gateway/handlers/3rdparty.go
  • internal/sms-gateway/handlers/inbox/3rdparty.go
  • internal/sms-gateway/handlers/inbox/permissions.go
  • internal/sms-gateway/handlers/messages/3rdparty.go
  • internal/sms-gateway/handlers/module.go
  • internal/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

Comment thread api/local.http
@capcom6 capcom6 force-pushed the codex/implement-/inbox-routes-returning-not-implemented branch from 2696e00 to beaa14c Compare April 17, 2026 01:39
@capcom6 capcom6 changed the title [inbox] add routes placeholders [inbox] introduce module and API placeholders Apr 17, 2026
@capcom6 capcom6 added the deployed The PR is deployed on staging label Apr 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

codex deployed The PR is deployed on staging

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant