-
Notifications
You must be signed in to change notification settings - Fork 4
[client] add mms:received event
#35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests.
🚀 New features to boost your workflow:
|
WalkthroughAdds a new exported webhook event constant "mms:received", registers it in the internal event list, and includes it in the WebhookEventTypes() helper return. Validation logic remains unchanged. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
smsgateway/domain_webhooks.go (3)
27-49: Reduce duplication: derive map and slice from a single ordered list.Right now we maintain the set (allEventTypes) and the slice (WebhookEventTypes) separately; this is easy to desync on future edits. Consider a single source of truth that preserves your preferred order.
Apply within this block:
-//nolint:gochecknoglobals // lookup table -var allEventTypes = map[WebhookEvent]struct{}{ - WebhookEventSmsReceived: {}, - WebhookEventSmsDataReceived: {}, - WebhookEventSmsSent: {}, - WebhookEventSmsDelivered: {}, - WebhookEventSmsFailed: {}, - WebhookEventSystemPing: {}, - WebhookEventMmsReceived: {}, -} - -// WebhookEventTypes returns a slice of all supported webhook event types. -func WebhookEventTypes() []WebhookEvent { - return []WebhookEvent{ - WebhookEventSmsReceived, - WebhookEventSmsDataReceived, - WebhookEventSmsSent, - WebhookEventSmsDelivered, - WebhookEventSmsFailed, - WebhookEventSystemPing, - WebhookEventMmsReceived, - } -} +// Keep a single ordered list and derive other structures from it. +var webhookEventTypesOrdered = []WebhookEvent{ + WebhookEventSmsReceived, + WebhookEventSmsDataReceived, + WebhookEventSmsSent, + WebhookEventSmsDelivered, + WebhookEventSmsFailed, + WebhookEventSystemPing, + WebhookEventMmsReceived, +} + +//nolint:gochecknoglobals // lookup table derived from the ordered list +var allEventTypes = func() map[WebhookEvent]struct{} { + m := make(map[WebhookEvent]struct{}, len(webhookEventTypesOrdered)) + for _, e := range webhookEventTypesOrdered { + m[e] = struct{}{} + } + return m +}() + +// WebhookEventTypes returns a copy of the ordered list of supported webhook event types. +func WebhookEventTypes() []WebhookEvent { + out := make([]WebhookEvent, len(webhookEventTypesOrdered)) + copy(out, webhookEventTypesOrdered) + return out +}
72-72: Optional: update the example to showcase the new event.This helps surface the feature in generated docs and examples.
Go snippet:
// The type of event the webhook is triggered for. Event WebhookEvent `json:"event" validate:"required" example:"mms:received"`
23-24: Add/adjust tests to cover the new event.Minimal additions to prevent regressions:
Go tests (new file suggestion: smsgateway/domain_webhooks_test.go):
package smsgateway import "testing" func TestIsValidWebhookEvent_MMSReceived(t *testing.T) { if !IsValidWebhookEvent(WebhookEventMmsReceived) { t.Fatalf("expected mms:received to be valid") } } func TestWebhook_Validate_MMSReceived(t *testing.T) { w := Webhook{ ID: "123e4567-e89b-12d3-a456-426614174000", DeviceID: nil, URL: "https://example.com/webhook", Event: WebhookEventMmsReceived, } if err := w.Validate(); err != nil { t.Fatalf("validate failed for mms:received: %v", err) } }If you’d like, I can push these tests as a follow-up.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
smsgateway/domain_webhooks.go(3 hunks)
🔇 Additional comments (4)
smsgateway/domain_webhooks.go (4)
23-24: New event constant looks correct and consistent.Naming, string value, and comment align with existing SMS event conventions.
35-35: Properly registered in the validation set.Adding the new event to allEventTypes ensures IsValidWebhookEvent() and Validate() accept it.
47-47: Exposed via WebhookEventTypes().Including the event in the helper keeps API consumers in sync.
23-24: Sanity check passed – no hardcoded event strings found outsidedomain_webhooks.go
domain_webhooks.goand its accompanying tests and lookup table now includeWebhookEventMmsReceived.- No occurrences of
sms:…ormms:receivedwere found in markdown docs (*.md), anexamples/directory, or adocs/folder.- No tests assume a specific ordering of
WebhookEventTypes().Everything appears in sync; no further updates to docs or samples are needed.
Summary by CodeRabbit