refactor: improve notification #2746
Conversation
chrisgacsal
added
the
release-note/ignore
📝 WalkthroughWalkthroughThis change set refactors the notification system's data model and repository layer, focusing on unifying the usage of event types. All references to the Changes
Possibly related PRs
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (1.64.8)Error: you are using a configuration file for golangci-lint v2 with golangci-lint v1: please use golangci-lint v2 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 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 (7)
✨ Finishing Touches
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:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🔭 Outside diff range comments (6)
openmeter/notification/rule.go (2)
174-186: 🛠️ Refactor suggestionValidate the provided
Typesslice
ListRulesInput.Validatecurrently returnsnil, so clients can pass arbitrary strings inTypeswhich will hit the DB layer and fail later (or worse, be mishandled). Iterate overi.Typesand callt.Validate()to fail fast.func (i ListRulesInput) Validate(_ context.Context, _ Service) error { - return nil + for _, t := range i.Types { + if err := t.Validate(); err != nil { + return err + } + } + return nil }
209-229: 🛠️ Refactor suggestionEnsure
TypeandConfig.Typestay in sync
CreateRuleInput.Validatechecksi.Typeand separately validatesi.Config, but never asserts that the two refer to the same event type. A client could send{type:A, config:{type:B}}and pass validation, leading to inconsistent data.if err := i.Type.Validate(); err != nil { return err } +if i.Config.Type != i.Type { + return ValidationError{ + Err: fmt.Errorf("config.type (%s) must match input.type (%s)", i.Config.Type, i.Type), + } +} + if i.Name == "" {openmeter/notification/service/rule.go (1)
42-57:⚠️ Potential issueExternal side-effects inside the DB transaction risk breaking atomicity
webhook.UpdateWebhookChannelsperforms network I/O. Invoking it inside the DB transaction:
- Prolongs the lock window.
- May leave the external system mutated even if the subsequent commit fails.
Consider:
• First committing the rule,
• Then updating webhooks (or using an outbox pattern).This comment applies to
DeleteRuleandUpdateRuleas well.openmeter/notification/service/channel.go (3)
199-223:⚠️ Potential issueUse the channel’s persisted type instead of the (possibly empty) request payload
params.Typecan be omitted by the caller during an update (it is optional in the API).
Relying on it after the record has already been fetched/updated may therefore resolve to the empty string and trigger the “invalid channel type” branch, even though the underlying record has a valid type.
Use the value that actually exists in the updated entity (channel.Type) when deciding which integration logic to execute.- switch params.Type { + switch channel.Type {
24-66: 🛠️ Refactor suggestionAvoid external IO (webhook creation) inside the DB transaction
transaction.Runkeeps the DB transaction open for the entire body.
Creating a webhook (s.webhook.CreateWebhook) is a network-bound operation that can ❶ hold DB locks for much longer than necessary and ❷ cannot be rolled back automatically if the DB commit later fails (e.g. becauseUpdateChannelerrors).A safer pattern is:
- Persist the channel inside the transaction (no webhook call yet) and commit.
- Execute the webhook creation outside the transaction.
- Persist the secret in a second lightweight update, or use a DB “after-commit hook” if your transaction helper supports it.
This minimises lock time and keeps the system in a consistent state even if the external call fails.
92-136: 🛠️ Refactor suggestionOrder of deletion can leave the system in an inconsistent state
Inside the transaction the webhook is deleted before the channel record (
s.adapter.DeleteChannel).
If the DB delete subsequently fails or the transaction rolls back, the webhook is already gone while the channel still exists, breaking referential integrity.Reverse the order (delete the channel first, commit, then delete the webhook) or perform the webhook deletion in an after-commit hook.
♻️ Duplicate comments (2)
openmeter/notification/rule.go (1)
259-279: 🛠️ Refactor suggestionMirror the
Type/Config.Typeconsistency check in updatesSame mismatch issue applies to updates; the extra guard should be added here as well.
openmeter/notification/service/rule.go (1)
175-200:⚠️ Potential issueRepeat: external webhook updates executed inside the same transaction
See rationale above.
🧹 Nitpick comments (9)
openmeter/ent/schema/notification.go (2)
293-306: Keep error messages & switch tags consistentThe
Varm now switches onnotification.EventTypeBalanceThreshold, but the default branch still talks about an “unknown rule config type”.
That wording is correct, whereas theSarm further below still says “unknown rule type”.To avoid future confusion, align both error messages:
- return ruleConfig, fmt.Errorf("unknown rule type: %s", meta.Type) + return ruleConfig, fmt.Errorf("unknown rule config type: %s", meta.Type)
321-341: Re-evaluate coupling between RuleConfig and EventTypeUsing
notification.EventType*to drive rule-config (de)serialization works for the singleBalanceThresholdrule, but conflates two separate concepts:
- EventType – originates from the emitted domain events (
entitlements.balance.threshold, etc.).- RuleConfig type – describes how to interpret the
datasection of the rule.If, in the future, you introduce multiple rule flavours that react to the same event type, this
switchwill no longer be a 1-to-1 mapping and will fail closed.Consider a dedicated
RuleConfigTypeenum (or embed a"kind"field) to decouple payload structure from the triggering event.openmeter/ent/db/notificationrule/where.go (1)
294-322: Predicate overload now matches the new type – consider zero-length guardWith the parameter changed to
notification.EventType, the generated predicates compile as expected.
One minor edge-case: callingTypeIn()/TypeNotIn()with zero arguments will yield an SQL fragment ofIN (), which Postgres rejects.Add a short-circuit to return
predicate.False()whenlen(vs) == 0to avoid runtime errors:func TypeIn(vs ...notification.EventType) predicate.NotificationRule { - v := make([]any, len(vs)) + if len(vs) == 0 { + return predicate.NotificationRule(func(*sql.Selector) {}) // no-op FALSE + } + v := make([]any, len(vs)) for i := range v { v[i] = vs[i] } return predicate.NotificationRule(sql.FieldIn(FieldType, v...)) }Same for
TypeNotIn.openmeter/notification/service/deliverystatus.go (1)
1-9: Use a pointer receiver for service methods for consistency & to prevent accidental copiesThroughout the service layer other files use the value receiver as well, which makes every call copy the
Servicestruct (contains a logger, adapter pointer, etc.). While the struct is small, pointer receivers are the de-facto standard for service objects and avoid surprises if fields are added later.No change required right now, but consider switching to
func (s *Service)consistently.openmeter/notification/adapter/adapter.go (1)
22-28: Clarify error wording to match generic SQL driver usageThe error message “postgres client is required” is misleading because the
entdb.Clientis an abstraction that could be backed by other SQL drivers in the future.
Consider re-phrasing to something like “database client is required” to avoid coupling the wording to Postgres.- return errors.New("postgres client is required") + return errors.New("database client is required")openmeter/notification/adapter/rule.go (2)
146-147: Typo in error messageThere is a small typo: “failed top delete” ➜ “failed to delete”.
- return fmt.Errorf("failed top delete notification rule: %w", err) + return fmt.Errorf("failed to delete notification rule: %w", err)
196-197: Reasonable validation: ensure provided Channel IDs existAfter saving the rule you load channels but don’t validate that every requested channel was found.
Consider verifying the lengths match and returning a descriptive error to prevent silent mis-configuration.openmeter/notification/adapter/deliverystatus.go (1)
124-126: Typo in error pathThe message reads “failed to udpate …” – minor spelling fix.
- return nil, fmt.Errorf("failed to udpate notification event delivery status: %w", err) + return nil, fmt.Errorf("failed to update notification event delivery status: %w", err)openmeter/notification/service/channel.go (1)
192-195: Incorrect error message on update failureOn update failure the code returns “failed to create channel” which is misleading and complicates debugging.
- return nil, fmt.Errorf("failed to create channel: %w", err) + return nil, fmt.Errorf("failed to update channel: %w", err)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
openmeter/ent/db/mutation.go(4 hunks)openmeter/ent/db/notificationrule.go(2 hunks)openmeter/ent/db/notificationrule/notificationrule.go(1 hunks)openmeter/ent/db/notificationrule/where.go(2 hunks)openmeter/ent/db/notificationrule_create.go(1 hunks)openmeter/ent/schema/notification.go(3 hunks)openmeter/notification/adapter/adapter.go(1 hunks)openmeter/notification/adapter/channel.go(1 hunks)openmeter/notification/adapter/deliverystatus.go(1 hunks)openmeter/notification/adapter/event.go(1 hunks)openmeter/notification/adapter/repository.go(0 hunks)openmeter/notification/adapter/rule.go(1 hunks)openmeter/notification/consumer/balancetreshold.go(1 hunks)openmeter/notification/deliverystatus.go(1 hunks)openmeter/notification/event.go(0 hunks)openmeter/notification/httpdriver/mapping.go(3 hunks)openmeter/notification/httpdriver/rule.go(1 hunks)openmeter/notification/repository.go(1 hunks)openmeter/notification/rule.go(6 hunks)openmeter/notification/service/channel.go(6 hunks)openmeter/notification/service/deliverystatus.go(1 hunks)openmeter/notification/service/event.go(2 hunks)openmeter/notification/service/rule.go(5 hunks)openmeter/notification/service/service.go(2 hunks)openmeter/notification/webhook/svix.go(2 hunks)test/notification/repository.go(1 hunks)test/notification/rule.go(1 hunks)
💤 Files with no reviewable changes (2)
- openmeter/notification/event.go
- openmeter/notification/adapter/repository.go
🧰 Additional context used
🧬 Code Graph Analysis (13)
openmeter/notification/httpdriver/rule.go (2)
openmeter/notification/event.go (2)
EventTypeBalanceThreshold(35-35)CreateEventInput(126-138)openmeter/notification/internal/rule.go (1)
NewTestEventPayload(20-94)
openmeter/notification/consumer/balancetreshold.go (1)
openmeter/notification/event.go (2)
EventType(38-38)EventTypeBalanceThreshold(35-35)
openmeter/notification/httpdriver/mapping.go (3)
openmeter/notification/event.go (2)
EventType(38-38)EventTypeBalanceThreshold(35-35)openmeter/ent/db/notificationrule/notificationrule.go (1)
DefaultDisabled(97-97)openmeter/notification/rule.go (2)
RuleConfig(93-98)RuleConfigMeta(84-86)
openmeter/notification/service/service.go (1)
openmeter/notification/repository.go (1)
Repository(10-16)
openmeter/notification/service/rule.go (6)
openmeter/notification/service/service.go (2)
Service(21-30)New(45-83)openmeter/notification/rule.go (5)
CreateRuleInput(192-205)Rule(26-42)DeleteRuleInput(317-317)GetRuleInput(297-297)UpdateRuleInput(241-257)pkg/models/validator.go (1)
Validate(16-26)pkg/framework/transaction/transaction.go (2)
Run(31-49)RunWithNoValue(23-28)openmeter/notification/adapter/adapter.go (1)
New(33-42)openmeter/notification/channel.go (2)
Channel(38-52)ListChannelsInput(114-123)
openmeter/notification/service/deliverystatus.go (3)
openmeter/notification/service/service.go (1)
Service(21-30)openmeter/notification/deliverystatus.go (4)
UpdateEventDeliveryStatusInput(122-135)EventDeliveryStatus(41-54)ListEventsDeliveryStatusInput(58-77)GetEventDeliveryStatusInput(93-102)pkg/framework/transaction/transaction.go (1)
Run(31-49)
openmeter/notification/repository.go (1)
pkg/framework/entutils/transaction.go (1)
TxCreator(176-176)
openmeter/notification/rule.go (1)
openmeter/notification/event.go (2)
EventType(38-38)EventTypeBalanceThreshold(35-35)
openmeter/notification/adapter/deliverystatus.go (4)
openmeter/notification/deliverystatus.go (4)
ListEventsDeliveryStatusInput(58-77)EventDeliveryStatus(41-54)GetEventDeliveryStatusInput(93-102)UpdateEventDeliveryStatusInput(122-135)openmeter/notification/adapter/entitymapping.go (1)
EventDeliveryStatusFromDBEntity(115-128)pkg/framework/entutils/transaction.go (1)
TransactingRepo(199-221)openmeter/notification/adapter/adapter.go (1)
New(33-42)
openmeter/ent/schema/notification.go (1)
openmeter/notification/event.go (2)
EventType(38-38)EventTypeBalanceThreshold(35-35)
test/notification/repository.go (2)
openmeter/notification/event.go (1)
EventTypeBalanceThreshold(35-35)openmeter/notification/rule.go (2)
RuleConfig(93-98)RuleConfigMeta(84-86)
openmeter/ent/db/notificationrule/where.go (3)
openmeter/ent/db/notificationrule.go (2)
NotificationRule(17-41)NotificationRule(73-90)openmeter/ent/schema/notification.go (5)
NotificationRule(66-68)NotificationRule(70-76)NotificationRule(78-98)NotificationRule(100-106)NotificationRule(108-113)openmeter/ent/db/notificationrule/notificationrule.go (1)
FieldType(29-29)
openmeter/notification/adapter/channel.go (8)
openmeter/notification/channel.go (6)
ListChannelsInput(114-123)Channel(38-52)CreateChannelInput(133-144)DeleteChannelInput(240-240)GetChannelInput(220-220)UpdateChannelInput(172-186)openmeter/ent/db/notificationrule/where.go (1)
Disabled(95-97)pkg/sortx/order.go (2)
OrderDefault(8-8)Order(3-3)openmeter/notification/service.go (5)
OrderBy(16-16)OrderByCreatedAt(12-12)OrderByUpdatedAt(13-13)OrderByType(11-11)OrderByID(10-10)openmeter/notification/adapter/entitymapping.go (1)
ChannelFromDBEntity(13-37)pkg/framework/entutils/transaction.go (2)
TransactingRepo(199-221)TransactingRepoWithNoValue(224-236)openmeter/notification/adapter/adapter.go (1)
Config(16-19)pkg/clock/clock.go (1)
Now(14-21)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: Artifacts / Container image
- GitHub Check: Test
- GitHub Check: Quickstart
- GitHub Check: Lint
- GitHub Check: E2E
- GitHub Check: Developer environment
- GitHub Check: CI
- GitHub Check: Analyze (go)
- GitHub Check: Commit hooks
- GitHub Check: Build
🔇 Additional comments (26)
openmeter/notification/httpdriver/rule.go (2)
288-288: Type unification implemented correctly.The switch case has been updated to use
notification.EventTypeBalanceThresholdinstead of the previousRuleTypeBalanceThreshold, aligning with the broader refactoring effort to consolidate event types across the codebase.
294-294: Type casting removed correctly.The explicit cast to
notification.EventTypehas been removed as it's no longer needed, sincerule.Typeis now directly of typeEventType. This simplifies the code while maintaining type safety.openmeter/notification/service/service.go (2)
24-24: Repository field renamed to adapter.The field has been renamed from
repotoadapterwhile maintaining thenotification.Repositoryinterface type. This naming change better reflects the adapter pattern being adopted in this refactoring.
77-77: Constructor updated to match field name change.The constructor assignment has been properly updated to use the renamed field
adapter, ensuring consistency with the struct field change.test/notification/repository.go (2)
55-55: Type constant updated in test setup.The
Typefield in theCreateRuleInputhas been correctly updated to usenotification.EventTypeBalanceThresholdinstead ofRuleTypeBalanceThreshold, maintaining consistency with the type consolidation in the main codebase.
60-60: Nested type updated in rule configuration.The
Typefield in theRuleConfigMetastruct has also been updated to useEventTypeBalanceThreshold, ensuring full consistency in the test fixtures with the type unification refactoring.openmeter/ent/db/notificationrule.go (2)
30-30: Database model field type updated.The
Typefield in theNotificationRulestruct has been changed fromnotification.RuleTypetonotification.EventType, ensuring the database model properly represents the unified type system.
135-135: Type casting in SQL value assignment updated.The assignment of the SQL string value now correctly casts to
notification.EventTypeinstead of the previousRuleType, maintaining consistency in the database layer with the type unification refactoring.openmeter/ent/db/notificationrule/notificationrule.go (1)
107-107: Type parameter updated to use EventType instead of RuleTypeThe function parameter type has been changed from
notification.RuleTypetonotification.EventTypeas part of the broader type consolidation effort mentioned in the PR objectives.openmeter/notification/consumer/balancetreshold.go (1)
48-48: Updated type references in ListRules filterThe code now correctly uses
notification.EventTypeandEventTypeBalanceThresholdinstead of the previousRuleType, aligning with the consolidated type system across the notification domain.test/notification/rule.go (2)
26-27: Updated type field to use EventTypeThis test helper now correctly uses
notification.EventTypeBalanceThresholdinstead of the previousRuleTypeBalanceThreshold, maintaining consistency with the production code changes.
31-32: Updated nested type references to use EventTypeThe RuleConfigMeta.Type field is now properly using the
notification.EventTypeBalanceThresholdconstant, ensuring consistency throughout the test data structure.openmeter/ent/db/notificationrule_create.go (1)
78-81: Updated SetType method parameter typeThe method now accepts
notification.EventTypeinstead ofnotification.RuleType, aligning with the PR's type consolidation objectives across the notification domain.openmeter/notification/webhook/svix.go (2)
586-586: Fixed misleading error message.The previous error message incorrectly referenced "delete Svix endpoint" instead of accurately reflecting the attempted operation. This update corrects the message to "failed to send message", which properly represents the operation being performed in the SendMessage method.
659-666: Improved error handling for Svix errors.This is a good enhancement to the error handling logic. The previous implementation only captured the first error message from the Svix API response, potentially losing valuable debug information. The updated code properly collects all error messages from the response body and joins them together, providing more comprehensive error details to the caller.
openmeter/notification/httpdriver/mapping.go (3)
99-99: Type consolidation: RuleType replaced with EventType.This change aligns with the PR objective of consolidating types in the notification system. Using EventType instead of RuleType creates a more consistent type system across the codebase.
Also applies to: 103-104
120-121: Consistent type usage in update request function.Similar to the create request function, this change ensures consistent use of EventType instead of RuleType, maintaining alignment with the rest of the notification system.
Also applies to: 124-125
139-141: Standardized event type matching in switch statement.Updated the switch statement to use EventTypeBalanceThreshold instead of RuleTypeBalanceThreshold, completing the type consolidation across the mapping layer.
openmeter/ent/db/mutation.go (5)
37424-37424: Type field updated to use EventType consistentlyThis change aligns with the consolidation of notification types by replacing
RuleTypewithEventTypein the struct definition. This is part of the broader refactoring to standardize on a single event type across the codebase.
37702-37703: Method signature updated to use EventTypeThe
SetTypemethod signature has been properly updated to usenotification.EventTypeinstead ofRuleType, maintaining consistent type usage throughout the codebase.
37707-37710: Return type updated to use EventTypeThe
GetTypemethod signature has been correctly updated to returnnotification.EventTypeinstead ofRuleType, ensuring consistency with other changes in the PR.
37718-37721: OldType method updated to use EventTypeThe
OldTypemethod now correctly usesnotification.EventTypeas its return type, maintaining consistency with the type consolidation effort.
38112-38115: Type assertion updated to use EventTypeThe type assertion in the field update handler has been properly updated to use
notification.EventType, ensuring that type checking during field updates works correctly with the new type.openmeter/ent/schema/notification.go (1)
78-84: Ensure enum migration is applied after switching from RuleType → EventTypeChanging the Go type of the
"type"enum fromnotification.RuleTypetonotification.EventTypeis source-level only.
If the underlying column was created with a Postgres ENUM (or CHECK constraint) that still references the old values, reads/writes will start failing at runtime.Please double-check the existing DB schema. If the column is backed by an ENUM, you will need a migration similar to:
ALTER TYPE notification_rule_type RENAME TO notification_event_type;(or recreate the constraint) before deploying the refactor.
Let me know if you want a full migration snippet generated.openmeter/notification/repository.go (1)
6-12: 👍 Embedding TxCreator cleans up ad-hoc helpersReplacing the bespoke
WithTx*helpers with theentutils.TxCreatorinterface makes the transaction story consistent across packages and drops ~80 lines of boiler-plate.
Implementation structs just need to forwardTx(ctx)andWithTx(...), which are already provided by the adapter layer.openmeter/notification/adapter/deliverystatus.go (1)
48-58: Unstable pagination without explicit ordering
ListEventsDeliveryStatuspaginates without anORDER BY. The underlying SQL engine’s default ordering is implementation-dependent, leading to item duplication or omission between pages.
Add a deterministic order (e.g.,UpdatedAt DESC, ID ASC) before callingPaginate.- paged, err := query.Paginate(ctx, params.Page) + paged, err := query. + Order(statusdb.ByUpdatedAt(entutils.GetOrdering(sortx.OrderDesc)...), statusdb.ByID()). + Paginate(ctx, params.Page)
|
I support the general direction 👍 |
chrisgacsal
force-pushed
the
refactor/notification
branch
from
d2d9091 to
66a30cc
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
openmeter/notification/adapter/event.go (1)
219-234: Consider logging when no delivery statuses are createdWhile the code correctly handles rules with no channels (and won't panic), consider adding a warning log when
statusBulkQueryremains empty after processing all channels, to help with debugging.statusBulkQuery = append(statusBulkQuery, q) } + +if len(statusBulkQuery) == 0 { + a.logger.WarnContext(ctx, "no delivery statuses created for event", + "eventID", eventRow.ID, + "ruleID", params.RuleID, + "namespace", params.Namespace) +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
openmeter/notification/adapter/adapter.go(1 hunks)openmeter/notification/adapter/channel.go(1 hunks)openmeter/notification/adapter/deliverystatus.go(1 hunks)openmeter/notification/adapter/event.go(1 hunks)openmeter/notification/adapter/rule.go(1 hunks)openmeter/notification/deliverystatus.go(1 hunks)openmeter/notification/repository.go(1 hunks)openmeter/notification/service/channel.go(6 hunks)openmeter/notification/service/deliverystatus.go(1 hunks)openmeter/notification/service/event.go(2 hunks)openmeter/notification/service/rule.go(5 hunks)openmeter/notification/service/service.go(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (11)
- openmeter/notification/service/service.go
- openmeter/notification/adapter/deliverystatus.go
- openmeter/notification/adapter/adapter.go
- openmeter/notification/adapter/rule.go
- openmeter/notification/service/deliverystatus.go
- openmeter/notification/service/rule.go
- openmeter/notification/service/event.go
- openmeter/notification/deliverystatus.go
- openmeter/notification/adapter/channel.go
- openmeter/notification/repository.go
- openmeter/notification/service/channel.go
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Quickstart
- GitHub Check: E2E
- GitHub Check: CI
- GitHub Check: Developer environment
- GitHub Check: Commit hooks
- GitHub Check: Lint
- GitHub Check: Test
- GitHub Check: Build
- GitHub Check: Analyze (go)
🔇 Additional comments (5)
openmeter/notification/adapter/event.go (5)
21-128: Well-structured transaction pattern in ListEventsThe implementation correctly wraps the core query logic with the transaction pattern using
entutils.TransactingRepo. The filtering, pagination, and error handling look solid.
47-49: Good improvement on filtering by delivery statusNow correctly using
eventdb.HasDeliveryStatusesWithto filter parent events based on child delivery status states, which ensures you only get events with at least one matching status - addressing a known issue.
130-165: GetEvent implementation follows consistent transaction patternThe method correctly implements the transaction pattern for retrieving a single event by ID, with proper error handling for not found cases and nil results.
167-254: CreateEvent correctly implements transaction patternThe method successfully wraps the event creation logic in a transaction, properly handles error cases, and creates delivery statuses for all channels associated with the rule.
236-241: Bulk creation of delivery statuses looks goodThe implementation correctly handles the creation of delivery statuses for all channels. Note that as previously confirmed, Ent's
CreateBulkwon't panic with an empty slice if there are no channels.
4 participants
Overview
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Tests