feat(subs): link subscriptions via annotations #3565
Conversation
|
Warning Rate limit exceeded@GAlexIHU has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 20 minutes and 21 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (37)
📝 WalkthroughWalkthroughAdds an Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as HTTP Handler
participant Service as Subscription Service
participant Hooks as Registered Hooks
participant Repo as Subscription Repo
participant DB as DB
Client->>API: CreateSubscription(spec{annotations?})
API->>Service: Create(spec)
Service->>Hooks: BeforeCreate(namespace, spec)
Hooks-->>Service: (ok / error)
Service->>Repo: Create(entity with annotations)
Repo->>DB: INSERT subscriptions (annotations jsonb)
DB-->>Repo: OK
Repo-->>Service: Subscription
Service->>Hooks: AfterCreate(subscription)
Hooks-->>Service: (ok)
Service-->>API: 201 Created (subscription)
sequenceDiagram
participant Caller
participant Service
participant Repo
participant Hooks
Caller->>Service: ChangeToPlan(id, newPlan)
Service->>Repo: Get(current)
Repo-->>Service: current
Service->>Service: build createInputAnnotations(previousID)
Service->>Service: Create new subscription (with annotations)
Service->>Repo: Create(new)
Repo-->>Service: new
Service->>Service: set superseding on old annotations
Service->>Repo: UpdateAnnotations(oldID, annotations)
Repo->>DB: UPDATE subscriptions SET annotations = ...
Repo-->>Service: updated old
Service->>Hooks: AfterCreate(new) / AfterUpdate(old)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Focus areas:
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
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 |
GAlexIHU
force-pushed
the
feat/subscription/annotations-on-change
branch
from
d8d2a60 to
56a5e15
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
openmeter/subscription/workflow/service.go (1)
29-29: Consider adding a json tag for consistency.Other fields in this struct have json tags (like
billingAnchoron line 28). IfAnnotationswill be serialized/deserialized, you might want to addjson:"annotations,omitempty"here for consistency.openmeter/subscription/repo/mapping.go (1)
36-60: LGTM! Annotations mapping looks solid.The logic correctly handles both populated and empty annotations, ensuring the domain model always has a non-nil Annotations field. This is good for consistency downstream.
One tiny optional note: the conditional initialization could technically be simplified to just
annotations := sub.Annotations(since Go maps can be nil), but explicitly ensuring a non-nil map is a reasonable design choice for the domain layer.openmeter/server/server_test.go (1)
1228-1230: Prefer returning a non-nil stub Subscription.If a handler ever calls this stub and tries to read the returned subscription, the current
nil, nilresponse will blow up with a nil deref even though we reported success. Let’s hand back an empty struct instead so the mock mirrors real behavior a bit better.func (n NoopSubscriptionService) UpdateAnnotations(ctx context.Context, subscriptionID models.NamespacedID, annotations models.Annotations) (*subscription.Subscription, error) { - return nil, nil + return &subscription.Subscription{}, nil }openmeter/subscription/service/service.go (1)
73-82: Let's call this a hook everywhere.Could we rename the parameter and tweak the error so we stop referring to validators? It’ll keep the logs and API surface consistent with the new hook-based setup.
-func (s *service) RegisterHook(validator subscription.SubscriptionCommandHook) error { - if validator == nil { - return errors.New("invalid subscription validator: nil") +func (s *service) RegisterHook(hook subscription.SubscriptionCommandHook) error { + if hook == nil { + return errors.New("invalid subscription hook: nil") } s.mu.Lock() defer s.mu.Unlock() - s.Hooks = append(s.Hooks, validator) + s.Hooks = append(s.Hooks, hook) return nil }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
api/client/go/client.gen.gois excluded by!api/client/**api/client/javascript/src/client/schemas.tsis excluded by!api/client/**api/client/python/openmeter/_generated/models/_models.pyis excluded by!**/_generated/**,!api/client/**api/openapi.cloud.yamlis excluded by!**/openapi.cloud.yamlapi/openapi.yamlis excluded by!**/openapi.yamlgo.sumis excluded by!**/*.sum,!**/*.sumtools/migrate/migrations/atlas.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (37)
api/api.gen.go(3 hunks)api/spec/src/productcatalog/subscription.tsp(1 hunks)app/common/billing.go(1 hunks)app/common/subscription.go(2 hunks)openmeter/billing/validators/subscription/validator.go(3 hunks)openmeter/ent/db/migrate/schema.go(4 hunks)openmeter/ent/db/mutation.go(9 hunks)openmeter/ent/db/runtime.go(1 hunks)openmeter/ent/db/setorclear.go(1 hunks)openmeter/ent/db/subscription.go(5 hunks)openmeter/ent/db/subscription/subscription.go(2 hunks)openmeter/ent/db/subscription/where.go(1 hunks)openmeter/ent/db/subscription_create.go(6 hunks)openmeter/ent/db/subscription_update.go(5 hunks)openmeter/ent/schema/subscription.go(1 hunks)openmeter/productcatalog/subscription/http/mapping.go(2 hunks)openmeter/server/server_test.go(1 hunks)openmeter/subscription/annotations.go(2 hunks)openmeter/subscription/hook.go(1 hunks)openmeter/subscription/hooks/annotations/hook.go(1 hunks)openmeter/subscription/repo/mapping.go(2 hunks)openmeter/subscription/repo/subscriptionrepo.go(2 hunks)openmeter/subscription/repository.go(2 hunks)openmeter/subscription/service.go(2 hunks)openmeter/subscription/service/service.go(12 hunks)openmeter/subscription/service/service_test.go(6 hunks)openmeter/subscription/subscription.go(2 hunks)openmeter/subscription/subscriptionspec.go(2 hunks)openmeter/subscription/testutils/mock.go(2 hunks)openmeter/subscription/testutils/service.go(2 hunks)openmeter/subscription/validator.go(0 hunks)openmeter/subscription/validators/subscription/validator.go(5 hunks)openmeter/subscription/workflow/service.go(1 hunks)openmeter/subscription/workflow/service/subscription.go(3 hunks)openmeter/subscription/workflow/service/subscription_test.go(1 hunks)tools/migrate/migrations/20251102163348_add-subscription-annotations.down.sql(1 hunks)tools/migrate/migrations/20251102163348_add-subscription-annotations.up.sql(1 hunks)
💤 Files with no reviewable changes (1)
- openmeter/subscription/validator.go
🧰 Additional context used
📓 Path-based instructions (3)
**/*.go
⚙️ CodeRabbit configuration file
**/*.go: In general when reviewing the Golang code make readability and maintainability a priority, even potentially suggest restructuring the code to improve them.Performance should be a priority in critical code paths. Anything related to event ingestion, message processing, database operations (regardless of database) should be vetted for potential performance bottlenecks.
Files:
app/common/subscription.goopenmeter/ent/db/subscription/subscription.goopenmeter/ent/db/subscription.goopenmeter/ent/db/subscription/where.goopenmeter/ent/db/setorclear.goopenmeter/ent/schema/subscription.goopenmeter/subscription/subscriptionspec.goopenmeter/productcatalog/subscription/http/mapping.goopenmeter/subscription/workflow/service/subscription_test.goapp/common/billing.goopenmeter/ent/db/subscription_update.goopenmeter/subscription/subscription.goopenmeter/ent/db/runtime.goopenmeter/server/server_test.goopenmeter/subscription/repo/subscriptionrepo.goopenmeter/billing/validators/subscription/validator.goopenmeter/subscription/service.goopenmeter/subscription/repo/mapping.goapi/api.gen.goopenmeter/subscription/repository.goopenmeter/subscription/annotations.goopenmeter/subscription/hook.goopenmeter/subscription/validators/subscription/validator.goopenmeter/subscription/hooks/annotations/hook.goopenmeter/subscription/testutils/service.goopenmeter/subscription/testutils/mock.goopenmeter/subscription/workflow/service/subscription.goopenmeter/ent/db/subscription_create.goopenmeter/subscription/service/service_test.goopenmeter/ent/db/migrate/schema.goopenmeter/subscription/workflow/service.goopenmeter/ent/db/mutation.goopenmeter/subscription/service/service.go
**/*_test.go
⚙️ CodeRabbit configuration file
**/*_test.go: Make sure the tests are comprehensive and cover the changes. Keep a strong focus on unit tests and in-code integration tests.
When appropriate, recommend e2e tests for critical changes.
Files:
openmeter/subscription/workflow/service/subscription_test.goopenmeter/server/server_test.goopenmeter/subscription/service/service_test.go
**/*.tsp
⚙️ CodeRabbit configuration file
**/*.tsp: Review the TypeSpec code for conformity with TypeSpec best practices. When recommending changes also consider the fact that multiple codegeneration toolchains depend on the TypeSpec code, each of which have their idiosyncrasies and bugs.The declared API should be accurate, in parity with the actual implementation, and easy to understand for the user.
Files:
api/spec/src/productcatalog/subscription.tsp
🧠 Learnings (2)
📚 Learning: 2025-08-29T12:31:52.802Z
Learnt from: chrisgacsal
Repo: openmeterio/openmeter PR: 3291
File: app/common/customer.go:88-89
Timestamp: 2025-08-29T12:31:52.802Z
Learning: In Go projects using Google's wire dependency injection framework, named types (without =) should be used instead of type aliases (with =) to work around wire limitations. For example, use `type CustomerSubjectValidatorHook customerservicehooks.SubjectValidatorHook` instead of `type CustomerSubjectValidatorHook = customerservicehooks.SubjectValidatorHook` when wire is involved.
Applied to files:
app/common/billing.goopenmeter/subscription/service/service.go
📚 Learning: 2025-03-07T12:17:43.129Z
Learnt from: GAlexIHU
Repo: openmeterio/openmeter PR: 2383
File: openmeter/entitlement/metered/lateevents_test.go:37-45
Timestamp: 2025-03-07T12:17:43.129Z
Learning: In the OpenMeter codebase, test files like `openmeter/entitlement/metered/lateevents_test.go` may use variables like `meterSlug` and `namespace` without explicit declarations visible in the same file. This appears to be an accepted pattern in their test structure.
Applied to files:
openmeter/subscription/service/service_test.go
🧬 Code graph analysis (33)
app/common/subscription.go (1)
openmeter/subscription/hooks/annotations/hook.go (1)
NewAnnotationCleanupHook(20-33)
openmeter/ent/db/subscription/subscription.go (3)
openmeter/ent/db/entitlement/entitlement.go (1)
FieldAnnotations(67-67)openmeter/ent/db/billinginvoiceline/billinginvoiceline.go (1)
FieldAnnotations(21-21)openmeter/ent/db/subscriptionitem/subscriptionitem.go (1)
FieldAnnotations(31-31)
openmeter/ent/db/subscription.go (7)
api/api.gen.go (1)
Annotations(1042-1042)api/client/python/openmeter/_generated/models/_models.py (1)
Annotations(341-342)api/client/go/client.gen.go (1)
Annotations(957-957)openmeter/ent/db/subscription/subscription.go (2)
FieldAnnotations(22-22)FieldMetadata(30-30)openmeter/ent/db/entitlement/entitlement.go (2)
FieldAnnotations(67-67)FieldMetadata(23-23)openmeter/ent/db/billinginvoiceline/billinginvoiceline.go (2)
FieldAnnotations(21-21)FieldMetadata(25-25)openmeter/ent/db/subscriptionitem/subscriptionitem.go (2)
FieldAnnotations(31-31)FieldMetadata(29-29)
openmeter/ent/db/subscription/where.go (3)
openmeter/ent/db/entitlement/where.go (1)
AnnotationsIsNil(1265-1267)openmeter/ent/db/subscription.go (2)
Subscription(23-63)Subscription(143-160)openmeter/ent/db/subscription/subscription.go (1)
FieldAnnotations(22-22)
openmeter/ent/db/setorclear.go (3)
openmeter/ent/db/subscription_update.go (2)
SubscriptionUpdate(27-31)SubscriptionUpdateOne(702-707)api/api.gen.go (1)
Annotations(1042-1042)pkg/models/annotation.go (1)
Annotations(3-3)
openmeter/ent/schema/subscription.go (1)
pkg/framework/entutils/mixins.go (3)
AnnotationsMixin(157-159)AnnotationsMixin(162-170)AnnotationsMixin(172-181)
openmeter/subscription/subscriptionspec.go (2)
api/api.gen.go (1)
Annotations(1042-1042)pkg/models/annotation.go (1)
Annotations(3-3)
openmeter/productcatalog/subscription/http/mapping.go (5)
api/api.gen.go (1)
Annotations(1042-1042)api/client/javascript/src/client/schemas.ts (1)
Annotations(11535-11535)api/client/python/openmeter/_generated/models/_models.py (1)
Annotations(341-342)api/client/go/client.gen.go (1)
Annotations(957-957)pkg/models/annotation.go (1)
Annotations(3-3)
openmeter/subscription/workflow/service/subscription_test.go (9)
openmeter/subscription/service/service_test.go (1)
TestSubscriptionChangeTrackingAnnotations(675-951)openmeter/subscription/workflow/service.go (3)
Service(13-21)CreateSubscriptionWorkflowInput(23-30)ChangeSubscriptionWorkflowInput(32-39)openmeter/subscription/testutils/db.go (2)
DBDeps(15-19)SetupDBDeps(41-75)openmeter/subscription/testutils/service.go (2)
SubscriptionDependencies(48-66)NewService(68-275)openmeter/subscription/testutils/plan.go (1)
GetExamplePlanInput(19-27)openmeter/testutils/time.go (1)
GetRFC3339Time(8-15)pkg/clock/clock.go (2)
SetTime(23-27)Now(14-21)openmeter/subscription/annotations.go (1)
AnnotationParser(30-30)openmeter/subscription/workflow/annotations.go (1)
AnnotationParser(15-15)
app/common/billing.go (3)
openmeter/subscription/service.go (1)
Service(37-40)openmeter/customer/service.go (1)
Service(11-16)openmeter/subject/service.go (1)
Service(11-24)
openmeter/ent/db/subscription_update.go (2)
api/api.gen.go (1)
Annotations(1042-1042)openmeter/ent/db/subscription/subscription.go (1)
FieldAnnotations(22-22)
openmeter/subscription/subscription.go (5)
api/api.gen.go (1)
Annotations(1042-1042)api/client/javascript/src/client/schemas.ts (1)
Annotations(11535-11535)api/client/python/openmeter/_generated/models/_models.py (1)
Annotations(341-342)api/client/go/client.gen.go (1)
Annotations(957-957)pkg/models/annotation.go (1)
Annotations(3-3)
openmeter/ent/db/runtime.go (2)
openmeter/ent/schema/subscription.go (5)
Subscription(18-20)Subscription(22-31)Subscription(33-58)Subscription(60-65)Subscription(67-81)openmeter/ent/db/subscription/subscription.go (2)
NamespaceValidator(142-142)DefaultCreatedAt(144-144)
openmeter/server/server_test.go (4)
pkg/models/id.go (1)
NamespacedID(7-10)api/api.gen.go (2)
Annotations(1042-1042)Subscription(6915-6972)openmeter/subscription/subscription.go (1)
Subscription(13-33)openmeter/subscription/hook.go (1)
SubscriptionCommandHook(9-21)
openmeter/subscription/repo/subscriptionrepo.go (6)
pkg/models/id.go (1)
NamespacedID(7-10)openmeter/ent/db/subscription.go (2)
Subscription(23-63)Subscription(143-160)pkg/framework/entutils/transaction.go (1)
TransactingRepo(199-221)openmeter/ent/db/subscription/where.go (2)
ID(16-18)Namespace(71-73)openmeter/subscription/errors.go (1)
NewSubscriptionNotFoundError(167-173)openmeter/subscription/repo/mapping.go (1)
MapDBSubscription(15-70)
openmeter/billing/validators/subscription/validator.go (1)
openmeter/subscription/hook.go (10)
NoOpSubscriptionCommandHook(25-25)NoOpSubscriptionCommandHook(27-29)NoOpSubscriptionCommandHook(31-33)NoOpSubscriptionCommandHook(35-37)NoOpSubscriptionCommandHook(39-41)NoOpSubscriptionCommandHook(43-45)NoOpSubscriptionCommandHook(47-49)NoOpSubscriptionCommandHook(51-53)NoOpSubscriptionCommandHook(55-57)SubscriptionCommandHook(9-21)
openmeter/subscription/service.go (4)
pkg/models/id.go (1)
NamespacedID(7-10)pkg/models/annotation.go (1)
Annotations(3-3)openmeter/subscription/subscription.go (1)
Subscription(13-33)openmeter/subscription/hook.go (1)
SubscriptionCommandHook(9-21)
openmeter/subscription/repo/mapping.go (5)
api/api.gen.go (1)
Annotations(1042-1042)api/client/javascript/src/client/schemas.ts (1)
Annotations(11535-11535)api/client/python/openmeter/_generated/models/_models.py (1)
Annotations(341-342)api/client/go/client.gen.go (1)
Annotations(957-957)pkg/models/annotation.go (1)
Annotations(3-3)
api/api.gen.go (3)
api/client/javascript/src/client/schemas.ts (1)
Annotations(11535-11535)api/client/python/openmeter/_generated/models/_models.py (1)
Annotations(341-342)api/client/go/client.gen.go (1)
Annotations(957-957)
openmeter/subscription/repository.go (3)
pkg/models/annotation.go (1)
Annotations(3-3)openmeter/ent/db/subscription.go (2)
Subscription(23-63)Subscription(143-160)openmeter/subscription/subscription.go (1)
Subscription(13-33)
openmeter/subscription/annotations.go (3)
api/api.gen.go (1)
Annotations(1042-1042)api/client/javascript/src/client/schemas.ts (1)
Annotations(11535-11535)api/client/go/client.gen.go (1)
Annotations(957-957)
openmeter/subscription/hook.go (3)
openmeter/subscription/subscriptionspec.go (1)
SubscriptionSpec(54-60)openmeter/subscription/subscriptionview.go (1)
SubscriptionView(19-24)pkg/models/id.go (1)
NamespacedID(7-10)
openmeter/subscription/validators/subscription/validator.go (4)
openmeter/subscription/hook.go (10)
NoOpSubscriptionCommandHook(25-25)NoOpSubscriptionCommandHook(27-29)NoOpSubscriptionCommandHook(31-33)NoOpSubscriptionCommandHook(35-37)NoOpSubscriptionCommandHook(39-41)NoOpSubscriptionCommandHook(43-45)NoOpSubscriptionCommandHook(47-49)NoOpSubscriptionCommandHook(51-53)NoOpSubscriptionCommandHook(55-57)SubscriptionCommandHook(9-21)openmeter/subscription/subscriptionspec.go (1)
SubscriptionSpec(54-60)pkg/models/id.go (1)
NamespacedID(7-10)openmeter/subscription/subscriptionview.go (1)
SubscriptionView(19-24)
openmeter/subscription/hooks/annotations/hook.go (6)
openmeter/subscription/hook.go (9)
NoOpSubscriptionCommandHook(25-25)NoOpSubscriptionCommandHook(27-29)NoOpSubscriptionCommandHook(31-33)NoOpSubscriptionCommandHook(35-37)NoOpSubscriptionCommandHook(39-41)NoOpSubscriptionCommandHook(43-45)NoOpSubscriptionCommandHook(47-49)NoOpSubscriptionCommandHook(51-53)NoOpSubscriptionCommandHook(55-57)openmeter/subscription/service.go (1)
QueryService(9-18)openmeter/subscription/repository.go (1)
SubscriptionRepository(40-59)openmeter/subscription/subscriptionview.go (1)
SubscriptionView(19-24)pkg/models/id.go (1)
NamespacedID(7-10)openmeter/subscription/annotations.go (1)
AnnotationPreviousSubscriptionID(20-20)
openmeter/subscription/testutils/service.go (1)
openmeter/subscription/hooks/annotations/hook.go (1)
NewAnnotationCleanupHook(20-33)
openmeter/subscription/testutils/mock.go (4)
pkg/models/id.go (1)
NamespacedID(7-10)openmeter/subscription/subscription.go (1)
Subscription(13-33)openmeter/subscription/hook.go (1)
SubscriptionCommandHook(9-21)openmeter/subscription/service/service.go (1)
New(44-63)
openmeter/subscription/workflow/service/subscription.go (5)
openmeter/subscription/annotations.go (1)
AnnotationParser(30-30)openmeter/subscription/workflow/annotations.go (1)
AnnotationParser(15-15)openmeter/subscription/workflow/service.go (3)
CreateSubscriptionWorkflowInput(23-30)ChangeSubscriptionWorkflowInput(32-39)Service(13-21)openmeter/ent/db/subscription.go (2)
Subscription(23-63)Subscription(143-160)openmeter/subscription/subscription.go (1)
Subscription(13-33)
openmeter/ent/db/subscription_create.go (2)
api/api.gen.go (1)
Annotations(1042-1042)openmeter/ent/db/subscription/subscription.go (1)
FieldAnnotations(22-22)
openmeter/subscription/service/service_test.go (6)
api/api.gen.go (4)
Annotations(1042-1042)CustomerId(2322-2325)Currency(1930-1942)Subscription(6915-6972)openmeter/subscription/subscriptionspec.go (1)
CreateSubscriptionCustomerInput(42-52)openmeter/ent/schema/subscription.go (5)
Subscription(18-20)Subscription(22-31)Subscription(33-58)Subscription(60-65)Subscription(67-81)openmeter/subscription/subscription.go (1)
Subscription(13-33)openmeter/subscription/workflow/service.go (2)
CreateSubscriptionWorkflowInput(23-30)ChangeSubscriptionWorkflowInput(32-39)openmeter/subscription/annotations.go (1)
AnnotationParser(30-30)
openmeter/ent/db/migrate/schema.go (1)
openmeter/ent/db/subscription/subscription.go (1)
Columns(110-128)
openmeter/subscription/workflow/service.go (3)
api/api.gen.go (1)
Annotations(1042-1042)api/client/javascript/src/client/schemas.ts (1)
Annotations(11535-11535)api/client/go/client.gen.go (1)
Annotations(957-957)
openmeter/ent/db/mutation.go (2)
pkg/models/annotation.go (1)
Annotations(3-3)openmeter/ent/db/subscription/subscription.go (1)
FieldAnnotations(22-22)
openmeter/subscription/service/service.go (3)
openmeter/subscription/hook.go (1)
SubscriptionCommandHook(9-21)openmeter/ent/db/subscription.go (2)
Subscription(23-63)Subscription(143-160)openmeter/subscription/subscription.go (1)
Subscription(13-33)
⏰ 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: Code Generators
- GitHub Check: Analyze (go)
🔇 Additional comments (28)
openmeter/ent/db/runtime.go (1)
1349-1350: Looks good—mixin reordering reflected correctly.The subscription descriptor initialization properly accounts for the mixin shift. Adding
AnnotationsMixinpushedTimeMixinfrom index 2 to index 3, and the created_at/updated_at descriptor sources are now correctly sourced fromsubscriptionMixinFields3[0]andsubscriptionMixinFields3[1]respectively. Since this is generated code, the changes are exactly what we'd expect.Also applies to: 1358-1358, 1362-1362
api/api.gen.go (3)
6925-6926: LGTM! Generated code looks good.The
Annotationsfield addition is clean and consistent with the PR's goal of tracking subscription changes via annotations. The pointer type withomitemptymakes sense for an optional, system-managed field.
7153-7154: LGTM! Consistent addition.This mirrors the annotations field added in the previous struct, maintaining consistency across the generated API types.
21020-21492: LGTM! Regenerated embedded spec.This large block appears to be the regenerated embedded OpenAPI/Swagger specification that now includes the Annotations field. This is expected when the API schema is updated.
tools/migrate/migrations/20251102163348_add-subscription-annotations.up.sql (1)
1-4: LGTM! Clean migration for annotations support.The GIN index on the JSONB column is perfect for efficient annotation queries, and making it nullable gives you the flexibility you need.
tools/migrate/migrations/20251102163348_add-subscription-annotations.down.sql (1)
1-4: LGTM! Rollback looks good.Nice job dropping the index before the column—proper cleanup order.
openmeter/ent/schema/subscription.go (1)
22-31: LGTM! Mixin integration looks solid.Adding
AnnotationsMixinhere aligns perfectly with your migration and keeps the schema definition clean and consistent with other entities.app/common/billing.go (1)
91-91: LGTM! Hook registration looks good.The validator → hook rename is clean and the error handling is preserved correctly.
openmeter/subscription/subscriptionspec.go (2)
42-52: LGTM! Field addition looks clean.The
Annotationsfield fits nicely into the customer input struct with proper typing and json tag.
62-82: LGTM! Propagation logic is solid.The annotations flow cleanly from the customer input to the entity input, maintaining consistency with how other fields are handled.
app/common/subscription.go (1)
124-131: LGTM! Hook wiring looks solid.The annotation cleanup hook is properly initialized with the right dependencies and registered with appropriate error handling. Nice clean integration.
api/spec/src/productcatalog/subscription.tsp (1)
67-72: LGTM! API spec addition looks perfect.The
annotationsfield is properly marked as read-only with clear documentation that it's system-managed. Making it optional gives you the right flexibility.openmeter/subscription/subscription.go (2)
32-32: Nice addition!The Annotations field is cleanly integrated into the Subscription struct, consistent with other metadata fields.
42-42: Proper propagation.Annotations are correctly passed through in AsEntityInput(), ensuring they flow through the creation pipeline.
openmeter/ent/db/subscription/where.go (1)
202-210: Standard ent predicate generation.These predicate constructors follow the established pattern in the codebase and enable filtering subscriptions by annotation presence/absence. All good!
openmeter/subscription/testutils/service.go (2)
36-36: Clean import.The annotation hook package is properly imported for test setup.
242-244: Hook registration looks great!The AnnotationCleanupHook is correctly instantiated with the required dependencies (service and repo) and registered with proper error handling. This ensures annotations are cleaned up properly during subscription deletion in tests.
openmeter/subscription/repository.go (2)
21-22: Clean field addition.The Annotations field is properly added to CreateSubscriptionEntityInput, ensuring annotations can be set during subscription creation.
57-58: Well-designed repository method.The UpdateAnnotations method has a clear, focused signature for updating just the annotations field. Returning
*Subscriptionallows for proper handling of not-found cases.openmeter/subscription/repo/subscriptionrepo.go (2)
53-69: Solid implementation of UpdateAnnotations!The method correctly:
- Wraps the operation in a transaction using TransactingRepo
- Updates only the annotations field with SetAnnotations
- Filters by namespace for proper multi-tenancy
- Handles not-found errors with the domain-specific error type
- Maps the database result back to the domain model
All the pieces fit together nicely.
101-101: Annotations properly persisted on creation.The SetAnnotations call ensures annotations are stored when creating new subscriptions. Perfect!
openmeter/productcatalog/subscription/http/mapping.go (1)
179-195: Great API mapping approach!The conditional inclusion of annotations is a smart design choice—only exposing them when present keeps API responses clean. The use of
lo.ToPtrfor the conversion is also consistent with how other optional fields (like Metadata) are handled in this mapping.openmeter/ent/db/setorclear.go (1)
2873-2885: Consistent SetOrClear pattern.These helper methods follow the established pattern throughout the file, making it easy to conditionally set or clear annotations during updates. The implementation is consistent with all other SetOrClear methods, which is exactly what we want for generated code.
openmeter/billing/validators/subscription/validator.go (1)
15-46: Smooth hook migration, nicely done.Love how you swapped in the hook interface without losing the billing setup guardrails—reads clean and keeps behavior identical. 👍
openmeter/subscription/workflow/service/subscription.go (1)
73-237: Great call on wiring the annotation links.Passing annotations through create and neatly stitching previous/superseding IDs (with the clone to dodge shared-map surprises) makes the upgrade flow a lot easier to trace. 🙌
openmeter/subscription/annotations.go (1)
19-165: Helpers look super handy.These parser helpers hit the right balance—clean constants, safe nil handling, and clear intent for previous/superseding IDs. Nice touch keeping the API consistent with the existing helpers.
openmeter/ent/db/migrate/schema.go (1)
2264-2330: Schema updates look consistent.The new annotations column/index slot right in, and the FK/index offsets all line up after the shift—thanks for keeping the generated schema tidy.
openmeter/subscription/service/service_test.go (1)
94-951: Loving the annotation coverage.These tests nail the big cases—preserving user data, linking previous/superseding subs, and cleaning up on delete (even with nil annotations). Feels rock solid.
GAlexIHU
force-pushed
the
feat/subscription/annotations-on-change
branch
from
56a5e15 to
cfaf100
Compare
GAlexIHU
force-pushed
the
feat/subscription/annotations-on-change
branch
from
cfaf100 to
3f7225a
Compare
2 participants
Overview
Adds
annotationsto subscriptions. To be able to track upgrades/downgrades,subscription.previous.id: holds the id of the previous subsubscription.superseding.id: holds the id of the superseding subso changes can be correlated across subscriptions
workflow.Service.ChangeToPlan.subscription/hooks/annotationproperly handles not leaving dangling annotationsSummary by CodeRabbit
New Features
Chores