[handlers] fix inbox export path #179
Conversation
WalkthroughCorrected a router annotation for the inbox export route and updated OpenAPI docs: added Cloud Server notes for passphrase/signingKey, extended the WebhookEvent enum with Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant MP as MMS Provider
participant SG as SMS Gateway
participant WD as Webhook Dispatcher
participant CE as Client Endpoint
MP->>SG: Deliver MMS
SG->>WD: Emit event "mms:received"
WD->>CE: POST webhook (event: mms:received)
CE-->>WD: 2xx/4xx response
WD-->>SG: Delivery result
note over SG,WD: New OpenAPI event type and exported constant added
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
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/openapi/docs.go (1)
150-207: OpenAPI path still wrong: /3rdparty/v1/inbox/export (missing /messages)Swagger still advertises /3rdparty/v1/inbox/export, which will 404. Regenerate Swagger from the updated annotation, don’t hand-edit this file.
Suggested quick patch if regeneration isn’t possible right now:
- "/3rdparty/v1/inbox/export": { + "/3rdparty/v1/messages/inbox/export": {
🧹 Nitpick comments (1)
internal/sms-gateway/openapi/docs.go (1)
1473-1475: Enforce Cloud ban for webhook signing_key — ignore or reject in Cloud buildsDocs state signing_key "Must not be used with Cloud Server" but the setting is still exposed in defaults/samples (internal/sms-gateway/modules/settings/utils.go; internal/sms-gateway/openapi/docs.go; api/requests.http; api/local.http). Add a Cloud-mode guard in settings loading/validation and in the webhook signing/dispatch path to either strip/ignore signing_key or fail validation when running in Cloud.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
internal/sms-gateway/handlers/messages/3rdparty.go(1 hunks)internal/sms-gateway/openapi/docs.go(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
internal/sms-gateway/openapi/docs.go (1)
internal/sms-gateway/modules/webhooks/service.go (1)
s(72-107)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test
- GitHub Check: E2E
- GitHub Check: lint
- GitHub Check: Analyze (go)
🔇 Additional comments (1)
internal/sms-gateway/openapi/docs.go (1)
1549-1560: New enum value mms:received — verify end-to-end supportEnum is present only in OpenAPI docs (internal/sms-gateway/openapi/docs.go:1550,1559); no runtime references were found. Verify:
- smsgateway.IsValidWebhookEvent accepts "mms:received".
- Producers actually emit the "mms:received" webhook.
- Consumers and automated tests (SSE/webhook handlers, unit/integration) cover this event.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
internal/sms-gateway/openapi/docs.go (1)
1371-1373: Cloud Server prohibition needs runtime enforcement (passphrase)Docs add “Must not be used with Cloud Server.” Please ensure the server rejects/ignores encryption.passphrase in Cloud mode; this was raised previously and still applies.
🧹 Nitpick comments (2)
internal/sms-gateway/openapi/docs.go (2)
391-391: Clarify SMS vs MMS webhook(s) for inbox exportDescription says only sms:received is triggered, but WebhookEvent now also lists mms:received. If MMS are exported, document both events; if not, explicitly say “SMS only.” Also minor copy polish.
Proposed doc tweak:
- "description": "Initiates process of inbox messages export via webhooks. For each message the `sms:received` webhook will be triggered. The webhooks will be triggered without specific order.", + "description": "Initiates export of inbox messages via webhooks. For each SMS message, `sms:received` is triggered; for MMS, `mms:received` is triggered. Webhooks are emitted without a guaranteed order.",
384-441: Optional: return a correlation ID for async export202 response has an opaque “object” schema and no headers. Consider including a requestId (body or Location header) to help users trace webhook batches.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
internal/sms-gateway/openapi/docs.go(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build / Docker image (linux/amd64)
- GitHub Check: Analyze (go)
🔇 Additional comments (4)
internal/sms-gateway/openapi/docs.go (4)
384-441: Corrected export path under messages — LGTMThe path now reflects /3rdparty/v1/messages/inbox/export and matches the intended namespace.
384-441: Resolved — handler @router annotation matches updated pathNo stale /3rdparty/v1/inbox/export annotations found; handler annotation in internal/sms-gateway/handlers/messages/3rdparty.go references /3rdparty/v1/messages/inbox/export.
1473-1475: Enforce Cloud-mode prohibition for signing_key at runtimeMirror the passphrase guard: reject or strip SettingsWebhooks.signing_key when running in Cloud mode and surface a clear validation error. Add this check at config load and on webhook-settings save (where the passphrase guard is implemented) and include unit tests to cover the behavior.
1549-1560: Verify new 'mms:received' event and x-enum-varname changeinternal/sms-gateway/openapi/docs.go (≈lines 1549–1560) contains "mms:received" and x-enum-varnames "WebhookEventSystemPing" and "WebhookEventMmsReceived". Ripgrep shows these only in the generated docs; no code-level constant or handler matches were found.
- Declare/confirm the WebhookEventMmsReceived constant where other WebhookEvent* constants are defined.
- Ensure event validation/routing (switch/case or handler registration) accepts "mms:received" and maps it to WebhookEventMmsReceived.
- Add a release note for the x-enum-varname change (WebhookEventSystemPing) — SDKs that generate enums from OpenAPI may be impacted.
capcom6
force-pushed
the
handlers/fix-inbox-export-path
branch
from
786c5ea to
073972b
Compare
Summary by CodeRabbit
New Features
Documentation